From 14ebf61eadba15774269c85fa4c8da1e562df0e8 Mon Sep 17 00:00:00 2001 From: luckyzd Date: Sun, 18 Oct 2026 16:38:33 +0800 Subject: [PATCH 1/3] fix(plan): wait for active generation after a no-op response --- DOCUMENTATION.md | 2 +- src/commands/test.plan.spec.ts | 46 +++++++++++++++ src/lib/plan-poll.spec.ts | 100 +++++++++++++++++++++++++++++++++ src/lib/plan-poll.ts | 56 +++++++----------- 4 files changed, 167 insertions(+), 37 deletions(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index aac4beeb..1e1684af 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -643,7 +643,7 @@ Behavior worth knowing: - **Progress is a single stderr line**, updated in place, and only in an interactive terminal. `--output json` and CI runs get no ticker — just the final result object on stdout. - **Re-running joins a stage already in progress.** If a stage is already running — including one you started in the Portal — the command waits for it instead of failing or starting a second one. -- **If proposals are already staged, nothing is started.** The command prints the existing batch and says so. Regenerating a batch you don't like is a Portal action for now: accept or discard the staged batch there first. +- **If proposals are already staged, nothing is started.** The command prints the existing batch and says so. If another client accepts that batch and starts generation before the next read, the command follows the new active state until it settles or the wait budget expires. Regenerating a batch you don't like is a Portal action for now: accept or discard the staged batch there first. - **Ctrl-C pauses, it does not cancel.** The stage in progress finishes on its own; the stages after it wait until you run the command again. Exit 130/143/129; stdout still gets a partial `{ projectId, status: "running", stagesRemaining, … }` object so a redirected file is never empty, and stderr says which stage you paused in and how many are left. - **On `--timeout` (exit 7)** the same partial is printed with a `Continue:` line; running the identical command again picks up where it left off. Raise `--timeout` if you want one run to cover every stage. - **Stage progress lines.** In text mode one `stage i/N done ` line is printed as each stage finishes and stays in scrollback; the live progress line (`exploring app… (resources 3/8, 4m10s)`) redraws in place beneath it on a terminal and is cleared before each stage line is written, so the two never share a row. diff --git a/src/commands/test.plan.spec.ts b/src/commands/test.plan.spec.ts index e3f9d89a..49c20c6d 100644 --- a/src/commands/test.plan.spec.ts +++ b/src/commands/test.plan.spec.ts @@ -697,6 +697,52 @@ describe('runPlanGenerate — failed stage', () => { }); }); +describe.each(['initial', 'later'] as const)( + 'runPlanGenerate — %s no-op followed by active generation', + position => { + it.each(['json', 'text'] as const)( + 'waits for the final proposals in %s output', + async output => { + const active = (status: 'strategizing' | 'proposing'): CliGetPlansResponse => ({ + generation: { status, errorCode: null, errorMessage: null }, + proposals: [], + }); + const noop = { body: { ...accepted(null, []), status: 'nothing_to_start' } }; + const backend = makePlanBackend({ + triggers: + position === 'initial' ? [noop] : [{ body: accepted('strategy', ['proposals']) }, noop], + reads: [ + { body: stagedPlans([]) }, // best-effort pre-trigger baseline + ...(position === 'later' + ? [{ body: active('strategizing') }, { body: stagedPlans([]) }] + : []), + { body: active('proposing') }, + { body: stagedPlans() }, + ], + }); + const capture: Capture = { stdout: [], stderr: [] }; + await runPlanGenerate(baseOpts({ output }), makeDeps(backend.fetchImpl, capture) as never); + const stdout = capture.stdout.join('\n'); + if (output === 'json') { + expect(JSON.parse(stdout)).toMatchObject({ + generation: { status: 'idle' }, + proposals: STAGED, + }); + } else { + expect(stdout).toContain('3 test-case proposals staged'); + expect(stdout).not.toContain('0 test-case proposals staged'); + } + expect(backend.calls.filter(call => call.method === 'POST')).toHaveLength( + position === 'initial' ? 1 : 2, + ); + expect(backend.calls.filter(call => call.method === 'GET')).toHaveLength( + position === 'initial' ? 3 : 5, + ); + }, + ); + }, +); + describe('runPlanGenerate — typed exit-7 timeout conversion', () => { it('converts the raw ladder timeout into the UNSUPPORTED envelope (exit 7, NOT 1) with a partial on stdout', async () => { const backend = makePlanBackend({ diff --git a/src/lib/plan-poll.spec.ts b/src/lib/plan-poll.spec.ts index dc0c59ce..d745c795 100644 --- a/src/lib/plan-poll.spec.ts +++ b/src/lib/plan-poll.spec.ts @@ -336,6 +336,106 @@ describe('runGenerationLadder — stage progression', () => { // Billing guards // --------------------------------------------------------------------------- +describe.each(['initial', 'later'] as const)( + 'runGenerationLadder — %s nothing_to_start observation', + position => { + /** Prefix a script with a completed strategy when testing the later POST. */ + function clientAfterNoop(reads: Array) { + return makeClient( + position === 'initial' + ? [nothingToStart()] + : [accepted('strategy', ['proposals']), nothingToStart()], + position === 'initial' ? reads : [plansRead('strategizing'), plansRead('idle'), ...reads], + ); + } + + const expectedPosts = position === 'initial' ? 1 : 2; + const prefixReads = position === 'initial' ? 0 : 2; + + it.each(['exploring', 'strategizing', 'proposing'] as const)( + 'waits for staged plans after the next read reports %s', + async status => { + const client = clientAfterNoop([plansRead(status), plansRead('idle', STAGED)]); + const result = await ladder(client); + expect(result.plans.generation.status).toBe('idle'); + expect(result.plans.proposals).toEqual(STAGED); + expect(client.readCount).toBe(prefixReads + 2); + expect(client.triggerCount).toBe(expectedPosts); + }, + ); + + it('keeps polling an idle visibility gap after observing proposals run', async () => { + const client = clientAfterNoop([ + plansRead('proposing'), + plansRead('idle'), + plansRead('idle'), + plansRead('idle', STAGED), + ]); + const result = await ladder(client); + expect(result.plans.proposals).toEqual(STAGED); + expect(client.readCount).toBe(prefixReads + 4); + expect(client.triggerCount).toBe(expectedPosts); + }); + + it('preserves an immediate empty-idle read when another client accepted the batch', async () => { + const client = clientAfterNoop([plansRead('idle')]); + const result = await ladder(client); + expect(result.plans.proposals).toEqual([]); + expect(client.readCount).toBe(prefixReads + 1); + expect(client.triggerCount).toBe(expectedPosts); + }); + + it('returns a later failed state instead of the earlier active observation', async () => { + const client = clientAfterNoop([ + plansRead('proposing'), + plansRead('failed', [], { errorCode: 'proposal_generation_aborted' }), + ]); + const result = await ladder(client); + expect(result.plans.generation.status).toBe('failed'); + expect(client.triggerCount).toBe(expectedPosts); + }); + + it('retains the active observation when the shared deadline expires', async () => { + const client = clientAfterNoop([plansRead('proposing')]); + const realDateNow = Date.now; + let now = realDateNow(); + Date.now = () => now; + try { + await expect( + ladder(client, { + timeoutSeconds: 1, + onTick: () => { + if (client.triggerCount === expectedPosts) now += 1001; + }, + }), + ).rejects.toMatchObject({ + name: 'PlanGenerationTimeoutError', + lastPlans: { generation: { status: 'proposing' } }, + }); + expect(client.triggerCount).toBe(expectedPosts); + } finally { + Date.now = realDateNow; + } + }); + }, +); + +it('starts the remaining stage after a no-op read observes a new strategy run finish', async () => { + const client = makeClient( + [nothingToStart(), accepted('proposals', [])], + [ + plansRead('strategizing'), + plansRead('idle'), + plansRead('proposing'), + plansRead('idle', STAGED), + ], + ); + const result = await ladder(client); + expect(result.plans.proposals).toEqual(STAGED); + expect(client.triggerCount).toBe(2); + expect(client.triggerKeys).toEqual([IDEMPOTENCY_KEY, IDEMPOTENCY_KEY + ':post2']); +}); + describe('runGenerationLadder — billing guards', () => { it('NEVER re-POSTs after the proposals rung (stagesRemaining: [])', async () => { // The append listener sets its in-flight flags asynchronously: reads can diff --git a/src/lib/plan-poll.ts b/src/lib/plan-poll.ts index df9ac119..d520a26e 100644 --- a/src/lib/plan-poll.ts +++ b/src/lib/plan-poll.ts @@ -183,8 +183,6 @@ export const MAX_ACCEPTED_TRIGGER_POSTS = PLAN_GENERATION_STAGES.length; */ export const MAX_TRIGGER_POSTS_TOTAL = PLAN_GENERATION_STAGES.length * 2; -type TriggerOutcome = 'accepted' | 'nothing_to_start' | 'attached'; - /** * Drive the generate ladder to a settled state: trigger the next missing * stage, poll, re-trigger between stages, and return the settled plans read. @@ -273,6 +271,10 @@ async function ladderLoop( skippedCategories: null as number | null, expectedRemaining: null as CliGenerationStage[] | null, proposalsObserved: false, + // A no-op can settle on its next idle read (even if another client has + // accepted the batch). Observing active generation revokes this shortcut: + // the new generation must follow the normal wait and billing guards. + noopSettlesIdle: false, observedActiveSinceTrigger: false, consecutiveIdleGaps: 0, }; @@ -302,7 +304,7 @@ async function ladderLoop( } }; - const doTrigger = async (): Promise => { + const doTrigger = async (): Promise => { // Fresh idempotency namespace per POST attempt — ATTACHES INCLUDED: a // 409's key is never committed server-side, but reusing it would // silently depend on that; a fresh key never can. Transport-level @@ -317,8 +319,12 @@ async function ladderLoop( ); state.triggerPosts += 1; if (response.status === 'nothing_to_start') { + state.noopSettlesIdle = true; + state.expectedRemaining = null; + state.observedActiveSinceTrigger = false; + state.consecutiveIdleGaps = 0; options.onTrigger?.(response, state.acceptedPosts); - return 'nothing_to_start'; + return; } state.acceptedPosts += 1; if (state.firstTrigger === null) state.firstTrigger = response; @@ -329,7 +335,7 @@ async function ladderLoop( state.observedActiveSinceTrigger = false; state.consecutiveIdleGaps = 0; options.onTrigger?.(response, state.acceptedPosts); - return 'accepted'; + return; } catch (err) { if ( err instanceof ApiError && @@ -345,7 +351,7 @@ async function ladderLoop( state.expectedRemaining = null; state.observedActiveSinceTrigger = false; state.consecutiveIdleGaps = 0; - return 'attached'; + return; } throw err; } @@ -384,19 +390,7 @@ async function ladderLoop( try { // ── initial trigger ── - const initial = await doTrigger(); - if (initial === 'nothing_to_start') { - // Proposals already staged — one bounded read, render, done. - const plans = await readPlans(); - lastPlans = plans; - options.onTick?.(plans, Date.now() - startMs); - return { - plans, - firstTrigger: state.firstTrigger, - acceptedPosts: state.acceptedPosts, - skippedCategories: state.skippedCategories, - }; - } + await doTrigger(); // ── poll / re-trigger loop ── while (true) { @@ -416,6 +410,7 @@ async function ladderLoop( }; } if (isActiveGenerationStatus(status)) { + state.noopSettlesIdle = false; state.observedActiveSinceTrigger = true; // Seeing the proposals stage RUN arms billing guard #1 regardless of // who started it — the 409-attach path has no trigger response to @@ -429,8 +424,9 @@ async function ladderLoop( } // status === 'idle' - if (plans.proposals.length > 0) { - // Staged batch present — success. + if (plans.proposals.length > 0 || state.noopSettlesIdle) { + // Staged batch present, or an idle read confirms the no-op. A batch + // accepted by another client may be empty; do not regenerate it. return { plans, firstTrigger: state.firstTrigger, @@ -499,21 +495,9 @@ async function ladderLoop( }); } - const outcome = await doTrigger(); - if (outcome === 'nothing_to_start') { - // Proposals got staged between our read and the POST (or another - // client staged them) — settle on a final read. - const finalPlans = await readPlans(); - lastPlans = finalPlans; - options.onTick?.(finalPlans, Date.now() - startMs); - return { - plans: finalPlans, - firstTrigger: state.firstTrigger, - acceptedPosts: state.acceptedPosts, - skippedCategories: state.skippedCategories, - }; - } - // 'accepted' or 'attached' → back to polling. + await doTrigger(); + // Every trigger outcome is checked against the next observed state: + // another client may have accepted and regenerated a staged batch. } } finally { clearTimeout(deadlineTimer); From 4441eaa43773c8a38f01e40139622c0206b61f7d Mon Sep 17 00:00:00 2001 From: luckyzd Date: Sun, 18 Oct 2026 19:38:52 +0800 Subject: [PATCH 2/3] docs(plan): document generation polling helpers --- src/lib/plan-poll.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/lib/plan-poll.ts b/src/lib/plan-poll.ts index d520a26e..643b798b 100644 --- a/src/lib/plan-poll.ts +++ b/src/lib/plan-poll.ts @@ -212,6 +212,16 @@ export async function runGenerationLadder( } } +/** + * Run the generation state machine within one deadline and request-signal + * scope. Every trigger outcome is checked against subsequent reads: active + * generation keeps polling, while an idle read may settle a no-op response. + * Re-trigger only between stages, preserving the proposals billing guards. + * + * @returns The settled plans read and metadata from accepted triggers. + * @throws `PlanGenerationTimeoutError` with the last read when time expires. + * @throws Interrupt and API errors after releasing the session's resources. + */ async function ladderLoop( client: PlanLadderClient, projectId: string, @@ -304,6 +314,11 @@ async function ladderLoop( } }; + /** + * Attempt the next stage with a fresh POST key, updating counters and + * polling guards for accepted, no-op, or in-flight outcomes. The caller + * must read the current state afterward; no trigger outcome settles it. + */ const doTrigger = async (): Promise => { // Fresh idempotency namespace per POST attempt — ATTACHES INCLUDED: a // 409's key is never committed server-side, but reusing it would From a9b8aa4226568bd6e625ed1623f6ec1c14bdc32b Mon Sep 17 00:00:00 2001 From: luckyzd Date: Sun, 18 Oct 2026 19:45:15 +0800 Subject: [PATCH 3/3] docs(plan): clarify the existing post-accept no-op outcome --- DOCUMENTATION.md | 1 + src/commands/test.plan.spec.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/DOCUMENTATION.md b/DOCUMENTATION.md index 1e1684af..a976132f 100644 --- a/DOCUMENTATION.md +++ b/DOCUMENTATION.md @@ -644,6 +644,7 @@ Behavior worth knowing: - **Progress is a single stderr line**, updated in place, and only in an interactive terminal. `--output json` and CI runs get no ticker — just the final result object on stdout. - **Re-running joins a stage already in progress.** If a stage is already running — including one you started in the Portal — the command waits for it instead of failing or starting a second one. - **If proposals are already staged, nothing is started.** The command prints the existing batch and says so. If another client accepts that batch and starts generation before the next read, the command follows the new active state until it settles or the wait budget expires. Regenerating a batch you don't like is a Portal action for now: accept or discard the staged batch there first. +- **A no-op can return an empty batch after another client accepts it.** If the first read after `nothing_to_start` is idle with no proposals, the command exits 0 with that current empty batch; it does not start a replacement or claim that a new generation completed. Once active generation is observed, empty idle reads no longer settle the command: it continues waiting for staged proposals, failure, or timeout. - **Ctrl-C pauses, it does not cancel.** The stage in progress finishes on its own; the stages after it wait until you run the command again. Exit 130/143/129; stdout still gets a partial `{ projectId, status: "running", stagesRemaining, … }` object so a redirected file is never empty, and stderr says which stage you paused in and how many are left. - **On `--timeout` (exit 7)** the same partial is printed with a `Continue:` line; running the identical command again picks up where it left off. Raise `--timeout` if you want one run to cover every stage. - **Stage progress lines.** In text mode one `stage i/N done ` line is printed as each stage finishes and stays in scrollback; the live progress line (`exploring app… (resources 3/8, 4m10s)`) redraws in place beneath it on a terminal and is cleared before each stage line is written, so the two never share a row. diff --git a/src/commands/test.plan.spec.ts b/src/commands/test.plan.spec.ts index 49c20c6d..e4dcb2ca 100644 --- a/src/commands/test.plan.spec.ts +++ b/src/commands/test.plan.spec.ts @@ -366,7 +366,8 @@ describe('runPlanGenerate — happy path', () => { it('the note still prints when zero proposals were staged, worded for nothing staged', () => { // Renderer-level: the ladder never settles on idle+empty after the - // proposals rung (it polls to staged/failed/timeout), so this branch is + // proposals rung (it polls to staged/failed/timeout). The immediate no-op + // post-accept exception is covered separately below, so this branch is // pinned directly. The skip count matters most here — it points at the // strategy rather than inviting a re-run that bills again. const text = renderPlanGenerateResultText(stagedPlans([]), PROJECT_ID, 2, 2);