diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index 5346f1d43..33d589847 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -1666,8 +1666,11 @@ if command -v jq >/dev/null 2>&1; then # Filter to current branch + recent commits, then keep records for the # latest run_id only. (Single phase may have multiple files if the user # re-ran the review; aggregator takes the newest.) + # Bind the record's .commit to $c BEFORE entering the split-array pipe — + # inside ($commits | split("|") | ...) the input is the array, so a bare + # index(.commit) would index the array with the string "commit" and error. jq -c --arg branch "$BRANCH" --arg commits "$COMMITS_RECENT" \ - 'select(.branch == $branch and ($commits | split("|") | index(.commit) != null))' \ + 'select(.branch == $branch and (.commit as $c | ($commits | split("|") | index($c)) != null))' \ "$f" 2>/dev/null >> "$ALL_JSONL" || true done < <(find "$TASKS_DIR" -maxdepth 1 -name "tasks-$phase-*.jsonl" 2>/dev/null | sort) # Reduce to latest run_id per phase diff --git a/scripts/resolvers/tasks-section.ts b/scripts/resolvers/tasks-section.ts index 2f86f588b..13bc72d86 100644 --- a/scripts/resolvers/tasks-section.ts +++ b/scripts/resolvers/tasks-section.ts @@ -118,8 +118,11 @@ if command -v jq >/dev/null 2>&1; then # Filter to current branch + recent commits, then keep records for the # latest run_id only. (Single phase may have multiple files if the user # re-ran the review; aggregator takes the newest.) + # Bind the record's .commit to \$c BEFORE entering the split-array pipe — + # inside (\$commits | split("|") | ...) the input is the array, so a bare + # index(.commit) would index the array with the string "commit" and error. jq -c --arg branch "$BRANCH" --arg commits "$COMMITS_RECENT" \\ - 'select(.branch == $branch and ($commits | split("|") | index(.commit) != null))' \\ + 'select(.branch == $branch and (.commit as $c | ($commits | split("|") | index($c)) != null))' \\ "$f" 2>/dev/null >> "$ALL_JSONL" || true done < <(find "$TASKS_DIR" -maxdepth 1 -name "tasks-$phase-*.jsonl" 2>/dev/null | sort) # Reduce to latest run_id per phase diff --git a/test/autoplan-tasks-aggregator.test.ts b/test/autoplan-tasks-aggregator.test.ts new file mode 100644 index 000000000..4311a3b84 --- /dev/null +++ b/test/autoplan-tasks-aggregator.test.ts @@ -0,0 +1,101 @@ +import { describe, test, expect, beforeAll } from "bun:test"; +import { readFileSync } from "fs"; +import { join } from "path"; +import { spawnSync } from "child_process"; + +// Regression test for issue #2018: the autoplan Phase 4 aggregator's +// branch+commit filter used `index(.commit)` INSIDE a `$commits | split("|")` +// pipe, where the input is the split array, so jq tried to index an array with +// the string "commit" and errored. The error was swallowed by `2>/dev/null`, +// so every aggregation silently returned zero tasks. +// +// We extract the real `select(...)` filter from the generated autoplan/SKILL.md +// and run it through jq so this test tracks whatever actually ships, not a copy. + +const SKILL = join(import.meta.dir, "..", "autoplan", "SKILL.md"); + +function hasJq(): boolean { + return spawnSync("jq", ["--version"], { encoding: "utf-8" }).status === 0; +} + +function extractSelectFilter(): string { + const text = readFileSync(SKILL, "utf-8"); + // The shipped line looks like: + // 'select(.branch == $branch and (.commit as $c | ...))' \ + const m = text.match(/'(select\(\.branch == \$branch and [^']*\))'/); + if (!m) throw new Error("could not find branch+commit select filter in autoplan/SKILL.md"); + return m[1]; +} + +// Run the extracted filter over one JSONL record, returning jq's result. +function runFilter(filter: string, record: object, branch: string, commits: string) { + return spawnSync( + "jq", + ["-c", "--arg", "branch", branch, "--arg", "commits", commits, filter], + { input: JSON.stringify(record), encoding: "utf-8", timeout: 10000 } + ); +} + +describe("autoplan tasks aggregator branch+commit filter (issue #2018)", () => { + let filter: string; + + beforeAll(() => { + filter = extractSelectFilter(); + }); + + test("filter does not reference the buggy bare index(.commit)", () => { + // The fix binds the record's commit to a variable first; the regression was + // an unbound `.commit` evaluated against the split array. + expect(filter).not.toContain("index(.commit)"); + expect(filter).toContain("as $c"); + }); + + test("matching branch + commit is selected (no jq error)", () => { + const r = runFilter( + filter, + { branch: "dev", commit: "abc123", phase: "ceo-review" }, + "dev", + "abc123|def456" + ); + expect(r.status).toBe(0); + expect(r.stderr).toBe(""); + expect(r.stdout.trim()).not.toBe(""); + expect(JSON.parse(r.stdout).commit).toBe("abc123"); + }); + + test("non-matching commit is excluded (no jq error)", () => { + const r = runFilter( + filter, + { branch: "dev", commit: "zzz999", phase: "ceo-review" }, + "dev", + "abc123|def456" + ); + expect(r.status).toBe(0); + expect(r.stderr).toBe(""); + expect(r.stdout.trim()).toBe(""); + }); + + test("wrong branch is excluded (no jq error)", () => { + const r = runFilter( + filter, + { branch: "other", commit: "abc123", phase: "ceo-review" }, + "dev", + "abc123|def456" + ); + expect(r.status).toBe(0); + expect(r.stderr).toBe(""); + expect(r.stdout.trim()).toBe(""); + }); + + test("record missing commit field does not error, just excludes", () => { + const r = runFilter( + filter, + { branch: "dev", phase: "ceo-review" }, + "dev", + "abc123|def456" + ); + expect(r.status).toBe(0); + expect(r.stderr).toBe(""); + expect(r.stdout.trim()).toBe(""); + }); +});