diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index b7db9d9f27..4805122cf7 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -2,6 +2,11 @@ ## [Unreleased] +### Fixed + +- Anthropic `ping` keepalives no longer reset stream progress, so responses that stop producing content now reach the idle timeout instead of hanging indefinitely. +- The documented `GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS` environment variable now takes effect: the stream-watchdog idle-timeout helpers resolve it GJC-first before the legacy `PI_OPENAI_STREAM_IDLE_TIMEOUT_MS` / `PI_STREAM_IDLE_TIMEOUT_MS` aliases (previously only the `PI_`-prefixed names were read, so setting the documented GJC name was a silent no-op). + ## [0.11.10] - 2026-07-25 ## [0.11.9] - 2026-07-24 diff --git a/packages/ai/src/providers/anthropic.ts b/packages/ai/src/providers/anthropic.ts index 67cd55244f..cd917d79da 100644 --- a/packages/ai/src/providers/anthropic.ts +++ b/packages/ai/src/providers/anthropic.ts @@ -1182,6 +1182,40 @@ function shouldIgnoreAnthropicPreambleEvent(eventType: unknown): boolean { return !ANTHROPIC_PRE_MESSAGE_START_EVENT_TYPES.has(eventType); } +function createAnthropicStreamProgressPredicate(): (event: unknown) => boolean { + let outputTokens = -1; + + return event => { + if (!isRecord(event) || typeof event.type !== "string") return false; + if ( + event.type === "message_start" || + event.type === "content_block_start" || + event.type === "content_block_stop" || + event.type === "message_stop" + ) { + return true; + } + if (event.type === "content_block_delta") { + if (!isRecord(event.delta)) return false; + const delta = event.delta; + return ( + (typeof delta.text === "string" && delta.text.length > 0) || + (typeof delta.thinking === "string" && delta.thinking.length > 0) || + (typeof delta.partial_json === "string" && delta.partial_json.length > 0) || + (typeof delta.signature === "string" && delta.signature.length > 0) + ); + } + if (event.type === "message_delta") { + if (isRecord(event.delta) && event.delta.stop_reason != null) return true; + if (!isRecord(event.usage) || typeof event.usage.output_tokens !== "number") return false; + if (event.usage.output_tokens <= outputTokens) return false; + outputTokens = event.usage.output_tokens; + return true; + } + return false; + }; +} + function isTransientStreamEnvelopeError(error: unknown): boolean { if (!(error instanceof Error)) return false; return ( @@ -1457,6 +1491,7 @@ export const streamAnthropic: StreamFunction<"anthropic-messages"> = ( let sawEvent = false; let sawMessageStart = false; let sawTerminalEnvelope = false; + const isProgressEvent = createAnthropicStreamProgressPredicate(); for await (const event of iterateWithIdleTimeout(anthropicStream, { idleTimeoutMs, @@ -1466,6 +1501,7 @@ export const streamAnthropic: StreamFunction<"anthropic-messages"> = ( onIdle: () => activeAbortTracker.abortLocally(idleTimeoutAbortError), onFirstItemTimeout: () => activeAbortTracker.abortLocally(firstEventTimeoutAbortError), abortSignal: options?.signal, + isProgressItem: isProgressEvent, })) { sawEvent = true; if (sawProviderSafetyStop) { diff --git a/packages/ai/src/utils/idle-iterator.ts b/packages/ai/src/utils/idle-iterator.ts index 8f99b60ebc..ae6d9e4e14 100644 --- a/packages/ai/src/utils/idle-iterator.ts +++ b/packages/ai/src/utils/idle-iterator.ts @@ -19,7 +19,7 @@ function normalizeIdleTimeoutMs(value: string | undefined, fallback: number): nu /** * Returns the idle timeout used for provider streaming transports. * - * `PI_OPENAI_STREAM_IDLE_TIMEOUT_MS` is accepted as a backward-compatible alias. + * `GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS` is honored first; `PI_OPENAI_STREAM_IDLE_TIMEOUT_MS` is a backward-compatible alias. * Set `PI_STREAM_IDLE_TIMEOUT_MS=0` to disable the watchdog. * * Providers that legitimately stream much slower than the global default can pass @@ -27,17 +27,20 @@ function normalizeIdleTimeoutMs(value: string | undefined, fallback: number): nu * Caller options still take precedence; env overrides still trump the fallback. */ export function getStreamIdleTimeoutMs(fallbackMs: number = DEFAULT_STREAM_IDLE_TIMEOUT_MS): number | undefined { - return normalizeIdleTimeoutMs($env.PI_STREAM_IDLE_TIMEOUT_MS ?? $env.PI_OPENAI_STREAM_IDLE_TIMEOUT_MS, fallbackMs); + return normalizeIdleTimeoutMs( + $env.GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS ?? $env.PI_STREAM_IDLE_TIMEOUT_MS ?? $env.PI_OPENAI_STREAM_IDLE_TIMEOUT_MS, + fallbackMs, + ); } /** * Returns the idle timeout used for OpenAI-family streaming transports. * - * Set `PI_OPENAI_STREAM_IDLE_TIMEOUT_MS=0` to disable the watchdog. + * Honors `GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS` first (`PI_OPENAI_STREAM_IDLE_TIMEOUT_MS` is the legacy alias). Set `=0` to disable. */ export function getOpenAIStreamIdleTimeoutMs(): number | undefined { return normalizeIdleTimeoutMs( - $env.PI_OPENAI_STREAM_IDLE_TIMEOUT_MS ?? $env.PI_STREAM_IDLE_TIMEOUT_MS, + $env.GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS ?? $env.PI_OPENAI_STREAM_IDLE_TIMEOUT_MS ?? $env.PI_STREAM_IDLE_TIMEOUT_MS, DEFAULT_STREAM_IDLE_TIMEOUT_MS, ); } @@ -173,8 +176,6 @@ export async function* iterateWithIdleTimeout( } } - const nextResultPromise = withRacy(iterator.next()); - const racers: Array< Promise< | { kind: "next"; result: IteratorResult } @@ -182,7 +183,7 @@ export async function* iterateWithIdleTimeout( | { kind: "timeout" } | { kind: "abort" } > - > = [nextResultPromise]; + > = []; let timer: NodeJS.Timeout | undefined; let resolveTimeout: ((value: { kind: "timeout" }) => void) | undefined; @@ -207,6 +208,13 @@ export async function* iterateWithIdleTimeout( racers.push(promise); } + // Arm timeout/abort races before asking the source for its next item. A + // periodic keepalive iterator commonly registers its own timer inside + // `next()`; registering that first lets equal-deadline keepalives win every + // race and extend the idle window forever. Already-buffered items still + // settle as microtasks before a 0ms watchdog. + racers.unshift(withRacy(iterator.next())); + try { const outcome = await Promise.race(racers); if (outcome.kind === "abort") { diff --git a/packages/ai/test/anthropic-stream-timeout.test.ts b/packages/ai/test/anthropic-stream-timeout.test.ts index 4ecd3b5db5..38f3ccbb54 100644 --- a/packages/ai/test/anthropic-stream-timeout.test.ts +++ b/packages/ai/test/anthropic-stream-timeout.test.ts @@ -300,4 +300,55 @@ describe("anthropic first-event timeout retries", () => { }, ]); }); + + it("does not let Anthropic ping events keep a stalled response alive", async () => { + const create = ((_body: unknown, requestOptions?: { signal?: AbortSignal }) => { + const response = new Response(null, { status: 200, headers: { "request-id": "req_ping_stall" } }); + const data: MockAnthropicStream = { + async *[Symbol.asyncIterator]() { + yield { + type: "message_start", + message: { + id: "msg_ping_stall", + usage: { + input_tokens: 12, + output_tokens: 0, + cache_read_input_tokens: 0, + cache_creation_input_tokens: 0, + }, + }, + }; + yield { + type: "content_block_start", + index: 0, + content_block: { type: "text", text: "" }, + }; + yield { + type: "content_block_delta", + index: 0, + delta: { type: "text_delta", text: "checking" }, + }; + while (!requestOptions?.signal?.aborted) { + await Bun.sleep(1); + yield { type: "ping" }; + } + }, + }; + return { + async withResponse() { + return { data, response, request_id: "req_ping_stall" }; + }, + } as never; + }) as unknown as Anthropic["messages"]["create"]; + const client = { messages: { create } } as Anthropic; + + const result = await streamAnthropic(model, context, { + client, + streamFirstEventTimeoutMs: 5000, + streamIdleTimeoutMs: 5, + }).result(); + + expect(result.stopReason).toBe("error"); + expect(result.errorMessage).toBe("Anthropic stream stalled while waiting for the next event"); + }); }); diff --git a/packages/ai/test/stream-timeout-defaults.test.ts b/packages/ai/test/stream-timeout-defaults.test.ts index 07ad7ca1a7..63c2bb1863 100644 --- a/packages/ai/test/stream-timeout-defaults.test.ts +++ b/packages/ai/test/stream-timeout-defaults.test.ts @@ -1,5 +1,6 @@ import { afterEach, beforeEach, describe, expect, it } from "bun:test"; import { + getOpenAIStreamIdleTimeoutMs, getProviderFirstEventTimeoutFallbackMs, getStreamFirstEventTimeoutMs, getStreamIdleTimeoutMs, @@ -17,6 +18,7 @@ import { const ENV_KEYS = [ "PI_STREAM_IDLE_TIMEOUT_MS", "PI_OPENAI_STREAM_IDLE_TIMEOUT_MS", + "GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS", "PI_STREAM_FIRST_EVENT_TIMEOUT_MS", ] as const; @@ -63,6 +65,35 @@ describe("getStreamIdleTimeoutMs(fallbackMs)", () => { Bun.env.PI_STREAM_IDLE_TIMEOUT_MS = "0"; expect(getStreamIdleTimeoutMs(300_000)).toBeUndefined(); }); + + it("honors the documented GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS override", () => { + Bun.env.GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS = "77"; + expect(getStreamIdleTimeoutMs(300_000)).toBe(77); + }); + + it("resolves GJC-first: GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS wins over legacy PI_STREAM_IDLE_TIMEOUT_MS", () => { + Bun.env.GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS = "77"; + Bun.env.PI_STREAM_IDLE_TIMEOUT_MS = "42"; + expect(getStreamIdleTimeoutMs(300_000)).toBe(77); + }); + + it("treats GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS=0 as a watchdog disable", () => { + Bun.env.GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS = "0"; + expect(getStreamIdleTimeoutMs(300_000)).toBeUndefined(); + }); +}); + +describe("getOpenAIStreamIdleTimeoutMs()", () => { + it("honors the documented GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS first", () => { + Bun.env.GJC_OPENAI_STREAM_IDLE_TIMEOUT_MS = "88"; + Bun.env.PI_OPENAI_STREAM_IDLE_TIMEOUT_MS = "42"; + expect(getOpenAIStreamIdleTimeoutMs()).toBe(88); + }); + + it("falls back to the legacy PI_OPENAI_STREAM_IDLE_TIMEOUT_MS alias", () => { + Bun.env.PI_OPENAI_STREAM_IDLE_TIMEOUT_MS = "42"; + expect(getOpenAIStreamIdleTimeoutMs()).toBe(42); + }); }); describe("getStreamFirstEventTimeoutMs(idleTimeoutMs, fallbackMs)", () => { diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 56ed32737c..68ac449917 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,9 +2,17 @@ ## [Unreleased] +### Added + +- Ralplan consensus planning now enforces a finite planner/revision iteration budget at the native write path (default 5, configurable via `gjc.ralplan.maxIterations`). Opening another planner/revision pass past the cap fails closed with exit code 3 and an operator-visible `PLANNING-STUCK` marker instead of silent unbounded re-review; `final`/post-interview escalation remains allowed without auto-implementation. The cap also floors against on-disk `stage-*-{planner,revision}.md` artifacts so a wiped, truncated, or malformed `index.jsonl` cannot fail open after prior openers (#3165). + ### Fixed +- Questions about `ultragoal` behavior now stay on the direct-answer path instead of being misclassified as requests to start the durable workflow. +- Aligned the startup GJC Forge splash border with the composer trailing gutter, including the one-row constrained fallback. - `gjc resume` and delete no longer pay a durable (fsync-backed) lock acquisition for managed session tombstones that have nothing left to reconcile; a scope with many accumulated already-completed tombstones opens noticeably faster (#3067). +- `gjc deep-interview apply-round-result` no longer fails with `DI_INTERNAL_ERROR` on every call, which made the deep-interview workflow unable to score a single round. Three defects stacked: the Round-0 topology gate is recorded as a permanently unscorable `answered` shell (`--round` must be >= 1) yet counted toward the "earlier rounds must be scored" precondition, deadlocking every later round; the round-result decoder materializes omitted optional keys as `undefined`, which canonical JSON rejected outright, so any request omitting `targeting`/`ontology`/`bookkeeping` could not be digested; and `scoreToUnits` tested the raw float product, so ordinary scores whose scaling misses the integer grid (`0.69 * 10_000` is `6900.000000000001`) were rejected as non-integral 1e-4 units. Round-0 gate shells are now excluded from the ordering precondition, canonical JSON drops `undefined` object properties like `JSON.stringify` (array elements and the top-level value stay strict), and 1e-4 unit conversion is decided from the shortest round-trip decimal so genuinely off-grid precision such as `0.00005` and `0.05000000000000001` is still rejected. +- Task output-limit environment overrides now accept only complete positive decimal safe integers; malformed, fractional, exponent-form, whitespace-padded, and precision-losing values fall back to the documented defaults instead of being partially parsed (#3175). ## [0.11.10] - 2026-07-25 ### Changed @@ -28,6 +36,8 @@ - Delegated-task and subagent status surfaces now distinguish provider recovery from normal running, identify first-event versus idle-stream stalls, show retry budget and provider-progress age, and aggregate concurrent degradation by provider (#3071). - Telegram notification daemon ownership hardening (#3048): Bot API outcomes now share one honest classifier so both the initiating `429` response and cooldown-suppressed calls settle retryably instead of being lost or falsely rejected, including selected acknowledgements; exclusive operator work is registered before its callback can throw; notification health degrades corrupt daemon-state JSON to a warning; root-registration ownership tokens propagate through injected and built-in ensure, rollback, reconciliation, teardown, and abandoned-startup cleanup seams, with token-bearing rows refusing tokenless cleanup while genuinely legacy rows retain root-match behavior; and initial daemon readiness is published only after the matching heartbeat sidecar rename is durable, so no waiter can attach during the proof window. - `/new`, `fork()`, handoff, `/resume`, and branch/tree-jump transitions now complete verified managed `local://` legacy-root migration for the successor session identity *before* that identity is published to the agent, the workflow-gate emitter, or extension hooks, so those consumers cannot resolve `local://` against an ungated root. Matches cold-start `createAgentSession()` (#2797) and extends `/resume` (#2925). Sending a prompt right after `/new` no longer fails with "local:// legacy migration must complete before path resolution". The `SessionManager` rotates its own session id before this gate runs, so a residual window remains between that rotation and gate completion; it has no reachable in-process synchronous `local://` consumer under the session-transition admission lease. Closing it atomically is tracked in #3138. +- Telegram notification topics now fence malformed successful `createForumTopic` responses per session endpoint, preventing repeated ambiguous topic creation while keeping explicit Bot API failures retryable. +- Workflow-state readers and handoff paths no longer write corrupt-state warnings straight to `process.stderr`, which painted raw bytes over the live TUI composer during interactive sessions. Warnings now route through the TUI-safe file logger while `gjc state read`/`status`/`handoff` still surface them on the structured command-result `stderr`, so corrupt state stays distinguishable from absent state for CLI/automation (#3002). - Managed model fallback now gives each exhausted entry at most one retry with a rotated credential before advancing, so repeated quota failures cannot consume the attempts reserved for downstream models. - Telegram notification topics now fence malformed successful `createForumTopic` responses per session endpoint, preventing repeated ambiguous topic creation while keeping explicit Bot API failures retryable. diff --git a/packages/coding-agent/src/config/settings-schema.ts b/packages/coding-agent/src/config/settings-schema.ts index ad5a5b32a5..82087bfdd7 100644 --- a/packages/coding-agent/src/config/settings-schema.ts +++ b/packages/coding-agent/src/config/settings-schema.ts @@ -501,6 +501,11 @@ export const SETTINGS_SCHEMA = { default: 0.05, validate: (value: number) => Number.isFinite(value) && value > 0 && value <= 1, }, + "gjc.ralplan.maxIterations": { + type: "number", + default: 5, + validate: (value: number) => Number.isInteger(value) && value >= 1 && value <= 20, + }, // ──────────────────────────────────────────────────────────────────────── // Appearance diff --git a/packages/coding-agent/src/defaults/gjc/skills/ralplan/SKILL.md b/packages/coding-agent/src/defaults/gjc/skills/ralplan/SKILL.md index a4a00e8019..306f3d5c51 100644 --- a/packages/coding-agent/src/defaults/gjc/skills/ralplan/SKILL.md +++ b/packages/coding-agent/src/defaults/gjc/skills/ralplan/SKILL.md @@ -67,14 +67,15 @@ The consensus workflow: - **Plan-only Critic lane**: independently check quality, principle-option consistency, alternatives, risks, acceptance criteria, and verification; when the plan is thin, request concrete expansion rather than only defects. Persist with `gjc ralplan --write --stage critic --stage_n --artifact-env GJC_RALPLAN_ARTIFACT --json`, then return receipt/path plus `OKAY`/`ITERATE`/`REJECT`. - **Sequential fallback**: if Critic must evaluate Architect findings, verdict, antithesis, tradeoffs, synthesis, status, or any Architect-produced artifact, await the Architect result before issuing that Architect-dependent Critic pass. 4. **Review join gate**: before consensus, revision, reconciliation, finalization, or approval, verify both Architect and Critic receipts/verdicts exist for the same Planner artifact/pass (`path`, `sha256`, `stage_n`). A non-`CLEAR` Architect verdict, non-`APPROVE` Architect decision, or any non-`OKAY` Critic verdict routes back to Planner revision; do not finalize from only one review lane. -5. **Re-review loop** (max 5 iterations): Any non-`OKAY` Critic verdict (`ITERATE` or `REJECT`) or Architect result that is not `CLEAR`/`APPROVE` MUST run the same full closed loop: +5. **Re-review loop** (max 5 iterations; **runtime-enforced**): Any non-`OKAY` Critic verdict (`ITERATE` or `REJECT`) or Architect result that is not `CLEAR`/`APPROVE` MUST run the same full closed loop: a. Collect Architect + Critic feedback b. Revise the plan by resuming the SAME persisted Planner subagent with consolidated Architect + Critic feedback (see **Persisted Planner** below); fall back to a fresh Planner spawn only per the fallback routing table c. Return to the review fan-out or sequential fallback path above - Persist each Planner revision with `gjc ralplan --write --stage revision --stage_n --artifact-env GJC_RALPLAN_ARTIFACT --json` before re-review, then pass the receipt/path forward instead of duplicating the full revision markdown in the parent conversation. d. Re-join Architect and Critic verdicts for the same revised Planner artifact/pass e. Repeat this loop until Critic returns `OKAY` **and** Architect is `CLEAR`/`APPROVE` for the same Planner artifact/pass, or 5 iterations are reached - f. If 5 iterations are reached without Critic `OKAY` plus Architect `CLEAR`/`APPROVE`, present the best version to the user + f. If 5 iterations are reached without Critic `OKAY` plus Architect `CLEAR`/`APPROVE`, **stop opening further planner/revision passes**. Present the best version to the user (interactive) or surface `PLANNING-STUCK` (headless). Do **not** auto-start implementation. + g. **Runtime budget (#3165):** native `gjc ralplan --write` refuses a new `planner`/`revision` that would open consensus iteration **> max** (default **5**, overridable via `gjc.ralplan.maxIterations` in project/user `.gjc/settings.json`, integer 1..20). Cap uses the same iteration definition as the HUD (`planner`/`revision` openers in `index.jsonl`). Overflow exits **3**, prints operator-visible **`PLANNING-STUCK`** on stdout (and stderr detail; JSON includes `planning_stuck: true`), and still allows `architect`/`critic` within an already-opened pass plus `post-interview`/`adr`/`final` so the best plan can be escalated to `pending approval` without auto-execution. A new `--run-id` starts a fresh budget. 6. **Post-ralplan interview** (intent reconciliation gate): After the review join gate has both Critic `OKAY` and Architect `CLEAR`/`APPROVE` for the same Planner artifact/pass, and before the plan is finalized, reconcile the consensus plan against the user's actual intent. The goal is to make sure ralplan did not silently bake in assumptions that conflict with what the user wants. a. **Collect open items** from the run: every assumption the Planner/Architect/Critic resolved by assumption rather than by stated fact, every ambiguity flagged during review, and every decision the loop made without explicit user input. Source these from the persisted `planner`/`architect`/`critic`/`revision` stage artifacts, not from memory. b. **Cross-check prior context for conflicts**: glob `.gjc/_session-{sessionid}/specs/deep-interview-*.md` and other prior specs/plans/context relevant by topic. For each, list points where the consensus plan contradicts, weakens, or expands beyond a previously crystallized decision, constraint, or non-goal. Cite the conflicting artifact and line/section. @@ -103,6 +104,24 @@ The consensus workflow: > **Important:** Architect and Critic MAY run in the same parallel batch only for the plan-only Critic lane after Planner persistence. Any Architect-dependent Critic pass MUST remain sequential: await Architect before issuing Critic, then apply the same review join gate before consensus. +## Consensus iteration cap (operator contract) + +- Default max consensus iterations: **5** (`gjc.ralplan.maxIterations`). +- On cap: exit code **3**, marker **`PLANNING-STUCK`** (stdout), no silent re-loop, no automatic ultragoal/team handoff. Opener budget is `max(index.jsonl openers, on-disk stage-*-{planner,revision}.md count)` so a missing/empty/malformed ledger cannot fail open after prior openers. +- Headless/CI: treat `PLANNING-STUCK` / exit 3 as terminal planning failure for orchestration/watchdogs. +- Interactive: present best existing plan via the final approval gate; residual critic findings stay as caveats. +- Override example (project `.gjc/settings.json`): + +```json +{ + "gjc": { + "ralplan": { + "maxIterations": 3 + } + } +} +``` + Follow this ralplan-internal consensus workflow for consensus mode details. diff --git a/packages/coding-agent/src/gjc-runtime/deep-interview-ambiguity.ts b/packages/coding-agent/src/gjc-runtime/deep-interview-ambiguity.ts index fe18fdbe51..c41f2627e9 100644 --- a/packages/coding-agent/src/gjc-runtime/deep-interview-ambiguity.ts +++ b/packages/coding-agent/src/gjc-runtime/deep-interview-ambiguity.ts @@ -207,8 +207,22 @@ export function deriveAmbiguityMilestone( export function scoreToUnits(score: number): number { if (!Number.isFinite(score) || score < 0 || score > 1) throw new RangeError("score must be finite in [0, 1]"); - const units = score * 10_000; - if (!Number.isSafeInteger(units)) throw new RangeError("score must be expressed in integral 1e-4 units"); + /** + * Scores carry at most four decimal places, but `score * 10_000` misses the + * integer grid for ordinary values: `0.69 * 10_000` is `6900.000000000001` and + * `0.07 * 10_000` is `700.0000000000001`. Ambiguity is fed straight back in as + * `units / 10_000`, so testing the float product rejected most real scores. + * + * Decide from the shortest round-trip decimal instead. That is exact: every + * genuine 1e-4 value has at most four fraction digits, while off-grid + * precision keeps its digits (`0.00005`, `0.05000000000000001`) and is + * rejected. An epsilon on the product cannot separate those two cases -- both + * residuals are ~1e-13. The `[0, 1]` guard plus the four-digit cap already + * bound the result to an integer in `[0, 10_000]`. + */ + const decimal = /^(\d+)(?:\.(\d{1,4}))?$/.exec(String(score)); + if (!decimal) throw new RangeError("score must be expressed in integral 1e-4 units"); + const units = Number(decimal[1]) * 10_000 + Number((decimal[2] ?? "").padEnd(4, "0")); return units; } diff --git a/packages/coding-agent/src/gjc-runtime/deep-interview-state.ts b/packages/coding-agent/src/gjc-runtime/deep-interview-state.ts index af7c5ad8a0..3b9e632848 100644 --- a/packages/coding-agent/src/gjc-runtime/deep-interview-state.ts +++ b/packages/coding-agent/src/gjc-runtime/deep-interview-state.ts @@ -937,8 +937,20 @@ function canonicalJsonValue(value: unknown): unknown { if (value === undefined) throw new TypeError("canonical JSON rejects undefined"); return value; } + /** + * Object properties set to `undefined` are dropped, matching `JSON.stringify`: + * an omitted optional key and a present-but-undefined one must digest + * identically. Decoded requests routinely materialize every optional key (an + * omitted `targeting` becomes `targeting: undefined`), so rejecting them here + * would make every request carrying an absent optional field unserializable. + * Array elements and the top-level value stay strict: `undefined` there is a + * real encoding bug, not an absent field. + */ const output: Record = {}; - for (const key of Object.keys(value).sort()) output[key] = canonicalJsonValue(value[key]); + for (const key of Object.keys(value).sort()) { + if (value[key] === undefined) continue; + output[key] = canonicalJsonValue(value[key]); + } return output; } @@ -1004,7 +1016,7 @@ export function deepInterviewAnswerIdentityEqual( component: a.component ?? null, dimension: a.dimension ?? null, question_text: a.question_text ?? null, - question_hash: a.question_hash, + question_hash: a.question_hash ?? null, selected_options: a.selected_options ?? [], custom_input: a.custom_input ?? null, }) === @@ -1016,7 +1028,7 @@ export function deepInterviewAnswerIdentityEqual( component: b.component ?? null, dimension: b.dimension ?? null, question_text: b.question_text ?? null, - question_hash: b.question_hash, + question_hash: b.question_hash ?? null, selected_options: b.selected_options ?? [], custom_input: b.custom_input ?? null, }) @@ -1061,10 +1073,20 @@ export function applyDeepInterviewRoundResultV1( typeof value === "number" && Number.isFinite(value) && value >= 0 && value <= 1; if (!dimensions.every(dimension => finiteScore(result.global_scores[dimension]))) throw new Error("DI_STATE_SCHEMA_INVALID"); + /** + * Rounds must be scored in order, but the Round 0 topology gate is not a + * scorable round: the recorder persists it as an `answered` shell to bind the + * locked intent contract, `apply-round-result` rejects `--round 0`, and + * `validateDeepInterviewV1Envelope` refuses to persist a round-0 record that + * is anything other than an answered, score-less shell. Counting it here would + * deadlock every later round forever, so the gate is excluded by identity + * rather than by an ordering range. + */ if ( rounds.some( round => round.lifecycle !== "scored" && + round.round !== 0 && (round.round < shell.round || (round.round === shell.round && round.round_key < shell.round_key)), ) ) diff --git a/packages/coding-agent/src/gjc-runtime/ralplan-runtime.ts b/packages/coding-agent/src/gjc-runtime/ralplan-runtime.ts index 30adf96000..90ad8ac11d 100644 --- a/packages/coding-agent/src/gjc-runtime/ralplan-runtime.ts +++ b/packages/coding-agent/src/gjc-runtime/ralplan-runtime.ts @@ -1,5 +1,6 @@ import { createHash, randomBytes } from "node:crypto"; import * as fs from "node:fs/promises"; +import * as os from "node:os"; import * as path from "node:path"; import { syncSkillActiveState } from "../skill-state/active-state"; import { buildRalplanHudSummary } from "../skill-state/workflow-hud"; @@ -21,7 +22,7 @@ import { RepositoryBindingError, } from "./repository-binding"; import { GJC_RALPLAN_ARTIFACT_ENV, isRestrictedRoleAgentBash } from "./restricted-role-agent-bash"; -import { modeStatePath, sessionPlansDir } from "./session-layout"; +import { gjcRoot, modeStatePath, sessionPlansDir } from "./session-layout"; import { resolveGjcSessionForWrite, writeSessionActivityMarker } from "./session-resolution"; import { migrateWorkflowState } from "./state-migrations"; import { runNativeStateCommand } from "./state-runtime"; @@ -63,6 +64,231 @@ export interface RalplanCommandResult { const KNOWN_STAGES = ["planner", "architect", "critic", "revision", "post-interview", "adr", "final"] as const; type RalplanStage = (typeof KNOWN_STAGES)[number]; +/** Default consensus iterations (planner + revision openers) per run. Matches SKILL.md re-review cap. */ +export const RALPLAN_DEFAULT_MAX_ITERATIONS = 5; +/** Inclusive upper bound for `gjc.ralplan.maxIterations` settings overrides. */ +export const RALPLAN_MAX_ITERATIONS_LIMIT = 20; +/** Operator-visible stuck signal for headless/CI orchestration (#3165). */ +export const PLANNING_STUCK_MARKER = "PLANNING-STUCK"; + +const RALPLAN_ITERATION_OPENER_STAGES = new Set(["planner", "revision"]); + +export type RalplanIterationCapDecision = + | { + allowed: true; + currentIterations: number; + projectedIterations: number; + maxIterations: number; + } + | { + allowed: false; + currentIterations: number; + projectedIterations: number; + maxIterations: number; + reason: string; + }; + +/** + * Pure consensus-iteration budget gate (#3165). + * + * A `planner` or `revision` write opens a new iteration (same definition as + * `summarizeRalplanIndex`). Other stages never open iterations and are always + * allowed by this gate — including `final` after the cap is already reached. + * + * `iterationFloor` raises the observed opener count when on-disk evidence or a + * recovered ledger is higher than the parsed index (fail-closed vs wipe/truncate). + */ +export function evaluateRalplanIterationCap(input: { + rows: readonly RalplanIndexRow[]; + stage: string; + maxIterations?: number; + /** Minimum opener count (e.g. on-disk stage-*-{planner,revision}.md). */ + iterationFloor?: number; +}): RalplanIterationCapDecision { + const maxIterations = + typeof input.maxIterations === "number" && + Number.isInteger(input.maxIterations) && + input.maxIterations >= 1 && + input.maxIterations <= RALPLAN_MAX_ITERATIONS_LIMIT + ? input.maxIterations + : RALPLAN_DEFAULT_MAX_ITERATIONS; + const fromIndex = summarizeRalplanIndex(input.rows).iteration; + const floor = + typeof input.iterationFloor === "number" && Number.isInteger(input.iterationFloor) && input.iterationFloor > 0 + ? input.iterationFloor + : 0; + const currentIterations = Math.max(fromIndex, floor); + if (!RALPLAN_ITERATION_OPENER_STAGES.has(input.stage as RalplanStage)) { + return { + allowed: true, + currentIterations, + projectedIterations: currentIterations, + maxIterations, + }; + } + const projectedIterations = currentIterations + 1; + if (projectedIterations > maxIterations) { + const ledgerNote = floor > fromIndex ? ` (ledger under-count: index=${fromIndex}, on-disk openers=${floor})` : ""; + return { + allowed: false, + currentIterations, + projectedIterations, + maxIterations, + reason: + `ralplan consensus iteration cap exceeded: opening ${input.stage} would start ` + + `iteration ${projectedIterations} (max ${maxIterations})${ledgerNote}`, + }; + } + return { + allowed: true, + currentIterations, + projectedIterations, + maxIterations, + }; +} + +/** Filename pattern for persisted planner/revision stage artifacts. */ +const OPENER_ARTIFACT_RE = /^stage-\d{2,}-(planner|revision)\.md$/; + +/** + * Count on-disk planner/revision stage artifacts for a run. Used as a floor when + * `index.jsonl` is missing, empty, truncated, or otherwise under-counts openers. + */ +export async function countRalplanOnDiskOpeners(cwd: string, sessionId: string, runId: string): Promise { + const runDir = path.join(sessionPlansDir(cwd, sessionId), "ralplan", runId); + try { + const entries = await fs.readdir(runDir); + let count = 0; + for (const name of entries) { + if (OPENER_ARTIFACT_RE.test(name)) count += 1; + } + return count; + } catch { + return 0; + } +} + +/** + * Load index rows for cap enforcement. Unlike HUD reads, returns structural + * signals so callers can fail closed when the ledger is empty/malformed while + * opener artifacts already exist on disk. + */ +export async function loadRalplanIndexForCap( + cwd: string, + sessionId: string, + runId: string, +): Promise<{ rows: RalplanIndexRow[]; indexPresent: boolean; parseableLines: number; rawLineCount: number }> { + const indexPath = path.join(sessionPlansDir(cwd, sessionId), "ralplan", runId, "index.jsonl"); + try { + const text = await fs.readFile(indexPath, "utf8"); + const lines = text.split(/\r?\n/).filter(line => line.trim().length > 0); + const rows: RalplanIndexRow[] = []; + for (const line of lines) { + const row = parseRalplanIndexLine(line); + if (row) rows.push(row); + } + return { + rows, + indexPresent: true, + parseableLines: rows.length, + rawLineCount: lines.length, + }; + } catch (error) { + const code = + error && typeof error === "object" && "code" in error ? (error as { code?: string }).code : undefined; + if (code === "ENOENT") { + return { rows: [], indexPresent: false, parseableLines: 0, rawLineCount: 0 }; + } + // Unreadable index: treat as present-but-untrusted empty parse. + return { rows: [], indexPresent: true, parseableLines: 0, rawLineCount: 0 }; + } +} + +function parseMaxIterationsValue(value: unknown): number | null { + return typeof value === "number" && + Number.isFinite(value) && + Number.isInteger(value) && + value >= 1 && + value <= RALPLAN_MAX_ITERATIONS_LIMIT + ? value + : null; +} + +async function readSettingsMaxIterations(settingsPath: string): Promise { + try { + const raw = await Bun.file(settingsPath).text(); + const parsed = JSON.parse(raw) as Record; + const flat = parseMaxIterationsValue(parsed["gjc.ralplan.maxIterations"]); + if (flat !== null) return flat; + const gjc = parsed.gjc; + if (gjc && typeof gjc === "object") { + const ralplan = (gjc as Record).ralplan; + if (ralplan && typeof ralplan === "object") { + return parseMaxIterationsValue((ralplan as Record).maxIterations); + } + } + return null; + } catch { + return null; + } +} + +/** + * Resolve ralplan consensus iteration cap. Project `./.gjc/settings.json` overrides + * user settings, else default 5. + */ +export async function resolveRalplanMaxIterations(cwd: string): Promise<{ maxIterations: number; source: string }> { + const projectPath = path.join(gjcRoot(cwd), "settings.json"); + const project = await readSettingsMaxIterations(projectPath); + if (project !== null) return { maxIterations: project, source: projectPath }; + const userDir = process.env.GJC_CONFIG_DIR?.trim() || path.join(os.homedir(), ".gjc"); + const userPath = path.join(userDir, "settings.json"); + const user = await readSettingsMaxIterations(userPath); + if (user !== null) return { maxIterations: user, source: userPath }; + return { maxIterations: RALPLAN_DEFAULT_MAX_ITERATIONS, source: "default" }; +} + +function buildPlanningStuckResult(input: { + json: boolean; + stage: RalplanStage; + stageN: number; + runId: string; + decision: Extract; + source: string; +}): RalplanCommandResult { + const detail = + `${PLANNING_STUCK_MARKER}: ${input.decision.reason} ` + + `(run_id=${input.runId}, stage=${input.stage}, stage_n=${input.stageN}, source=${input.source}). ` + + `Stop opening planner/revision passes; escalate the best existing plan via final/pending-approval without auto-implementation.`; + if (input.json) { + return { + status: 3, + stdout: `${JSON.stringify( + { + ok: false, + planning_stuck: true, + marker: PLANNING_STUCK_MARKER, + run_id: input.runId, + stage: input.stage, + stage_n: input.stageN, + iteration: input.decision.currentIterations, + projected_iteration: input.decision.projectedIterations, + max_iterations: input.decision.maxIterations, + max_iterations_source: input.source, + reason: input.decision.reason, + }, + null, + 2, + )}\n`, + stderr: `${detail}\n`, + }; + } + return { + status: 3, + stdout: `${PLANNING_STUCK_MARKER}\n`, + stderr: `${detail}\n`, + }; +} const KNOWN_ARCHITECT_KINDS = new Set(["openai-code"]); const KNOWN_CRITIC_KINDS = new Set(["openai-code"]); @@ -738,6 +964,31 @@ async function handleArtifactWrite(args: readonly string[], cwd: string): Promis return buildDeduplicatedResult(resolved, existingArtifact, sha256, cwd, repositoryBinding); } + // Consensus iteration budget (#3165): refuse new planner/revision openers past the cap. + // Dedupe returns above so identical re-writes never stuck-signal. Non-openers (architect, + // critic, final, …) remain allowed so operators can escalate without auto-implementation. + // On-disk opener artifacts floor the count so a wiped/truncated/malformed index.jsonl cannot + // under-count and fail open after prior planner/revision writes. + const indexLoad = await loadRalplanIndexForCap(cwd, resolved.sessionId, resolved.runId); + const onDiskOpeners = await countRalplanOnDiskOpeners(cwd, resolved.sessionId, resolved.runId); + const { maxIterations, source: maxIterationsSource } = await resolveRalplanMaxIterations(cwd); + const capDecision = evaluateRalplanIterationCap({ + rows: indexLoad.rows, + stage: resolved.stage, + maxIterations, + iterationFloor: onDiskOpeners, + }); + if (!capDecision.allowed) { + return buildPlanningStuckResult({ + json: resolved.json, + stage: resolved.stage, + stageN: resolved.stageN, + runId: resolved.runId, + decision: capDecision, + source: maxIterationsSource, + }); + } + // Keep run-state `current_phase` coherent with the stage being persisted. await persistActiveRunId(cwd, resolved.sessionId, resolved.runId, resolved.stage); const persisted = await persistArtifact(resolved, cwd, content, sha256); diff --git a/packages/coding-agent/src/gjc-runtime/state-runtime.ts b/packages/coding-agent/src/gjc-runtime/state-runtime.ts index 7c55be3f3d..75b723e7e4 100644 --- a/packages/coding-agent/src/gjc-runtime/state-runtime.ts +++ b/packages/coding-agent/src/gjc-runtime/state-runtime.ts @@ -1,6 +1,9 @@ import { createHash } from "node:crypto"; import * as fs from "node:fs/promises"; import * as path from "node:path"; +// Subpath import keeps this module native-free for the gjc-state-gates shards: +// the package barrel pulls procmgr/ptree → @gajae-code/natives. +import * as logger from "@gajae-code/utils/logger"; import type { WorkflowHudSummary } from "../skill-state/active-state"; import { applyHandoffToActiveState, @@ -265,7 +268,21 @@ async function describeStaleClearState( return undefined; } -async function readJsonFile(filePath: string): Promise | null> { +/** + * Route a workflow-state warning through the TUI-safe centralized file logger + * (console transport off by default) so interactive sessions never paint raw + * bytes into the alternate-screen stream (#3002). CLI command handlers may also + * collect the warning via an `onWarning` sink to surface it on the structured + * {@link StateCommandResult.stderr} channel, so `gjc state` automation still + * distinguishes corrupt state from absent state. + */ +function emitStateWarning(warning: string, context?: Record): void { + logger.warn(warning, context); +} + +type StateWarningSink = (warning: string) => void; + +async function readJsonFile(filePath: string, onWarning?: StateWarningSink): Promise | null> { try { const raw = await fs.readFile(filePath, "utf-8"); const parsed = JSON.parse(raw); @@ -276,18 +293,22 @@ async function readJsonFile(filePath: string): Promise | } catch (error) { const err = error as NodeJS.ErrnoException; if (err.code === "ENOENT") return null; - process.stderr.write(`WARNING: failed to read ${filePath}; ignoring corrupt state: ${err.message}\n`); + const warning = `WARNING: failed to read ${filePath}; ignoring corrupt state: ${err.message}`; + emitStateWarning(warning, { filePath, error: err.message }); + onWarning?.(warning); return null; } } -async function readJsonValue(filePath: string): Promise { +async function readJsonValue(filePath: string, onWarning?: StateWarningSink): Promise { try { return JSON.parse(await fs.readFile(filePath, "utf-8")); } catch (error) { const err = error as NodeJS.ErrnoException; if (err.code === "ENOENT") return null; - process.stderr.write(`WARNING: failed to read ${filePath}; ignoring corrupt state: ${err.message}\n`); + const warning = `WARNING: failed to read ${filePath}; ignoring corrupt state: ${err.message}`; + emitStateWarning(warning, { filePath, error: err.message }); + onWarning?.(warning); return null; } } @@ -1101,21 +1122,30 @@ export async function readWorkflowStateJson( cwd: string, skill: CanonicalGjcWorkflowSkill, sessionId?: string, + onWarning?: StateWarningSink, ): Promise> { const session = await resolveGjcSessionForRead(cwd, { payloadSessionId: sessionId, envSessionId: process.env.GJC_SESSION_ID, }); - return (await readJsonFile(modeStateFile(cwd, skill, session.gjcSessionId))) ?? {}; + return (await readJsonFile(modeStateFile(cwd, skill, session.gjcSessionId), onWarning)) ?? {}; } async function handleRead(args: readonly string[], cwd: string): Promise { const selectors = await resolveSelectors(args, cwd, "read"); const mode = selectors.mode ?? (await inferModeFromActiveState(cwd, selectors.gjcSessionId)); const fields = parseFieldsFlag(args); + // Corrupt-state warnings are TUI-safe file-logged inside the readers; the CLI + // path also surfaces them on the command result so `gjc state read` + // automation can tell corrupt state from absent state (#3002). + const warnings: string[] = []; + const warningStderr = (): Pick => + warnings.length ? { stderr: warnings.map(warning => `${warning}\n`).join("") } : {}; if (mode) { const filePath = modeStateFile(cwd, mode, selectors.gjcSessionId); - const existing = await readWorkflowStateJson(cwd, mode, selectors.gjcSessionId); + const existing = await readWorkflowStateJson(cwd, mode, selectors.gjcSessionId, warning => + warnings.push(warning), + ); const envelope = { skill: mode, state: existing, storage_path: filePath }; const manifest = getSkillManifest(mode); if (fields) { @@ -1125,6 +1155,7 @@ async function handleRead(args: readonly string[], cwd: string): Promise warnings.push(warning)); const existing = isPlainObject(existingRaw) ? existingRaw : null; - return { status: 0, stdout: `${JSON.stringify(existing ?? {}, null, 2)}\n` }; + return { status: 0, stdout: `${JSON.stringify(existing ?? {}, null, 2)}\n`, ...warningStderr() }; } async function handleStatus(args: readonly string[], cwd: string): Promise { @@ -1159,7 +1192,8 @@ async function handleStatus(args: readonly string[], cwd: string): Promise warnings.push(warning)); const summary = buildStateStatusSummary( mode, { skill: mode, state: existing, storage_path: filePath }, @@ -1169,6 +1203,7 @@ async function handleStatus(args: readonly string[], cwd: string): Promise `${warning}\n`).join("") } : {}), }; } @@ -1603,7 +1638,7 @@ async function handleHandoffUnlocked(args: readonly string[], cwd: string): Prom toPhase: "handoff", }); await updateWorkflowTransactionJournal(cwd, sessionId, mutationId, { steps: ["caller-mode-state"] }); - if (callerWrite.warning) process.stderr.write(`${callerWrite.warning}\n`); + if (callerWrite.warning) emitStateWarning(callerWrite.warning); const stampedCallerReceipt = isPlainObject(callerWrite.stamped.receipt) ? callerWrite.stamped.receipt : {}; await syncSkillActiveState({ cwd, @@ -1646,6 +1681,7 @@ async function handleHandoffUnlocked(args: readonly string[], cwd: string): Prom active_state: activeStateFile(cwd, sessionId), }, }), + ...(callerWrite.warning ? { stderr: `${callerWrite.warning}\n` } : {}), }; } @@ -1746,7 +1782,7 @@ async function handleHandoffUnlocked(args: readonly string[], cwd: string): Prom ); const stampedCallerReceipt = isPlainObject(callerWrite.stamped.receipt) ? callerWrite.stamped.receipt : {}; const stampedCalleeReceipt = isPlainObject(calleeWrite.stamped.receipt) ? calleeWrite.stamped.receipt : {}; - for (const warning of warnings) process.stderr.write(`${warning}\n`); + for (const warning of warnings) emitStateWarning(warning); if (process.env.GJC_STATE_HANDOFF_FAIL_AFTER_CALLER === mutationId) { throw new StateCommandError(1, `injected handoff failure after caller write for ${mutationId}`); } @@ -1818,6 +1854,7 @@ async function handleHandoffUnlocked(args: readonly string[], cwd: string): Prom active_state: activeStateFile(cwd, sessionId), }, }), + ...(warnings.length ? { stderr: warnings.map(warning => `${warning}\n`).join("") } : {}), }; } diff --git a/packages/coding-agent/src/modes/components/model-selector.ts b/packages/coding-agent/src/modes/components/model-selector.ts index bf17833c90..7f177f0d53 100644 --- a/packages/coding-agent/src/modes/components/model-selector.ts +++ b/packages/coding-agent/src/modes/components/model-selector.ts @@ -1245,8 +1245,14 @@ export class ModelSelectorComponent extends Container { new Text(`${prefix}${i === this.#presetScopeIndex ? theme.fg("accent", label) : label}`, 0, 0), ); } + this.#listContainer.addChild(new Spacer(1)); + this.#listContainer.addChild( + new Text(theme.fg("muted", " Enter: apply | d: set as default | Esc: back"), 0, 0), + ); } else { - this.#listContainer.addChild(new Text(theme.fg("muted", " Press Enter to apply this preset"), 0, 0)); + this.#listContainer.addChild( + new Text(theme.fg("muted", " Press Enter to apply or d to set as default"), 0, 0), + ); } } @@ -1558,6 +1564,14 @@ export class ModelSelectorComponent extends Container { } #handlePresetLandingInput(keyData: string): void { + if (keyData === "d" || keyData === "D") { + if (this.#previewProfileName) { + this.#presetScopeMenuOpen = true; + this.#presetScopeIndex = 1; + this.#handlePresetEnter(); + return; + } + } if (isPrintableCharacter(keyData)) { this.#switchToModelMode(keyData); return; diff --git a/packages/coding-agent/src/modes/components/welcome.ts b/packages/coding-agent/src/modes/components/welcome.ts index c4c854f840..8a3dce4ff7 100644 --- a/packages/coding-agent/src/modes/components/welcome.ts +++ b/packages/coding-agent/src/modes/components/welcome.ts @@ -20,6 +20,7 @@ export interface WelcomeComponentOptions { getViewportRows?: () => number | undefined; getReservedBottomRows?: (termWidth: number) => number; changelogMarkdown?: string; + rightGutterWidth?: number; collapseChangelog?: boolean; buildLabel?: string; keyDisplayContext?: KeyDisplayContext; @@ -110,7 +111,8 @@ export class WelcomeComponent implements Component { } render(termWidth: number): string[] { - const boxWidth = Math.max(0, termWidth); + const rightGutterWidth = this.#rightGutterWidth(termWidth); + const boxWidth = Math.max(0, termWidth - rightGutterWidth); if (boxWidth < 4) { return []; } @@ -245,7 +247,7 @@ export class WelcomeComponent implements Component { lines.push(tl + titleStyled + theme.fg("dim", hChar.repeat(afterTitle)) + tr); } if (outputRows === 1) { - return lines; + return this.#withRightGutter(lines, rightGutterWidth); } if (showRightColumn) { @@ -273,7 +275,7 @@ export class WelcomeComponent implements Component { lines.push(bl + h.repeat(leftCol) + br); } - return lines; + return this.#withRightGutter(lines, rightGutterWidth); } /** Center text within a given width */ @@ -295,6 +297,19 @@ export class WelcomeComponent implements Component { } return str + padding(width - visLen); } + #rightGutterWidth(termWidth: number): number { + const configured = this.options.rightGutterWidth ?? 0; + if (!Number.isFinite(configured) || configured <= 0) return 0; + const gutterWidth = Math.floor(configured); + return Math.min(gutterWidth, Math.max(0, termWidth - 4)); + } + + #withRightGutter(lines: string[], rightGutterWidth: number): string[] { + if (rightGutterWidth <= 0) return lines; + const gutter = padding(rightGutterWidth); + return lines.map(line => line + gutter); + } + #targetRows(termWidth: number): number | undefined { const viewportRows = this.options.getViewportRows?.(); if (typeof viewportRows !== "number" || !Number.isFinite(viewportRows) || viewportRows <= 0) { diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index c849675f50..cfda3fc432 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -168,6 +168,7 @@ export function getComposerPlaceholder( return buildComposerPlaceholder(keybindings, context, options); } const WELCOME_RESERVED_CONTAINER_CHILD_LIMIT = 8; +const COMPOSER_RIGHT_GUTTER_WIDTH = 1; const IRC_SIDEBAR_TOGGLE_SHADOWING_ACTIONS: readonly AppKeybinding[] = [ "app.plan.toggle", @@ -214,7 +215,7 @@ function configureDefaultComposerChrome(editor: CustomEditor): void { editor.setInputPrefix(getDefaultInputPrefix()); editor.setPlaceholder(getDefaultComposerPlaceholder()); editor.setPaddingX(1); - editor.setRightGutterWidth(1); + editor.setRightGutterWidth(COMPOSER_RIGHT_GUTTER_WIDTH); editor.setTopBorder(undefined); } @@ -713,6 +714,7 @@ export class InteractiveMode implements InteractiveModeContext { getViewportRows: () => this.ui.terminal.rows, getReservedBottomRows: getWelcomeReservedBottomRows, changelogMarkdown: this.#changelogMarkdown, + rightGutterWidth: COMPOSER_RIGHT_GUTTER_WIDTH, collapseChangelog: settings.get("collapseChangelog"), keyDisplayContext: this.#keyDisplayContext, }, diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 141c52abf5..e0ce8cd975 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -341,6 +341,7 @@ import { import { type ConfiguredFallbackChain, cappedExponentialWithFullJitter, + compactionRetryDelay, effectiveFallbackDelay, FallbackChainController, } from "./fallback-chain-controller"; @@ -12504,8 +12505,14 @@ export class AgentSession { break; } - const baseDelayMs = retrySettings.baseDelayMs * 2 ** attempt; - const delayMs = retryAfterMs !== undefined ? Math.max(baseDelayMs, retryAfterMs) : baseDelayMs; + // Legacy parsed Retry-After is capped at retry.maxDelayMs (see + // compactionRetryDelay); only managed fallback is uncapped. + const delayMs = compactionRetryDelay( + retrySettings.baseDelayMs, + retrySettings.maxDelayMs, + attempt, + retryAfterMs, + ); // If retry delay is too long (>30s), try next candidate instead of waiting const maxAcceptableDelayMs = 30_000; diff --git a/packages/coding-agent/src/session/fallback-chain-controller.ts b/packages/coding-agent/src/session/fallback-chain-controller.ts index f80b45e9f7..27e332db2f 100644 --- a/packages/coding-agent/src/session/fallback-chain-controller.ts +++ b/packages/coding-agent/src/session/fallback-chain-controller.ts @@ -186,6 +186,33 @@ export function cappedExponentialWithFullJitter( return Math.floor(Math.max(0, cap) * Math.max(0, Math.min(1, random()))); } +/** + * Legacy auto-compaction retry delay. + * + * Deliberately the mirror image of `effectiveFallbackDelay`: this path recovers + * Retry-After by regex over provider error prose (`#parseRetryAfterMsFromError`), + * so it follows the documented legacy rule — `retry.maxDelayMs` caps every + * legacy session retry delay, including provider retry-after hints. Managed + * fallback stays uncapped because it retries within its own per-entry budget; + * compaction has no such budget, so the final candidate would otherwise sleep + * for the full server-suggested duration. + * + * `maxDelayMs <= 0` means "no cap", matching `cappedExponentialWithFullJitter`. + * A missing, NaN, or infinite hint collapses to "no usable hint". + */ +export function compactionRetryDelay( + baseDelayMs: number, + maxDelayMs: number, + attempt: number, + retryAfterMs: number | undefined, +): number { + const exponential = baseDelayMs * 2 ** Math.max(0, attempt); + const hint = Number.isFinite(retryAfterMs) ? Math.max(0, retryAfterMs as number) : 0; + const hinted = Math.max(exponential, hint); + const bounded = maxDelayMs > 0 ? Math.min(hinted, maxDelayMs) : hinted; + return Math.max(0, bounded); +} + /** Retry-After is intentionally uncapped. */ export function effectiveFallbackDelay( baseDelayMs: number, diff --git a/packages/coding-agent/src/task/types.ts b/packages/coding-agent/src/task/types.ts index 3b5356d3f7..7b3fe182a5 100644 --- a/packages/coding-agent/src/task/types.ts +++ b/packages/coding-agent/src/task/types.ts @@ -1,6 +1,5 @@ import type { ThinkingLevel } from "@gajae-code/agent-core"; import type { Usage } from "@gajae-code/ai"; -import { $pickenv } from "@gajae-code/utils"; import * as z from "zod/v4"; import { isValidTaskId, TASK_ID_DESCRIPTION } from "./id"; import type { TaskResultReceipt } from "./receipt"; @@ -14,23 +13,28 @@ export type AgentSource = "bundled" | "user" | "project"; export type ForkContextPolicy = "forbidden" | "allowed"; export type ForkContextMode = "none" | "receipt" | "last-turn" | "bounded" | "full"; -const parseNumber = (value: string | undefined, defaultValue: number): number => { - if (value) { - try { - const number = Number.parseInt(value, 10); - if (!Number.isNaN(number) && number > 0) { - return number; - } - } catch {} +const parsePositiveIntegerEnvironment = (keys: string[], defaultValue: number): number => { + for (const key of keys) { + const value = Bun.env[key]; + if (!value || value.trim().length === 0) continue; + if (!/^\d+$/.test(value)) return defaultValue; + const number = Number(value); + return Number.isSafeInteger(number) && number > 0 ? number : defaultValue; } return defaultValue; }; /** Maximum output bytes per agent */ -export const MAX_OUTPUT_BYTES = parseNumber($pickenv("GJC_TASK_MAX_OUTPUT_BYTES", "PI_TASK_MAX_OUTPUT_BYTES"), 500_000); +export const MAX_OUTPUT_BYTES = parsePositiveIntegerEnvironment( + ["GJC_TASK_MAX_OUTPUT_BYTES", "PI_TASK_MAX_OUTPUT_BYTES"], + 500_000, +); /** Maximum output lines per agent */ -export const MAX_OUTPUT_LINES = parseNumber($pickenv("GJC_TASK_MAX_OUTPUT_LINES", "PI_TASK_MAX_OUTPUT_LINES"), 5000); +export const MAX_OUTPUT_LINES = parsePositiveIntegerEnvironment( + ["GJC_TASK_MAX_OUTPUT_LINES", "PI_TASK_MAX_OUTPUT_LINES"], + 5000, +); /** EventBus channel for raw subagent events */ export const TASK_SUBAGENT_EVENT_CHANNEL = "task:subagent:event"; diff --git a/packages/coding-agent/src/workflow/workflow-intent-diff.ts b/packages/coding-agent/src/workflow/workflow-intent-diff.ts index 2f6ee3e4cf..a3ace91095 100644 --- a/packages/coding-agent/src/workflow/workflow-intent-diff.ts +++ b/packages/coding-agent/src/workflow/workflow-intent-diff.ts @@ -40,7 +40,9 @@ interface RouteMatch { const PROMPT_PREVIEW_LIMIT = 240; const DURABLE_TRACKING_PATTERNS = [ - /\bultragoal\b/i, + /\/skill:ultragoal\b/i, + /\b(?:use|run|start|create|activate|invoke|execute)\s+(?:the\s+)?ultragoal\b/i, + /\bultragoal(?:로|으로)[^.!?\n]{0,40}(?:해|진행|실행|관리|처리)/i, /\bdurable (?:goal|tracking|ledger|plan)\b/i, /\b(?:goal|tracking|plan) ledger\b/i, /\bcheckpoint(?:ed|ing)? (?:goal|plan|workflow|release|work)\b/i, diff --git a/packages/coding-agent/test/agent-session-workflow-intent-diff.test.ts b/packages/coding-agent/test/agent-session-workflow-intent-diff.test.ts index dd9d69293c..dce0c497fa 100644 --- a/packages/coding-agent/test/agent-session-workflow-intent-diff.test.ts +++ b/packages/coding-agent/test/agent-session-workflow-intent-diff.test.ts @@ -156,8 +156,10 @@ describe("AgentSession workflow intent-diff tracking", () => { await session.prompt("I'm not sure what this product should be, don't assume the requirements"); await session.prompt("create a durable goal ledger for this multi-step release"); await session.prompt("create a durable goal ledger for this production release"); + await session.prompt("use ultragoal to track this release"); + await session.prompt("ultragoal로 이 작업 처리해줘"); - const [deepInterview, ultragoal, productionUltragoal] = workflowIntentEntries(); + const [deepInterview, ultragoal, productionUltragoal, namedUltragoal, koreanUltragoal] = workflowIntentEntries(); expect(deepInterview?.data).toMatchObject({ route: "deep-interview", recommendedSkill: "deep-interview", @@ -177,6 +179,28 @@ describe("AgentSession workflow intent-diff tracking", () => { directTracking: "not-direct", rootCausePhase: { status: "active", triggers: ["high-risk transition"] }, }); + expect(namedUltragoal?.data).toMatchObject({ + route: "ultragoal", + recommendedSkill: "ultragoal", + }); + expect(koreanUltragoal?.data).toMatchObject({ + route: "ultragoal", + recommendedSkill: "ultragoal", + }); + }); + + it("keeps questions about ultragoal behavior on the direct path", async () => { + await session.prompt("How many consensus rounds does ultragoal run, and can I limit them?"); + await session.prompt( + "ultragoal 같은거 쓰면 합의 몇번이나 하게 되어 있음? 끝도 없이 하는 경우도 있는거 같은데 제약 할수 있는 옵션 있음?", + ); + + for (const entry of workflowIntentEntries()) { + expect(entry.data).toMatchObject({ + route: "direct", + directTracking: "custom-entry-only", + }); + } }); it("lets ambiguous requirements take precedence over durable tracking words", async () => { diff --git a/packages/coding-agent/test/g002-ws1-redteam.test.ts b/packages/coding-agent/test/g002-ws1-redteam.test.ts index b527258063..642efefa78 100644 --- a/packages/coding-agent/test/g002-ws1-redteam.test.ts +++ b/packages/coding-agent/test/g002-ws1-redteam.test.ts @@ -51,6 +51,7 @@ function controller(messages: AgentMessage[], revealViewportAnchor = vi.fn((_id: const session = { messages }; const ctx = { session, + editor: { getText: () => "draft" }, ui: { revealViewportAnchor }, showTranscriptViewer, showError: vi.fn(), diff --git a/packages/coding-agent/test/g003-ws2-redteam.test.ts b/packages/coding-agent/test/g003-ws2-redteam.test.ts index dd8b2dfa72..dad06b8c35 100644 --- a/packages/coding-agent/test/g003-ws2-redteam.test.ts +++ b/packages/coding-agent/test/g003-ws2-redteam.test.ts @@ -26,10 +26,12 @@ function createControllerContext(overrides: Partial = {} model: undefined, messages: [], queuedMessageCount: 0, + getQueuedMessageEntries: () => [], isStreaming: false, getRoleModelCycleCandidateCount: () => 0, hasForegroundBashBackgroundRequestHandler: () => false, }, + compactionQueuedMessages: [], chatContainer: { children: [] }, goalModeController: { enabled: false, paused: false, handleCommand: () => {} }, planModeController: { enabled: true, paused: false, handleCommand: () => {} }, diff --git a/packages/coding-agent/test/gjc-runtime/deep-interview-ambiguity.test.ts b/packages/coding-agent/test/gjc-runtime/deep-interview-ambiguity.test.ts index fb82fe8f23..5860ceb05d 100644 --- a/packages/coding-agent/test/gjc-runtime/deep-interview-ambiguity.test.ts +++ b/packages/coding-agent/test/gjc-runtime/deep-interview-ambiguity.test.ts @@ -1,7 +1,7 @@ import { afterAll, afterEach, beforeAll, describe, expect, it } from "bun:test"; import * as fs from "node:fs/promises"; import * as path from "node:path"; -import { deriveAmbiguityMilestone } from "@gajae-code/coding-agent/gjc-runtime/deep-interview-ambiguity"; +import { deriveAmbiguityMilestone, scoreToUnits } from "@gajae-code/coding-agent/gjc-runtime/deep-interview-ambiguity"; import { answerHash, appendOrMergeDeepInterviewRound, @@ -338,6 +338,40 @@ describe("deep-interview v1 core contracts", () => { deepInterviewRoundResultDigest({ result: { a: { alpha: 1, beta: 2 }, z: ["x"] }, round: 1, question_id: "q" }), ); expect(canonicalDeepInterviewJson({ b: 1, a: 2 })).toBe('{"a":2,"b":1}'); + // Decoded requests materialize every optional key, so an absent optional field + // arrives as `undefined`. It must serialize like an omitted key instead of + // throwing, and both spellings must digest identically. + expect(canonicalDeepInterviewJson({ a: 1, targeting: undefined })).toBe('{"a":1}'); + expect( + deepInterviewRoundResultDigest({ + round: 1, + question_id: "q", + result: { global_scores: { goal: 0.5 }, targeting: undefined, ontology: undefined }, + }), + ).toBe(deepInterviewRoundResultDigest({ round: 1, question_id: "q", result: { global_scores: { goal: 0.5 } } })); + // Array elements and the top-level value stay strict: `undefined` there is an + // encoding bug, not an absent field. + expect(() => canonicalDeepInterviewJson([1, undefined])).toThrow("canonical JSON rejects undefined"); + expect(() => canonicalDeepInterviewJson(undefined)).toThrow("canonical JSON rejects undefined"); + // `null` must survive the collapse. If the skip predicate ever loosened to + // `!value[key]` or `== undefined`, null/0/""/false would fold into "absent" + // and silently change the meaning of every persisted round_result_digest. + expect(canonicalDeepInterviewJson({ a: null })).toBe('{"a":null}'); + expect(canonicalDeepInterviewJson({ a: 0, b: "", c: false })).toBe('{"a":0,"b":"","c":false}'); + + // `0.69 * 10_000` is `6900.000000000001` in IEEE-754; ambiguity round-trips + // through `units / 10_000`, so every value on the 1e-4 grid must convert + // exactly while genuinely off-grid precision stays rejected. The sweep is the + // real contract: spot checks alone cannot show which floats miss the grid. + const offGrid: number[] = []; + for (let units = 0; units <= 10_000; units += 1) if (scoreToUnits(units / 10_000) !== units) offGrid.push(units); + expect(offGrid).toEqual([]); + expect(scoreToUnits(-0)).toBe(0); + expect(scoreToUnits(0.9999)).toBe(9_999); + for (const rejected of [0.00005, 0.05000000000000001, 0.30000000000000004, 1e-7]) + expect(() => scoreToUnits(rejected)).toThrow("integral 1e-4 units"); + for (const rejected of [1.5, -0.1, Number.NaN, Number.POSITIVE_INFINITY]) + expect(() => scoreToUnits(rejected)).toThrow("finite in [0, 1]"); const filePath = "/tmp/state.json"; const envelope = { @@ -452,5 +486,64 @@ describe("deep-interview v1 core contracts", () => { "2026-01-01T00:02:00.000Z", ), ).toThrow("DI_ROUND_RESULT_CONFLICT"); + + // The Round-0 gate is excluded from the "earlier rounds must be scored" + // precondition only because a round-0 record can never carry scoring. Pin + // that premise with a positive control and a negative case differing by + // exactly one field, so the assertion cannot pass for an unrelated reason: if + // the validator's round-0 rule is deleted, the scored fixture below becomes + // valid and this test goes red. + const gateShell = { + round: 0, + round_key: "r0", + question_id: "round0-topology", + question_text: "Confirm locked intent", + question_hash: "question", + answer_hash: "answer", + lifecycle: "answered", + answered_at: "2026-01-01T00:00:00.000Z", + }; + const withRoundZero = (record: Record): Record => { + const candidate = structuredClone(applied.envelope) as Record; + (candidate.state as { rounds: Record[] }).rounds.unshift(record); + return candidate; + }; + validateDeepInterviewV1Envelope(withRoundZero({ ...gateShell })); + expect(() => + validateDeepInterviewV1Envelope( + withRoundZero({ ...gateShell, scores: { goal: 0.2, constraints: 0.3, criteria: 0.4 } }), + ), + ).toThrow("DI_STATE_SCHEMA_INVALID"); + // The gate's `review-topology`/`topology` metadata is accepted only while the + // record stays a score-less shell, which is what keeps it unscorable. + validateDeepInterviewV1Envelope( + withRoundZero({ ...gateShell, component: "review-topology", dimension: "topology" }), + ); + }); + + it("refuses to score the Round-0 topology gate through the repair CLI", async () => { + const cwd = await tempDir(); + await seedRecorderState(cwd); + const rejected = await runDeepInterviewRepairCommand( + [ + "apply-round-result", + "--session-id", + TEST_SESSION_ID, + "--schema-version", + "1", + "--expected-revision", + "1", + "--round", + "0", + "--question-id", + "round0-topology", + "--result-json", + JSON.stringify({ global_scores: { goal: 0.4, constraints: 0.3, criteria: 0.2 } }), + "--json", + ], + cwd, + ); + expect(rejected.status).toBe(2); + expect(rejected.stderr).toContain("DI_INVALID_ROUND"); }); }); diff --git a/packages/coding-agent/test/gjc-runtime/deep-interview-recorder.test.ts b/packages/coding-agent/test/gjc-runtime/deep-interview-recorder.test.ts index ecb6f3fea1..3cf9f20da0 100644 --- a/packages/coding-agent/test/gjc-runtime/deep-interview-recorder.test.ts +++ b/packages/coding-agent/test/gjc-runtime/deep-interview-recorder.test.ts @@ -718,6 +718,126 @@ describe("deep-interview recorder: persistence (state-writer backed)", () => { expect(compact.pending_shells).toEqual([]); }); + it("scores Round 1 even though the unscorable Round-0 topology gate shell stays answered", async () => { + // Regression: the Round-0 topology gate is recorded as an `answered` shell so the + // locked intent contract can bind to its answer hash, but `apply-round-result` + // rejects `--round 0`, so that shell can never reach `scored`. Counting it in the + // "earlier rounds must be scored" precondition deadlocked every later round, making + // the whole interview unable to score a single answer. + const cwd = await tempDir(); + const statePath = statePathFor(cwd); + await seedRecorderState(cwd); + expect( + ( + await runDeepInterviewRepairCommand( + [ + "confirm-topology", + "--session-id", + TEST_SESSION_ID, + "--schema-version", + "1", + "--expected-revision", + "1", + "--input-json", + '{"components":[{"id":"alpha","name":"Alpha"}],"deferred_components":[]}', + "--json", + ], + cwd, + ) + ).status, + ).toBe(0); + await appendOrMergeDeepInterviewRound( + cwd, + statePath, + { + round: 0, + questionId: "round0-topology", + questionText: "Confirm locked intent", + component: "review-topology", + dimension: "topology", + selectedOptions: ["Confirm"], + intent_contract: { + items: [{ id: "surface:review", category: "surface" as const, statement: "Provide a reviewer surface" }], + confirmation_options: ["Confirm"], + }, + }, + { sessionId: TEST_SESSION_ID }, + ); + await appendOrMergeDeepInterviewRound( + cwd, + statePath, + { + round: 1, + questionId: "q1", + questionText: "Q?", + component: "alpha", + dimension: "goal", + selectedOptions: ["a"], + }, + { sessionId: TEST_SESSION_ID }, + ); + const before = JSON.parse(await fs.readFile(statePath, "utf-8")) as { + state_revision: number; + state: { rounds: DeepInterviewRoundRecord[] }; + }; + expect(before.state.rounds.find(round => round.round === 0)?.lifecycle).toBe("answered"); + const scored = await runDeepInterviewRepairCommand( + [ + "apply-round-result", + "--session-id", + TEST_SESSION_ID, + "--schema-version", + "1", + "--expected-revision", + String(before.state_revision), + "--round", + "1", + "--question-id", + "q1", + "--result-json", + JSON.stringify({ + global_scores: { goal: 0.4, constraints: 0.3, criteria: 0.2 }, + component_updates: [{ component_id: "alpha", scores: { goal: 0.4, constraints: 0.3, criteria: 0.2 } }], + }), + "--json", + ], + cwd, + ); + expect(scored.status, scored.stderr).toBe(0); + const persisted = JSON.parse(await fs.readFile(statePath, "utf-8")) as { + state: { rounds: DeepInterviewRoundRecord[] }; + }; + expect(persisted.state.rounds.find(round => round.round === 0)?.lifecycle).toBe("answered"); + expect(persisted.state.rounds.find(round => round.round === 1)?.lifecycle).toBe("scored"); + // The result JSON above omits `targeting`/`ontology`/`bookkeeping`, so the + // decoder materializes them as `undefined`. Replaying it must digest to the + // same value and settle as an idempotent noop rather than a conflict. + const replay = await runDeepInterviewRepairCommand( + [ + "apply-round-result", + "--session-id", + TEST_SESSION_ID, + "--schema-version", + "1", + "--expected-revision", + String(before.state_revision + 1), + "--round", + "1", + "--question-id", + "q1", + "--result-json", + JSON.stringify({ + global_scores: { goal: 0.4, constraints: 0.3, criteria: 0.2 }, + component_updates: [{ component_id: "alpha", scores: { goal: 0.4, constraints: 0.3, criteria: 0.2 } }], + }), + "--json", + ], + cwd, + ); + expect(replay.status, replay.stderr).toBe(0); + expect(JSON.parse(replay.stdout ?? "{}")).toMatchObject({ ok: true, written: false }); + }); + it("canonicalizes an agent-supplied dimension label before persisting the shell", () => { // `deepInterview.dimension` is free text on post-topology asks; the persisted envelope // only accepts canonical ids, so a display label must not reach state verbatim. diff --git a/packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts b/packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts index efae74c955..3031a8dfd6 100644 --- a/packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts +++ b/packages/coding-agent/test/gjc-runtime/ralplan-runtime.test.ts @@ -1,7 +1,12 @@ import { afterAll, afterEach, beforeAll, describe, expect, it } from "bun:test"; import * as fs from "node:fs/promises"; import * as path from "node:path"; -import { runNativeRalplanCommand } from "@gajae-code/coding-agent/gjc-runtime/ralplan-runtime"; +import { + evaluateRalplanIterationCap, + PLANNING_STUCK_MARKER, + RALPLAN_DEFAULT_MAX_ITERATIONS, + runNativeRalplanCommand, +} from "@gajae-code/coding-agent/gjc-runtime/ralplan-runtime"; import { GJC_RALPLAN_ARTIFACT_ENV, GJC_RESTRICTED_ROLE_AGENT_BASH_ENV, @@ -1154,3 +1159,259 @@ describe("native gjc ralplan runtime — post-clear re-activation (#644)", () => expect(after.current_phase).toBe("complete"); }); }); +describe("ralplan consensus iteration cap (#3165)", () => { + it("evaluateRalplanIterationCap allows openers up to max and rejects the next", () => { + const rows = [ + { stage: "planner", stageN: 1 }, + { stage: "architect", stageN: 1 }, + { stage: "critic", stageN: 1 }, + { stage: "revision", stageN: 2 }, + { stage: "revision", stageN: 3 }, + { stage: "revision", stageN: 4 }, + { stage: "revision", stageN: 5 }, + ]; + expect(evaluateRalplanIterationCap({ rows, stage: "revision" })).toMatchObject({ + allowed: false, + currentIterations: 5, + projectedIterations: 6, + maxIterations: RALPLAN_DEFAULT_MAX_ITERATIONS, + }); + expect(evaluateRalplanIterationCap({ rows, stage: "final" }).allowed).toBe(true); + expect(evaluateRalplanIterationCap({ rows, stage: "architect" }).allowed).toBe(true); + expect( + evaluateRalplanIterationCap({ + rows: [{ stage: "planner", stageN: 1 }], + stage: "revision", + maxIterations: 2, + }).allowed, + ).toBe(true); + expect( + evaluateRalplanIterationCap({ + rows: [ + { stage: "planner", stageN: 1 }, + { stage: "revision", stageN: 2 }, + ], + stage: "revision", + maxIterations: 2, + }).allowed, + ).toBe(false); + // Floor from on-disk openers wins over an empty/under-counted index. + expect( + evaluateRalplanIterationCap({ + rows: [], + stage: "revision", + maxIterations: 5, + iterationFloor: 5, + }), + ).toMatchObject({ + allowed: false, + currentIterations: 5, + projectedIterations: 6, + }); + expect( + evaluateRalplanIterationCap({ + rows: [{ stage: "planner", stageN: 1 }], + stage: "revision", + maxIterations: 5, + iterationFloor: 3, + }).allowed, + ).toBe(true); + }); + + it("rejects a 6th revision opener with PLANNING-STUCK and still allows final", async () => { + const root = await tempDir(); + const runId = "cap-run"; + const write = async (stage: string, stageN: number, body: string) => + runNativeRalplanCommand( + ["--write", "--stage", stage, "--stage_n", String(stageN), "--artifact", body, "--run-id", runId, "--json"], + root, + ); + + expect((await write("planner", 1, "# p1")).status).toBe(0); + expect((await write("architect", 1, "# a1")).status).toBe(0); + expect((await write("critic", 1, "Verdict: ITERATE")).status).toBe(0); + for (let n = 2; n <= 5; n++) { + expect((await write("revision", n, `# r${n}`)).status).toBe(0); + expect((await write("architect", n, `# a${n}`)).status).toBe(0); + expect((await write("critic", n, "Verdict: ITERATE")).status).toBe(0); + } + + const stuck = await write("revision", 6, "# r6 perpetual iterate"); + expect(stuck.status).toBe(3); + expect(stuck.stdout).toContain(PLANNING_STUCK_MARKER); + expect(stuck.stderr).toContain(PLANNING_STUCK_MARKER); + const payload = JSON.parse(stuck.stdout ?? "{}"); + expect(payload).toMatchObject({ + ok: false, + planning_stuck: true, + marker: PLANNING_STUCK_MARKER, + max_iterations: 5, + projected_iteration: 6, + }); + + const final = await write("final", 6, "# best effort pending approval"); + expect(final.status).toBe(0); + expect(final.stdout).toContain("pending_approval_path"); + }); + + it("honors project settings maxIterations=2 and resets budget on new run_id", async () => { + const root = await tempDir(); + await fs.mkdir(path.join(root, ".gjc"), { recursive: true }); + await fs.writeFile( + path.join(root, ".gjc", "settings.json"), + JSON.stringify({ gjc: { ralplan: { maxIterations: 2 } } }), + "utf-8", + ); + + const write = async (runId: string, stage: string, stageN: number, body: string) => + runNativeRalplanCommand( + ["--write", "--stage", stage, "--stage_n", String(stageN), "--artifact", body, "--run-id", runId, "--json"], + root, + ); + + expect((await write("run-a", "planner", 1, "# p")).status).toBe(0); + expect((await write("run-a", "revision", 2, "# r2")).status).toBe(0); + const stuck = await write("run-a", "revision", 3, "# r3"); + expect(stuck.status).toBe(3); + expect(JSON.parse(stuck.stdout ?? "{}").max_iterations).toBe(2); + + // Fresh run_id must not inherit the stuck budget. + expect((await write("run-b", "planner", 1, "# p-b")).status).toBe(0); + expect((await write("run-b", "revision", 2, "# r2-b")).status).toBe(0); + }); + + it("dedupes an identical revision write at the cap without PLANNING-STUCK", async () => { + const root = await tempDir(); + const runId = "dedupe-cap"; + const write = async (stage: string, stageN: number, body: string) => + runNativeRalplanCommand( + ["--write", "--stage", stage, "--stage_n", String(stageN), "--artifact", body, "--run-id", runId, "--json"], + root, + ); + + expect((await write("planner", 1, "# p")).status).toBe(0); + for (let n = 2; n <= 5; n++) { + expect((await write("revision", n, `# r${n}`)).status).toBe(0); + } + const first = await write("revision", 5, "# r5"); + expect(first.status).toBe(0); + const payload = JSON.parse(first.stdout ?? "{}"); + expect(payload.deduplicated).toBe(true); + expect(payload.planning_stuck).toBeUndefined(); + }); + it("fails closed when index.jsonl is emptied after max openers (ledger wipe)", async () => { + const root = await tempDir(); + const runId = "wipe-cap"; + const write = async (stage: string, stageN: number, body: string) => + runNativeRalplanCommand( + ["--write", "--stage", stage, "--stage_n", String(stageN), "--artifact", body, "--run-id", runId, "--json"], + root, + ); + + expect((await write("planner", 1, "# p")).status).toBe(0); + for (let n = 2; n <= 5; n++) { + expect((await write("revision", n, `# r${n}`)).status).toBe(0); + } + + const indexPath = path.join(ralplanRunDir(root, runId), "index.jsonl"); + await fs.writeFile(indexPath, "", "utf-8"); + + const stuck = await write("revision", 6, "# after wipe"); + expect(stuck.status).toBe(3); + const payload = JSON.parse(stuck.stdout ?? "{}"); + expect(payload.planning_stuck).toBe(true); + expect(payload.reason).toContain("on-disk openers"); + // Non-openers still escalate after untrusted ledger. + expect((await write("final", 6, "# final after wipe")).status).toBe(0); + }); + + it("fails closed when index.jsonl is truncated under on-disk openers", async () => { + const root = await tempDir(); + const runId = "trunc-cap"; + const write = async (stage: string, stageN: number, body: string) => + runNativeRalplanCommand( + ["--write", "--stage", stage, "--stage_n", String(stageN), "--artifact", body, "--run-id", runId, "--json"], + root, + ); + + expect((await write("planner", 1, "# p")).status).toBe(0); + for (let n = 2; n <= 5; n++) { + expect((await write("revision", n, `# r${n}`)).status).toBe(0); + } + + const indexPath = path.join(ralplanRunDir(root, runId), "index.jsonl"); + const full = await fs.readFile(indexPath, "utf-8"); + const firstLine = full.split(/\r?\n/).find(line => line.trim().length > 0) ?? ""; + await fs.writeFile(indexPath, `${firstLine}\n`, "utf-8"); + + const stuck = await write("revision", 6, "# after truncate"); + expect(stuck.status).toBe(3); + expect(JSON.parse(stuck.stdout ?? "{}").planning_stuck).toBe(true); + }); + + it("fails closed when index.jsonl is only malformed lines while openers exist on disk", async () => { + const root = await tempDir(); + const runId = "malformed-cap"; + const write = async (stage: string, stageN: number, body: string) => + runNativeRalplanCommand( + ["--write", "--stage", stage, "--stage_n", String(stageN), "--artifact", body, "--run-id", runId, "--json"], + root, + ); + + expect((await write("planner", 1, "# p")).status).toBe(0); + for (let n = 2; n <= 5; n++) { + expect((await write("revision", n, `# r${n}`)).status).toBe(0); + } + + const indexPath = path.join(ralplanRunDir(root, runId), "index.jsonl"); + await fs.writeFile(indexPath, '{not-json\nnot a row\n{"stage":1}\n', "utf-8"); + + const stuck = await write("revision", 6, "# after malformed"); + expect(stuck.status).toBe(3); + expect(JSON.parse(stuck.stdout ?? "{}").planning_stuck).toBe(true); + // architect/critic remain allowed (not openers) + expect((await write("architect", 6, "# a")).status).toBe(0); + expect((await write("critic", 6, "Verdict: ITERATE")).status).toBe(0); + }); + + it("fails closed when index is deleted but opener stage files remain", async () => { + const root = await tempDir(); + const runId = "delete-index-cap"; + const write = async (stage: string, stageN: number, body: string) => + runNativeRalplanCommand( + ["--write", "--stage", stage, "--stage_n", String(stageN), "--artifact", body, "--run-id", runId, "--json"], + root, + ); + + expect((await write("planner", 1, "# p")).status).toBe(0); + for (let n = 2; n <= 5; n++) { + expect((await write("revision", n, `# r${n}`)).status).toBe(0); + } + + await fs.rm(path.join(ralplanRunDir(root, runId), "index.jsonl"), { force: true }); + + const stuck = await write("revision", 6, "# after delete index"); + expect(stuck.status).toBe(3); + expect(JSON.parse(stuck.stdout ?? "{}").planning_stuck).toBe(true); + }); + + it("clean new run_id still allows openers after another run is ledger-stuck", async () => { + const root = await tempDir(); + const write = async (runId: string, stage: string, stageN: number, body: string) => + runNativeRalplanCommand( + ["--write", "--stage", stage, "--stage_n", String(stageN), "--artifact", body, "--run-id", runId, "--json"], + root, + ); + + expect((await write("run-old", "planner", 1, "# p")).status).toBe(0); + for (let n = 2; n <= 5; n++) { + expect((await write("run-old", "revision", n, `# r${n}`)).status).toBe(0); + } + await fs.writeFile(path.join(ralplanRunDir(root, "run-old"), "index.jsonl"), "", "utf-8"); + expect((await write("run-old", "revision", 6, "# stuck")).status).toBe(3); + + // Fresh run is independent even while the old run remains at cap under wipe. + expect((await write("run-new", "planner", 1, "# p-new")).status).toBe(0); + expect((await write("run-new", "revision", 2, "# r2-new")).status).toBe(0); + }); +}); diff --git a/packages/coding-agent/test/gjc-runtime/state-write-hardening.test.ts b/packages/coding-agent/test/gjc-runtime/state-write-hardening.test.ts index 3475f3a567..1ba873ac86 100644 --- a/packages/coding-agent/test/gjc-runtime/state-write-hardening.test.ts +++ b/packages/coding-agent/test/gjc-runtime/state-write-hardening.test.ts @@ -2,7 +2,8 @@ import { afterAll, afterEach, beforeAll, describe, expect, it, spyOn } from "bun import * as fs from "node:fs/promises"; import * as path from "node:path"; import { modeStatePath, sessionStateDir } from "@gajae-code/coding-agent/gjc-runtime/session-layout"; -import { runNativeStateCommand } from "@gajae-code/coding-agent/gjc-runtime/state-runtime"; +import { readWorkflowStateJson, runNativeStateCommand } from "@gajae-code/coding-agent/gjc-runtime/state-runtime"; +import * as logger from "@gajae-code/utils/logger"; const TEST_SESSION_ID = "test-session"; @@ -175,13 +176,24 @@ describe("gjc state write hardening", () => { const root = await tempDir(); await writeRawState(root, "ralplan", "{broken json"); const stderr = captureStderrWrites(); + const warn = spyOn(logger, "warn").mockImplementation(() => {}); try { const read = await runNativeStateCommand(["read", "--mode", "ralplan", "--json"], root); expect(read.status).toBe(0); const status = await runNativeStateCommand(["status", "--mode", "ralplan", "--json"], root); expect(status.status).toBe(0); - expect(stderr.writes.join("")).toContain("ignoring corrupt state"); + // #3002: never paint raw warning bytes onto the (TUI alternate-screen) stderr stream, + // but still surface the corrupt-state signal to CLI/automation via the structured + // command result and the TUI-safe file logger — so corrupt state stays distinguishable + // from absent state. + expect(stderr.writes.join("")).not.toContain("ignoring corrupt state"); + expect(read.stderr ?? "").toContain("ignoring corrupt state"); + expect(status.stderr ?? "").toContain("ignoring corrupt state"); + expect( + warn.mock.calls.some(call => typeof call[0] === "string" && call[0].includes("ignoring corrupt state")), + ).toBe(true); } finally { + warn.mockRestore(); stderr.restore(); } @@ -202,6 +214,27 @@ describe("gjc state write hardening", () => { expect(receiptFrom(forcedClear.stdout).current_phase).toBe("complete"); }); + it("in-process readWorkflowStateJson never writes corrupt-state warnings to process.stderr (#3002)", async () => { + // The interactive (in-process, no command-result) read path — e.g. ultragoal reading + // its own state while the TUI composer is live — must route the warning through the + // file logger only, never onto the terminal stream, and must fail open as {}. + const root = await tempDir(); + await writeRawState(root, "ralplan", "{broken json"); + const stderr = captureStderrWrites(); + const warn = spyOn(logger, "warn").mockImplementation(() => {}); + try { + const state = await readWorkflowStateJson(root, "ralplan", TEST_SESSION_ID); + expect(state).toEqual({}); + expect(stderr.writes.join("")).not.toContain("ignoring corrupt state"); + expect( + warn.mock.calls.some(call => typeof call[0] === "string" && call[0].includes("ignoring corrupt state")), + ).toBe(true); + } finally { + warn.mockRestore(); + stderr.restore(); + } + }); + it("allows seeds with no prior phase", async () => { const root = await tempDir(); const result = await writeState(root, "ralplan", { current_phase: "final" }); diff --git a/packages/coding-agent/test/model-selector-profiles-redteam.test.ts b/packages/coding-agent/test/model-selector-profiles-redteam.test.ts index 9599b07d35..10e07ed058 100644 --- a/packages/coding-agent/test/model-selector-profiles-redteam.test.ts +++ b/packages/coding-agent/test/model-selector-profiles-redteam.test.ts @@ -207,6 +207,19 @@ describe("model selector profile red-team", () => { ]); }); + test("shortcut 'd' key activates profile with Set as default (setDefault: true)", async () => { + const selections: ModelSelectorSelection[] = []; + const selector = createSelector(selection => { + selections.push(selection); + }); + await renderSelector(selector); + selector.handleInput("\x1b[C"); + selector.handleInput("\x1b[B"); + selector.handleInput("\n"); + selector.handleInput("d"); + + expect(selections).toEqual([{ kind: "profile", profileName: "profile-a", setDefault: true }]); + }); test("controller persists only Set as default and leaves Apply for this session non-default", async () => { const sessionOnly = createControllerContext(); await selectProfileThroughController(new SelectorController(sessionOnly.ctx as never), false); diff --git a/packages/coding-agent/test/routing-adversarial.test.ts b/packages/coding-agent/test/routing-adversarial.test.ts index e726a89377..076080f8b9 100644 --- a/packages/coding-agent/test/routing-adversarial.test.ts +++ b/packages/coding-agent/test/routing-adversarial.test.ts @@ -13,6 +13,7 @@ import { } from "@gajae-code/coding-agent/session/cache-economics"; import { cappedExponentialWithFullJitter, + compactionRetryDelay, effectiveFallbackDelay, FallbackChainController, } from "@gajae-code/coding-agent/session/fallback-chain-controller"; @@ -60,6 +61,74 @@ describe("routing adversarial contract probes", () => { expect(effectiveFallbackDelay(100, 1_000, 1, THREE_HOURS_MS, () => 1)).toBe(THREE_HOURS_MS); }); + // Mirror image of the contract above: auto-compaction recovers Retry-After by + // regex over provider error prose, so it is legacy and MUST stay capped. + // Managed fallback is uncapped only because it retries within its own + // per-entry budget; compaction has no such budget. + test("caps legacy compaction retry-after at retry.maxDelayMs", () => { + // A hostile/misconfigured provider asking for 3h cannot outrun the cap. + expect(compactionRetryDelay(100, 1_000, 0, THREE_HOURS_MS)).toBe(1_000); + // Same hint, managed fallback path: still honoured verbatim. + expect(effectiveFallbackDelay(100, 1_000, 1, THREE_HOURS_MS, () => 1)).toBe(THREE_HOURS_MS); + }); + + test("compaction retry delay honours hints below the cap and keeps exponential growth", () => { + // No hint → plain exponential (base * 2**attempt), unchanged behaviour. + expect(compactionRetryDelay(2_000, 300_000, 0, undefined)).toBe(2_000); + expect(compactionRetryDelay(2_000, 300_000, 3, undefined)).toBe(16_000); + // Hint below the cap wins over the exponential, exactly as before. + expect(compactionRetryDelay(2_000, 300_000, 0, 45_000)).toBe(45_000); + // Hint smaller than the exponential never shortens the backoff. + expect(compactionRetryDelay(2_000, 300_000, 3, 1_000)).toBe(16_000); + }); + + test("compaction retry delay never yields a negative, NaN, or infinite sleep", () => { + for (const hint of [undefined, Number.NaN, Number.POSITIVE_INFINITY, -5_000]) { + const delay = compactionRetryDelay(2_000, 300_000, 0, hint); + expect(Number.isFinite(delay)).toBe(true); + expect(delay).toBeGreaterThanOrEqual(0); + expect(delay).toBeLessThanOrEqual(300_000); + } + // maxDelayMs <= 0 means "no cap", matching cappedExponentialWithFullJitter. + expect(compactionRetryDelay(2_000, 0, 0, 45_000)).toBe(45_000); + }); + + // Both legacy surfaces recover Retry-After from prose, so the documented rule + // ("retry.maxDelayMs caps every legacy session retry delay, including provider + // retry-after hints") must bind them identically. This pins the two together so + // a future change to one cannot silently drift from the other. + test("legacy compaction agrees with the legacy non-compaction retry-after cap", () => { + const maxDelayMs = 300_000; + const baseDelayMs = 2_000; + for (const hint of [1_000, 45_000, 299_999, 300_000, 300_001, THREE_HOURS_MS]) { + // agent-session.ts, legacy non-compaction path: Math.min(retryAfterMs, maxDelayMs) + const legacyNonCompaction = Math.min(hint, maxDelayMs); + const compaction = compactionRetryDelay(baseDelayMs, maxDelayMs, 0, hint); + expect(compaction).toBeLessThanOrEqual(maxDelayMs); + // Compaction may floor at its exponential, but never exceeds the legacy bound. + expect(compaction).toBeLessThanOrEqual(Math.max(legacyNonCompaction, baseDelayMs)); + } + }); + + test("compaction retry delay invariant holds across the whole parameter grid", () => { + const hints = [undefined, 0, 1_000, THREE_HOURS_MS, Number.NaN, Number.POSITIVE_INFINITY, -1]; + let checked = 0; + for (const baseDelayMs of [0, 1, 500, 2_000, 60_000]) { + for (const maxDelayMs of [0, 1_000, 30_000, 300_000]) { + for (const attempt of [0, 1, 3, 10, 30]) { + for (const hint of hints) { + const delay = compactionRetryDelay(baseDelayMs, maxDelayMs, attempt, hint); + expect(Number.isFinite(delay)).toBe(true); + expect(delay).toBeGreaterThanOrEqual(0); + if (maxDelayMs > 0) expect(delay).toBeLessThanOrEqual(maxDelayMs); + checked++; + } + } + } + } + expect(checked).toBe(700); + }); + test("tries a rotated Fable credential once before falling back to Opus", () => { const fable = "anthropic/claude-fable-5:high"; const opus = "anthropic/claude-opus-5:high"; diff --git a/packages/coding-agent/test/sdk-daemon-cli-e2e.test.ts b/packages/coding-agent/test/sdk-daemon-cli-e2e.test.ts index 76d2d2eced..1e6712142d 100644 --- a/packages/coding-agent/test/sdk-daemon-cli-e2e.test.ts +++ b/packages/coding-agent/test/sdk-daemon-cli-e2e.test.ts @@ -12,6 +12,17 @@ type CliResult = { exitCode: number; stdout: string; stderr: string }; // Capture through files rather than pipes: a piped child that outlives the // parent's read teardown can be killed by SIGPIPE (exit 141) under CI load, // which masks the CLI's real exit contract. +function closeCaptureFd(fd: number): void { + // Bun.spawn may close inherited capture FDs when a short-lived child exits, + // especially on fail-closed CLI paths. Ignore EBADF so teardown does not + // mask the CLI exit contract under CI load (see shard-6 post-#3076 red). + try { + closeSync(fd); + } catch (error) { + if ((error as NodeJS.ErrnoException | undefined)?.code !== "EBADF") throw error; + } +} + async function runCli(repo: string, agentDir: string, args: string[]): Promise { const captureDir = await fs.mkdtemp(path.join(os.tmpdir(), "gjc-sdk-cli-capture-")); const stdoutPath = path.join(captureDir, "stdout"); @@ -26,14 +37,18 @@ async function runCli(repo: string, agentDir: string, args: string[]): Promise = {}): Args { } function fakeSessionResult(): CreateAgentSessionResult { + let activeModel = testModel; const session = { - model: testModel, + get model() { + return activeModel; + }, extensionRunner: undefined, + getConfiguredModelChain: () => undefined, + setConfiguredModelChain: () => {}, + setModelTemporary: async (model: typeof testModel) => { + activeModel = model; + }, dispose: async () => {}, } as unknown as AgentSession; return { @@ -751,6 +759,7 @@ describe("startup update contract", () => { const exitSpy = vi.spyOn(process, "exit").mockImplementation((): never => { throw exit; }); + const getApiKeySpy = vi.spyOn(AuthStorage.prototype, "getApiKey").mockResolvedValue(undefined); const stderr: string[] = []; const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation((chunk: string | Uint8Array) => { stderr.push(typeof chunk === "string" ? chunk : Buffer.from(chunk).toString("utf8")); @@ -777,6 +786,7 @@ describe("startup update contract", () => { expect(stderr.join("")).toContain('Model profile "codex-medium" requires credentials for: openai-codex'); } finally { stderrSpy.mockRestore(); + getApiKeySpy.mockRestore(); exitSpy.mockRestore(); authStorage.close(); } diff --git a/packages/coding-agent/test/task-output-limit-env.test.ts b/packages/coding-agent/test/task-output-limit-env.test.ts new file mode 100644 index 0000000000..0a7df263f7 --- /dev/null +++ b/packages/coding-agent/test/task-output-limit-env.test.ts @@ -0,0 +1,80 @@ +import { describe, expect, it } from "bun:test"; +import * as path from "node:path"; + +interface TaskOutputLimits { + bytes: number; + lines: number; +} + +const taskTypesPath = path.resolve(import.meta.dir, "../src/task/types.ts"); +const defaults: TaskOutputLimits = { bytes: 500_000, lines: 5000 }; + +async function readTaskOutputLimits(overrides: Record = {}): Promise { + const env = { ...process.env }; + delete env.GJC_TASK_MAX_OUTPUT_BYTES; + delete env.PI_TASK_MAX_OUTPUT_BYTES; + delete env.GJC_TASK_MAX_OUTPUT_LINES; + delete env.PI_TASK_MAX_OUTPUT_LINES; + Object.assign(env, overrides); + + const script = ` + const taskTypes = await import(${JSON.stringify(taskTypesPath)}); + process.stdout.write(JSON.stringify({ + bytes: taskTypes.MAX_OUTPUT_BYTES, + lines: taskTypes.MAX_OUTPUT_LINES, + })); + `; + const child = Bun.spawn([process.execPath, "--eval", script], { + env, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(child.stdout).text(), + new Response(child.stderr).text(), + child.exited, + ]); + if (exitCode !== 0) throw new Error(`task limit probe failed (${exitCode}): ${stderr}`); + return JSON.parse(stdout) as TaskOutputLimits; +} + +describe("task output limit environment parsing", () => { + it("uses documented defaults when overrides are absent", async () => { + expect(await readTaskOutputLimits()).toEqual(defaults); + }); + + it("accepts complete positive decimal integers from canonical variables", async () => { + expect( + await readTaskOutputLimits({ + GJC_TASK_MAX_OUTPUT_BYTES: "00064000", + GJC_TASK_MAX_OUTPUT_LINES: "250", + }), + ).toEqual({ bytes: 64_000, lines: 250 }); + }); + + it("keeps compatibility aliases when canonical variables are absent", async () => { + expect( + await readTaskOutputLimits({ + PI_TASK_MAX_OUTPUT_BYTES: "32000", + PI_TASK_MAX_OUTPUT_LINES: "125", + }), + ).toEqual({ bytes: 32_000, lines: 125 }); + }); + + it.each([ + "500000oops", + "1.5", + "1e3", + " 12 ", + "0", + "-1", + "9007199254740992", + ])("falls back for invalid or inexact override %j", async value => { + expect( + await readTaskOutputLimits({ + GJC_TASK_MAX_OUTPUT_BYTES: value, + GJC_TASK_MAX_OUTPUT_LINES: value, + }), + ).toEqual(defaults); + }); +}); diff --git a/packages/coding-agent/test/welcome-viewport.test.ts b/packages/coding-agent/test/welcome-viewport.test.ts index 5d8c5427db..28e6ab156b 100644 --- a/packages/coding-agent/test/welcome-viewport.test.ts +++ b/packages/coding-agent/test/welcome-viewport.test.ts @@ -76,6 +76,28 @@ describe("WelcomeComponent viewport sizing", () => { } }); + it("reserves the composer gutter for normal and one-row welcome layouts", () => { + const normal = new WelcomeComponent("1.2.3", "test-model", "test-provider", [], [], "ascii", { + rightGutterWidth: 1, + }); + for (const line of normal.render(100).map(stripRenderControls)) { + expect(visibleWidth(line)).toBe(100); + expect(line.endsWith(" ")).toBe(true); + expect(visibleWidth(line.trimEnd())).toBe(99); + } + + const constrained = new WelcomeComponent("1.2.3", "test-model", "test-provider", [], [], "ascii", { + rightGutterWidth: 1, + getViewportRows: () => 1, + getReservedBottomRows: () => 0, + }); + const lines = constrained.render(100).map(stripRenderControls); + expect(lines).toHaveLength(1); + expect(visibleWidth(lines[0]!)).toBe(100); + expect(lines[0]!.endsWith(" ")).toBe(true); + expect(visibleWidth(lines[0]!.trimEnd())).toBe(99); + }); + it("renders the build label from metadata instead of defaulting to dev", () => { const welcome = new WelcomeComponent("1.2.3", "test-model", "test-provider", [], [], "ascii", { buildLabel: "release build", diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index dc3ef0ce2a..6e660a6ed4 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Strict CLI commands now reject unexpected positional arguments with usage guidance instead of silently ignoring typos or unsupported trailing input; non-strict passthrough commands and variadic arguments retain their existing behavior (#3173). + ## [0.11.10] - 2026-07-25 ## [0.11.9] - 2026-07-24 @@ -9,6 +13,11 @@ ### Fixed - Fatal crashes (`uncaughtException` / `unhandledRejection`) are now also persisted to a dedicated, append-only crash log (`~/.gjc/agent/gjc-crash.log`) before any stderr output, and the fatal handler prints the crash-log path. The daily logger file is gzip-archived independently by every gjc process at date rollover; that shared-archive race can truncate a day's log to an empty `.gz` and destroy the `logger.error` crash record, leaving crashes undiagnosable. The rotation-immune crash log is capped at 512 KB, bounds every individual record (UTF-8-safe truncation with a marker), scrubs credential material (bearer/auth headers, key=value credential fields, and well-known vendor token shapes) before persisting, and enforces owner-only file permissions. +### Fixed + +- Integer CLI flags now reject trailing characters, decimals, exponent notation, surrounding whitespace, and values outside JavaScript's safe-integer range instead of silently truncating or rounding them. + +## [0.11.8] - 2026-07-23 ## [0.11.7] - 2026-07-22 ### Added diff --git a/packages/utils/src/cli.ts b/packages/utils/src/cli.ts index 3626d65bcf..438c9d3ddf 100644 --- a/packages/utils/src/cli.ts +++ b/packages/utils/src/cli.ts @@ -211,8 +211,11 @@ export abstract class Command { if (raw === undefined || typeof raw === "boolean") { flags[name] = desc.default ?? undefined; } else { - const n = Number.parseInt(raw as string, 10); - if (Number.isNaN(n)) { + if (typeof raw !== "string" || !/^-?\d+$/.test(raw)) { + throw new CliParseError(`Expected integer for --${name}, got "${String(raw)}"`); + } + const n = Number(raw); + if (!Number.isSafeInteger(n)) { throw new CliParseError(`Expected integer for --${name}, got "${raw}"`); } flags[name] = n; @@ -267,6 +270,12 @@ export abstract class Command { } } + if (strict && posIdx < positionals.length) { + const unexpected = positionals.slice(posIdx); + const rendered = unexpected.map(value => JSON.stringify(value)).join(", "); + throw new CliParseError(`Unexpected argument${unexpected.length === 1 ? "" : "s"}: ${rendered}`); + } + return { flags, args, argv: positionals } as never; } } diff --git a/packages/utils/test/cli.test.ts b/packages/utils/test/cli.test.ts index a6dc114333..1211afc763 100644 --- a/packages/utils/test/cli.test.ts +++ b/packages/utils/test/cli.test.ts @@ -12,6 +12,7 @@ class Demo extends Command { }; static flags = { scope: Flags.string({ description: "scope", options: ["user", "project"] }), + count: Flags.integer({ description: "count" }), }; async run(): Promise { const { args } = await this.parse(Demo); @@ -103,6 +104,60 @@ describe("cli parse — CliParseError for invalid input", () => { await expect(cmd.parse(Demo)).rejects.toThrow(/Expected --scope to be one of: user, project/); }); + it.each([ + "12oops", + "1.5", + "1e3", + " 12 ", + "9007199254740992", + ])("throws CliParseError for a non-exact integer flag value %j", async value => { + const cmd = new Demo(["build", "--count", value], CFG); + await expect(cmd.parse(Demo)).rejects.toBeInstanceOf(CliParseError); + await expect(cmd.parse(Demo)).rejects.toThrow(`Expected integer for --count, got "${value}"`); + }); + + it.each([ + ["--count=0", 0], + ["--count=-12", -12], + ["--count=0012", 12], + ])("accepts exact integer flag token %j", async (flag, expected) => { + const cmd = new Demo(["build", flag], CFG); + const { flags } = await cmd.parse(Demo); + expect(flags.count).toBe(expected); + }); + + it("throws CliParseError for extra positional arguments in strict commands", async () => { + const cmd = new Demo(["build", "unexpected", "also-unexpected"], CFG); + await expect(cmd.parse(Demo)).rejects.toBeInstanceOf(CliParseError); + await expect(cmd.parse(Demo)).rejects.toThrow('Unexpected arguments: "unexpected", "also-unexpected"'); + }); + + it("preserves extra positional arguments for commands that opt out of strict parsing", async () => { + class NonStrict extends Command { + static strict = false; + static args = { action: Args.string({}) }; + async run(): Promise { + await this.parse(NonStrict); + } + } + const cmd = new NonStrict(["build", "passthrough"], CFG); + const parsed = await cmd.parse(NonStrict); + expect(parsed.args.action).toBe("build"); + expect(parsed.argv).toEqual(["build", "passthrough"]); + }); + + it("allows a multiple positional descriptor to consume every remaining argument", async () => { + class Multiple extends Command { + static args = { values: Args.string({ multiple: true }) }; + async run(): Promise { + await this.parse(Multiple); + } + } + const cmd = new Multiple(["one", "two"], CFG); + const parsed = await cmd.parse(Multiple); + expect(parsed.args.values).toEqual(["one", "two"]); + }); + it("wraps node:util unknown-flag errors as CliParseError (strict command)", async () => { class Strict extends Command { static flags = { verbose: Flags.boolean({}) }; @@ -143,6 +198,15 @@ describe("cli run — usage instead of uncaught crash", () => { expect(sideEffect.ran).toBe(false); // command body never ran }); + it("renders usage and skips execution for extra positional arguments", async () => { + sideEffect.ran = false; + const { err, out, exitCode } = await runCapturing(["demo", "build", "extra"]); + expect(err).toContain('Unexpected argument: "extra"'); + expect(out.toLowerCase()).toContain("usage"); + expect(exitCode).toBe(2); + expect(sideEffect.ran).toBe(false); + }); + it("runs the command normally for valid input and leaves exitCode unset", async () => { sideEffect.ran = false; const { exitCode } = await runCapturing(["demo", "build"]);