Skip to content

fix(improvement): surface:'code' proposer never ran — headless flag + zero-candidate guard#501

Merged
drewstone merged 4 commits into
mainfrom
feat/raw-trace-context
Jul 9, 2026
Merged

fix(improvement): surface:'code' proposer never ran — headless flag + zero-candidate guard#501
drewstone merged 4 commits into
mainfrom
feat/raw-trace-context

Conversation

@drewstone

Copy link
Copy Markdown
Contributor

Two fixes that together made improve(surface:'code') a no-op, plus a regression test.

Fix 1: claude harness spawn used the removed --headless CLI flag

The claude harness spawn in src/mcp/local-harness.ts still passed the removed --headless CLI flag, so the code proposer's editing agent could never start. Now spawns with -p <prompt> --dangerously-skip-permissions.

Fix 2: improve(surface:'code') generated zero candidates on single-generation runs

The findings-empty guard in the improvement driver returned [] when a generation had no analyst findings — and the trace-distiller never runs at gen < maxGen - 1, so a single-generation run always hit the empty case and produced zero candidates. The agentic code generator reads the repo and raw traces on disk itself, so it now opts out of that guard via proposesWithoutFindings: true.

Test

Regression test in tests/improvement-driver.test.ts: the driver still proposes populationSize candidates on EMPTY findings when the generator sets proposesWithoutFindings.

Proven live: [proposer:claude] spawns and writes a real trace-grounded scaffold edit.

pnpm vitest run src/improvement tests/improvement-driver.test.ts — Test Files 3 passed (3), Tests 19 passed (19).

🤖 Generated with Claude Code

drewstone added 3 commits July 9, 2026 01:25
improve(surface:'code') -> agenticGenerator -> runLocalHarness spawned
'claude --headless -p <prompt>', but --headless is no longer a valid CLI
option (exit 1, unknown option), so the code-surface proposer could never
edit a candidate worktree -> every candidate empty -> the meta-harness
could not run on the code surface at all. Use '-p <prompt>
--dangerously-skip-permissions' (headless print mode + autonomous edits in
the isolated candidate worktree).
…generation runs

improvementDriver's guard 'if (findings.length === 0 && report === undefined) return []'
short-circuited the agentic code proposer whenever findings were empty. At
GENERATIONS=1 the trace-distiller (analyzeGeneration) never runs (gen < maxGen-1 is
0<0), so findings are ALWAYS empty -> the claude code-editing harness was never
spawned -> zero candidates, empty worktree, no proposal. The guard was written for
the reflective patch-applier (no findings = no patch to draft); it is wrong for an
agentic coder whose signal is the repo + raw traces on disk.

Fix: agenticGenerator declares proposesWithoutFindings:true; the driver guard honors
it and runs populationSize candidates from the seed + raw traces. Also fixes an
empty-seed fallback in raw-trace-distiller that dropped the raw-trace instruction.
Proven: [proposer:claude] now spawns and Claude Code makes a real scaffold edit
grounded in the raw traces (was grep-count 0 before). 32/32 improvement tests pass.

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 092fa863

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-07-09T07:27:35Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Value Audit — sound

Verdict sound
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.0s
Interrogation 267.9s (2 bridge agents)
Total 267.9s

💰 Value — sound

Fixes two no-op bugs in the code-surface improvement path (claude CLI spawn flag and a zero-candidate guard on first-generation runs) and adds a regression test; the approach is coherent and in the codebase's grain.

  • What it does: 1) Replaces the removed --headless claude CLI flag with -p <prompt> --dangerously-skip-permissions so the agentic code proposer can spawn (src/mcp/local-harness.ts:50). 2) Fixes rawTraceDistiller so an empty fallbackFindings array no longer wipes the steering context to []; it falls through to the default raw-trace instruction (src/improvement/raw-trace-distiller.ts:121-137). 3) Adds
  • Goals it achieves: Makes improve(surface: 'code') actually produce candidates instead of silently no-op'ing. Before this, the claude harness could not start (unknown --headless option), and even with a working harness a single-generation run always returned zero candidates because the driver's empty-findings guard fired before the raw-trace distiller had run. After this, the agentic coder can read the repo and r
  • Assessment: Good. The change is tightly scoped to the observed failure, reuses the existing pluggable CandidateGenerator seam, preserves the no-op optimization for reflective generators, and fixes the distiller fallback at the right layer (the distiller itself) rather than at every call site. The docstrings explain the contract clearly. The regression test directly targets the zero-candidate path.
  • Better / existing approach: none — this is the right approach. I checked: the only two implementations of the improvement CandidateGenerator are reflectiveGenerator (needs findings) and agenticGenerator (can use repo + traces), so a boolean opt-in is the minimal correct abstraction. The alternative of always calling the generator and letting it return applied:false would force the driver to create and discard worktre
  • Model: opencode/kimi-for-coding/k2p7
  • Bridge attempts: 1

🎯 Usefulness — sound

Two root-cause fixes that unblock a previously complete no-op (improve(surface:'code') could neither spawn its harness nor generate any candidates), on a reachable, used, public path — no materially better approach, no existing equivalent.

  • Integration: Fully reachable. improve() is publicly exported (src/index.ts:117). codeProposerFor (improve.ts:284-296) wires agenticGenerator as the default generator for surface:'code', and agenticGenerator sets proposesWithoutFindings: true (agentic-generator.ts:93). improvementDriver.propose (improvement-driver.ts:87-93) is driven by agent-eval's selfImprove loop. The claude buildArgs (loca
  • Fit with existing patterns: Fits the codebase's grain exactly. proposesWithoutFindings is a single optional boolean on the existing CandidateGenerator seam (improvement-driver.ts:48) — the one place that already varies by generator kind. The asymmetry is well-justified and documented: reflectiveGenerator drafts patches FROM findings (reflective-generator.ts:29-31), so with no findings there is genuinely nothing to appl
  • Real-world viability: Holds up on the real paths. The flag replacement is verified against the compat matrix (the old --headless was removed from the claude CLI; -p IS headless mode). The proposesWithoutFindings guard correctly preserves the reflective no-op short-circuit while unblocking the agentic first-generation case (empty seed findings + distiller not-yet-run). The empty-fallback fix closes a real silent-d
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260709T073350Z

@tangletools

Copy link
Copy Markdown
Contributor

❌ Needs Work — 092fa863

Review health 100/100 · Reviewer score 67/100 · Confidence 75/100 · 4 findings (1 high, 1 medium, 2 low)

deepseek kimi-code aggregate
Readiness 82 67 67
Confidence 75 75 75
Correctness 82 67 67
Security 82 67 67
Testing 82 67 67
Architecture 82 67 67

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 3/3 planned shots over 6 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 6 changed files. Global verifier still owns final merge decision.

Blocking

🔴 HIGH Unit tests still expect removed --headless flag — src/mcp/local-harness.ts

tests/mcp/local-harness.test.ts lines 130, 146, 150, 186, and 191 assert the legacy claude argv shape ['--headless', '-p', ...]. The changed buildArgs now emits ['-p', prompt, '--dangerously-skip-permissions'], and harnessInvocation (which calls buildArgs) will likewise produce the new shape. These stale expectations will cause test failures in CI. Update the test expectations and the 'byte-identical legacy shape' assertion to reflect the new argv order and the added permission-skip flag.

Other

🟠 MEDIUM Flag change breaks 4 existing test assertions (not updated in this PR) — src/mcp/local-harness.ts

The claude buildArgs now emits ['-p', , '--dangerously-skip-permissions'] instead of ['--headless', '-p', ]. tests/mcp/local-harness.test.ts:130 asserts ['--headless', '-p', 'go'], line 186 asserts ['--headless', '-p', 'SYS\n\ntask', '-m', 'kimi-k2.7'], and lines 190-191 assert ['--headless', '-p', 'go']. All three will fail. The test at line 146-150 passes because it uses an explicit

🟡 LOW fallbackFindings typed as unknown[] — no compile-time guard on what gets returned as findings — src/improvement/raw-trace-distiller.ts

The fallbackFindings option is typed unknown[], but the distiller internally produces AnalystFinding[] via makeFinding(). When a non-empty fallback is returned directly (line 123), the caller receives unknown[] which may contain non-AnalystFinding shapes. This is a pre-existing pattern (generateFailureDistiller also uses unknown[]; improve() takes findings: unknown[]), not introduced by this PR, but it means any consumer that assumes findings are AnalystFinding (e.g. agenticGenerator's defaultBuildPrompt line 153

🟡 LOW --dangerously-skip-permissions is unconditional for all claude invocations — src/mcp/local-harness.ts

The skip-permissions flag is now baked into HARNESS_INVOCATIONS.claude.buildArgs, so every caller of runLocalHarness (including direct callers of the exported function) gets it. Production paths route through runWorktreeHarness, which creates an isolated worktree, so the blast radius is scoped. Still, because runLocalHarness is exported publicly, a future caller passing an arbitrary cwd would also bypass permission checks. Acceptable for the documented autonomous-improvement use case, but worth a code-level warning or opt-in gate if direct usage expands.


tangletools · 2026-07-09T07:38:16Z · trace

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ 1 Blocking Finding — 092fa863

Full multi-shot audit completed 3/3 planned shots over 6 changed files. Global verifier still owns final merge decision. | Full multi-shot audit completed 3/3 planned shots over 6 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary


tangletools · 2026-07-09T07:38:16Z · immutable trace

@drewstone

Copy link
Copy Markdown
Contributor Author

Addressed the review findings in 9b37fd2.

  • Removed every stale executable test expectation for the retired --headless flag.
  • Restored a safe public default: direct runLocalHarness and harnessInvocation calls do not bypass Claude permissions.
  • Added an explicit dangerouslySkipPermissions opt-in and enabled it only after the code-proposer and worktree runner have created isolated candidate checkouts.
  • Added default, opt-in, and both isolated-worktree propagation assertions. Deliberately removing each propagation made its test fail before restoration.
  • Regenerated API docs, including the earlier proposesWithoutFindings contract from this PR.

The fallbackFindings unknown[] note is pre-existing across rawTraceDistiller, generateFailureDistiller, and the public improve(findings: unknown[]) contract. Narrowing only this one option would make the existing API inconsistent, so I did not mix that unrelated breaking type change into this fix.

Local proof: pnpm lint; pnpm typecheck; pnpm build; pnpm test (131 files, 1,278 passed, 2 skipped); pnpm docs:check; pnpm verify:package; git merge-tree --write-tree origin/main HEAD.

@drewstone

Copy link
Copy Markdown
Contributor Author

@tangletools review now

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Auto-approved drewstone PR — 9b37fd24

This PR was opened by the trusted drewstone account.
The full PR reviewer audit still runs separately and will publish findings if it detects issues.

tangletools · auto-approval · reason: drewstone_author · 2026-07-09T20:58:11Z

@tangletools tangletools left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Value Audit — sound

Verdict sound
Concerns 0 (none)
Heuristic 0.0s
Duplication 0.0s
Interrogation 102.5s (2 bridge agents)
Total 102.5s

💰 Value — sound

Makes improve(surface:'code') actually run by replacing a removed claude CLI flag, letting an agentic generator propose from an empty findings set, and stopping the distiller from wiping steering context on a clean generation — a coherent, in-grain fix with no better approach available.

  • What it does: Three coordinated behavior deltas. (1) src/mcp/local-harness.ts:47 drops the removed --headless flag (claude now spawns as -p <prompt>, which IS print/headless mode) and adds an opt-in dangerouslySkipPermissions flag (buildHarnessArgs, :74-84) threaded through both harnessInvocation (:101) and runLocalHarness (:145, :219) — default off, so the legacy/public path stays permission-safe
  • Goals it achieves: Unblocks improve(surface:'code'), which was a guaranteed no-op: the claude proposer subprocess could never start (dead flag), and even fixing that, a single-generation run always hit the empty-findings guard and produced zero candidates because rawTraceDistiller does not run at gen < maxGen-1. After merge, the agentic coder runs the full populationSize from the repo + raw traces on disk (t
  • Assessment: Good on its merits and built in the grain of the codebase. Each change lands at the natural seam: the per-harness arg table for the CLI fix; the CandidateGenerator interface for the propose-without-findings decision; the distiller's own fallback branch for the context preservation. The permission bypass is correctly fail-closed (default safe, opt-in per isolated-worktree call site, ignored by no
  • Better / existing approach: none — this is the right approach. I checked for existing equivalents and better designs: (a) the only empty-findings short-circuit in src/improvement is the one being fixed (improvement-driver.ts:88-92); the reflective generator has no internal guard and relies on the driver's, so nothing duplicates the new flag. (b) Pushing the empty-findings decision into generate() would force the driver
  • Model: opencode/kimi-for-coding/k2p7
  • Bridge attempts: 1

🎯 Usefulness — sound

Two coherent fixes that unblock the real improve(surface:'code') path — a removed-flag spawn fix and an empty-findings guard opt-out — both wired into live call sites and following the codebase's additive-opt-in grain.

  • Integration: Fully reachable. The code proposer reaches the driver via improve.ts:282-296 (codeProposerFor → improvementDriver(agenticGenerator)); the new proposesWithoutFindings flag on agentic-generator.ts:93 is read at improvement-driver.ts:90. The rawTraceDistiller empty-fallback fix is exercised by improve.ts:400 when rawTraceContext:true. The --headless removal + --dangerously-skip-permissions reaches ru
  • Fit with existing patterns: Fits the grain precisely. proposesWithoutFindings is an optional additive flag on the existing CandidateGenerator interface; reflectiveGenerator leaves it unset so the old short-circuit is preserved for patch-appliers (the documented agentic-vs-reflective distinction). dangerouslySkipPermissions follows the existing HarnessInvocationOptions opt-in pattern (additive, explicit, never the public defa
  • Real-world viability: Holds up. defaultBuildPrompt (agentic-generator.ts:149) handles empty findings cleanly, and toolBuildPrompt/mcpBuildPrompt are findings-agnostic, so opting out of the guard never starves the prompt. The driver loops populationSize sequentially (no new concurrency); the existing try/finally worktree-cleanup path (improvement-driver.ts:119-123) covers harness spawn failures. The empty-fallback chang
  • Model: opencode/zai-coding-plan/glm-5.2
  • Bridge attempts: 1

No concerns — sound change, no better or existing approach found. ✅


What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass What it asks
Heuristic Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication Do added function/class names already exist elsewhere in the repo?
Value Audit What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

value-audit · 20260709T210013Z

@drewstone drewstone merged commit dcb36a8 into main Jul 9, 2026
1 check passed
@drewstone drewstone deleted the feat/raw-trace-context branch July 9, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants