From 104359cfe61d7a634805bf02ef2b30d39c532ada Mon Sep 17 00:00:00 2001 From: nicolascukas Date: Mon, 8 Jun 2026 11:55:39 +0200 Subject: [PATCH 1/6] =?UTF-8?q?feat(cli):=20add=20'Other=20=E2=80=94=20rev?= =?UTF-8?q?ise=20the=20plan'=20option=20to=20plan=20approval?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plan approval was binary (Approve / Reject). When a plan was almost-right, the user had no in-band way to say 'adjust step 2' — typing into the composer was silently swallowed by the awaiting-approval keyboard guard, and Reject was destructive (killed the proposal text). Add a third slot 'Other — revise the plan': - PlanProposalView renders 1) Approve, 3) Other, 2) Reject - keyboard.kern cycles 3 values; y/1, n/2, o/3 jump directly - Arrows wrap (down: 0→1→2→0; up: 0→2) - 'revise' action clears the proposal from the live pane, pre-fills the composer with 'Revise the plan: ', and shows a brief hint. User finishes typing and presses Enter; the message routes to Cesar who (per session.kern RULE 9) re-proposes with the user's changes — the old plan is replaced, not killed. - planControl.action union widens to 'approve'|'cancel'|'revise' Tests: 43/43 keyboard tests pass (added one covering the 3-way cycle, o/3 jumps, and the revise action). 2209/2209 total. --- .../src/generated/blocks/plan-view.entry.tsx | 4 ++-- .../cli/src/generated/blocks/plan-view.tsx | 5 ++-- .../cli/src/generated/signals/keyboard.ts | 14 +++++++---- packages/cli/src/generated/surfaces/app.tsx | 13 +++++++++- packages/cli/src/kern/blocks/plan-view.kern | 3 ++- packages/cli/src/kern/signals/keyboard.kern | 14 +++++++---- packages/cli/src/kern/surfaces/app.kern | 11 +++++++++ tests/unit/keyboard.test.ts | 24 +++++++++++++++++++ 8 files changed, 74 insertions(+), 14 deletions(-) diff --git a/packages/cli/src/generated/blocks/plan-view.entry.tsx b/packages/cli/src/generated/blocks/plan-view.entry.tsx index 7722884c..2a9a5885 100644 --- a/packages/cli/src/generated/blocks/plan-view.entry.tsx +++ b/packages/cli/src/generated/blocks/plan-view.entry.tsx @@ -1,7 +1,7 @@ #!/usr/bin/env node -// @generated by kern v3.5.7 — DO NOT EDIT. Source: src/kern/blocks/plan-view.kern +// @generated by kern v3.5.8 — DO NOT EDIT. Source: src/kern/blocks/plan-view.kern -// @kern-source: plan-view:220 +// @kern-source: plan-view:221 import React from 'react'; import { render } from 'ink'; diff --git a/packages/cli/src/generated/blocks/plan-view.tsx b/packages/cli/src/generated/blocks/plan-view.tsx index 91edc396..adcf149a 100644 --- a/packages/cli/src/generated/blocks/plan-view.tsx +++ b/packages/cli/src/generated/blocks/plan-view.tsx @@ -188,8 +188,9 @@ const PlanProposalView = React.memo(function PlanProposalView({ plan, markdown, {'Awaiting approval'} {(selectedIndex ?? 0) === 0 ? '❯ ' : ' '}{'1. Approve & run'} + {(selectedIndex ?? 0) === 2 ? '❯ ' : ' '}{'3. Other — revise the plan'} {(selectedIndex ?? 0) === 1 ? '❯ ' : ' '}{'2. Reject'} - {' ↑↓ move · Enter select · ✎ type to change · /approve · /cancel'} + {' ↑↓ move · Enter select · 1/2/3 jump · y/n/o · /approve · /cancel'} )} @@ -197,7 +198,7 @@ const PlanProposalView = React.memo(function PlanProposalView({ plan, markdown, }); export { PlanProposalView }; -// @kern-source: plan-view:220 +// @kern-source: plan-view:221 export function PlanExecutionView({ plan }: { plan:any }) { const steps: any[] = plan.steps ?? []; const doneSteps = steps.filter((s: any) => s.state === 'done'); diff --git a/packages/cli/src/generated/signals/keyboard.ts b/packages/cli/src/generated/signals/keyboard.ts index 1844ec1f..9864d28f 100644 --- a/packages/cli/src/generated/signals/keyboard.ts +++ b/packages/cli/src/generated/signals/keyboard.ts @@ -73,7 +73,7 @@ export type KeyboardAction = | { type: 'togglePauseMenu' } | { type: 'movePauseCursor'; direction: 'up'|'down' } | { type: 'selectPauseAction' } - | { type: 'planControl'; action: 'approve'|'cancel' } + | { type: 'planControl'; action: 'approve'|'cancel'|'revise' } | { type: 'movePlanApproval'; index: number } | { type: 'updateBanner'; action: 'update'|'changelog'|'dismiss' }; @@ -255,15 +255,21 @@ export function resolveKeyboardInput(ctx: KeyboardCtx): KeyboardAction { && !ctx.reviewEventOpen && !ctx.toolDetailOpen && !ctx.questionState ) { - const pcur = (ctx.planApprovalIndex ?? 0) === 1 ? 1 : 0; - if (key.upArrow || key.downArrow) return { type: 'movePlanApproval', index: pcur === 0 ? 1 : 0 }; + // 3-option plan approval: 0=Approve, 1=Reject, 2=Other (revise / type feedback). + const pcur = Math.max(0, Math.min(2, ctx.planApprovalIndex ?? 0)); + const next = pcur === 0 ? 1 : pcur === 1 ? 2 : 0; + const prev = pcur === 0 ? 2 : pcur === 1 ? 0 : 1; + if (key.downArrow) return { type: 'movePlanApproval', index: next }; + if (key.upArrow) return { type: 'movePlanApproval', index: prev }; if (key.return || input === '\r' || input === '\n') { - return { type: 'planControl', action: pcur === 1 ? 'cancel' : 'approve' }; + const action = pcur === 1 ? 'cancel' : pcur === 2 ? 'revise' : 'approve'; + return { type: 'planControl', action }; } if (typeof input === 'string' && input.length === 1) { const lowered = input.toLowerCase(); if (lowered === 'y' || lowered === '1') return { type: 'movePlanApproval', index: 0 }; if (lowered === 'n' || lowered === '2') return { type: 'movePlanApproval', index: 1 }; + if (lowered === 'o' || lowered === '3') return { type: 'movePlanApproval', index: 2 }; } } diff --git a/packages/cli/src/generated/surfaces/app.tsx b/packages/cli/src/generated/surfaces/app.tsx index d8364328..ca23f460 100644 --- a/packages/cli/src/generated/surfaces/app.tsx +++ b/packages/cli/src/generated/surfaces/app.tsx @@ -1887,6 +1887,17 @@ export function App() { // into the composer after we route the approval. ctrlKeyHandledRef.current = true; setPlanApprovalIndex(0); + if (action.action === 'revise') { + // Drop the proposal from the live pane but leave the plan in + // activePlanRef so /cancel on the next turn is a no-op. Pre-fill + // the composer with a revision prompt; pressing Enter routes the + // message to Cesar, who (per session.kern RULE 9) will see the + // pending plan and ProposePlan again with the user's changes. + setPendingPlanProposal(null); + setInputValue('Revise the plan: '); + dispatch({ type: 'info', message: 'Type your revision and press Enter (Esc clears).' } as any); + return; + } handleSubmit(action.action === 'approve' ? '/approve' : '/cancel'); return; case 'movePlanApproval': @@ -2921,7 +2932,7 @@ export const _lastSigintAt: { value: number } = { value: 0 }; // @kern-source: app:93 export const _pauseState: { value: PauseState | null } = { value: null }; -// @kern-source: app:2656 +// @kern-source: app:2667 export async function startRepl(): Promise { ensureAgonHome(); ensureCurrentWorkspace(process.cwd()); diff --git a/packages/cli/src/kern/blocks/plan-view.kern b/packages/cli/src/kern/blocks/plan-view.kern index 1eb2a3cf..f54846d5 100644 --- a/packages/cli/src/kern/blocks/plan-view.kern +++ b/packages/cli/src/kern/blocks/plan-view.kern @@ -207,8 +207,9 @@ screen name=PlanProposalView target=ink memo=true {'Awaiting approval'} {(selectedIndex ?? 0) === 0 ? '❯ ' : ' '}{'1. Approve & run'} + {(selectedIndex ?? 0) === 2 ? '❯ ' : ' '}{'3. Other — revise the plan'} {(selectedIndex ?? 0) === 1 ? '❯ ' : ' '}{'2. Reject'} - {' ↑↓ move · Enter select · ✎ type to change · /approve · /cancel'} + {' ↑↓ move · Enter select · 1/2/3 jump · y/n/o · /approve · /cancel'} )} diff --git a/packages/cli/src/kern/signals/keyboard.kern b/packages/cli/src/kern/signals/keyboard.kern index 9cfa3a24..caafc9d0 100644 --- a/packages/cli/src/kern/signals/keyboard.kern +++ b/packages/cli/src/kern/signals/keyboard.kern @@ -80,7 +80,7 @@ union name=KeyboardAction discriminant=type field name=direction type="'up'|'down'" variant name=selectPauseAction variant name=planControl - field name=action type="'approve'|'cancel'" + field name=action type="'approve'|'cancel'|'revise'" variant name=movePlanApproval field name=index type=number variant name=updateBanner @@ -262,15 +262,21 @@ fn name=resolveKeyboardInput params="ctx:KeyboardCtx" returns="KeyboardAction" && !ctx.reviewEventOpen && !ctx.toolDetailOpen && !ctx.questionState ) { - const pcur = (ctx.planApprovalIndex ?? 0) === 1 ? 1 : 0; - if (key.upArrow || key.downArrow) return { type: 'movePlanApproval', index: pcur === 0 ? 1 : 0 }; + // 3-option plan approval: 0=Approve, 1=Reject, 2=Other (revise / type feedback). + const pcur = Math.max(0, Math.min(2, ctx.planApprovalIndex ?? 0)); + const next = pcur === 0 ? 1 : pcur === 1 ? 2 : 0; + const prev = pcur === 0 ? 2 : pcur === 1 ? 0 : 1; + if (key.downArrow) return { type: 'movePlanApproval', index: next }; + if (key.upArrow) return { type: 'movePlanApproval', index: prev }; if (key.return || input === '\r' || input === '\n') { - return { type: 'planControl', action: pcur === 1 ? 'cancel' : 'approve' }; + const action = pcur === 1 ? 'cancel' : pcur === 2 ? 'revise' : 'approve'; + return { type: 'planControl', action }; } if (typeof input === 'string' && input.length === 1) { const lowered = input.toLowerCase(); if (lowered === 'y' || lowered === '1') return { type: 'movePlanApproval', index: 0 }; if (lowered === 'n' || lowered === '2') return { type: 'movePlanApproval', index: 1 }; + if (lowered === 'o' || lowered === '3') return { type: 'movePlanApproval', index: 2 }; } } diff --git a/packages/cli/src/kern/surfaces/app.kern b/packages/cli/src/kern/surfaces/app.kern index 9e29bc11..29ee8abc 100644 --- a/packages/cli/src/kern/surfaces/app.kern +++ b/packages/cli/src/kern/surfaces/app.kern @@ -2103,6 +2103,17 @@ ${streamCtx ? 'Recent live output from the running task:\n' + streamCtx + '\n\n' // into the composer after we route the approval. ctrlKeyHandledRef.current = true; setPlanApprovalIndex(0); + if (action.action === 'revise') { + // Drop the proposal from the live pane but leave the plan in + // activePlanRef so /cancel on the next turn is a no-op. Pre-fill + // the composer with a revision prompt; pressing Enter routes the + // message to Cesar, who (per session.kern RULE 9) will see the + // pending plan and ProposePlan again with the user's changes. + setPendingPlanProposal(null); + setInputValue('Revise the plan: '); + dispatch({ type: 'info', message: 'Type your revision and press Enter (Esc clears).' } as any); + return; + } handleSubmit(action.action === 'approve' ? '/approve' : '/cancel'); return; case 'movePlanApproval': diff --git a/tests/unit/keyboard.test.ts b/tests/unit/keyboard.test.ts index 6fdf02b0..5424ca47 100644 --- a/tests/unit/keyboard.test.ts +++ b/tests/unit/keyboard.test.ts @@ -312,6 +312,30 @@ describe('resolveKeyboardInput', () => { .toEqual({ type: 'planControl', action: 'cancel' }); }); + it('plan approval: third "Other" slot — arrows cycle 0→1→2→0, o/3 jump to it, Enter on it revises', () => { + // Down from Approve goes to Reject (1) — unchanged behavior. + expect(resolveKeyboardInput(baseCtx({ key: { downArrow: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 0 }))) + .toEqual({ type: 'movePlanApproval', index: 1 }); + // Down from Reject (1) wraps to Other (2) — new. + expect(resolveKeyboardInput(baseCtx({ key: { downArrow: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 1 }))) + .toEqual({ type: 'movePlanApproval', index: 2 }); + // Down from Other (2) wraps back to Approve (0). + expect(resolveKeyboardInput(baseCtx({ key: { downArrow: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 2 }))) + .toEqual({ type: 'movePlanApproval', index: 0 }); + // Up from Approve (0) wraps to Other (2). + expect(resolveKeyboardInput(baseCtx({ key: { upArrow: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 0 }))) + .toEqual({ type: 'movePlanApproval', index: 2 }); + // o/3 jump straight to Other. + expect(resolveKeyboardInput(baseCtx({ input: 'o', key: {}, activePlanState: 'awaiting_approval' }))) + .toEqual({ type: 'movePlanApproval', index: 2 }); + expect(resolveKeyboardInput(baseCtx({ input: '3', key: {}, activePlanState: 'awaiting_approval' }))) + .toEqual({ type: 'movePlanApproval', index: 2 }); + // Enter on Other emits the 'revise' action (new union member) — the app + // route clears the proposal and pre-fills the composer for free-form feedback. + expect(resolveKeyboardInput(baseCtx({ input: '\r', key: { return: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 2 }))) + .toEqual({ type: 'planControl', action: 'revise' }); + }); + it('leaves PgUp/PgDn/Home/End to the terminal — main-buffer + native scrollback owns scroll', () => { // In Agon's current architecture (no alt-screen, transcript rows committed // to Static → terminal scrollback), the app has no in-app scroll surface. From bcd93506c2976f0aac1a8639bab5d4938dfefe64 Mon Sep 17 00:00:00 2001 From: nicolascukas Date: Mon, 8 Jun 2026 12:11:12 +0200 Subject: [PATCH 2/6] fix(review): announce the real review target + warn on cwd/repo mismatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit agon resolves the review directory from the active workspace (resolveWorkingDir), not process.cwd() — so running a review from repo X while the active workspace is repo Y silently reviews Y. This produced a 6-engine review of agon's OWN repo instead of the user's code, with no visible signal; the reviewers never saw the intended changes. Adds announceReviewTarget(), called in handleReview + handleReviewMany BEFORE the empty-diff check: prints the repo name/path/branch actually being reviewed, and warns loudly when process.cwd() is a DIFFERENT git repo than the one being reviewed (the active-workspace footgun). A wrong-repo review can no longer pass silently — even "No changes to review" now names the repo it inspected. (Does not change which dir is used — that's the deliberate active-workspace model; switch workspace or pass branch:/commit: to review elsewhere. This makes the mistake impossible to miss.) review tests: 58/58 pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/cli/src/generated/handlers/review.ts | 28 ++++++++++++++++++- packages/cli/src/kern/handlers/review.kern | 24 ++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/generated/handlers/review.ts b/packages/cli/src/generated/handlers/review.ts index 209c4021..d3d30279 100644 --- a/packages/cli/src/generated/handlers/review.ts +++ b/packages/cli/src/generated/handlers/review.ts @@ -590,6 +590,9 @@ export async function handleReview(dispatch: Dispatch, ctx: HandlerContext, targ return; } + // Announce the real target (and warn on a cwd-vs-reviewed-repo mismatch) + // before anything else, so an empty or wrong-repo review is never silent. + announceReviewTarget(dispatch, cwd, label); if (!diff.trim()) { dispatch({ type: 'info', message: `No changes to review (${label}).` }); return; @@ -696,10 +699,29 @@ export async function handleReview(dispatch: Dispatch, ctx: HandlerContext, targ } } +/** + * Make the review's actual target unmistakable BEFORE engines run. Prints the repo name/path/branch being reviewed, and — critically — warns when the directory you're standing in is a DIFFERENT git repo than the one being reviewed. agon resolves the review dir from the active workspace (resolveWorkingDir), not process.cwd(), so running `agon review` from repo X while the active workspace is repo Y silently reviews Y. That footgun produced a 6-engine review of agon's own repo instead of the user's code; this turns it into a loud, actionable signal instead of a silent wrong-repo pass. + */ +// @kern-source: review:641 +function announceReviewTarget(dispatch: Dispatch, cwd: string, label: string): void { + let reviewRoot = cwd; + try { reviewRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], { cwd, encoding: 'utf-8' }).trim() || cwd; } catch { /* not a git repo — keep cwd */ } + let branch = ''; + try { branch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { cwd, encoding: 'utf-8' }).trim(); } catch { /* detached / no repo */ } + const name = reviewRoot.split(sep).filter(Boolean).pop() ?? reviewRoot; + dispatch({ type: 'info', message: `Reviewing ${label} in ${name} (${reviewRoot})${branch ? ` on ${branch}` : ''}` } as any); + try { + const pwdRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], { cwd: process.cwd(), encoding: 'utf-8' }).trim(); + if (pwdRoot && pwdRoot !== reviewRoot) { + dispatch({ type: 'warning', message: `Heads up: you are in ${pwdRoot}, but this review targets the active workspace ${name} (${reviewRoot}). To review where you are, switch the active workspace (e.g. /workspace ${name === 'agon' ? '' : name}) or pass an explicit target (branch:NAME / commit:SHA).` } as any); + } + } catch { /* process.cwd() isn't a git repo — nothing to compare against */ } +} + /** * Run review for one or more explicitly requested engines. With 2+ engines they run in PARALLEL — each gets its own hard timeout, so a slow-but-excellent reviewer (codex) never blocks the others and a hung engine can't wedge the whole review. Each engine's block is dispatched as it finishes; findings are combined into ctx.lastReviewResult for Cesar follow-up/fix planning. A single engine delegates to the streaming handleReview path. */ -// @kern-source: review:638 +// @kern-source: review:658 export async function handleReviewMany(dispatch: Dispatch, ctx: HandlerContext, target?: string, requestedEngines?: string[]): Promise { const abort = new AbortController(); try { @@ -724,6 +746,10 @@ export async function handleReviewMany(dispatch: Dispatch, ctx: HandlerContext, dispatch({ type: 'error', message: err instanceof Error ? err.message : String(err) }); return; } + // Announce the real target (and warn on a cwd-vs-reviewed-repo mismatch) + // BEFORE the empty-diff check, so even "No changes to review" makes clear + // which repo was actually inspected. + announceReviewTarget(dispatch, cwd, label); if (!diff.trim()) { dispatch({ type: 'info', message: `No changes to review (${label}).` }); return; diff --git a/packages/cli/src/kern/handlers/review.kern b/packages/cli/src/kern/handlers/review.kern index c137b9e5..5a4e201e 100644 --- a/packages/cli/src/kern/handlers/review.kern +++ b/packages/cli/src/kern/handlers/review.kern @@ -529,6 +529,9 @@ fn name=handleReview params="dispatch:Dispatch, ctx:HandlerContext, target?:stri return; } + // Announce the real target (and warn on a cwd-vs-reviewed-repo mismatch) + // before anything else, so an empty or wrong-repo review is never silent. + announceReviewTarget(dispatch, cwd, label); if (!diff.trim()) { dispatch({ type: 'info', message: `No changes to review (${label}).` }); return; @@ -635,6 +638,23 @@ fn name=handleReview params="dispatch:Dispatch, ctx:HandlerContext, target?:stri ctx.setActiveAbort(null); >>> +fn name=announceReviewTarget params="dispatch:Dispatch, cwd:string, label:string" returns=void export=false + doc "Make the review's actual target unmistakable BEFORE engines run. Prints the repo name/path/branch being reviewed, and — critically — warns when the directory you're standing in is a DIFFERENT git repo than the one being reviewed. agon resolves the review dir from the active workspace (resolveWorkingDir), not process.cwd(), so running `agon review` from repo X while the active workspace is repo Y silently reviews Y. That footgun produced a 6-engine review of agon's own repo instead of the user's code; this turns it into a loud, actionable signal instead of a silent wrong-repo pass." + handler <<< + let reviewRoot = cwd; + try { reviewRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], { cwd, encoding: 'utf-8' }).trim() || cwd; } catch { /* not a git repo — keep cwd */ } + let branch = ''; + try { branch = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { cwd, encoding: 'utf-8' }).trim(); } catch { /* detached / no repo */ } + const name = reviewRoot.split(sep).filter(Boolean).pop() ?? reviewRoot; + dispatch({ type: 'info', message: `Reviewing ${label} in ${name} (${reviewRoot})${branch ? ` on ${branch}` : ''}` } as any); + try { + const pwdRoot = execFileSync('git', ['rev-parse', '--show-toplevel'], { cwd: process.cwd(), encoding: 'utf-8' }).trim(); + if (pwdRoot && pwdRoot !== reviewRoot) { + dispatch({ type: 'warning', message: `Heads up: you are in ${pwdRoot}, but this review targets the active workspace ${name} (${reviewRoot}). To review where you are, switch the active workspace (e.g. /workspace ${name === 'agon' ? '' : name}) or pass an explicit target (branch:NAME / commit:SHA).` } as any); + } + } catch { /* process.cwd() isn't a git repo — nothing to compare against */ } + >>> + fn name=handleReviewMany params="dispatch:Dispatch, ctx:HandlerContext, target?:string, requestedEngines?:string[]" returns="Promise" async=true export=true doc "Run review for one or more explicitly requested engines. With 2+ engines they run in PARALLEL — each gets its own hard timeout, so a slow-but-excellent reviewer (codex) never blocks the others and a hung engine can't wedge the whole review. Each engine's block is dispatched as it finishes; findings are combined into ctx.lastReviewResult for Cesar follow-up/fix planning. A single engine delegates to the streaming handleReview path." signal name=abort @@ -660,6 +680,10 @@ fn name=handleReviewMany params="dispatch:Dispatch, ctx:HandlerContext, target?: dispatch({ type: 'error', message: err instanceof Error ? err.message : String(err) }); return; } + // Announce the real target (and warn on a cwd-vs-reviewed-repo mismatch) + // BEFORE the empty-diff check, so even "No changes to review" makes clear + // which repo was actually inspected. + announceReviewTarget(dispatch, cwd, label); if (!diff.trim()) { dispatch({ type: 'info', message: `No changes to review (${label}).` }); return; From fd2cf8158c93a897c6d3f76fc4c97f46f2a21f78 Mon Sep 17 00:00:00 2001 From: nicolascukas Date: Mon, 8 Jun 2026 12:18:09 +0200 Subject: [PATCH 3/6] fix(review): dispatch CLI review engines in the repo the diff came from MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agon review` computed the diff from process.cwd() but called runReviewCore WITHOUT a cwdOverride, so each engine was dispatched into resolveWorkingDir() = the active workspace. Running `agon review` from repo X therefore reviewed X's diff while the reviewers gathered file context from the active-workspace repo (e.g. agon's own source) — so a multi-engine review "never saw the code" the diff referenced. This silently invalidated reviews run outside the active workspace. Pass cwd (process.cwd()) as runReviewCore's cwdOverride so diff + engine context stay in one repo, and print a "Repo: " header so the target repo is always visible. Verified: `agon review` from kern-guard now shows "Repo: .../kern-guard" and dispatches engines there. Pairs with the handler-side announce/mismatch-warning (REPL path). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/cli/src/commands/review.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/review.ts b/packages/cli/src/commands/review.ts index be88209c..493a6042 100644 --- a/packages/cli/src/commands/review.ts +++ b/packages/cli/src/commands/review.ts @@ -151,6 +151,7 @@ export const reviewCommand = defineCommand({ : 'single engine'; if (!quiet) { header(`Review: ${target.label}`); + info(`Repo: ${cwd}`); info(`Engines: ${requested.join(', ')} (${concurrencyNote})`); info(`Per-engine timeout: ${timeoutSec}s (auto-cancel, others unaffected)`); } @@ -188,7 +189,13 @@ export const reviewCommand = defineCommand({ // Flush a single labeled block so concurrent engines never interleave mid-line. const flush = (body: string[]) => { if (!quiet && body.length) console.log(`\n▸ Reviewer: ${bold(engineId)}\n${body.join('\n')}`); }; try { - const result = await runReviewCore(target.diff, target.label, engineId, ctx, controller.signal); + // Pin the engine dispatch to the SAME cwd the diff came from (process.cwd()). + // Without this cwdOverride, runReviewCore falls back to resolveWorkingDir() + // = the active workspace — so `agon review` run from repo X dispatched every + // engine into the active-workspace repo (e.g. agon's own source) to gather + // file context, while reviewing X's diff. The reviewers "never saw the code" + // the diff referenced. Passing cwd keeps diff + engine context in one repo. + const result = await runReviewCore(target.diff, target.label, engineId, ctx, controller.signal, undefined, cwd); const rawResponse = result.response ?? ''; writeOutput(rawResponse); if (timedOut) { From 6dc5e3559d5bd7cf56c76fcbebd2ee6d4af8ced4 Mon Sep 17 00:00:00 2001 From: nicolascukas Date: Mon, 8 Jun 2026 12:43:49 +0200 Subject: [PATCH 4/6] fix(review): pin the parse-repair dispatch to cwdOverride too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLI review cwd fix pinned the MAIN engine dispatch to process.cwd(), but runReviewCore's parse-repair retry (runReviewRepair) still recomputed resolveWorkingDir() = the active workspace. So when a review's first response didn't parse, the repair engine ran in the wrong repo — the same split-brain bug, one layer down. Caught by the panel review of our own branch. Thread cwdOverride into runReviewRepair and pass runReviewCore's resolved cwd. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/cli/src/generated/handlers/review.ts | 8 ++++---- packages/cli/src/kern/handlers/review.kern | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/generated/handlers/review.ts b/packages/cli/src/generated/handlers/review.ts index d3d30279..5451869f 100644 --- a/packages/cli/src/generated/handlers/review.ts +++ b/packages/cli/src/generated/handlers/review.ts @@ -324,12 +324,12 @@ export function summarizeReviewFindings(response: string): ReviewSeverityCounts } /** - * Repair pass (B): re-ask the engine for ONLY a bare JSON array of the findings it already wrote in prose. Asking for a bare array (no sentinel, no prose, no fence) is the format LLMs comply with most reliably — far better than 'an HTML-comment marker followed by JSON', which engines routinely truncate to just the marker. The caller (runReviewCore) prepends the sentinel itself before parsing, so the anti-injection anchor is preserved. Best-effort: if this still doesn't parse, the fail-closed/unstructured result stands. + * Repair pass (B): re-ask the engine for ONLY a bare JSON array of the findings it already wrote in prose. Asking for a bare array (no sentinel, no prose, no fence) is the format LLMs comply with most reliably — far better than 'an HTML-comment marker followed by JSON', which engines routinely truncate to just the marker. The caller (runReviewCore) prepends the sentinel itself before parsing, so the anti-injection anchor is preserved. Best-effort: if this still doesn't parse, the fail-closed/unstructured result stands. cwdOverride must match the main dispatch's cwd so the repair engine runs in the SAME repo (goal worktree / process.cwd()), never the active workspace. */ // @kern-source: review:308 -export async function runReviewRepair(priorReview: string, engineId: string, ctx: HandlerContext, signal?: AbortSignal): Promise { +export async function runReviewRepair(priorReview: string, engineId: string, ctx: HandlerContext, signal?: AbortSignal, cwdOverride?: string): Promise { const config = ctx.config; - const cwd = resolveWorkingDir(); + const cwd = cwdOverride ?? resolveWorkingDir(); const parts: string[] = []; parts.push('You previously produced this code review:'); parts.push(priorReview); @@ -497,7 +497,7 @@ export async function runReviewCore(diff: string, label: string, engineId: strin let unstructured = false; // Repair pass (B): the engine reviewed but didn't emit a parseable findings block (the common case: it ends with the sentinel and no JSON). Re-ask for ONLY a bare JSON array — the format LLMs comply with most reliably — then prepend the sentinel OURSELVES so parseReviewBlocking's anti-injection anchor still holds. Append the reconstructed block so the result is parseable downstream. Failure leaves the fail-closed/unstructured result intact. if (parseFailed && response.length > 0 && !signal?.aborted) { - const repairResp = await runReviewRepair(response, engineId, ctx, signal); + const repairResp = await runReviewRepair(response, engineId, ctx, signal, cwd); if (repairResp) { const repairBlock = `\n${repairResp}`; const parsed2 = parseReviewBlocking(repairBlock); diff --git a/packages/cli/src/kern/handlers/review.kern b/packages/cli/src/kern/handlers/review.kern index 5a4e201e..b2e186cf 100644 --- a/packages/cli/src/kern/handlers/review.kern +++ b/packages/cli/src/kern/handlers/review.kern @@ -305,11 +305,11 @@ fn name=summarizeReviewFindings params="response:string" returns="ReviewSeverity return { blocking, important, nit, total: findings.length }; >>> -fn name=runReviewRepair params="priorReview:string, engineId:string, ctx:HandlerContext, signal?:AbortSignal" returns="Promise" async=true - doc "Repair pass (B): re-ask the engine for ONLY a bare JSON array of the findings it already wrote in prose. Asking for a bare array (no sentinel, no prose, no fence) is the format LLMs comply with most reliably — far better than 'an HTML-comment marker followed by JSON', which engines routinely truncate to just the marker. The caller (runReviewCore) prepends the sentinel itself before parsing, so the anti-injection anchor is preserved. Best-effort: if this still doesn't parse, the fail-closed/unstructured result stands." +fn name=runReviewRepair params="priorReview:string, engineId:string, ctx:HandlerContext, signal?:AbortSignal, cwdOverride?:string" returns="Promise" async=true + doc "Repair pass (B): re-ask the engine for ONLY a bare JSON array of the findings it already wrote in prose. Asking for a bare array (no sentinel, no prose, no fence) is the format LLMs comply with most reliably — far better than 'an HTML-comment marker followed by JSON', which engines routinely truncate to just the marker. The caller (runReviewCore) prepends the sentinel itself before parsing, so the anti-injection anchor is preserved. Best-effort: if this still doesn't parse, the fail-closed/unstructured result stands. cwdOverride must match the main dispatch's cwd so the repair engine runs in the SAME repo (goal worktree / process.cwd()), never the active workspace." handler lang="kern" let name=config value="ctx.config" - let name=cwd value="resolveWorkingDir()" + let name=cwd value="cwdOverride ?? resolveWorkingDir()" let name=parts type="string[]" value="[]" do value="parts.push('You previously produced this code review:')" do value="parts.push(priorReview)" @@ -450,7 +450,7 @@ fn name=runReviewCore params="diff:string, label:string, engineId:string, ctx:Ha let name=unstructured kind=let value="false" comment raw="// Repair pass (B): the engine reviewed but didn't emit a parseable findings block (the common case: it ends with the sentinel and no JSON). Re-ask for ONLY a bare JSON array — the format LLMs comply with most reliably — then prepend the sentinel OURSELVES so parseReviewBlocking's anti-injection anchor still holds. Append the reconstructed block so the result is parseable downstream. Failure leaves the fail-closed/unstructured result intact." if cond="parseFailed && response.length > 0 && !(signal?.aborted)" - let name=repairResp value="await runReviewRepair(response, engineId, ctx, signal)" + let name=repairResp value="await runReviewRepair(response, engineId, ctx, signal, cwd)" if cond="repairResp" let name=repairBlock value="`\\n${repairResp}`" let name=parsed2 value="parseReviewBlocking(repairBlock)" From a6f8abc894d7143ca49888593d518aa782238198 Mon Sep 17 00:00:00 2001 From: nicolascukas Date: Mon, 8 Jun 2026 12:44:01 +0200 Subject: [PATCH 5/6] fix(cli): plan-approval cursor order + missing Other slot (taking over Cesar's work) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The panel review flagged real bugs in Cesar's 'Other — revise the plan' feature (104359cf): - Indices were 0=Approve, 1=Reject, 2=Other, but the options render Approve / Other / Reject and arrows cycled by index 0→1→2 — so ↓ from Approve jumped to Reject (bottom), skipping the middle Other slot; the cursor moved top→bottom→middle (felt broken). - The numbers displayed out of order (1, 3, 2). - A SECOND rendering (the markdown/pinned plan block) never got the Other option at all and still showed the old "✎ type to change" footer. Renumber everything to one consistent VISUAL order — 0=Approve, 1=Other, 2=Reject: - keyboard.kern: arrows cycle 0→1→2→0 (Approve→Other→Reject, no skip); keys y/1→Approve, o/2→Other, n/3→Reject; Enter maps 1→revise, 2→cancel. - plan-view.kern: both renderings show 1. Approve / 2. Other / 3. Reject with matching colors and the updated footer; the markdown block gains the Other slot it was missing. - keyboard.test.ts: updated the 3-way cycle / jump / revise expectations. (False positive dismissed: the panel's "sep not imported in review.ts" blocker — it is imported at review.ts:5 and tsc is clean.) Full suite: 2000/2000. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/generated/blocks/plan-view.entry.tsx | 2 +- .../cli/src/generated/blocks/plan-view.tsx | 13 ++++--- .../cli/src/generated/signals/keyboard.ts | 16 +++++--- packages/cli/src/kern/blocks/plan-view.kern | 11 +++--- packages/cli/src/kern/signals/keyboard.kern | 16 +++++--- tests/unit/keyboard.test.ts | 37 +++++++++++-------- 6 files changed, 56 insertions(+), 39 deletions(-) diff --git a/packages/cli/src/generated/blocks/plan-view.entry.tsx b/packages/cli/src/generated/blocks/plan-view.entry.tsx index 2a9a5885..5831d50d 100644 --- a/packages/cli/src/generated/blocks/plan-view.entry.tsx +++ b/packages/cli/src/generated/blocks/plan-view.entry.tsx @@ -1,7 +1,7 @@ #!/usr/bin/env node // @generated by kern v3.5.8 — DO NOT EDIT. Source: src/kern/blocks/plan-view.kern -// @kern-source: plan-view:221 +// @kern-source: plan-view:222 import React from 'react'; import { render } from 'ink'; diff --git a/packages/cli/src/generated/blocks/plan-view.tsx b/packages/cli/src/generated/blocks/plan-view.tsx index adcf149a..9fe6c0bb 100644 --- a/packages/cli/src/generated/blocks/plan-view.tsx +++ b/packages/cli/src/generated/blocks/plan-view.tsx @@ -60,8 +60,9 @@ const PlanProposalView = React.memo(function PlanProposalView({ plan, markdown, {'Awaiting approval'} {(selectedIndex ?? 0) === 0 ? '❯ ' : ' '}{'1. Approve & run'} - {(selectedIndex ?? 0) === 1 ? '❯ ' : ' '}{'2. Reject'} - {' ↑↓ move · Enter select · ✎ type to change · /approve · /cancel'} + {(selectedIndex ?? 0) === 1 ? '❯ ' : ' '}{'2. Other — revise the plan'} + {(selectedIndex ?? 0) === 2 ? '❯ ' : ' '}{'3. Reject'} + {' ↑↓ move · Enter select · 1/2/3 jump · y/o/n · /approve · /cancel'} )} @@ -188,9 +189,9 @@ const PlanProposalView = React.memo(function PlanProposalView({ plan, markdown, {'Awaiting approval'} {(selectedIndex ?? 0) === 0 ? '❯ ' : ' '}{'1. Approve & run'} - {(selectedIndex ?? 0) === 2 ? '❯ ' : ' '}{'3. Other — revise the plan'} - {(selectedIndex ?? 0) === 1 ? '❯ ' : ' '}{'2. Reject'} - {' ↑↓ move · Enter select · 1/2/3 jump · y/n/o · /approve · /cancel'} + {(selectedIndex ?? 0) === 1 ? '❯ ' : ' '}{'2. Other — revise the plan'} + {(selectedIndex ?? 0) === 2 ? '❯ ' : ' '}{'3. Reject'} + {' ↑↓ move · Enter select · 1/2/3 jump · y/o/n · /approve · /cancel'} )} @@ -198,7 +199,7 @@ const PlanProposalView = React.memo(function PlanProposalView({ plan, markdown, }); export { PlanProposalView }; -// @kern-source: plan-view:221 +// @kern-source: plan-view:222 export function PlanExecutionView({ plan }: { plan:any }) { const steps: any[] = plan.steps ?? []; const doneSteps = steps.filter((s: any) => s.state === 'done'); diff --git a/packages/cli/src/generated/signals/keyboard.ts b/packages/cli/src/generated/signals/keyboard.ts index 9864d28f..6384041a 100644 --- a/packages/cli/src/generated/signals/keyboard.ts +++ b/packages/cli/src/generated/signals/keyboard.ts @@ -255,21 +255,25 @@ export function resolveKeyboardInput(ctx: KeyboardCtx): KeyboardAction { && !ctx.reviewEventOpen && !ctx.toolDetailOpen && !ctx.questionState ) { - // 3-option plan approval: 0=Approve, 1=Reject, 2=Other (revise / type feedback). + // 3-option plan approval indexed in VISUAL (top-to-bottom) order: + // 0=Approve, 1=Other (revise / type feedback), 2=Reject. Arrow nav follows + // that order so ↓ moves Approve→Other→Reject. (Cesar's original 0=Approve, + // 1=Reject, 2=Other made ↓ from Approve jump to Reject, skipping the middle + // Other slot — the bug the panel review flagged.) const pcur = Math.max(0, Math.min(2, ctx.planApprovalIndex ?? 0)); - const next = pcur === 0 ? 1 : pcur === 1 ? 2 : 0; - const prev = pcur === 0 ? 2 : pcur === 1 ? 0 : 1; + const next = pcur === 2 ? 0 : pcur + 1; // 0→1→2→0 + const prev = pcur === 0 ? 2 : pcur - 1; // 0→2→1→0 if (key.downArrow) return { type: 'movePlanApproval', index: next }; if (key.upArrow) return { type: 'movePlanApproval', index: prev }; if (key.return || input === '\r' || input === '\n') { - const action = pcur === 1 ? 'cancel' : pcur === 2 ? 'revise' : 'approve'; + const action = pcur === 1 ? 'revise' : pcur === 2 ? 'cancel' : 'approve'; return { type: 'planControl', action }; } if (typeof input === 'string' && input.length === 1) { const lowered = input.toLowerCase(); if (lowered === 'y' || lowered === '1') return { type: 'movePlanApproval', index: 0 }; - if (lowered === 'n' || lowered === '2') return { type: 'movePlanApproval', index: 1 }; - if (lowered === 'o' || lowered === '3') return { type: 'movePlanApproval', index: 2 }; + if (lowered === 'o' || lowered === '2') return { type: 'movePlanApproval', index: 1 }; + if (lowered === 'n' || lowered === '3') return { type: 'movePlanApproval', index: 2 }; } } diff --git a/packages/cli/src/kern/blocks/plan-view.kern b/packages/cli/src/kern/blocks/plan-view.kern index f54846d5..2e5b04ab 100644 --- a/packages/cli/src/kern/blocks/plan-view.kern +++ b/packages/cli/src/kern/blocks/plan-view.kern @@ -79,8 +79,9 @@ screen name=PlanProposalView target=ink memo=true {'Awaiting approval'} {(selectedIndex ?? 0) === 0 ? '❯ ' : ' '}{'1. Approve & run'} - {(selectedIndex ?? 0) === 1 ? '❯ ' : ' '}{'2. Reject'} - {' ↑↓ move · Enter select · ✎ type to change · /approve · /cancel'} + {(selectedIndex ?? 0) === 1 ? '❯ ' : ' '}{'2. Other — revise the plan'} + {(selectedIndex ?? 0) === 2 ? '❯ ' : ' '}{'3. Reject'} + {' ↑↓ move · Enter select · 1/2/3 jump · y/o/n · /approve · /cancel'} )} @@ -207,9 +208,9 @@ screen name=PlanProposalView target=ink memo=true {'Awaiting approval'} {(selectedIndex ?? 0) === 0 ? '❯ ' : ' '}{'1. Approve & run'} - {(selectedIndex ?? 0) === 2 ? '❯ ' : ' '}{'3. Other — revise the plan'} - {(selectedIndex ?? 0) === 1 ? '❯ ' : ' '}{'2. Reject'} - {' ↑↓ move · Enter select · 1/2/3 jump · y/n/o · /approve · /cancel'} + {(selectedIndex ?? 0) === 1 ? '❯ ' : ' '}{'2. Other — revise the plan'} + {(selectedIndex ?? 0) === 2 ? '❯ ' : ' '}{'3. Reject'} + {' ↑↓ move · Enter select · 1/2/3 jump · y/o/n · /approve · /cancel'} )} diff --git a/packages/cli/src/kern/signals/keyboard.kern b/packages/cli/src/kern/signals/keyboard.kern index caafc9d0..036d391d 100644 --- a/packages/cli/src/kern/signals/keyboard.kern +++ b/packages/cli/src/kern/signals/keyboard.kern @@ -262,21 +262,25 @@ fn name=resolveKeyboardInput params="ctx:KeyboardCtx" returns="KeyboardAction" && !ctx.reviewEventOpen && !ctx.toolDetailOpen && !ctx.questionState ) { - // 3-option plan approval: 0=Approve, 1=Reject, 2=Other (revise / type feedback). + // 3-option plan approval indexed in VISUAL (top-to-bottom) order: + // 0=Approve, 1=Other (revise / type feedback), 2=Reject. Arrow nav follows + // that order so ↓ moves Approve→Other→Reject. (Cesar's original 0=Approve, + // 1=Reject, 2=Other made ↓ from Approve jump to Reject, skipping the middle + // Other slot — the bug the panel review flagged.) const pcur = Math.max(0, Math.min(2, ctx.planApprovalIndex ?? 0)); - const next = pcur === 0 ? 1 : pcur === 1 ? 2 : 0; - const prev = pcur === 0 ? 2 : pcur === 1 ? 0 : 1; + const next = pcur === 2 ? 0 : pcur + 1; // 0→1→2→0 + const prev = pcur === 0 ? 2 : pcur - 1; // 0→2→1→0 if (key.downArrow) return { type: 'movePlanApproval', index: next }; if (key.upArrow) return { type: 'movePlanApproval', index: prev }; if (key.return || input === '\r' || input === '\n') { - const action = pcur === 1 ? 'cancel' : pcur === 2 ? 'revise' : 'approve'; + const action = pcur === 1 ? 'revise' : pcur === 2 ? 'cancel' : 'approve'; return { type: 'planControl', action }; } if (typeof input === 'string' && input.length === 1) { const lowered = input.toLowerCase(); if (lowered === 'y' || lowered === '1') return { type: 'movePlanApproval', index: 0 }; - if (lowered === 'n' || lowered === '2') return { type: 'movePlanApproval', index: 1 }; - if (lowered === 'o' || lowered === '3') return { type: 'movePlanApproval', index: 2 }; + if (lowered === 'o' || lowered === '2') return { type: 'movePlanApproval', index: 1 }; + if (lowered === 'n' || lowered === '3') return { type: 'movePlanApproval', index: 2 }; } } diff --git a/tests/unit/keyboard.test.ts b/tests/unit/keyboard.test.ts index 5424ca47..6371d73c 100644 --- a/tests/unit/keyboard.test.ts +++ b/tests/unit/keyboard.test.ts @@ -296,43 +296,50 @@ describe('resolveKeyboardInput', () => { .toEqual({ type: 'exitOther' }); }); - it('plan approval: arrows and y/n move the approve/reject cursor without applying', () => { + it('plan approval: arrows and y/n/o move the cursor without applying', () => { + // Indices are in VISUAL order: 0=Approve, 1=Other, 2=Reject. expect(resolveKeyboardInput(baseCtx({ key: { downArrow: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 0 }))) .toEqual({ type: 'movePlanApproval', index: 1 }); expect(resolveKeyboardInput(baseCtx({ input: 'y', key: {}, activePlanState: 'awaiting_approval' }))) .toEqual({ type: 'movePlanApproval', index: 0 }); - expect(resolveKeyboardInput(baseCtx({ input: 'n', key: {}, activePlanState: 'awaiting_approval' }))) + expect(resolveKeyboardInput(baseCtx({ input: 'o', key: {}, activePlanState: 'awaiting_approval' }))) .toEqual({ type: 'movePlanApproval', index: 1 }); + expect(resolveKeyboardInput(baseCtx({ input: 'n', key: {}, activePlanState: 'awaiting_approval' }))) + .toEqual({ type: 'movePlanApproval', index: 2 }); }); it('plan approval: Enter confirms the highlighted action', () => { expect(resolveKeyboardInput(baseCtx({ input: '\r', key: { return: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 0 }))) .toEqual({ type: 'planControl', action: 'approve' }); + // index 1 = Other → revise; index 2 = Reject → cancel. expect(resolveKeyboardInput(baseCtx({ input: '\r', key: { return: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 1 }))) + .toEqual({ type: 'planControl', action: 'revise' }); + expect(resolveKeyboardInput(baseCtx({ input: '\r', key: { return: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 2 }))) .toEqual({ type: 'planControl', action: 'cancel' }); }); - it('plan approval: third "Other" slot — arrows cycle 0→1→2→0, o/3 jump to it, Enter on it revises', () => { - // Down from Approve goes to Reject (1) — unchanged behavior. + it('plan approval: 3-way cursor cycles in visual order — Approve→Other→Reject; o/3 jump; Enter on Other revises', () => { + // Arrow cycle matches the on-screen top-to-bottom layout (0=Approve, + // 1=Other, 2=Reject). Down never skips the middle Other slot. expect(resolveKeyboardInput(baseCtx({ key: { downArrow: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 0 }))) - .toEqual({ type: 'movePlanApproval', index: 1 }); - // Down from Reject (1) wraps to Other (2) — new. + .toEqual({ type: 'movePlanApproval', index: 1 }); // Approve → Other expect(resolveKeyboardInput(baseCtx({ key: { downArrow: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 1 }))) - .toEqual({ type: 'movePlanApproval', index: 2 }); - // Down from Other (2) wraps back to Approve (0). + .toEqual({ type: 'movePlanApproval', index: 2 }); // Other → Reject expect(resolveKeyboardInput(baseCtx({ key: { downArrow: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 2 }))) - .toEqual({ type: 'movePlanApproval', index: 0 }); - // Up from Approve (0) wraps to Other (2). + .toEqual({ type: 'movePlanApproval', index: 0 }); // Reject → Approve (wrap) + // Up from Approve wraps to Reject (bottom). expect(resolveKeyboardInput(baseCtx({ key: { upArrow: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 0 }))) .toEqual({ type: 'movePlanApproval', index: 2 }); - // o/3 jump straight to Other. + // o/2 jump straight to Other (middle); n/3 to Reject (bottom). expect(resolveKeyboardInput(baseCtx({ input: 'o', key: {}, activePlanState: 'awaiting_approval' }))) - .toEqual({ type: 'movePlanApproval', index: 2 }); + .toEqual({ type: 'movePlanApproval', index: 1 }); + expect(resolveKeyboardInput(baseCtx({ input: '2', key: {}, activePlanState: 'awaiting_approval' }))) + .toEqual({ type: 'movePlanApproval', index: 1 }); expect(resolveKeyboardInput(baseCtx({ input: '3', key: {}, activePlanState: 'awaiting_approval' }))) .toEqual({ type: 'movePlanApproval', index: 2 }); - // Enter on Other emits the 'revise' action (new union member) — the app - // route clears the proposal and pre-fills the composer for free-form feedback. - expect(resolveKeyboardInput(baseCtx({ input: '\r', key: { return: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 2 }))) + // Enter on Other emits 'revise' — the app route clears the proposal and + // pre-fills the composer for free-form feedback. + expect(resolveKeyboardInput(baseCtx({ input: '\r', key: { return: true }, activePlanState: 'awaiting_approval', planApprovalIndex: 1 }))) .toEqual({ type: 'planControl', action: 'revise' }); }); From 53dfce9d633328e3fb3d37479da22f4ecd246d0a Mon Sep 17 00:00:00 2001 From: nicolascukas Date: Mon, 8 Jun 2026 12:52:37 +0200 Subject: [PATCH 6/6] fix(render): size CodeBlockView against the parent's available width MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code-block width fix sized rows against the raw terminal width (contentWidth(8)). When a block is nested inside padded/bordered containers (the streaming preview pane, plan view, etc.), the box could still be wider than the actual available Ink width and wrap again — the panel review flagged this as a residual edge case. Thread RenderedSegments' wrapWidth (the parent-computed content budget) into CodeBlockView and cap the body at wrapWidth-8 so the box (body+8 frame cols) always fits inside it. Falls back to the terminal budget when no wrapWidth is passed (no behavior change for top-level blocks). Verified: box width ≤ wrapWidth across nested/deeply-nested cases; 2000/2000 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/generated/blocks/rendering.entry.tsx | 2 +- .../cli/src/generated/blocks/rendering.tsx | 26 ++++++++++++------- packages/cli/src/kern/blocks/rendering.kern | 13 ++++++++-- 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/packages/cli/src/generated/blocks/rendering.entry.tsx b/packages/cli/src/generated/blocks/rendering.entry.tsx index 087f99e4..d499ae16 100644 --- a/packages/cli/src/generated/blocks/rendering.entry.tsx +++ b/packages/cli/src/generated/blocks/rendering.entry.tsx @@ -1,7 +1,7 @@ #!/usr/bin/env node // @generated by kern v3.5.8 — DO NOT EDIT. Source: src/kern/blocks/rendering.kern -// @kern-source: rendering:438 +// @kern-source: rendering:447 import React from 'react'; import { render } from 'ink'; diff --git a/packages/cli/src/generated/blocks/rendering.tsx b/packages/cli/src/generated/blocks/rendering.tsx index cf026dd9..3f2c08fb 100644 --- a/packages/cli/src/generated/blocks/rendering.tsx +++ b/packages/cli/src/generated/blocks/rendering.tsx @@ -61,8 +61,16 @@ export function SyntaxLine({ line, maxWidth }: { line:string; maxWidth:number }) } // @kern-source: rendering:199 -export function CodeBlockView({ segment, borderColor }: { segment:ContentSegment & { type: 'code' }; borderColor:string }) { - const codeWidth = contentWidth(8); +export function CodeBlockView({ segment, borderColor, wrapWidth }: { segment:ContentSegment & { type: 'code' }; borderColor:string; wrapWidth?:number }) { + // The box is `body + 8` cols wide (frame). When the parent passes the + // actual available content width (wrapWidth), the box must fit inside it — + // otherwise nesting/padding pushes the block past the available Ink width + // and every row wraps again. Cap the body budget at wrapWidth-8 in that + // case; fall back to the raw terminal budget when no width is provided. + const termBudget = contentWidth(8); + const codeWidth = (typeof wrapWidth === 'number' && wrapWidth > 0) + ? Math.max(8, Math.min(termBudget, wrapWidth - 8)) + : termBudget; const lines = (segment.code ?? '').split('\n'); const isDiff = segment.language === 'diff' || lines.some((l: string) => /^[+-@]/.test(l)); const capped = lines.slice(0, MAX_CODE_LINES); @@ -120,7 +128,7 @@ export function CodeBlockView({ segment, borderColor }: { segment:ContentSegment ); } -// @kern-source: rendering:264 +// @kern-source: rendering:273 export function RichSpanView({ span }: { span:InlineSpan }) { if (span.style.code) { return {span.text}; @@ -137,7 +145,7 @@ export function RichSpanView({ span }: { span:InlineSpan }) { return el; } -// @kern-source: rendering:285 +// @kern-source: rendering:294 export function RichLineView({ line, borderColor }: { line:RichLine; borderColor?:string }) { const border = borderColor ? {'\u2502 '} : null; const indent = line.indent > 0 ? ' '.repeat(line.indent) : ''; @@ -159,7 +167,7 @@ export function RichLineView({ line, borderColor }: { line:RichLine; borderColor return {border}{indent}{listIndent}{marker}{line.spans.map((s: InlineSpan, i: number) => )}; } -// @kern-source: rendering:312 +// @kern-source: rendering:321 export function MarkdownTableView({ headers, rows, alignments, borderColor }: { headers:string[]; rows:string[][]; alignments:('left' | 'center' | 'right')[]; borderColor:string }) { const colWidths = headers.map((h: string, i: number) => { let max = h.length; @@ -195,7 +203,7 @@ export function MarkdownTableView({ headers, rows, alignments, borderColor }: { ); } -// @kern-source: rendering:355 +// @kern-source: rendering:364 export function RenderedSegments({ segments, borderColor, wrapWidth }: { segments:ContentSegment[]; borderColor:string; wrapWidth:number }) { return ( <> @@ -248,7 +256,7 @@ export function RenderedSegments({ segments, borderColor, wrapWidth }: { segment return ( {spacer} - + ); })} @@ -256,7 +264,7 @@ export function RenderedSegments({ segments, borderColor, wrapWidth }: { segment ); } -// @kern-source: rendering:422 +// @kern-source: rendering:431 export function GradientLine({ text, colors }: { text:string; colors:readonly string[] }) { const step = Math.max(1, Math.ceil(text.length / colors.length)); return ( @@ -269,7 +277,7 @@ export function GradientLine({ text, colors }: { text:string; colors:readonly st ); } -// @kern-source: rendering:438 +// @kern-source: rendering:447 export function AnsiLine({ text, maxWidth, fallbackDim }: { text:string; maxWidth:number; fallbackDim?:boolean }) { if (!hasAnsiCodes(text)) { const display = text.length > maxWidth ? text.slice(0, maxWidth - 4) + '\u2026' : text; diff --git a/packages/cli/src/kern/blocks/rendering.kern b/packages/cli/src/kern/blocks/rendering.kern index fdc1d21d..5b15db91 100644 --- a/packages/cli/src/kern/blocks/rendering.kern +++ b/packages/cli/src/kern/blocks/rendering.kern @@ -199,9 +199,18 @@ screen name=SyntaxLine target=ink screen name=CodeBlockView target=ink prop name=segment type="ContentSegment & { type: 'code' }" prop name=borderColor type=string + prop name=wrapWidth type=number optional=true render handler <<< - const codeWidth = contentWidth(8); + // The box is `body + 8` cols wide (frame). When the parent passes the + // actual available content width (wrapWidth), the box must fit inside it — + // otherwise nesting/padding pushes the block past the available Ink width + // and every row wraps again. Cap the body budget at wrapWidth-8 in that + // case; fall back to the raw terminal budget when no width is provided. + const termBudget = contentWidth(8); + const codeWidth = (typeof wrapWidth === 'number' && wrapWidth > 0) + ? Math.max(8, Math.min(termBudget, wrapWidth - 8)) + : termBudget; const lines = (segment.code ?? '').split('\n'); const isDiff = segment.language === 'diff' || lines.some((l: string) => /^[+-@]/.test(l)); const capped = lines.slice(0, MAX_CODE_LINES); @@ -409,7 +418,7 @@ screen name=RenderedSegments target=ink return ( {spacer} - + ); })}