From 1948e54e88136e92dc698068042ae7d2652764e0 Mon Sep 17 00:00:00 2001 From: 0xDevNinja Date: Tue, 16 Jun 2026 12:16:34 +0530 Subject: [PATCH 1/2] fix(autoplan): bind commit before split-array index in tasks aggregator The Phase 4 implementation-tasks aggregator filtered per-phase JSONL by branch and recent commit with: select(.branch == $branch and ($commits | split("|") | index(.commit) != null)) Inside the ($commits | split("|") | ...) pipe the input is the split array, so index(.commit) tried to index an array with the string "commit" and jq errored with "Cannot index array with string". The surrounding invocation swallowed it via 2>/dev/null, so every run silently dropped all records and reported no actionable tasks even when valid task records existed for the current branch and commits. Bind the record's commit to $c before entering the split-array pipe so index() receives the commit string and the membership test works. Fixes #2018. --- autoplan/SKILL.md | 5 ++++- scripts/resolvers/tasks-section.ts | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/autoplan/SKILL.md b/autoplan/SKILL.md index 49db38ff9..2e4888b3b 100644 --- a/autoplan/SKILL.md +++ b/autoplan/SKILL.md @@ -1637,8 +1637,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 From 27c583af585ff083b6b01e6f5c8b0f44baeac3b4 Mon Sep 17 00:00:00 2001 From: 0xDevNinja Date: Tue, 16 Jun 2026 12:16:34 +0530 Subject: [PATCH 2/2] test(autoplan): regression test for tasks aggregator branch+commit filter Extracts the shipped select filter from autoplan/SKILL.md and runs it through jq over sample records: matching branch+commit is kept, a non-matching commit / wrong branch / missing commit field are excluded without a jq error. Pins the #2018 fix and fails on the bare index(.commit) form (which errors). Refs #2018. --- test/autoplan-tasks-aggregator.test.ts | 101 +++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 test/autoplan-tasks-aggregator.test.ts 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(""); + }); +});