diff --git a/benchmark/SKILL.md b/benchmark/SKILL.md index 56c76e733..ee09bbba4 100644 --- a/benchmark/SKILL.md +++ b/benchmark/SKILL.md @@ -412,7 +412,7 @@ TOP 10 SLOWEST RESOURCES RECOMMENDATIONS: - vendor.chunk.js: code-split — 320KB on initial load - analytics.js: load async/defer — blocks rendering for 250ms -- hero-image.webp: add width/height (CLS), lazy-load +- hero-image.webp: add width/height (CLS) ``` Third-party = any host but the page's origin. Each recommendation names a resource here or in a Phase 5 regression, the metric it moves and one remedy (split, defer, compress, lazy-load, cache). diff --git a/benchmark/SKILL.md.tmpl b/benchmark/SKILL.md.tmpl index 12a805eec..0a7448a29 100644 --- a/benchmark/SKILL.md.tmpl +++ b/benchmark/SKILL.md.tmpl @@ -201,7 +201,7 @@ TOP 10 SLOWEST RESOURCES RECOMMENDATIONS: - vendor.chunk.js: code-split — 320KB on initial load - analytics.js: load async/defer — blocks rendering for 250ms -- hero-image.webp: add width/height (CLS), lazy-load +- hero-image.webp: add width/height (CLS) ``` Third-party = any host but the page's origin. Each recommendation names a resource here or in a Phase 5 regression, the metric it moves and one remedy (split, defer, compress, lazy-load, cache). diff --git a/plan-eng-review/SKILL.md b/plan-eng-review/SKILL.md index a35ce149f..295c9eef5 100644 --- a/plan-eng-review/SKILL.md +++ b/plan-eng-review/SKILL.md @@ -50,7 +50,7 @@ Recommendation: A when a branch diff exists, otherwise B. Reply with A, B, or C. After target selection, use the preamble's full decision brief, transport and continuous D-numbering. Setup questions approve no engineering remedies. -**Format precedence:** Copy required command, output and question formats exactly. Apply Voice to newly composed prose. +**Format precedence:** Copy required command, output and question formats exactly, em dashes included. Apply Voice to newly composed prose. **Startup sequence** (after target selection): 1. Run the Preamble command and its startup instructions (Context Recovery and the preamble's own one-time setup prompts; the Prior Learnings cross-project question waits for Step 0). Defer Operational Self-Improvement, Telemetry and Plan Status Footer to finish; format/transport rules apply throughout. @@ -187,28 +187,6 @@ Before emitting a tool or prose decision brief, verify: - [ ] Follow Tool resolution: tool call unless Conductor or documented prose fallback; prose includes the mandatory triad + explicit reply selectors, then STOP. Spawned sessions follow their auto-choice rule. - [ ] Write non-ASCII directly, not \u-escaped. For 5+ options, split/batch into ≤4 without dropping; check dependencies and stop the chain immediately on Hold. -## Voice - -GStack voice: Garry-shaped product and engineering judgment. - -- Lead with the point. Say what it does, why it matters, and what changes for the builder. -- Be concrete. Name files, functions, line numbers, commands, outputs, evals, and real numbers. -- Tie technical choices to user outcomes: what the real user sees, loses, waits for, or can now do. -- Be direct about quality. Bugs matter. Edge cases matter. Fix the whole thing, not the demo path. -- Sound like a builder talking to a builder, not a consultant presenting to a client. -- Never corporate, academic, PR, or hype. Avoid filler, throat-clearing, generic optimism, and founder cosplay. -- Do not add em dashes in prose you compose during the review. Existing templates, quoted text, command output, and required copied labels may contain them. No AI vocabulary: delve, crucial, robust, comprehensive, nuanced, multifaceted, furthermore, moreover, additionally, pivotal, landscape, tapestry, underscore, foster, showcase, intricate, vibrant, fundamental, significant, load-bearing. -- Reply in the language of the user's latest message unless asked otherwise. Code, commands, paths, identifiers, quoted output and question markers (`D`, option letters, `(recommended)`) stay verbatim. -- The user has context you do not: domain knowledge, timing, relationships, taste. Cross-model agreement is a recommendation, not a decision. The user decides. - -Good: "auth.ts:47 returns undefined when the session cookie expires. Users hit a white screen. Fix: add a null check and redirect to /login. Two lines." -Bad: "I've identified a potential issue in the authentication flow that may cause problems under certain conditions." - -**Bounded closer.** After completing work, report in at most a few short lines: what changed, what was skipped, what to watch. No feature tours or unrequested design notes. Exempt: decision briefs, completion-status blocks, requested explanations, and a skill's mandated report (/qa-only, /plan-*-review, /retro, /document-generate). The rule limits prose around the deliverable, never the deliverable. - -Good closer: "Renamed the flag in 3 files, regenerated docs, tests green. Skipped the CLI alias (unused since v1.2); watch the Windows job." -Bad closer: a tour of every edit, a restatement of the plan, and three paragraphs justifying choices nobody questioned. - ## Question Tuning (skip entirely if `QUESTION_TUNING: false`) Before each decision brief (AskUserQuestion or Conductor/fallback prose), choose `question_id` from `~/.claude/skills/gstack/scripts/question-registry.ts` or `{skill}-{slug}`, then run `~/.claude/skills/gstack/bin/gstack-question-preference --check ""`; for an unregistered id, write the question summary to `.gstack/tmp/qt.txt` (file-write tool) and append `--summary-file .gstack/tmp/qt.txt` (one-way keyword check). `AUTO_DECIDE` means choose the recommended option and say "Auto-decided [summary] → [option] (your preference). Change with /plan-tune." `ASK_NORMALLY` means ask. diff --git a/plan-eng-review/SKILL.md.tmpl b/plan-eng-review/SKILL.md.tmpl index 66d1efa95..3bda37347 100644 --- a/plan-eng-review/SKILL.md.tmpl +++ b/plan-eng-review/SKILL.md.tmpl @@ -60,7 +60,7 @@ Recommendation: A when a branch diff exists, otherwise B. Reply with A, B, or C. After target selection, use the preamble's full decision brief, transport and continuous D-numbering. Setup questions approve no engineering remedies. -**Format precedence:** Copy required command, output and question formats exactly. Apply Voice to newly composed prose. +**Format precedence:** Copy required command, output and question formats exactly, em dashes included. Apply Voice to newly composed prose. **Startup sequence** (after target selection): 1. Run the Preamble command and its startup instructions (Context Recovery and the preamble's own one-time setup prompts; the Prior Learnings cross-project question waits for Step 0). Defer Operational Self-Improvement, Telemetry and Plan Status Footer to finish; format/transport rules apply throughout. diff --git a/review/SKILL.md b/review/SKILL.md index 9c54f19aa..cb0850c69 100644 --- a/review/SKILL.md +++ b/review/SKILL.md @@ -806,7 +806,7 @@ After outputting your own findings, if Greptile comments were classified in Step Before replying to any comment, run greptile-triage.md's **Escalation Detection** to choose Tier 1 (friendly) or Tier 2 (firm) reply templates. -1. **VALID & ACTIONABLE comments:** Use their Step 5a–5d disposition; do not ask a second fix question. Step 5c alone supplies A) Fix / B) Skip for ASK items. After a completed fix, use the **Fix reply template** with diff and explanation; cite the current diff if uncommitted, never invent a commit SHA. A Skip leaves the defect unresolved and grants no new fix permission. If evidence disproves the finding, reclassify it below. +1. **VALID & ACTIONABLE comments:** Use their Step 5a–5d disposition; do not ask a second fix question. After a completed fix, use the **Fix reply template** with diff and explanation; cite the current diff if uncommitted, never invent a commit SHA. A Skip leaves the defect unresolved and grants no new fix permission. If evidence disproves the finding, reclassify it below. 2. **FALSE POSITIVE comments:** These are reply decisions, not code approval. Show file:line (or [top-level]), summary, permalink and evidence, then ask: - A) Reply explaining why this is incorrect (recommended if clearly wrong) @@ -835,6 +835,8 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection* final: repeat Steps 3–5 with a full start. 2. On a repeat, execute Steps 3–5 in order. At Step 4.7, an edited pass supersedes every earlier QA capture (exploratory §4): rerun each probe the verdict relies on. + These reruns are revalidation, so they run even after smoke expiry; keep the + PROBE_DIR and run its once-only `materialize` on the final pass. A zero-edit pass reuses only this invocation's unchanged-input QA evidence, rerunning any probe whose inputs changed or are uncertain. Reusing never skips a review step. 3. **Verify completed actions.** On the final zero-edit pass, reconcile this @@ -846,9 +848,8 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection* defects and unresolved counts; earlier fixes do not suppress them. 4. **Recheck skipped advice.** Re-read its final-snapshot supporting source and reconfirm the decision; otherwise report its history without a reusable skip. - The logger computes `snapshot_covered_paths` from eligible paths whose raw bytes - equal the bound snapshot blobs (`[]` if none). Never carry prior-cycle, supplied - or prior-record coverage forward or build this proof yourself. Fixed advice + The logger computes `snapshot_covered_paths` itself; never carry prior-cycle, + supplied or prior-record coverage forward or build this proof yourself. Fixed advice needs no skip coverage. ### 2. Fill the record @@ -878,7 +879,8 @@ pass: report it with its reason. `helper_target`. Recheck source after fixes. The logger uses `sharedLibsFingerprint`, never supplied/model hashes. Actions: `auto-fixed` (Step 5b), `fixed` (approved **and completed** in Step 5d), - `skipped` (explicit Skip in Step 5c). Advice is never `auto-fixed`; pending + `skipped` (explicit Skip in Step 5c), + `open` (any other defect, e.g. unanswered). Advice is never `auto-fixed`; pending advice stays in the response, not the record. Exclude prior Step 5.0 suppressions; include this invocation's revalidated decisions. - Gate findings additionally carry `gate` (the RH tag, e.g. `"RH-15"`), `gate_id` @@ -896,9 +898,8 @@ pass: report it with its reason. `TIMESTAMP` is ISO 8601; `COMMIT` is `git rev-parse --short HEAD`. `quality_score` is Step 4.6's specialist score (`10.0` when small-diff specialists -were skipped or this host omits Review Army). This default is not completion evidence; -unresolved non-advisory core defects still count in `issues_found`, -`critical`, `informational`. The logger builds trusted `review_binding` from the +were skipped or this host omits Review Army). This default is not completion evidence +and never lowers defect counts. The logger builds trusted `review_binding` from the validated captured branch digest, discarding caller bindings. Never invent a binding or replace REVIEW_START at log time; finish only this pass's own token. diff --git a/review/SKILL.md.tmpl b/review/SKILL.md.tmpl index c613dae8c..4bc518bfb 100644 --- a/review/SKILL.md.tmpl +++ b/review/SKILL.md.tmpl @@ -357,7 +357,7 @@ After outputting your own findings, if Greptile comments were classified in Step Before replying to any comment, run greptile-triage.md's **Escalation Detection** to choose Tier 1 (friendly) or Tier 2 (firm) reply templates. -1. **VALID & ACTIONABLE comments:** Use their Step 5a–5d disposition; do not ask a second fix question. Step 5c alone supplies A) Fix / B) Skip for ASK items. After a completed fix, use the **Fix reply template** with diff and explanation; cite the current diff if uncommitted, never invent a commit SHA. A Skip leaves the defect unresolved and grants no new fix permission. If evidence disproves the finding, reclassify it below. +1. **VALID & ACTIONABLE comments:** Use their Step 5a–5d disposition; do not ask a second fix question. After a completed fix, use the **Fix reply template** with diff and explanation; cite the current diff if uncommitted, never invent a commit SHA. A Skip leaves the defect unresolved and grants no new fix permission. If evidence disproves the finding, reclassify it below. 2. **FALSE POSITIVE comments:** These are reply decisions, not code approval. Show file:line (or [top-level]), summary, permalink and evidence, then ask: - A) Reply explaining why this is incorrect (recommended if clearly wrong) @@ -386,6 +386,8 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection* final: repeat Steps 3–5 with a full start. 2. On a repeat, execute Steps 3–5 in order. At Step 4.7, an edited pass supersedes every earlier QA capture (exploratory §4): rerun each probe the verdict relies on. + These reruns are revalidation, so they run even after smoke expiry; keep the + PROBE_DIR and run its once-only `materialize` on the final pass. A zero-edit pass reuses only this invocation's unchanged-input QA evidence, rerunning any probe whose inputs changed or are uncertain. Reusing never skips a review step. 3. **Verify completed actions.** On the final zero-edit pass, reconcile this @@ -397,9 +399,8 @@ Before replying to any comment, run greptile-triage.md's **Escalation Detection* defects and unresolved counts; earlier fixes do not suppress them. 4. **Recheck skipped advice.** Re-read its final-snapshot supporting source and reconfirm the decision; otherwise report its history without a reusable skip. - The logger computes `snapshot_covered_paths` from eligible paths whose raw bytes - equal the bound snapshot blobs (`[]` if none). Never carry prior-cycle, supplied - or prior-record coverage forward or build this proof yourself. Fixed advice + The logger computes `snapshot_covered_paths` itself; never carry prior-cycle, + supplied or prior-record coverage forward or build this proof yourself. Fixed advice needs no skip coverage. ### 2. Fill the record @@ -429,7 +430,8 @@ pass: report it with its reason. `helper_target`. Recheck source after fixes. The logger uses `sharedLibsFingerprint`, never supplied/model hashes. Actions: `auto-fixed` (Step 5b), `fixed` (approved **and completed** in Step 5d), - `skipped` (explicit Skip in Step 5c). Advice is never `auto-fixed`; pending + `skipped` (explicit Skip in Step 5c), + `open` (any other defect, e.g. unanswered). Advice is never `auto-fixed`; pending advice stays in the response, not the record. Exclude prior Step 5.0 suppressions; include this invocation's revalidated decisions. - Gate findings additionally carry `gate` (the RH tag, e.g. `"RH-15"`), `gate_id` @@ -447,9 +449,8 @@ pass: report it with its reason. `TIMESTAMP` is ISO 8601; `COMMIT` is `git rev-parse --short HEAD`. `quality_score` is Step 4.6's specialist score (`10.0` when small-diff specialists -were skipped or this host omits Review Army). This default is not completion evidence; -unresolved non-advisory core defects still count in `issues_found`, -`critical`, `informational`. The logger builds trusted `review_binding` from the +were skipped or this host omits Review Army). This default is not completion evidence +and never lowers defect counts. The logger builds trusted `review_binding` from the validated captured branch digest, discarding caller bindings. Never invent a binding or replace REVIEW_START at log time; finish only this pass's own token. diff --git a/scripts/resolvers/preamble/generate-voice-directive.ts b/scripts/resolvers/preamble/generate-voice-directive.ts index 13813bf4c..4d220d53d 100644 --- a/scripts/resolvers/preamble/generate-voice-directive.ts +++ b/scripts/resolvers/preamble/generate-voice-directive.ts @@ -2,9 +2,7 @@ import type { TemplateContext } from '../types'; export function generateVoiceDirective(ctx: TemplateContext, tier: number): string { - const emDashRule = ctx.skillName === 'plan-eng-review' - ? 'Do not add em dashes in prose you compose during the review. Existing templates, quoted text, command output, and required copied labels may contain them.' - : 'No em dashes.'; + const emDashRule = 'No em dashes.'; if (tier <= 1) { return `## Voice diff --git a/test/eng-scope-entry-ap.test.ts b/test/eng-scope-entry-ap.test.ts index 9989b2f67..7ffc8ef4f 100644 --- a/test/eng-scope-entry-ap.test.ts +++ b/test/eng-scope-entry-ap.test.ts @@ -163,3 +163,16 @@ test('calibration keeps its future hook but no shipped source enables the absent } expect(mentions).toEqual(['scripts/resolvers/gbrain.ts']); }); + +test('the judged Eng bundle carries the shared Voice once and states its em-dash precedence in the Scope gate (paid screen 40634787316)', () => { + // Five of six samples (red panel and rerun) named Eng's near-copy of the shared Voice, whose only difference was + // its em-dash clause, as a rule the agent must reconcile; the Scope gate's Format precedence already owned it. + // fails_when=a second Voice section returns to the judged bundle or the precedence stops naming em dashes. + const input = readWorkflowJudgeInput({root: path.join(import.meta.dir, '..'), skillPath: 'plan-eng-review/SKILL.md', + startMarker: '# Plan Review Mode', endMarker: null}); + expect(input.text.match(/^## Voice$/gm)).toHaveLength(1); + expect(input.text).not.toContain('Do not add em dashes in prose you compose'); + expect(scope).toContain('**Format precedence:** Copy required command, output and question formats exactly, em dashes included. Apply Voice to newly composed prose.'); + const startup = input.files[0]!.content; + expect(startup.indexOf('**Format precedence:**')).toBeLessThan(startup.indexOf('## Shared preamble (load first)')); +}); diff --git a/test/fixtures/skill-md-size-allowlist.json b/test/fixtures/skill-md-size-allowlist.json index 686b6b63d..b801716cf 100644 --- a/test/fixtures/skill-md-size-allowlist.json +++ b/test/fixtures/skill-md-size-allowlist.json @@ -5,50 +5,50 @@ "agy/gstack-autoplan": 109810, "agy/gstack-plan-design-review": 110044, "agy/gstack-plan-devex-review": 116496, - "agy/gstack-plan-eng-review": 147241, - "agy/gstack-review": 103755, + "agy/gstack-plan-eng-review": 144907, + "agy/gstack-review": 103750, "codex/gstack-autoplan": 108553, "codex/gstack-land-and-deploy": 100284, "codex/gstack-plan-design-review": 111116, "codex/gstack-plan-devex-review": 118004, - "codex/gstack-plan-eng-review": 148406, - "codex/gstack-review": 103278, + "codex/gstack-plan-eng-review": 146072, + "codex/gstack-review": 103273, "copilot/gstack-autoplan": 109253, "copilot/gstack-plan-design-review": 110355, "copilot/gstack-plan-devex-review": 115958, - "copilot/gstack-plan-eng-review": 147406, - "copilot/gstack-review": 103336, + "copilot/gstack-plan-eng-review": 145072, + "copilot/gstack-review": 103331, "cursor/gstack-autoplan": 108485, "cursor/gstack-plan-design-review": 108900, "cursor/gstack-plan-devex-review": 115228, - "cursor/gstack-plan-eng-review": 145996, - "cursor/gstack-review": 121626, + "cursor/gstack-plan-eng-review": 143662, + "cursor/gstack-review": 121621, "factory/gstack-autoplan": 110763, "factory/gstack-land-and-deploy": 100105, "factory/gstack-plan-design-review": 111675, "factory/gstack-plan-devex-review": 118591, - "factory/gstack-plan-eng-review": 149104, - "factory/gstack-review": 125399, + "factory/gstack-plan-eng-review": 146770, + "factory/gstack-review": 125394, "gbrain/gstack-autoplan": 108892, - "gbrain/gstack-plan-eng-review": 128623, + "gbrain/gstack-plan-eng-review": 126289, "hermes/gstack-autoplan": 108982, - "hermes/gstack-plan-eng-review": 128708, + "hermes/gstack-plan-eng-review": 126374, "kiro/gstack-autoplan": 108259, "kiro/gstack-plan-design-review": 108752, "kiro/gstack-plan-devex-review": 115080, - "kiro/gstack-plan-eng-review": 145858, - "kiro/gstack-review": 121450, + "kiro/gstack-plan-eng-review": 143524, + "kiro/gstack-review": 121445, "openclaw/gstack-autoplan": 108715, - "openclaw/gstack-plan-eng-review": 127138, + "openclaw/gstack-plan-eng-review": 124804, "opencode/gstack-autoplan": 109234, "opencode/gstack-plan-design-review": 110187, "opencode/gstack-plan-devex-review": 115836, - "opencode/gstack-plan-eng-review": 147265, - "opencode/gstack-review": 122248, + "opencode/gstack-plan-eng-review": 144931, + "opencode/gstack-review": 122243, "slate/gstack-autoplan": 108372, "slate/gstack-plan-design-review": 108826, "slate/gstack-plan-devex-review": 115154, - "slate/gstack-plan-eng-review": 145927, - "slate/gstack-review": 121538 + "slate/gstack-plan-eng-review": 143593, + "slate/gstack-review": 121533 } } diff --git a/test/helpers/llm-judge.ts b/test/helpers/llm-judge.ts index a3177665b..6f742f6a0 100644 --- a/test/helpers/llm-judge.ts +++ b/test/helpers/llm-judge.ts @@ -146,34 +146,64 @@ export async function callJudge( ...(opts?.effort === undefined ? {} : { effort: opts.effort }) } }), messages: [{ role: 'user' as const, content: prompt }], }; - const makeRequest = () => opts?.stream - ? client.messages.stream(request, signal ? { signal } : undefined).finalMessage() - : client.messages.create(request, signal ? { signal } : undefined); + // What a cancelled request had received, so a judge deadline names rate-limit + // waits, a stream with no events, or a long thinking phase. Counts only, never content. + const started = performance.now(); + const elapsedMs = () => Math.round(performance.now() - started); + const progress = { retryWaitsMs: [] as number[], firstEventMs: null as number | null, lastEventMs: null as number | null, + block: null as string | null, thinkingChars: 0, textChars: 0, messageId: null as string | null, requestId: null as string | null }; + const makeRequest = () => { + if (!opts?.stream) return client.messages.create(request, signal ? { signal } : undefined); + const stream = client.messages.stream(request, signal ? { signal } : undefined); + stream.on('streamEvent', event => { + progress.firstEventMs ??= elapsedMs(); + progress.lastEventMs = elapsedMs(); + progress.requestId ??= stream.request_id ?? null; + if (event.type === 'message_start') progress.messageId = event.message.id; + if (event.type === 'content_block_start') progress.block = event.content_block.type; + if (event.type === 'content_block_delta' && event.delta.type === 'thinking_delta') progress.thinkingChars += event.delta.thinking.length; + if (event.type === 'content_block_delta' && event.delta.type === 'text_delta') progress.textChars += event.delta.text.length; + }); + return stream.finalMessage(); + }; // 429s under CI concurrency: jittered exponential backoff over 3 retries // (~1s/4s/16s + jitter), honoring the server's retry-after when present. // The old single fixed 1s retry lost races reliably at 40-way concurrency. let response; let attempt = 0; - for (;;) { - try { - signal?.throwIfAborted(); - response = await makeRequest(); - signal?.throwIfAborted(); - break; - } catch (err: any) { - signal?.throwIfAborted(); - if (err?.status !== 429 || attempt >= 3) throw err; - const retryAfterSecs = Number(err?.headers?.['retry-after']); - const baseMs = Number.isFinite(retryAfterSecs) && retryAfterSecs > 0 - ? retryAfterSecs * 1000 - : 1000 * 4 ** attempt; - await delay(baseMs + Math.random() * 500, undefined, { signal }).catch(error => { + try { + for (;;) { + try { signal?.throwIfAborted(); - throw error; - }); - attempt += 1; + response = await makeRequest(); + signal?.throwIfAborted(); + break; + } catch (err: any) { + signal?.throwIfAborted(); + if (err?.status !== 429 || attempt >= 3) throw err; + const retryAfterSecs = Number(err?.headers?.['retry-after']); + const baseMs = Number.isFinite(retryAfterSecs) && retryAfterSecs > 0 + ? retryAfterSecs * 1000 + : 1000 * 4 ** attempt; + const waitMs = baseMs + Math.random() * 500; + progress.retryWaitsMs.push(Math.round(waitMs)); + await delay(waitMs, undefined, { signal }).catch(error => { + signal?.throwIfAborted(); + throw error; + }); + attempt += 1; + } } + } catch (error) { + if (signal?.aborted) { + console.error(JSON.stringify({ + type: 'llm-judge-aborted', model: resolvedModel, stream: opts?.stream === true, elapsedMs: elapsedMs(), + requests: attempt + 1, ...progress, + reason: signal.reason instanceof Error ? signal.reason.message : String(signal.reason), + })); + } + throw error; } opts?.onResponse?.({ diff --git a/test/llm-judge-abort.test.ts b/test/llm-judge-abort.test.ts index ea3303d88..7723a963d 100644 --- a/test/llm-judge-abort.test.ts +++ b/test/llm-judge-abort.test.ts @@ -85,6 +85,25 @@ describe('callJudge cancellation', () => { expect(create).toHaveBeenCalledTimes(1); }, 3000); + test('an abort during a 429 wait retains the wait it interrupted', async () => { + const diagnostics = spyOn(console, 'error').mockImplementation(() => {}); + try { + const controller = new AbortController(); + create.mockRejectedValue({ status: 429, headers: { 'retry-after': '30' } }); + const pending = callJudge('Rate-limited request.', 'test-judge-model', { signal: controller.signal }); + await new Promise(resolve => setImmediate(resolve)); + controller.abort(new Error('Expired during rate-limit backoff.')); + await expect(pending).rejects.toThrow('Expired during rate-limit backoff.'); + expect(diagnostics).toHaveBeenCalledTimes(1); + expect(JSON.parse(diagnostics.mock.calls[0]![0])).toMatchObject({ type: 'llm-judge-aborted', model: 'test-judge-model', + stream: false, requests: 1, retryWaitsMs: [30000], firstEventMs: null, reason: 'Expired during rate-limit backoff.' }); + create.mockResolvedValue(response as any); + diagnostics.mockClear(); + expect(await callJudge('Judge this.', undefined, { signal: new AbortController().signal })).toEqual({ passed: true }); + expect(diagnostics).not.toHaveBeenCalled(); + } finally { diagnostics.mockRestore(); } + }, 3000); + test('keeps three 429 retries for callers without a signal', async () => { const rateLimit = { status: 429, headers: { 'retry-after': '0.001' } }; create.mockRejectedValue(rateLimit); diff --git a/test/llm-judge-stream.test.ts b/test/llm-judge-stream.test.ts index 589571e75..d484f7d4a 100644 --- a/test/llm-judge-stream.test.ts +++ b/test/llm-judge-stream.test.ts @@ -103,3 +103,59 @@ test('streaming retains the caller abort signal instead of extending its deadlin expect(await rejected).toBe(failure); expect(transport).toHaveBeenCalledTimes(1); }); + +test('a judge aborted mid-stream retains what it had received, as counts without content', async () => { + const encoder = new TextEncoder(); + const partial = [ + { type: 'message_start', message: { id: 'msg_stalled', type: 'message', role: 'assistant', model: 'claude-fable-5-1', + content: [], stop_reason: null, stop_sequence: null, usage: { input_tokens: 99023, output_tokens: 1 } } }, + { type: 'content_block_start', index: 0, content_block: { type: 'thinking', thinking: '', signature: '' } }, + { type: 'content_block_delta', index: 0, delta: { type: 'thinking_delta', thinking: 'PRIVATE_THINKING' } }, + ]; + let delivered!: () => void; + const received = new Promise(resolve => { delivered = resolve; }); + transport.mockImplementation(async (_url: string, init: RequestInit) => new Response(new ReadableStream({ + start(stream) { + for (const event of partial) stream.enqueue(encoder.encode(`event: ${event.type}\ndata: ${JSON.stringify(event)}\n\n`)); + init.signal!.addEventListener('abort', () => stream.error(init.signal!.reason), { once: true }); + setTimeout(delivered, 20); + }, + }), { headers: { 'content-type': 'text/event-stream', 'request-id': 'req_stalled' } })); + const controller = new AbortController(); + const rejected = callJudge('score the whole bundle', 'claude-fable-5-1', { + max_tokens: 65_536, stream: true, jsonSchema: WORKFLOW_JUDGE_RESPONSE_SCHEMA, signal: controller.signal, + }).catch(error => error); + await received; + const failure = new Error('ship/SKILL.md workflow exceeded its workflow judge work deadline'); + controller.abort(failure); + expect(await rejected).toBe(failure); + expect(diagnostics).toHaveBeenCalledTimes(1); + const diagnostic = diagnostics.mock.calls[0]![0]; + expect(diagnostic).not.toContain('PRIVATE_'); + const event = JSON.parse(diagnostic); + expect(event).toMatchObject({ type: 'llm-judge-aborted', model: 'claude-fable-5-1', stream: true, requests: 1, + retryWaitsMs: [], block: 'thinking', thinkingChars: 'PRIVATE_THINKING'.length, textChars: 0, + messageId: 'msg_stalled', requestId: 'req_stalled', reason: failure.message }); + expect(event.firstEventMs).toBeGreaterThanOrEqual(0); + expect(event.lastEventMs).toBeGreaterThanOrEqual(event.firstEventMs); + expect(event.elapsedMs).toBeGreaterThanOrEqual(event.lastEventMs); +}); + +test('a stream that never sent an event is retained as such, and provider errors without an abort are not', async () => { + transport.mockImplementation((_url: string, init: RequestInit) => new Promise((_resolve, reject) => { + init.signal!.addEventListener('abort', () => reject(init.signal!.reason), { once: true }); + })); + const controller = new AbortController(); + const rejected = callJudge('score', 'claude-fable-5-1', { max_tokens: 65_536, stream: true, signal: controller.signal }).catch(error => error); + await new Promise(resolve => setTimeout(resolve, 10)); + controller.abort(new Error('deadline')); + await rejected; + expect(JSON.parse(diagnostics.mock.calls[0]![0])).toMatchObject({ type: 'llm-judge-aborted', firstEventMs: null, + lastEventMs: null, block: null, thinkingChars: 0, messageId: null, requestId: null, reason: 'deadline' }); + + diagnostics.mockClear(); + transport.mockResolvedValue(new Response(JSON.stringify({ type: 'error', error: { type: 'api_error', message: 'boom' } }), + { status: 500, headers: { 'content-type': 'application/json', 'x-should-retry': 'false' } })); + await expect(callJudge('score', 'claude-fable-5-1', { max_tokens: 65_536, stream: true, signal: new AbortController().signal })).rejects.toThrow(); + expect(diagnostics).not.toHaveBeenCalled(); +}); diff --git a/test/review-workflow-clarity.test.ts b/test/review-workflow-clarity.test.ts index 4144d3e25..9fc9f56c9 100644 --- a/test/review-workflow-clarity.test.ts +++ b/test/review-workflow-clarity.test.ts @@ -684,3 +684,18 @@ test('review suppressions allow a clean result and skip common false positives b expectTokens(suppressions, ['"Function too long"', '"Possible null dereference"', '"N+1 query"', '"Hardcoded value"', '`Math.random()`'], 'suppressions'); expectAbsent(suppressions, [/missing input validation/i, /missing await/i], 'suppressions'); }); + +test('judged repeat-pass QA reruns and unresolved record actions each have one rule (paid screen 40617200857)', () => { + // Every sample of the red panel and its rerun named one of these: "post-expiry smoke rechecks are not-run" vs + // Step 5.8's rerun of superseded probes, materialize's once-per-PROBE_DIR limit vs reruns into the same report, + // and no record action for an unresolved non-gate defect (an unanswered question or an unproven repair). + // fails_when=the rerun stops being revalidation, materialize may run per pass, or such a defect has no action. + const flat = skill.replace(/\s+/g, ' '); + const repeat = flat.slice(flat.indexOf('2. On a repeat, execute Steps 3–5 in order.'), flat.indexOf('3. **Verify completed actions.**')); + expect(repeat).toContain('These reruns are revalidation, so they run even after smoke expiry; keep the PROBE_DIR and run its once-only `materialize` on the final pass.'); + expect(repeat.indexOf('rerun each probe the verdict relies on')).toBeLessThan(repeat.indexOf('These reruns are revalidation')); + const actions = flat.slice(flat.indexOf('Actions: `auto-fixed` (Step 5b)'), flat.indexOf('Advice is never `auto-fixed`')); + expect(actions).toContain('`fixed` (approved **and completed** in Step 5d)'); + expect(actions).toContain('`skipped` (explicit Skip in Step 5c), `open` (any other defect, e.g. unanswered).'); + expect(flat).toContain('Do not record an unanswered question as skipped'); +}); diff --git a/test/workflow-judge-cache.test.ts b/test/workflow-judge-cache.test.ts index f71a95421..5ad942656 100644 --- a/test/workflow-judge-cache.test.ts +++ b/test/workflow-judge-cache.test.ts @@ -410,7 +410,7 @@ test('Ship sends its authorized 64k cap and compact response contract through th expect(WORKFLOW_JUDGE_RESPONSE_SCHEMA.properties.reasoning).not.toHaveProperty('maxLength'); expect(source.match(/compactReasoning: true/g)).toHaveLength(1); const f = fixture(); - const stream = spyOn(Messages.prototype, 'stream').mockReturnValue({ finalMessage: async () => ({ + const stream = spyOn(Messages.prototype, 'stream').mockReturnValue({ on() { return this; }, finalMessage: async () => ({ stop_reason: 'end_turn', content: [{ type: 'text', text: JSON.stringify(scores) }], }) } as any); try { @@ -516,7 +516,7 @@ test('the structured callback rejects incomplete, schema-invalid and below-thres try { for (const response of responses) { const f = fixture(); - stream.mockReturnValue({ finalMessage: async () => ({ stop_reason: response.stop_reason, + stream.mockReturnValue({ on() { return this; }, finalMessage: async () => ({ stop_reason: response.stop_reason, content: [{ type: 'text', text: JSON.stringify(response.value) }] }) } as any); const h = actualCallback(f, { judge: (prompt, model, request) => callJudge(prompt, model, request) }); await expect(h.run({ ...h.options, schemaTransport: true, compactReasoning: true, maxTokens: 65_536, stream: true })).rejects.toThrow();