Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion DOCUMENTATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,8 @@ 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.
- **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 <name> done <elapsed>` 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.
Expand Down
49 changes: 48 additions & 1 deletion src/commands/test.plan.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -697,6 +698,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({
Expand Down
100 changes: 100 additions & 0 deletions src/lib/plan-poll.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<CliGetPlansResponse | ApiError>) {
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
Expand Down
71 changes: 35 additions & 36 deletions src/lib/plan-poll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -214,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,
Expand Down Expand Up @@ -273,6 +281,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,
};
Expand Down Expand Up @@ -302,7 +314,12 @@ async function ladderLoop(
}
};

const doTrigger = async (): Promise<TriggerOutcome> => {
/**
* 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<void> => {
// 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
Expand All @@ -317,8 +334,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;
Expand All @@ -329,7 +350,7 @@ async function ladderLoop(
state.observedActiveSinceTrigger = false;
state.consecutiveIdleGaps = 0;
options.onTrigger?.(response, state.acceptedPosts);
return 'accepted';
return;
} catch (err) {
if (
err instanceof ApiError &&
Expand All @@ -345,7 +366,7 @@ async function ladderLoop(
state.expectedRemaining = null;
state.observedActiveSinceTrigger = false;
state.consecutiveIdleGaps = 0;
return 'attached';
return;
}
throw err;
}
Expand Down Expand Up @@ -384,19 +405,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) {
Expand All @@ -416,6 +425,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
Expand All @@ -429,8 +439,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,
Expand Down Expand Up @@ -499,21 +510,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);
Expand Down
Loading