From 95fa5a35ac0b90a667e165e8f6e817bd565c42e9 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 30 Sep 2026 06:21:31 +0000 Subject: [PATCH] refactor(pty): decompose launchClaudePty and engNumberedFindingAUQ under 150 lines launchClaudePty (349 lines) becomes launch preparation (args, hermetic child env, owned state roots), recorder creation, spawn, the trust-dialog watcher, close, and the session handle over one PtyProcess state object. The failure order is unchanged: abort the viewport, dispose any recorders created so far, dispose the viewport, rethrow. The --model / --strict-mcp-config ordering and seedSkills wiring stay pinned by the behavioral fake-CLI tests. engNumberedFindingAUQ (345 lines) keeps its guards and dispatch; each self-contained issue family (declared cache, library retry hooks, cache owner, injected singleton, shared writers, injected export) moves verbatim into its own function. Every pty/ module is now <= 800 lines and every top-level function <= 150 lines. --- test/helpers/pty/boundaries.ts | 590 +++++++++++++++++---------------- test/helpers/pty/launch.ts | 511 +++++++++++++++------------- 2 files changed, 580 insertions(+), 521 deletions(-) diff --git a/test/helpers/pty/boundaries.ts b/test/helpers/pty/boundaries.ts index 87fcc137c..2aa820f0f 100644 --- a/test/helpers/pty/boundaries.ts +++ b/test/helpers/pty/boundaries.ts @@ -2,6 +2,7 @@ * Per-skill Step-0 / review boundary predicates. Moved from claude-pty-runner.ts. * Import through test/helpers/claude-pty-runner.ts from tests; pty/ modules import siblings directly. */ +import type { NativePlanQuestionCall } from '../plan-count-transcript'; import { engCacheWriterDecision } from '../eng-cache-writer-decision'; import { nativePlanCallFingerprint, planCountPrerequisitePick } from './auq'; import type { AskUserQuestionFingerprint, Step0BoundaryPredicate } from './auq'; @@ -234,262 +235,23 @@ function engNumberedFindingAUQ(fp: AskUserQuestionFingerprint): boolean { // an interrogative title. Require its native decision, current assessment, // and opposed implementation options; metadata alone never opens review. const declaredCache = /^(?:Issue|Finding) ([1-9]\d*) \[P[0-3]\] \(confidence (?:10|[1-9])\/10\) [A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?: ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) share a global mutable ([A-Za-z_$][\w$]*) via module-level export, and both mutate it\.$/.exec(title); - if (declaredCache) { - if (!/^Shared cache$/i.test(q.header.trim()) || declaredCache[2] === declaredCache[3]) return false; - const escaped = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - const escapedCache = escaped(declaredCache[4]!); - const decision = /^D([1-9]\d*)\s*[—–:-]/.exec(q.question)?.[1]; - const owner = `(?:(?:this|the|that) (?:finding|issue|gap|remedy|amendment|assessment|option|risk)|(?:Issue|Finding) ${declaredCache[1]}${decision ? `|D${decision}` : ''})`; - const boundary = '(?:^|[.!?;]\\s+|\\n|[✅❌]\\s*)(?:Correction:\\s*)?'; - const scalarOwner = new RegExp(`${boundary}${owner} (?:is|was|has been) (?:(?:now|already) )?$`, 'i'); - const current = (value: string) => value - .replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') - .replace(/^(?:\s*>| {4}|\t).*$/gm, '') - .replace(/"[^"\n]*"|“[^”\n]*”|(? - /^(?:withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|not current|no longer current)$/i.test(quoted.slice(1, -1)) && - scalarOwner.test(source.slice(0, index)) ? quoted.slice(1, -1) : '') - .replace(/`([^`\n]*)`/g, (_, code: string) => /^[A-Za-z_$][\w$]*$/.test(code) ? code : '') - .replace(/\*\*/g, ''); - const framed = /\b(?:source|quoted|historical|hypothetical|earlier|previous)\s+(?:review\s+)?(?:example|excerpt|assessment|finding|material|text)\b|(?:^|[.!?;:]\s+|\n|[✅❌]\s*)(?:if|when|unless|provided|assuming|suppose|imagine|source|example)\b/i; - const closed = new RegExp(`${boundary}${owner} (?:is|was|has been) (?:(?:now|already) )?(?:withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\\b|${boundary}no current (?:gap|risk|finding) (?:remains|exists)\\b`, 'i'); - const removed = new RegExp(`${boundary}(?:(?:${escapedCache}|(?:the|this|that) (?:cache|export|global|writers?|writes?|services?)) (?:is|are|has been|have been) (?:no longer (?:global|mutable|shared|unordered)|(?:now |already )?(?:ordered|serialized|removed))|(?:${escaped(declaredCache[2]!)}|${escaped(declaredCache[3]!)}|(?:the|this|both) services?) no longer (?:share|mutate|write)[a-z]*)\\b`, 'i'); - const cancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) (?:inject|receive|remove|delete|serialize|order|keep|proceed|accept)\b/i; - const inactive = (value: string) => framed.test(value) || closed.test(value) || cancelled.test(value); - const text = current(q.question), assessment = [...text.matchAll(/^ELI10: (.+)$/gm)]; - const preface = text.slice(text.indexOf('\n') + 1, assessment[0]?.index ?? 0).trim().split('\n').filter(Boolean); - if (assessment.length !== 1 || preface.length !== 1 || - !/^Project\/branch\/task: \S[^\n]*\bSection [1-9]\d* Architecture\b/.test(preface[0]!) || - !/^Two services write to the same cache through a global variable\./.test(assessment[0]![1]!) || - !/\b(?:writes can interleave|no ordering|unordered writes)\b/.test(assessment[0]![1]!) || inactive(text) || - removed.test(text)) return false; - const optionIds = q.options.map(o => /^([1-9]\d*)([A-D])[:.)]\s+(\S[\s\S]*)$/.exec(o.label)); - if (optionIds.some(id => id?.[1] !== declaredCache[1]) || new Set(optionIds.map(id => id![2])).size !== q.options.length) return false; - const actions = optionIds.map(id => id![3]!.replace(/\s*\(recommended\)$/i, '')); - return q.options.some((remedy, index) => { - if (!new RegExp(`^Inject ${escapedCache};`).test(actions[index]!)) return false; - const body = current(remedy.description ?? '').trim(); - if (inactive(body) || - !new RegExp(`^✅\\s*Both services receive the one ${escapedCache} instance via constructor; the module export goes away\\b`).test(body) || - !new RegExp(`✅\\s*${escapedCache} exposes only [^✅❌.!?]{1,180} and serializes writes per key\\b`).test(body) || - /\b(?:module export (?:stays|remains|is retained)|writes (?:remain|stay) unordered|(?:do not|never) serialize)\b/i.test(body)) return false; - return q.options.some((opposed, other) => other !== index && /^Proceed as written$/i.test(actions[other]!) && - !inactive(current(opposed.description ?? '')) && !removed.test(current(opposed.description ?? '')) && - new RegExp(`❌\\s*Ships an? (?:auth cache|${escapedCache}) with two unordered writers and shared test state\\b`).test(current(opposed.description ?? '')) && - !/\b(?:risk|race) (?:is |has been )?(?:resolved|closed|fixed)|\bno (?:race|risk) remains\b/i.test(current(opposed.description ?? ''))); - }); - } + if (declaredCache) return engDeclaredCacheFinding(q, declaredCache); if (engCacheWriterDecision(q)) return true; - // The category may precede the issue number. Bind this library choice to - // the current scheduling defect and both concrete outcomes, not its label. - const declaredLibraryHooks = /^Issue ([1-9]\d*): Custom inline scheduler vs\. the job library's built-in retry hooks \([A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?\)$/i.exec(title); - const implicitLibraryHooks = /^D([1-9]\d*)\s*[—–:-]\s*Custom inline backoff scheduler vs the job library's built-in retry hooks$/i.exec(q.question.split('\n')[0]!); - const scopedLibraryHooks = /^Issue ([1-9]\d*): custom inline scheduler per worker, or the job library's retry hook with a custom curve\?$/i.exec(title) ?? implicitLibraryHooks; - const libraryHooks = /^Architecture issue ([1-9]\d*): custom inline scheduler vs the job library's built-in retry hooks\?$/i.exec(title) ?? declaredLibraryHooks ?? scopedLibraryHooks; - if (libraryHooks) { - if (!(implicitLibraryHooks ? /^Architecture$/i : new RegExp(`^${declaredLibraryHooks ? 'Issue' : 'Arch(?:itecture)?'} ${libraryHooks[1]}$`, 'i')).test(q.header.trim())) return false; - const ordinal = (declaredLibraryHooks || scopedLibraryHooks) && /^D([1-9]\d*)\s*[—–:-]/.exec(q.question)?.[1]; - const declaredOwner = `(?:(?:this|the|that) (?:finding|issue|gap|remedy|amendment|assessment|option|deferral|(?:unchanged )?risk)|Issue ${libraryHooks[1]}${ordinal ? `|D${ordinal}` : ''})`; - const declaredBoundary = '(?:^|[.!?;]\\s+|\\n|[✅❌]\\s*)(?:Correction:\\s*)?'; - const scalarOwner = new RegExp(`${declaredBoundary}${declaredOwner} (?:is|was|has been) (?:(?:now|already) )?$`, 'i'); - const current = (text: string) => { - const prose = text.replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') - .replace(/^(?:\s*>| {4}|\t).*$/gm, ''); - if (declaredLibraryHooks || scopedLibraryHooks) return prose.replace(/\*\*/g, '') - .replace(/"[^"\n]*"|“[^”\n]*”|(? { - const status = /^["“'‘`](withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)["”'’`]$/i.exec(quoted); - return status && scalarOwner.test(source.slice(0, at)) ? status[1]! : ''; - }).replace(/\*\*/g, ''); - return prose - .replace(/["“](withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|not current|no longer current)["”]/gi, '$1') - .replace(/"[^"\n]*"|“[^”\n]*”|`[^`\n]*`/g, '') - .replace(/\*\*/g, ''); - }; - const framed = /\b(?:source|quoted|historical|hypothetical|earlier|previous)\s+(?:review\s+)?(?:example|excerpt|assessment|finding|material|text)\b|(?:^|[.!?;:]\s+|\n|[✅❌]\s*)(?:if|when|unless|provided|assuming|suppose|imagine|source|example)\b/i; - const closed = new RegExp(`\\b(?:(?:this|the|that) (?:finding|issue|gap|remedy|amendment|assessment|option|deferral|(?:unchanged )?risk)|Issue ${libraryHooks[1]}) (?:is|was|has been) (?:(?:now|already) )?(?:withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\\b|\\bno current (?:gap|risk|finding) (?:remains|exists)\\b`, 'i'); - const text = current(q.question), lines = text.split('\n'); - const contexts = lines.filter(line => /^Project\/branch\/task: \S/.test(line)); - const assessments = [...text.matchAll(/^ELI10: (.+)$/gm)]; - const preface = text.slice(0, assessments[0]?.index ?? 0).split('\n').filter(line => line.trim()).slice(1); - // A crash consequence explains the current defect; approval conditions - // still suspend the owned decision and are checked below. - const framingText = scopedLibraryHooks ? text.replace(/(?:^|[.!?]\s+)If (?:that|the|this) (?:worker )?process (?:dies|restarts|crashes)\b[^.!?\n]*[.!?]?/gi, '') : text; - if (contexts.length !== 1 || assessments.length !== 1 || preface.length !== 1 || preface[0] !== contexts[0] || - framed.test(framingText) || closed.test(text) || /\b(?:the|this) plan no longer rebuilds retry scheduling\b|\bretry scheduling no longer runs inside each worker\b/i.test(text)) return false; - const assessment = assessments[0]![1]!; - // A declarative issue with a source location can own the same concrete - // scheduling decision. Bind the assessment and each offered outcome; - // the issue number and source location alone do not begin review. - if (declaredLibraryHooks || scopedLibraryHooks) { - const ownClosed = new RegExp(`${declaredBoundary}${declaredOwner} (?:is|was|has been) (?:(?:now|already) )?(?:withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\\b`, 'i'); - // Read approval clauses in the original text, including an owned - // condition after a crash premise or after an option's tradeoffs. - const approvalBoundary = scopedLibraryHooks ? `(?:${declaredBoundary}|,\\s+)` : declaredBoundary; - const approvalPremise = scopedLibraryHooks ? '(?:(?:if|when|once) (?:approved|accepted)|(?:assuming|provided) (?:approval|acceptance))' : '(?:if|when|once) (?:approved|accepted)'; - const conditional = new RegExp(`${approvalBoundary}(?:${declaredOwner} (?:(?:applies|holds) (?:only )?(?:if|when|once|unless) (?:approved|accepted)|is (?:conditional|contingent|dependent) on (?:approval|acceptance)|requires (?:approval|acceptance))|${approvalPremise},? (?:use|adopt|accept|keep|proceed|choose|preserve)\\b)`, 'i'); - if (ownClosed.test(text) || conditional.test(text)) return false; - if (scopedLibraryHooks) { - const workers = /(?:^|[.!?]\s+)The plan writes its own\s+loop inside each of the ([1-9]\d*) workers\./i.exec(assessment)?.[1] ?? - /(?:^|[.!?]\s+)The plan says \([A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?\) to ignore it and hand-roll a scheduler inside each of the ([1-9]\d*) workers\b/i.exec(assessment)?.[1]; - if (!workers || Number(workers) < 2) return false; - const ids = q.options.map(o => /^([1-9]\d*)([A-D])[).:]\s+(\S[\s\S]*)$/.exec(o.label)); - if (ids.some(id => id?.[1] !== libraryHooks[1]) || new Set(ids.map(id => id![2])).size !== q.options.length) return false; - const actions = ids.map(id => id![3]!.replace(/\s*\(recommended\)$/i, '')); - const repair = actions.findIndex(action => /^Library hooks? \+ (?:custom curve|shared backoff fn)$/i.test(action)); - const keep = actions.findIndex(action => /^Custom inline scheduler as planned$/i.test(action) || - new RegExp(`^Proceed as planned \\(inline in ${workers} workers\\)$`, 'i').test(action)); - if (repair < 0 || keep < 0 || repair === keep) return false; - const remedy = current(q.options[repair]!.description ?? '').trim(), unchanged = current(q.options[keep]!.description ?? '').trim(); - const cancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) (?:use|accept|keep|proceed|adopt|choose|preserve|register)\b/i; - if ([remedy, unchanged].some(value => framed.test(value.split('❌')[0]!) || closed.test(value) || ownClosed.test(value) || conditional.test(value) || cancelled.test(value)) || - /\bthe library will not own (?:persistence|attempt counting|crash safety)\b/i.test(remedy) || - /\b(?:the|this) (?:unchanged |per-worker )?scheduler is (?:now |already )?crash-safe\b|\bretry state no longer lives in-process\b/i.test(unchanged)) return false; - const persisted = /^(?:✅\s*)?Retry state persisted by the library: [^.!?✅❌]*\bsurvives\b[^.!?✅❌]*\b(?:crash|restart|deploy)\b[^.!?✅❌]*/i.exec(remedy); - const registered = /^Register the library's retry hook in each worker, pass one shared pure [A-Za-z_$][\w$]*\(attempt\) for the curve\./i.test(remedy); - const lost = /(?:^|❌\s*)Retry state lives in process memory: [^.!?✅❌]*\b(?:crash|restart)\b[^.!?✅❌]*\b(?:drops|loses)\b[^.!?✅❌]*\b(?:job|retry|retries)\b/i.exec(unchanged); - const drift = /^Keep the plan as written\.[\s\S]*?\bRetries die with the process; ([a-z]+|[1-9]\d*) copies drift\./i.exec(unchanged); - const count = drift && (/^[1-9]\d*$/.test(drift[1]!) ? Number(drift[1]) : - ['zero', 'one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine', 'ten'].indexOf(drift[1]!.toLowerCase())); - return Boolean((registered || (persisted && !/\b(?:not|never|no longer)\b/i.test(persisted[0]))) && - ((lost && !/\b(?:not|never|no longer)\b/i.test(lost[0])) || (drift && count === Number(workers)))); - } - if (!/^The job library already knows how to retry a failed job later; you just tell it how long to wait\. The plan instead rebuilds that waiting-and-rescheduling machinery by hand inside each worker\./.test(assessment) || - !/\bjobs get lost \(worker dies mid-sleep\)/.test(assessment) || - !/\bno attempt cap or dead-letter path\b/.test(assessment)) return false; - const ids = q.options.map(o => /^([1-9]\d*)([A-D])[).:]\s+(\S[\s\S]*)$/.exec(o.label)); - if (ids.some(id => id?.[1] !== libraryHooks[1]) || new Set(ids.map(id => id![2])).size !== q.options.length) return false; - const actions = ids.map(id => id![3]!.replace(/\s*\(recommended\)$/i, '')); - const remedyIndex = actions.indexOf('Use library retry hook + custom curve fn'); - const unchangedIndex = actions.indexOf('Custom scheduler inline per worker, as planned'); - if (remedyIndex < 0 || unchangedIndex < 0 || remedyIndex === unchangedIndex) return false; - const remedy = current(q.options[remedyIndex]!.description ?? '').trim(); - const unchanged = current(q.options[unchangedIndex]!.description ?? '').trim(); - const cancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) (?:use|accept|keep|proceed|adopt|choose|preserve)\b/i; - if ([remedy, unchanged].some(value => framed.test(value.split("❌")[0]!) || closed.test(value) || ownClosed.test(value) || conditional.test(value) || cancelled.test(value)) || - /\bthe library will not own (?:persistence|attempt counting|crash safety)\b/i.test(remedy) || - /\b(?:the|this) (?:unchanged |per-worker )?scheduler is (?:now |already )?crash-safe\b|\bretry state no longer lives in-process\b/i.test(unchanged)) return false; - return /^✅\s*Persistence, attempt counting, max-attempts, and dead-letter come from the library; you own only delay\(attempt\) with full jitter\b/.test(remedy) && - /✅\s*Curve is still 100% yours: a pure function, trivially unit-tested\./.test(remedy) && - /❌\s*Retry state lives in-process, so a crash or deploy mid-backoff drops the retry; (?:[a-z]+|[1-9]\d*) copies of scheduler logic drift\b/.test(unchanged); - } - const workers = /^The plan rebuilds retry scheduling by hand inside each of ([1-9]\d*) workers\b/.exec(assessment)?.[1]; - if (!workers || Number(workers) < 2 || - !/\bpersisting attempt counts across process restarts, not double-scheduling when a worker crashes mid-dispatch\b/.test(assessment) || - !/\bthe plan does not mention any of it\./.test(assessment)) return false; - const optionIds = q.options.map(o => /^([1-9]\d*)([A-D])[:.)]\s+(\S[\s\S]*)$/.exec(o.label)); - if (optionIds.some(id => id?.[1] !== libraryHooks[1]) || new Set(optionIds.map(id => id![2])).size !== q.options.length) return false; - const actions = optionIds.map(id => id![3]!.replace(/\s*\(recommended\)$/i, '')); - const remedyIndex = actions.indexOf('Library hooks + custom backoff fn'); - const unchangedIndex = actions.indexOf('Proceed as written (inline in each worker)'); - if (remedyIndex < 0 || unchangedIndex < 0 || remedyIndex === unchangedIndex) return false; - const remedy = current(q.options[remedyIndex]!.description ?? '').trim(); - const unchanged = current(q.options[unchangedIndex]!.description ?? '').trim(); - const cancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) (?:use|accept|keep|proceed|adopt|choose)\b/i; - if (framed.test(remedy) || framed.test(unchanged) || closed.test(remedy) || closed.test(unchanged) || - cancelled.test(remedy) || cancelled.test(unchanged) || - /\bthe library will not own (?:attempt counting|crash safety)\b|\b(?:do not|don't|never|cancel|withdraw) preserve the exported backoff function\b/i.test(remedy) || - /\bthe unchanged per-worker scheduler is (?:now )?crash-safe\b/i.test(unchanged)) return false; - const risk = /❌\s*([A-Za-z]+|[1-9]\d*) copies of crash-unsafe scheduling logic, each drifting independently; every bug gets fixed ([A-Za-z]+|[1-9]\d*) times\./.exec(unchanged); - const number = (value: string) => /^[1-9]\d*$/.test(value) ? Number(value) : - ['zero', 'one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine', 'ten'].indexOf(value.toLowerCase()); - return Boolean(risk && number(risk[1]!) === Number(workers) && number(risk[2]!) === Number(workers) && - /✅\s*Attempt counting, crash safety, and dashboard visibility come from the library for free\./.test(remedy) && - /✅\s*The backoff curve lives in one exported function, so\s+is preserved and testable in isolation\./.test(remedy)); - } + const libraryHooks = engLibraryHooksFinding(q, title); + if (libraryHooks !== undefined) return libraryHooks; // A cache-ownership brief can name its actors in the current assessment // instead of the headline. Bind those actors to the offered single writer. const cacheOwner = /^(?:Issue|Finding) ([1-9]\d*): two services mutate (?:one|the same) shared cache with no (?:serialized writes|serialization)\. How should cache ownership work\?$/i.exec(title); - if (cacheOwner) { - if (!/^(?:Cache owner(?:ship)?|Shared cache)$/i.test(q.header.trim()) && - !new RegExp(`^(?:Issue|Finding|Architecture) ${cacheOwner[1]}$`, 'i').test(q.header.trim())) return false; - const current = (text: string) => text.replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') - .replace(/^(?:\s*>| {4}|\t).*$/gm, '') - .replace(/["“](withdrawn|rejected|cancelled|canceled|resolved|closed|not current)["”]/gi, '$1') - .replace(/"[^"\n]*"|“[^”\n]*”|`[^`]*`/g, ''); - const framing = /\b(?:source|quoted|historical|hypothetical|proposed|earlier|previous)\s+(?:review\s+)?(?:example|excerpt|assessment|finding|text)\b|(?:^|\n|:\s*)(?:if|unless|suppose|imagine)\b/i; - const withdrawn = new RegExp(`\\b(?:(?:this|the|that) (?:finding|issue|gap|remedy|amendment|assessment|option|race|single-writer requirement)|Issue ${cacheOwner[1]}) (?:is|was|has been) (?:(?:now|already) )?(?:withdrawn|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\\b|\\bno current (?:gap|defect|finding) (?:remains|exists)\\b`, 'i'); - const text = current(q.question), assessments = [...text.matchAll(/^ELI10: (.+)$/gm)]; - const prefix = text.slice(text.indexOf('\n') + 1, assessments[0]?.index ?? 0).trim().split('\n').filter(Boolean); - const actors = assessments.length === 1 && /^([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) both (?:write into|mutate) the same (?:tenant-keyed )?cache, and the plan says nothing orders those writes\./.exec(assessments[0]![1]!); - if (!actors || actors[1] === actors[2] || !prefix.length || - !prefix.every(line => /^Project\/branch\/task:/.test(line)) || framing.test(text) || withdrawn.test(text)) return false; - const ids = q.options.map(option => /^([A-D])\)\s+/.exec(option.label)?.[1]); - if (ids.some(id => !id) || new Set(ids).size !== ids.length) return false; - const active = (description: string) => !framing.test(description) && !withdrawn.test(description) && - !/(?:^|[.!?]\s+)(?:Correction:\s*)?(?:do not|don't|never|cancel|withdraw) (?:inject|use|keep|apply)\b/i.test(description); - return q.options.some(remedy => { - const body = current(remedy.description ?? '').trim(); - const owner = /^Constructor-inject the existing adapter into both services\. Only ([A-Za-z_$][\w$]*) writes; ([A-Za-z_$][\w$]*) returns minted material to the broker, which stores it\./.exec(body); - if (!owner || owner[1] === owner[2] || ![actors[1], actors[2]].includes(owner[1]) || - ![actors[1], actors[2]].includes(owner[2]) || !active(body)) return false; - const otherWriter = new RegExp(`(?:^|[.!?]\\s+)(?:Correction:\\s*)?${owner[2]} (?:will |can |may |still )?(?:also )?write(?:s)? (?:directly )?(?:to|into) (?:the )?cache\\b`, 'i'); - if (otherWriter.test(body)) return false; - return q.options.some(opposed => opposed !== remedy && - /^(?:[A-D]\) )?Keep (?:the )?module-level global as planned(?: \(recommended\))?$/i.test(opposed.label) && - /^Do nothing here; both services import and mutate the singleton\./.test(current(opposed.description ?? '').trim()) && - /❌\s*Race stays open and tests share mutable state across the whole suite\./.test(current(opposed.description ?? '')) && - active(current(opposed.description ?? ''))); - }); - } + if (cacheOwner) return engCacheOwnerFinding(q, cacheOwner); // A descriptive native header can carry the decision ordinal while the // current brief owns the architecture finding and its cache remedy. const injected = /^Module-level ([A-Za-z_$][\w$]*) singleton (?:→|->) constructor injection with a single writer\?$/i.exec(title); - if (injected) { - const ordinal = /^D([1-9]\d*)\s*[—–:-]/i.exec(q.question); - if (!ordinal || !new RegExp(`^D${ordinal[1]} DI$`, 'i').test(q.header.trim())) return false; - const current = (text: string) => text.replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') - .replace(/^(?:\s*>| {4}|\t).*$/gm, '') - .replace(/["“](withdrawn|rejected|cancelled|canceled|resolved|closed|not current)["”]/gi, '$1') - .replace(/"(?:[^"\\]|\\.)*"|“[^”]*”|`[^`]*`/g, ''); - const text = current(q.question), assessment = [...text.matchAll(/^ELI10: (.+)$/gm)]; - const preface = text.slice(text.indexOf('\n') + 1, assessment[0]?.index ?? 0).trim().split('\n').filter(Boolean); - const framing = /\b(?:source|quoted|historical|hypothetical|proposed|unrelated|earlier|previous)\s+(?:review\s+)?(?:example|excerpt|assessment|finding|text)\b|(?:^|\n)\s*(?:if|unless|suppose|imagine)\b/i; - const finding = /\bArchitecture finding (A[1-9]\d*)\b/.exec(preface.join(' ')); - const closed = /\b(?:this|the|that) (?:finding|issue|gap|remedy|amendment|explanation|assessment) (?:is|was|has been) (?:withdrawn|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\b|\bno current (?:gap|defect|finding) (?:remains|exists)\b|\bno longer (?:share|write|mutate)\b/i; - if (assessment.length !== 1 || !finding || !preface.length || - !preface.every(line => /^Project\/branch\/task:/.test(line)) || framing.test(preface.join(' ')) || - /\b(?:if|unless|when|suppose|imagine)\b/i.test(preface.join(' ').slice(0, finding.index)) || - !q.question.split('\n').some(line => /^Project\/branch\/task:/.test(line) && new RegExp(`\\b${injected[1]}\\b`).test(line)) || - !/^(?:Right now|Today) both services (?:grab|import) the same global cache (?:object|instance) from a module import and both (?:write to|mutate) it\./i.test(assessment[0]![1]!) || - framing.test(assessment[0]![1]!) || closed.test(text)) return false; - const letters = q.options.map(o => /^([A-D])\)\s+/.exec(o.label)?.[1]); - const recommendation = /^Recommendation: ([A-D]) because\b/m.exec(text)?.[1]; - if (letters.some(letter => !letter) || new Set(letters).size !== letters.length || - !recommendation || !letters.includes(recommendation) || - q.options.filter(o => /\(recommended\)/i.test(o.label)).length !== 1 || - !q.options[letters.indexOf(recommendation)]!.label.toLowerCase().includes('(recommended)')) return false; - const affirmative = (option: typeof q.options[number]) => { - const body = current(option.description ?? '').trim(); - if (!/^✅/.test(body) || framing.test(body) || closed.test(body)) return []; - return [...body.matchAll(/✅\s*([^✅❌]+)/g)].map(m => m[1]!.trim()).filter(pro => - !/^(?:if|unless|when|source|historical|hypothetical|example|previously)\b/i.test(pro)); - }; - const remedy = q.options.find(o => /^[A-D]\) Composition-root injection, single writer(?: \(recommended\))?$/i.test(o.label)); - const unchanged = q.options.find(o => /^[A-D]\) Do nothing(?: \(recommended\))?$/i.test(o.label)); - const owner = remedy && affirmative(remedy).map(pro => /^([A-Za-z_$][\w$]*) is the only session writer and ([A-Za-z_$][\w$]*) gets a read-only port, enforced by types not convention\./.exec(pro)).find(Boolean); - const remaining = unchanged && current(unchanged.description ?? '').trim(); - return Boolean(owner && owner[1] !== owner[2] && remaining && /^✅/.test(remaining) && - !framing.test(remaining) && !closed.test(remaining) && - new RegExp(`❌\\s*Both ${finding[1]} failure scenarios stay live and the plan's own test coverage cannot isolate state\\.$`).test(remaining)); - } + if (injected) return engInjectedSingletonFinding(q, injected); // The category can live in the title while the header carries the issue // number. Require the direct shared-state defect and technical choices; // an Issue heading on setup or report navigation is insufficient. const sharedWriters = /^(Issue|Finding)\s+([1-9]\d*(?:\.[1-9]\d*)*)\s+\(Architecture\)\s*[—–:-]\s*([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) both mutate (?:one|the same) shared cache with no owner and no serialization\. How should shared[- ]state access be structured\?$/i.exec(title); - if (sharedWriters) { - const header = /^(Issue|Finding)\s+([1-9]\d*(?:\.[1-9]\d*)*)$/i.exec(q.header.trim()); - if (!header || header[1]!.toLowerCase() !== sharedWriters[1]!.toLowerCase() || - header[2] !== sharedWriters[2] || sharedWriters[3] === sharedWriters[4] || q.options.length !== 3 || - q.options.some(option => !option.description?.trim())) return false; - const labels = q.options.map(option => option.label.trim() - .replace(/^[1-9]\d*[A-Z]\)\s*/i, '').replace(/\s*\(recommended\)$/i, '')); - return [ - /^Constructor[- ]inject the adapter; single[- ]writer ownership per operation; per[- ]key serialization on the [A-Za-z][\w-]* path; race test$/i, - /^Constructor[- ]inject the adapter only; both services keep writing freely$/i, - /^Keep the module[- ]level shared export as planned$/i, - ].every(pattern => labels.filter(label => pattern.test(label)).length === 1); - } + if (sharedWriters) return engSharedWritersFinding(q, sharedWriters); const issue = /^(?:Issue|Finding)\s+([1-9]\d*(?:\.[1-9]\d*)*)(?:\s*\(D[1-9]\d*\))?\s*[—–:-]\s*([^\n?]+\?)$/i.exec(title); const header = /^(?:Arch(?:itecture)?|Code\s+Q(?:uality)?|Tests?|Perf(?:ormance)?)\s+([1-9]\d*(?:\.[1-9]\d*)*)$/i.exec(q.header.trim()); if (!issue || !header || issue[1] !== header[1]) return false; @@ -504,45 +266,7 @@ function engNumberedFindingAUQ(fp: AskUserQuestionFingerprint): boolean { // numbered form states as a defect. Bind the current actors and both offered // outcomes; an Issue label or the word "inject" cannot open review alone. const injectedExport = /^Replace the module-level mutable ([A-Za-z_$][\w$]*) export with injected ownership\?$/i.exec(body); - if (injectedExport) { - if (!/^[1-9]\d*$/.test(issue[1]!) || !/^Arch(?:itecture)? [1-9]\d*$/i.test(q.header.trim())) return false; - const current = (text: string) => text - .replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') - .replace(/^(?:\s*>| {4}|\t).*$/gm, '') - .replace(/["“](withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|not current|no longer current)["”]/gi, '$1') - .replace(/"[^"\n]*"|“[^”\n]*”/g, '') - .replace(/`([^`\n]*)`/g, (_, code: string) => /^[A-Za-z_$][\w$]*$/.test(code) ? code : '') - .replace(/\*\*/g, ''); - const framed = /\b(?:source|quoted|historical|hypothetical|earlier|previous)\s+(?:review\s+)?(?:example|excerpt|assessment|finding|material|text)\b|(?:^|[.!?;:]\s+|\n|[✅❌]\s*)(?:if|when|unless|provided|assuming|suppose|imagine|source|example)\b/i; - const closed = new RegExp(`\\b(?:(?:this|the|that) (?:finding|issue|gap|remedy|amendment|assessment|option|deferral|(?:unchanged )?risk)|Issue ${issue[1]}) (?:is|was|has been) (?:(?:now|already) )?(?:withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\\b|\\bno current (?:gap|risk|finding) (?:remains|exists)\\b`, 'i'); - const removedGlobal = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:this|the|that) cache no longer has a module-level mutable export\b/i; - const text = current(q.question), lines = text.split('\n'); - const contexts = lines.filter(line => /^Project\/branch\/task: \S/.test(line)); - const assessments = [...text.matchAll(/^ELI10: (.+)$/gm)]; - const preface = text.slice(0, assessments[0]?.index ?? 0).split('\n').filter(line => line.trim()).slice(1); - if (contexts.length !== 1 || assessments.length !== 1 || preface.length !== 1 || preface[0] !== contexts[0] || - framed.test(text) || closed.test(text) || removedGlobal.test(text)) return false; - const assessment = assessments[0]![1]!; - if (!/^(?:Right now|Today) the cache is a global variable that two different services reach into and change\./i.test(assessment)) return false; - const actors = /\ba bug in ([A-Za-z_$][\w$]*) can silently corrupt what ([A-Za-z_$][\w$]*) reads\./.exec(assessment); - if (!actors || actors[1] === actors[2]) return false; - const optionIds = q.options.map(o => /^([1-9]\d*)([A-D])[:.)]\s+(\S[\s\S]*)$/.exec(o.label)); - if (optionIds.some(id => id?.[1] !== issue[1]) || new Set(optionIds.map(id => id![2])).size !== q.options.length) return false; - const actions = optionIds.map(id => id![3]!.replace(/\s*\(recommended\)$/i, '')); - const remedyIndex = actions.indexOf(`Inject ${injectedExport[1]}`), unchangedIndex = actions.indexOf('Do nothing'); - if (remedyIndex < 0 || unchangedIndex < 0 || remedyIndex === unchangedIndex) return false; - const remedy = current(q.options[remedyIndex]!.description ?? '').trim(); - const unchanged = current(q.options[unchangedIndex]!.description ?? '').trim(); - const removalCancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) (?:delete|remove) the module-level export\b/i; - const acceptanceCancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) accept the shared global as-is\b/i; - if (framed.test(remedy) || framed.test(unchanged) || closed.test(remedy) || closed.test(unchanged) || - removalCancelled.test(remedy) || acceptanceCancelled.test(unchanged)) return false; - const injection = /^Construct one ([A-Za-z_$][\w$]*) at the composition root, pass it into ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) constructors, (?:delete|remove) the module-level export, add a test that two service instances with separate caches never observe each other\./.exec(remedy); - return Boolean(injection && injection[1] === injectedExport[1] && injection[2] !== injection[3] && - [injection[2], injection[3]].every(actor => actor === actors[1] || actor === actors[2]) && - /^Accept the shared global as-is\./.test(unchanged) && - /❌\s*Documented [A-Za-z][\w-]* footgun for testability and request isolation; tenant leakage risk (?:stays|remains)\b/.test(unchanged)); - } + if (injectedExport) return engInjectedExportFinding(q, issue, injectedExport); return ( /^[A-Za-z_$][\w$]* is a (?:global|shared) mutable module-level export that (?:two|multiple|\d+) services mutate\. Inject it(?: instead)?\?$/i.test(body) && alternatives(/^Constructor[- ]inject$/i, /^Getter \+ reset hook$/i) @@ -561,6 +285,304 @@ function engNumberedFindingAUQ(fp: AskUserQuestionFingerprint): boolean { ); } +/** Declarative severity-labelled shared-cache issue (see engNumberedFindingAUQ). */ +function engDeclaredCacheFinding(q: NativePlanQuestionCall['questions'][number], declaredCache: RegExpExecArray): boolean { + if (!/^Shared cache$/i.test(q.header.trim()) || declaredCache[2] === declaredCache[3]) return false; + const escaped = (value: string) => value.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const escapedCache = escaped(declaredCache[4]!); + const decision = /^D([1-9]\d*)\s*[—–:-]/.exec(q.question)?.[1]; + const owner = `(?:(?:this|the|that) (?:finding|issue|gap|remedy|amendment|assessment|option|risk)|(?:Issue|Finding) ${declaredCache[1]}${decision ? `|D${decision}` : ''})`; + const boundary = '(?:^|[.!?;]\\s+|\\n|[✅❌]\\s*)(?:Correction:\\s*)?'; + const scalarOwner = new RegExp(`${boundary}${owner} (?:is|was|has been) (?:(?:now|already) )?$`, 'i'); + const current = (value: string) => value + .replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') + .replace(/^(?:\s*>| {4}|\t).*$/gm, '') + .replace(/"[^"\n]*"|“[^”\n]*”|(? + /^(?:withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|not current|no longer current)$/i.test(quoted.slice(1, -1)) && + scalarOwner.test(source.slice(0, index)) ? quoted.slice(1, -1) : '') + .replace(/`([^`\n]*)`/g, (_, code: string) => /^[A-Za-z_$][\w$]*$/.test(code) ? code : '') + .replace(/\*\*/g, ''); + const framed = /\b(?:source|quoted|historical|hypothetical|earlier|previous)\s+(?:review\s+)?(?:example|excerpt|assessment|finding|material|text)\b|(?:^|[.!?;:]\s+|\n|[✅❌]\s*)(?:if|when|unless|provided|assuming|suppose|imagine|source|example)\b/i; + const closed = new RegExp(`${boundary}${owner} (?:is|was|has been) (?:(?:now|already) )?(?:withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\\b|${boundary}no current (?:gap|risk|finding) (?:remains|exists)\\b`, 'i'); + const removed = new RegExp(`${boundary}(?:(?:${escapedCache}|(?:the|this|that) (?:cache|export|global|writers?|writes?|services?)) (?:is|are|has been|have been) (?:no longer (?:global|mutable|shared|unordered)|(?:now |already )?(?:ordered|serialized|removed))|(?:${escaped(declaredCache[2]!)}|${escaped(declaredCache[3]!)}|(?:the|this|both) services?) no longer (?:share|mutate|write)[a-z]*)\\b`, 'i'); + const cancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) (?:inject|receive|remove|delete|serialize|order|keep|proceed|accept)\b/i; + const inactive = (value: string) => framed.test(value) || closed.test(value) || cancelled.test(value); + const text = current(q.question), assessment = [...text.matchAll(/^ELI10: (.+)$/gm)]; + const preface = text.slice(text.indexOf('\n') + 1, assessment[0]?.index ?? 0).trim().split('\n').filter(Boolean); + if (assessment.length !== 1 || preface.length !== 1 || + !/^Project\/branch\/task: \S[^\n]*\bSection [1-9]\d* Architecture\b/.test(preface[0]!) || + !/^Two services write to the same cache through a global variable\./.test(assessment[0]![1]!) || + !/\b(?:writes can interleave|no ordering|unordered writes)\b/.test(assessment[0]![1]!) || inactive(text) || + removed.test(text)) return false; + const optionIds = q.options.map(o => /^([1-9]\d*)([A-D])[:.)]\s+(\S[\s\S]*)$/.exec(o.label)); + if (optionIds.some(id => id?.[1] !== declaredCache[1]) || new Set(optionIds.map(id => id![2])).size !== q.options.length) return false; + const actions = optionIds.map(id => id![3]!.replace(/\s*\(recommended\)$/i, '')); + return q.options.some((remedy, index) => { + if (!new RegExp(`^Inject ${escapedCache};`).test(actions[index]!)) return false; + const body = current(remedy.description ?? '').trim(); + if (inactive(body) || + !new RegExp(`^✅\\s*Both services receive the one ${escapedCache} instance via constructor; the module export goes away\\b`).test(body) || + !new RegExp(`✅\\s*${escapedCache} exposes only [^✅❌.!?]{1,180} and serializes writes per key\\b`).test(body) || + /\b(?:module export (?:stays|remains|is retained)|writes (?:remain|stay) unordered|(?:do not|never) serialize)\b/i.test(body)) return false; + return q.options.some((opposed, other) => other !== index && /^Proceed as written$/i.test(actions[other]!) && + !inactive(current(opposed.description ?? '')) && !removed.test(current(opposed.description ?? '')) && + new RegExp(`❌\\s*Ships an? (?:auth cache|${escapedCache}) with two unordered writers and shared test state\\b`).test(current(opposed.description ?? '')) && + !/\b(?:risk|race) (?:is |has been )?(?:resolved|closed|fixed)|\bno (?:race|risk) remains\b/i.test(current(opposed.description ?? ''))); + }); +} + +/** Custom scheduler vs library retry hooks; undefined when the title is another family. */ +function engLibraryHooksFinding(q: NativePlanQuestionCall['questions'][number], title: string): boolean | undefined { +// The category may precede the issue number. Bind this library choice to +// the current scheduling defect and both concrete outcomes, not its label. +const declaredLibraryHooks = /^Issue ([1-9]\d*): Custom inline scheduler vs\. the job library's built-in retry hooks \([A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?\)$/i.exec(title); +const implicitLibraryHooks = /^D([1-9]\d*)\s*[—–:-]\s*Custom inline backoff scheduler vs the job library's built-in retry hooks$/i.exec(q.question.split('\n')[0]!); +const scopedLibraryHooks = /^Issue ([1-9]\d*): custom inline scheduler per worker, or the job library's retry hook with a custom curve\?$/i.exec(title) ?? implicitLibraryHooks; +const libraryHooks = /^Architecture issue ([1-9]\d*): custom inline scheduler vs the job library's built-in retry hooks\?$/i.exec(title) ?? declaredLibraryHooks ?? scopedLibraryHooks; +if (!libraryHooks) return undefined; + if (!(implicitLibraryHooks ? /^Architecture$/i : new RegExp(`^${declaredLibraryHooks ? 'Issue' : 'Arch(?:itecture)?'} ${libraryHooks[1]}$`, 'i')).test(q.header.trim())) return false; + const ordinal = (declaredLibraryHooks || scopedLibraryHooks) && /^D([1-9]\d*)\s*[—–:-]/.exec(q.question)?.[1]; + const declaredOwner = `(?:(?:this|the|that) (?:finding|issue|gap|remedy|amendment|assessment|option|deferral|(?:unchanged )?risk)|Issue ${libraryHooks[1]}${ordinal ? `|D${ordinal}` : ''})`; + const declaredBoundary = '(?:^|[.!?;]\\s+|\\n|[✅❌]\\s*)(?:Correction:\\s*)?'; + const scalarOwner = new RegExp(`${declaredBoundary}${declaredOwner} (?:is|was|has been) (?:(?:now|already) )?$`, 'i'); + const current = (text: string) => { + const prose = text.replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') + .replace(/^(?:\s*>| {4}|\t).*$/gm, ''); + if (declaredLibraryHooks || scopedLibraryHooks) return prose.replace(/\*\*/g, '') + .replace(/"[^"\n]*"|“[^”\n]*”|(? { + const status = /^["“'‘`](withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)["”'’`]$/i.exec(quoted); + return status && scalarOwner.test(source.slice(0, at)) ? status[1]! : ''; + }).replace(/\*\*/g, ''); + return prose + .replace(/["“](withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|not current|no longer current)["”]/gi, '$1') + .replace(/"[^"\n]*"|“[^”\n]*”|`[^`\n]*`/g, '') + .replace(/\*\*/g, ''); + }; + const framed = /\b(?:source|quoted|historical|hypothetical|earlier|previous)\s+(?:review\s+)?(?:example|excerpt|assessment|finding|material|text)\b|(?:^|[.!?;:]\s+|\n|[✅❌]\s*)(?:if|when|unless|provided|assuming|suppose|imagine|source|example)\b/i; + const closed = new RegExp(`\\b(?:(?:this|the|that) (?:finding|issue|gap|remedy|amendment|assessment|option|deferral|(?:unchanged )?risk)|Issue ${libraryHooks[1]}) (?:is|was|has been) (?:(?:now|already) )?(?:withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\\b|\\bno current (?:gap|risk|finding) (?:remains|exists)\\b`, 'i'); + const text = current(q.question), lines = text.split('\n'); + const contexts = lines.filter(line => /^Project\/branch\/task: \S/.test(line)); + const assessments = [...text.matchAll(/^ELI10: (.+)$/gm)]; + const preface = text.slice(0, assessments[0]?.index ?? 0).split('\n').filter(line => line.trim()).slice(1); + // A crash consequence explains the current defect; approval conditions + // still suspend the owned decision and are checked below. + const framingText = scopedLibraryHooks ? text.replace(/(?:^|[.!?]\s+)If (?:that|the|this) (?:worker )?process (?:dies|restarts|crashes)\b[^.!?\n]*[.!?]?/gi, '') : text; + if (contexts.length !== 1 || assessments.length !== 1 || preface.length !== 1 || preface[0] !== contexts[0] || + framed.test(framingText) || closed.test(text) || /\b(?:the|this) plan no longer rebuilds retry scheduling\b|\bretry scheduling no longer runs inside each worker\b/i.test(text)) return false; + const assessment = assessments[0]![1]!; + // A declarative issue with a source location can own the same concrete + // scheduling decision. Bind the assessment and each offered outcome; + // the issue number and source location alone do not begin review. + if (declaredLibraryHooks || scopedLibraryHooks) { + const ownClosed = new RegExp(`${declaredBoundary}${declaredOwner} (?:is|was|has been) (?:(?:now|already) )?(?:withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\\b`, 'i'); + // Read approval clauses in the original text, including an owned + // condition after a crash premise or after an option's tradeoffs. + const approvalBoundary = scopedLibraryHooks ? `(?:${declaredBoundary}|,\\s+)` : declaredBoundary; + const approvalPremise = scopedLibraryHooks ? '(?:(?:if|when|once) (?:approved|accepted)|(?:assuming|provided) (?:approval|acceptance))' : '(?:if|when|once) (?:approved|accepted)'; + const conditional = new RegExp(`${approvalBoundary}(?:${declaredOwner} (?:(?:applies|holds) (?:only )?(?:if|when|once|unless) (?:approved|accepted)|is (?:conditional|contingent|dependent) on (?:approval|acceptance)|requires (?:approval|acceptance))|${approvalPremise},? (?:use|adopt|accept|keep|proceed|choose|preserve)\\b)`, 'i'); + if (ownClosed.test(text) || conditional.test(text)) return false; + if (scopedLibraryHooks) { + const workers = /(?:^|[.!?]\s+)The plan writes its own\s+loop inside each of the ([1-9]\d*) workers\./i.exec(assessment)?.[1] ?? + /(?:^|[.!?]\s+)The plan says \([A-Za-z][\w./-]*:[1-9]\d*(?:-[1-9]\d*)?\) to ignore it and hand-roll a scheduler inside each of the ([1-9]\d*) workers\b/i.exec(assessment)?.[1]; + if (!workers || Number(workers) < 2) return false; + const ids = q.options.map(o => /^([1-9]\d*)([A-D])[).:]\s+(\S[\s\S]*)$/.exec(o.label)); + if (ids.some(id => id?.[1] !== libraryHooks[1]) || new Set(ids.map(id => id![2])).size !== q.options.length) return false; + const actions = ids.map(id => id![3]!.replace(/\s*\(recommended\)$/i, '')); + const repair = actions.findIndex(action => /^Library hooks? \+ (?:custom curve|shared backoff fn)$/i.test(action)); + const keep = actions.findIndex(action => /^Custom inline scheduler as planned$/i.test(action) || + new RegExp(`^Proceed as planned \\(inline in ${workers} workers\\)$`, 'i').test(action)); + if (repair < 0 || keep < 0 || repair === keep) return false; + const remedy = current(q.options[repair]!.description ?? '').trim(), unchanged = current(q.options[keep]!.description ?? '').trim(); + const cancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) (?:use|accept|keep|proceed|adopt|choose|preserve|register)\b/i; + if ([remedy, unchanged].some(value => framed.test(value.split('❌')[0]!) || closed.test(value) || ownClosed.test(value) || conditional.test(value) || cancelled.test(value)) || + /\bthe library will not own (?:persistence|attempt counting|crash safety)\b/i.test(remedy) || + /\b(?:the|this) (?:unchanged |per-worker )?scheduler is (?:now |already )?crash-safe\b|\bretry state no longer lives in-process\b/i.test(unchanged)) return false; + const persisted = /^(?:✅\s*)?Retry state persisted by the library: [^.!?✅❌]*\bsurvives\b[^.!?✅❌]*\b(?:crash|restart|deploy)\b[^.!?✅❌]*/i.exec(remedy); + const registered = /^Register the library's retry hook in each worker, pass one shared pure [A-Za-z_$][\w$]*\(attempt\) for the curve\./i.test(remedy); + const lost = /(?:^|❌\s*)Retry state lives in process memory: [^.!?✅❌]*\b(?:crash|restart)\b[^.!?✅❌]*\b(?:drops|loses)\b[^.!?✅❌]*\b(?:job|retry|retries)\b/i.exec(unchanged); + const drift = /^Keep the plan as written\.[\s\S]*?\bRetries die with the process; ([a-z]+|[1-9]\d*) copies drift\./i.exec(unchanged); + const count = drift && (/^[1-9]\d*$/.test(drift[1]!) ? Number(drift[1]) : + ['zero', 'one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine', 'ten'].indexOf(drift[1]!.toLowerCase())); + return Boolean((registered || (persisted && !/\b(?:not|never|no longer)\b/i.test(persisted[0]))) && + ((lost && !/\b(?:not|never|no longer)\b/i.test(lost[0])) || (drift && count === Number(workers)))); + } + if (!/^The job library already knows how to retry a failed job later; you just tell it how long to wait\. The plan instead rebuilds that waiting-and-rescheduling machinery by hand inside each worker\./.test(assessment) || + !/\bjobs get lost \(worker dies mid-sleep\)/.test(assessment) || + !/\bno attempt cap or dead-letter path\b/.test(assessment)) return false; + const ids = q.options.map(o => /^([1-9]\d*)([A-D])[).:]\s+(\S[\s\S]*)$/.exec(o.label)); + if (ids.some(id => id?.[1] !== libraryHooks[1]) || new Set(ids.map(id => id![2])).size !== q.options.length) return false; + const actions = ids.map(id => id![3]!.replace(/\s*\(recommended\)$/i, '')); + const remedyIndex = actions.indexOf('Use library retry hook + custom curve fn'); + const unchangedIndex = actions.indexOf('Custom scheduler inline per worker, as planned'); + if (remedyIndex < 0 || unchangedIndex < 0 || remedyIndex === unchangedIndex) return false; + const remedy = current(q.options[remedyIndex]!.description ?? '').trim(); + const unchanged = current(q.options[unchangedIndex]!.description ?? '').trim(); + const cancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) (?:use|accept|keep|proceed|adopt|choose|preserve)\b/i; + if ([remedy, unchanged].some(value => framed.test(value.split("❌")[0]!) || closed.test(value) || ownClosed.test(value) || conditional.test(value) || cancelled.test(value)) || + /\bthe library will not own (?:persistence|attempt counting|crash safety)\b/i.test(remedy) || + /\b(?:the|this) (?:unchanged |per-worker )?scheduler is (?:now |already )?crash-safe\b|\bretry state no longer lives in-process\b/i.test(unchanged)) return false; + return /^✅\s*Persistence, attempt counting, max-attempts, and dead-letter come from the library; you own only delay\(attempt\) with full jitter\b/.test(remedy) && + /✅\s*Curve is still 100% yours: a pure function, trivially unit-tested\./.test(remedy) && + /❌\s*Retry state lives in-process, so a crash or deploy mid-backoff drops the retry; (?:[a-z]+|[1-9]\d*) copies of scheduler logic drift\b/.test(unchanged); + } + const workers = /^The plan rebuilds retry scheduling by hand inside each of ([1-9]\d*) workers\b/.exec(assessment)?.[1]; + if (!workers || Number(workers) < 2 || + !/\bpersisting attempt counts across process restarts, not double-scheduling when a worker crashes mid-dispatch\b/.test(assessment) || + !/\bthe plan does not mention any of it\./.test(assessment)) return false; + const optionIds = q.options.map(o => /^([1-9]\d*)([A-D])[:.)]\s+(\S[\s\S]*)$/.exec(o.label)); + if (optionIds.some(id => id?.[1] !== libraryHooks[1]) || new Set(optionIds.map(id => id![2])).size !== q.options.length) return false; + const actions = optionIds.map(id => id![3]!.replace(/\s*\(recommended\)$/i, '')); + const remedyIndex = actions.indexOf('Library hooks + custom backoff fn'); + const unchangedIndex = actions.indexOf('Proceed as written (inline in each worker)'); + if (remedyIndex < 0 || unchangedIndex < 0 || remedyIndex === unchangedIndex) return false; + const remedy = current(q.options[remedyIndex]!.description ?? '').trim(); + const unchanged = current(q.options[unchangedIndex]!.description ?? '').trim(); + const cancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) (?:use|accept|keep|proceed|adopt|choose)\b/i; + if (framed.test(remedy) || framed.test(unchanged) || closed.test(remedy) || closed.test(unchanged) || + cancelled.test(remedy) || cancelled.test(unchanged) || + /\bthe library will not own (?:attempt counting|crash safety)\b|\b(?:do not|don't|never|cancel|withdraw) preserve the exported backoff function\b/i.test(remedy) || + /\bthe unchanged per-worker scheduler is (?:now )?crash-safe\b/i.test(unchanged)) return false; + const risk = /❌\s*([A-Za-z]+|[1-9]\d*) copies of crash-unsafe scheduling logic, each drifting independently; every bug gets fixed ([A-Za-z]+|[1-9]\d*) times\./.exec(unchanged); + const number = (value: string) => /^[1-9]\d*$/.test(value) ? Number(value) : + ['zero', 'one', 'two', 'three', 'four', 'five', 'six', 'seven', 'eight', 'nine', 'ten'].indexOf(value.toLowerCase()); + return Boolean(risk && number(risk[1]!) === Number(workers) && number(risk[2]!) === Number(workers) && + /✅\s*Attempt counting, crash safety, and dashboard visibility come from the library for free\./.test(remedy) && + /✅\s*The backoff curve lives in one exported function, so\s+is preserved and testable in isolation\./.test(remedy)); +} + +/** Cache-ownership brief whose actors live in the current assessment. */ +function engCacheOwnerFinding(q: NativePlanQuestionCall['questions'][number], cacheOwner: RegExpExecArray): boolean { + if (!/^(?:Cache owner(?:ship)?|Shared cache)$/i.test(q.header.trim()) && + !new RegExp(`^(?:Issue|Finding|Architecture) ${cacheOwner[1]}$`, 'i').test(q.header.trim())) return false; + const current = (text: string) => text.replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') + .replace(/^(?:\s*>| {4}|\t).*$/gm, '') + .replace(/["“](withdrawn|rejected|cancelled|canceled|resolved|closed|not current)["”]/gi, '$1') + .replace(/"[^"\n]*"|“[^”\n]*”|`[^`]*`/g, ''); + const framing = /\b(?:source|quoted|historical|hypothetical|proposed|earlier|previous)\s+(?:review\s+)?(?:example|excerpt|assessment|finding|text)\b|(?:^|\n|:\s*)(?:if|unless|suppose|imagine)\b/i; + const withdrawn = new RegExp(`\\b(?:(?:this|the|that) (?:finding|issue|gap|remedy|amendment|assessment|option|race|single-writer requirement)|Issue ${cacheOwner[1]}) (?:is|was|has been) (?:(?:now|already) )?(?:withdrawn|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\\b|\\bno current (?:gap|defect|finding) (?:remains|exists)\\b`, 'i'); + const text = current(q.question), assessments = [...text.matchAll(/^ELI10: (.+)$/gm)]; + const prefix = text.slice(text.indexOf('\n') + 1, assessments[0]?.index ?? 0).trim().split('\n').filter(Boolean); + const actors = assessments.length === 1 && /^([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) both (?:write into|mutate) the same (?:tenant-keyed )?cache, and the plan says nothing orders those writes\./.exec(assessments[0]![1]!); + if (!actors || actors[1] === actors[2] || !prefix.length || + !prefix.every(line => /^Project\/branch\/task:/.test(line)) || framing.test(text) || withdrawn.test(text)) return false; + const ids = q.options.map(option => /^([A-D])\)\s+/.exec(option.label)?.[1]); + if (ids.some(id => !id) || new Set(ids).size !== ids.length) return false; + const active = (description: string) => !framing.test(description) && !withdrawn.test(description) && + !/(?:^|[.!?]\s+)(?:Correction:\s*)?(?:do not|don't|never|cancel|withdraw) (?:inject|use|keep|apply)\b/i.test(description); + return q.options.some(remedy => { + const body = current(remedy.description ?? '').trim(); + const owner = /^Constructor-inject the existing adapter into both services\. Only ([A-Za-z_$][\w$]*) writes; ([A-Za-z_$][\w$]*) returns minted material to the broker, which stores it\./.exec(body); + if (!owner || owner[1] === owner[2] || ![actors[1], actors[2]].includes(owner[1]) || + ![actors[1], actors[2]].includes(owner[2]) || !active(body)) return false; + const otherWriter = new RegExp(`(?:^|[.!?]\\s+)(?:Correction:\\s*)?${owner[2]} (?:will |can |may |still )?(?:also )?write(?:s)? (?:directly )?(?:to|into) (?:the )?cache\\b`, 'i'); + if (otherWriter.test(body)) return false; + return q.options.some(opposed => opposed !== remedy && + /^(?:[A-D]\) )?Keep (?:the )?module-level global as planned(?: \(recommended\))?$/i.test(opposed.label) && + /^Do nothing here; both services import and mutate the singleton\./.test(current(opposed.description ?? '').trim()) && + /❌\s*Race stays open and tests share mutable state across the whole suite\./.test(current(opposed.description ?? '')) && + active(current(opposed.description ?? ''))); + }); +} + +/** Module-level singleton to constructor injection with a single writer. */ +function engInjectedSingletonFinding(q: NativePlanQuestionCall['questions'][number], injected: RegExpExecArray): boolean { + const ordinal = /^D([1-9]\d*)\s*[—–:-]/i.exec(q.question); + if (!ordinal || !new RegExp(`^D${ordinal[1]} DI$`, 'i').test(q.header.trim())) return false; + const current = (text: string) => text.replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') + .replace(/^(?:\s*>| {4}|\t).*$/gm, '') + .replace(/["“](withdrawn|rejected|cancelled|canceled|resolved|closed|not current)["”]/gi, '$1') + .replace(/"(?:[^"\\]|\\.)*"|“[^”]*”|`[^`]*`/g, ''); + const text = current(q.question), assessment = [...text.matchAll(/^ELI10: (.+)$/gm)]; + const preface = text.slice(text.indexOf('\n') + 1, assessment[0]?.index ?? 0).trim().split('\n').filter(Boolean); + const framing = /\b(?:source|quoted|historical|hypothetical|proposed|unrelated|earlier|previous)\s+(?:review\s+)?(?:example|excerpt|assessment|finding|text)\b|(?:^|\n)\s*(?:if|unless|suppose|imagine)\b/i; + const finding = /\bArchitecture finding (A[1-9]\d*)\b/.exec(preface.join(' ')); + const closed = /\b(?:this|the|that) (?:finding|issue|gap|remedy|amendment|explanation|assessment) (?:is|was|has been) (?:withdrawn|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\b|\bno current (?:gap|defect|finding) (?:remains|exists)\b|\bno longer (?:share|write|mutate)\b/i; + if (assessment.length !== 1 || !finding || !preface.length || + !preface.every(line => /^Project\/branch\/task:/.test(line)) || framing.test(preface.join(' ')) || + /\b(?:if|unless|when|suppose|imagine)\b/i.test(preface.join(' ').slice(0, finding.index)) || + !q.question.split('\n').some(line => /^Project\/branch\/task:/.test(line) && new RegExp(`\\b${injected[1]}\\b`).test(line)) || + !/^(?:Right now|Today) both services (?:grab|import) the same global cache (?:object|instance) from a module import and both (?:write to|mutate) it\./i.test(assessment[0]![1]!) || + framing.test(assessment[0]![1]!) || closed.test(text)) return false; + const letters = q.options.map(o => /^([A-D])\)\s+/.exec(o.label)?.[1]); + const recommendation = /^Recommendation: ([A-D]) because\b/m.exec(text)?.[1]; + if (letters.some(letter => !letter) || new Set(letters).size !== letters.length || + !recommendation || !letters.includes(recommendation) || + q.options.filter(o => /\(recommended\)/i.test(o.label)).length !== 1 || + !q.options[letters.indexOf(recommendation)]!.label.toLowerCase().includes('(recommended)')) return false; + const affirmative = (option: typeof q.options[number]) => { + const body = current(option.description ?? '').trim(); + if (!/^✅/.test(body) || framing.test(body) || closed.test(body)) return []; + return [...body.matchAll(/✅\s*([^✅❌]+)/g)].map(m => m[1]!.trim()).filter(pro => + !/^(?:if|unless|when|source|historical|hypothetical|example|previously)\b/i.test(pro)); + }; + const remedy = q.options.find(o => /^[A-D]\) Composition-root injection, single writer(?: \(recommended\))?$/i.test(o.label)); + const unchanged = q.options.find(o => /^[A-D]\) Do nothing(?: \(recommended\))?$/i.test(o.label)); + const owner = remedy && affirmative(remedy).map(pro => /^([A-Za-z_$][\w$]*) is the only session writer and ([A-Za-z_$][\w$]*) gets a read-only port, enforced by types not convention\./.exec(pro)).find(Boolean); + const remaining = unchanged && current(unchanged.description ?? '').trim(); + return Boolean(owner && owner[1] !== owner[2] && remaining && /^✅/.test(remaining) && + !framing.test(remaining) && !closed.test(remaining) && + new RegExp(`❌\\s*Both ${finding[1]} failure scenarios stay live and the plan's own test coverage cannot isolate state\\.$`).test(remaining)); +} + +/** Category in the title, issue number in the header: two writers on one shared cache. */ +function engSharedWritersFinding(q: NativePlanQuestionCall['questions'][number], sharedWriters: RegExpExecArray): boolean { + const header = /^(Issue|Finding)\s+([1-9]\d*(?:\.[1-9]\d*)*)$/i.exec(q.header.trim()); + if (!header || header[1]!.toLowerCase() !== sharedWriters[1]!.toLowerCase() || + header[2] !== sharedWriters[2] || sharedWriters[3] === sharedWriters[4] || q.options.length !== 3 || + q.options.some(option => !option.description?.trim())) return false; + const labels = q.options.map(option => option.label.trim() + .replace(/^[1-9]\d*[A-Z]\)\s*/i, '').replace(/\s*\(recommended\)$/i, '')); + return [ + /^Constructor[- ]inject the adapter; single[- ]writer ownership per operation; per[- ]key serialization on the [A-Za-z][\w-]* path; race test$/i, + /^Constructor[- ]inject the adapter only; both services keep writing freely$/i, + /^Keep the module[- ]level shared export as planned$/i, + ].every(pattern => labels.filter(label => pattern.test(label)).length === 1); +} + +/** Imperative form of the injected-ownership cache amendment. */ +function engInjectedExportFinding(q: NativePlanQuestionCall['questions'][number], issue: RegExpExecArray, injectedExport: RegExpExecArray): boolean { + if (!/^[1-9]\d*$/.test(issue[1]!) || !/^Arch(?:itecture)? [1-9]\d*$/i.test(q.header.trim())) return false; + const current = (text: string) => text + .replace(/```[\s\S]*?(?:```|$)|~~~[\s\S]*?(?:~~~|$)/g, '') + .replace(/^(?:\s*>| {4}|\t).*$/gm, '') + .replace(/["“](withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|not current|no longer current)["”]/gi, '$1') + .replace(/"[^"\n]*"|“[^”\n]*”/g, '') + .replace(/`([^`\n]*)`/g, (_, code: string) => /^[A-Za-z_$][\w$]*$/.test(code) ? code : '') + .replace(/\*\*/g, ''); + const framed = /\b(?:source|quoted|historical|hypothetical|earlier|previous)\s+(?:review\s+)?(?:example|excerpt|assessment|finding|material|text)\b|(?:^|[.!?;:]\s+|\n|[✅❌]\s*)(?:if|when|unless|provided|assuming|suppose|imagine|source|example)\b/i; + const closed = new RegExp(`\\b(?:(?:this|the|that) (?:finding|issue|gap|remedy|amendment|assessment|option|deferral|(?:unchanged )?risk)|Issue ${issue[1]}) (?:is|was|has been) (?:(?:now|already) )?(?:withdrawn|superseded|rejected|cancelled|canceled|resolved|closed|hypothetical|not current|no longer current)\\b|\\bno current (?:gap|risk|finding) (?:remains|exists)\\b`, 'i'); + const removedGlobal = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:this|the|that) cache no longer has a module-level mutable export\b/i; + const text = current(q.question), lines = text.split('\n'); + const contexts = lines.filter(line => /^Project\/branch\/task: \S/.test(line)); + const assessments = [...text.matchAll(/^ELI10: (.+)$/gm)]; + const preface = text.slice(0, assessments[0]?.index ?? 0).split('\n').filter(line => line.trim()).slice(1); + if (contexts.length !== 1 || assessments.length !== 1 || preface.length !== 1 || preface[0] !== contexts[0] || + framed.test(text) || closed.test(text) || removedGlobal.test(text)) return false; + const assessment = assessments[0]![1]!; + if (!/^(?:Right now|Today) the cache is a global variable that two different services reach into and change\./i.test(assessment)) return false; + const actors = /\ba bug in ([A-Za-z_$][\w$]*) can silently corrupt what ([A-Za-z_$][\w$]*) reads\./.exec(assessment); + if (!actors || actors[1] === actors[2]) return false; + const optionIds = q.options.map(o => /^([1-9]\d*)([A-D])[:.)]\s+(\S[\s\S]*)$/.exec(o.label)); + if (optionIds.some(id => id?.[1] !== issue[1]) || new Set(optionIds.map(id => id![2])).size !== q.options.length) return false; + const actions = optionIds.map(id => id![3]!.replace(/\s*\(recommended\)$/i, '')); + const remedyIndex = actions.indexOf(`Inject ${injectedExport[1]}`), unchangedIndex = actions.indexOf('Do nothing'); + if (remedyIndex < 0 || unchangedIndex < 0 || remedyIndex === unchangedIndex) return false; + const remedy = current(q.options[remedyIndex]!.description ?? '').trim(); + const unchanged = current(q.options[unchangedIndex]!.description ?? '').trim(); + const removalCancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) (?:delete|remove) the module-level export\b/i; + const acceptanceCancelled = /(?:^|[.!?;]\s+|\n|\bCorrection:\s*)(?:do not|don't|never|skip|cancel|withdraw) accept the shared global as-is\b/i; + if (framed.test(remedy) || framed.test(unchanged) || closed.test(remedy) || closed.test(unchanged) || + removalCancelled.test(remedy) || acceptanceCancelled.test(unchanged)) return false; + const injection = /^Construct one ([A-Za-z_$][\w$]*) at the composition root, pass it into ([A-Za-z_$][\w$]*) and ([A-Za-z_$][\w$]*) constructors, (?:delete|remove) the module-level export, add a test that two service instances with separate caches never observe each other\./.exec(remedy); + return Boolean(injection && injection[1] === injectedExport[1] && injection[2] !== injection[3] && + [injection[2], injection[3]].every(actor => actor === actors[1] || actor === actors[2]) && + /^Accept the shared global as-is\./.test(unchanged) && + /❌\s*Documented [A-Za-z][\w-]* footgun for testability and request isolation; tenant leakage risk (?:stays|remains)\b/.test(unchanged)); +} + + /** An answered substantive finding can start review even in a mixed setup packet. */ export const engFirstReviewAUQ: Step0BoundaryPredicate = (fp) => { if (engNumberedFindingAUQ(fp) || engExplicitRepairAUQ(fp) || engArchitectureChoiceAUQ(fp) || engDependencyBindingAUQ(fp) || engSharedMutableCacheAUQ(fp)) return true; diff --git a/test/helpers/pty/launch.ts b/test/helpers/pty/launch.ts index c390fb8a6..86136d044 100644 --- a/test/helpers/pty/launch.ts +++ b/test/helpers/pty/launch.ts @@ -175,14 +175,73 @@ export async function launchClaudePty( const timeoutMs = opts.timeoutMs ?? 240_000; const wallDeadline = performance.now() + timeoutMs; const screenAbort = new AbortController(); + const launch = prepareLaunch(opts); - let buffer = ''; - let exited = false; - let closing = false; - let exitCodeCaptured: number | null = null; - const outputWaiters = new Set<() => void>(); - const notifyOutput = () => { for (const done of outputWaiters) done(); }; + // Construction must succeed before any CLI can be spawned. + const screen = opts.observeScreen ? await createPtyScreen(cols, rows, { + deadlineAt: Math.min(opts.screenDeadlineAt ?? wallDeadline, wallDeadline), signal: screenAbort.signal, + }) : undefined; + const pty: PtyProcess = { buffer: '', exited: false, closing: false, exitCode: null, outputWaiters: new Set(), + screen, screenAbort, screenClosing: undefined, screenFailure: undefined, proc: undefined, + exitedPromise: Promise.resolve(), wallDeadline, recorders: { pendingFiles: [] } }; + try { + createLaunchRecorders(opts, cwd, launch, pty.recorders); + pty.proc = (Bun as any).spawn([claudePath, ...launch.args], { + terminal: { + cols, + rows, + data(_t: unknown, chunk: Buffer) { + const text = chunk.toString('utf-8'); + pty.buffer += text; + if (screen && !pty.screenClosing) screen.write(text); + notifyOutput(pty); + }, + }, + cwd, + env: launch.childEnv, + }); } catch (error) { screenAbort.abort(error); disposeRecorders(pty.recorders); await disposeScreen(pty); throw error; } + + // Track exit so waitForAny can fail fast if claude crashes. + if (pty.proc.exited && typeof pty.proc.exited.then === 'function') { + pty.exitedPromise = pty.proc.exited + .then((code: number | null) => { + pty.exitCode = code; + pty.exited = true; + notifyOutput(pty); + void disposeScreen(pty); + }) + .catch(() => { + pty.exited = true; + notifyOutput(pty); + void disposeScreen(pty); + }); + } + + // Top-level timeout. If a test forgets to close, this kills it eventually. + const wallTimer = setTimeout(() => { + screenAbort.abort(new Error('PTY work deadline exceeded.')); + try { + pty.proc.kill?.('SIGKILL'); + } catch { + /* ignore */ + } + }, Math.max(0, wallDeadline - performance.now())); + const trust = watchTrustDialog(pty); + return sessionHandle(pty, launch, wallTimer, trust); +} + +/** Arguments, child environment and owned state roots, decided before any spawn. */ +interface LaunchSetup { + args: string[]; + childEnv: Record; + hermetic: boolean; + hermeticSkillStateRoot: string | undefined; + autoplanArtifactStateRoot: string | undefined; + autoplanEngTestPlanStateRoot: string | undefined; +} + +function prepareLaunch(opts: ClaudePtyOptions): LaunchSetup { const args: string[] = []; // Pin the model so smokes don't inherit the operator's settings.json model // (see ClaudePtyOptions.model). Chain mirrors session-runner.ts so PTY and @@ -236,275 +295,253 @@ export async function launchClaudePty( const autoplanEngTestPlanStateRoot = opts.approveAutoplanArtifactEdits && opts.autoplanArtifactState !== undefined ? hermeticSkillStateRoot : undefined; + return { args, childEnv, hermetic, hermeticSkillStateRoot, autoplanArtifactStateRoot, autoplanEngTestPlanStateRoot }; +} - // Construction must succeed before any CLI can be spawned. - const screen = opts.observeScreen ? await createPtyScreen(cols, rows, { - deadlineAt: Math.min(opts.screenDeadlineAt ?? wallDeadline, wallDeadline), signal: screenAbort.signal, - }) : undefined; - let screenClosing: Promise | undefined; - let screenFailure: unknown; - const disposeScreen = () => screenClosing ??= (screen?.dispose() ?? Promise.resolve()).catch(error => { screenFailure = error; }); +interface LaunchRecorders { + pendingExit?: ReturnType; + pendingQuestion?: ReturnType; + pendingArtifact?: ReturnType; + pendingFiles: Array<{ expected: string; recorder: NonNullable> }>; +} - // eslint-disable-next-line @typescript-eslint/no-explicit-any - let proc: any; - let pendingExit: ReturnType | undefined; - let pendingQuestion: ReturnType | undefined; - let pendingArtifact: ReturnType | undefined; - const pendingFiles: Array<{ expected: string; recorder: NonNullable> }> = []; - try { - if (opts.observePlanReady && hermetic && childEnv.CLAUDE_CONFIG_DIR) { - pendingExit = createPendingExitRecorder(cwd, childEnv.CLAUDE_CONFIG_DIR); - } - if (opts.observeSetupQuestions && hermetic && childEnv.CLAUDE_CONFIG_DIR) { - pendingQuestion = createPendingQuestionRecorder(cwd, childEnv.CLAUDE_CONFIG_DIR); - } - if (opts.observeAutoplanArtifacts && hermetic && childEnv.CLAUDE_CONFIG_DIR && autoplanArtifactStateRoot) { - pendingArtifact = createAutoplanArtifactRecorder(cwd, childEnv.CLAUDE_CONFIG_DIR, autoplanArtifactStateRoot, - opts.approveAutoplanArtifactEdits === true, opts.engTestPlanArtifactOnly === true, autoplanEngTestPlanStateRoot); - } - if (opts.observeFilePermissions && hermetic && childEnv.CLAUDE_CONFIG_DIR) { - for (const expected of new Set(opts.observeFilePermissions)) { - const recorder = createFilePermissionRecorder(cwd, childEnv.CLAUDE_CONFIG_DIR, expected); - if (recorder) pendingFiles.push({ expected, recorder }); - } - } - if (pendingFiles.length || pendingQuestion || pendingArtifact) { - const hooks = pendingExit ? JSON.parse(pendingExit.settings).hooks : {}; - for (const recorder of [...pendingFiles.map(p => p.recorder), ...(pendingQuestion ? [pendingQuestion] : []), ...(pendingArtifact ? [pendingArtifact] : [])]) for (const [event, entries] of Object.entries(recorder.hooks)) - hooks[event] = [...(hooks[event] ?? []), ...entries]; - args.push('--settings', JSON.stringify({hooks})); - } else if (pendingExit) args.push('--settings', pendingExit.settings); - proc = (Bun as any).spawn([claudePath, ...args], { - terminal: { - cols, - rows, - data(_t: unknown, chunk: Buffer) { - const text = chunk.toString('utf-8'); - buffer += text; - if (screen && !screenClosing) screen.write(text); - notifyOutput(); - }, - }, - cwd, - env: childEnv, - }); } catch (error) { screenAbort.abort(error); pendingFiles.forEach(({ recorder }) => recorder.dispose()); pendingExit?.dispose(); pendingQuestion?.dispose(); pendingArtifact?.dispose(); await disposeScreen(); throw error; } - - // Track exit so waitForAny can fail fast if claude crashes. - let exitedPromise: Promise = Promise.resolve(); - if (proc.exited && typeof proc.exited.then === 'function') { - exitedPromise = proc.exited - .then((code: number | null) => { - exitCodeCaptured = code; - exited = true; - notifyOutput(); - void disposeScreen(); - }) - .catch(() => { - exited = true; - notifyOutput(); - void disposeScreen(); - }); +/** Opted-in owned hook recorders, merged into one --settings argument. Each is + * added to `recorders` as it is created, so a throw leaves the partial set for + * the caller to dispose. */ +function createLaunchRecorders(opts: ClaudePtyOptions, cwd: string, launch: LaunchSetup, recorders: LaunchRecorders): void { + const { hermetic, childEnv, args, autoplanArtifactStateRoot } = launch; + if (opts.observePlanReady && hermetic && childEnv.CLAUDE_CONFIG_DIR) { + recorders.pendingExit = createPendingExitRecorder(cwd, childEnv.CLAUDE_CONFIG_DIR); } - - // Top-level timeout. If a test forgets to close, this kills it eventually. - const wallTimer = setTimeout(() => { - screenAbort.abort(new Error('PTY work deadline exceeded.')); - try { - proc.kill?.('SIGKILL'); - } catch { - /* ignore */ + if (opts.observeSetupQuestions && hermetic && childEnv.CLAUDE_CONFIG_DIR) { + recorders.pendingQuestion = createPendingQuestionRecorder(cwd, childEnv.CLAUDE_CONFIG_DIR); + } + if (opts.observeAutoplanArtifacts && hermetic && childEnv.CLAUDE_CONFIG_DIR && autoplanArtifactStateRoot) { + recorders.pendingArtifact = createAutoplanArtifactRecorder(cwd, childEnv.CLAUDE_CONFIG_DIR, autoplanArtifactStateRoot, + opts.approveAutoplanArtifactEdits === true, opts.engTestPlanArtifactOnly === true, launch.autoplanEngTestPlanStateRoot); + } + if (opts.observeFilePermissions && hermetic && childEnv.CLAUDE_CONFIG_DIR) { + for (const expected of new Set(opts.observeFilePermissions)) { + const recorder = createFilePermissionRecorder(cwd, childEnv.CLAUDE_CONFIG_DIR, expected); + if (recorder) recorders.pendingFiles.push({ expected, recorder }); } - }, Math.max(0, wallDeadline - performance.now())); + } + const { pendingExit, pendingQuestion, pendingArtifact, pendingFiles } = recorders; + if (pendingFiles.length || pendingQuestion || pendingArtifact) { + const hooks = pendingExit ? JSON.parse(pendingExit.settings).hooks : {}; + for (const recorder of [...pendingFiles.map(p => p.recorder), ...(pendingQuestion ? [pendingQuestion] : []), ...(pendingArtifact ? [pendingArtifact] : [])]) for (const [event, entries] of Object.entries(recorder.hooks)) + hooks[event] = [...(hooks[event] ?? []), ...entries]; + args.push('--settings', JSON.stringify({hooks})); + } else if (pendingExit) args.push('--settings', pendingExit.settings); +} - // Auto-handle the workspace-trust dialog. Runs once during the boot - // window, after both choices and the selected cursor are visible. Newer - // unnumbered menus default to "No, exit", so "1\r" would reject trust. - // The first paint can precede the input handler's readiness. Let startup - // settle, then deliver navigation and confirmation as separate events. +function disposeRecorders(recorders: LaunchRecorders): void { + recorders.pendingFiles.forEach(({ recorder }) => recorder.dispose()); + recorders.pendingExit?.dispose(); + recorders.pendingQuestion?.dispose(); recorders.pendingArtifact?.dispose(); +} + +/** One spawned CLI: output buffer, exit state, viewport and owned recorders. */ +interface PtyProcess { + buffer: string; + exited: boolean; + closing: boolean; + exitCode: number | null; + outputWaiters: Set<() => void>; + screen: Awaited> | undefined; + screenAbort: AbortController; + screenClosing: Promise | undefined; + screenFailure: unknown; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + proc: any; + exitedPromise: Promise; + wallDeadline: number; + recorders: LaunchRecorders; +} + +function notifyOutput(pty: PtyProcess): void { for (const done of pty.outputWaiters) done(); } + +function disposeScreen(pty: PtyProcess): Promise { + return pty.screenClosing ??= (pty.screen?.dispose() ?? Promise.resolve()).catch(error => { pty.screenFailure = error; }); +} + +function writeTerminal(pty: PtyProcess, data: string): void { + if (pty.exited) return; + try { + pty.proc.terminal?.write?.(data); + } catch { + /* ignore */ + } +} + +/** + * Auto-handle the workspace-trust dialog. Runs once during the boot + * window, after both choices and the selected cursor are visible. Newer + * unnumbered menus default to "No, exit", so "1\r" would reject trust. + * The first paint can precede the input handler's readiness. Let startup + * settle, then deliver navigation and confirmation as separate events. + */ +function watchTrustDialog(pty: PtyProcess) { let trustHandled = false; let trustVisibleAt: number | undefined; - const trustInputTimers: ReturnType[] = []; - const trustWatcher = setInterval(() => { - if (trustHandled || exited) return; - const input = trustDialogInput(buffer); + const inputTimers: ReturnType[] = []; + const watcher = setInterval(() => { + if (trustHandled || pty.exited) return; + const input = trustDialogInput(pty.buffer); if (input !== null) { trustVisibleAt ??= Date.now(); if (Date.now() - trustVisibleAt < 1_500) return; trustHandled = true; const keys = input.match(/\x1b\[[AB]|\r/g) ?? []; for (const [i, key] of keys.entries()) { - trustInputTimers.push(setTimeout(() => { - if (exited) return; - try { proc.terminal?.write?.(key); } catch { /* ignore */ } + inputTimers.push(setTimeout(() => { + if (pty.exited) return; + try { pty.proc.terminal?.write?.(key); } catch { /* ignore */ } }, i * 500)); } } }, 200); // Stop the watcher after 15s — by then the dialog has either fired or // doesn't exist on this run. - const trustWatcherStop = setTimeout(() => clearInterval(trustWatcher), 15_000); + const stop = setTimeout(() => clearInterval(watcher), 15_000); + return { watcher, stop, inputTimers }; +} - function send(data: string): void { - if (exited) return; - try { - proc.terminal?.write?.(data); - } catch { - /* ignore */ +async function waitForAnyOutput( + pty: PtyProcess, + patterns: Array, + waitOpts?: { timeoutMs?: number; pollMs?: number; since?: number }, +): Promise<{ matched: RegExp | string; index: number }> { + const wTimeout = waitOpts?.timeoutMs ?? 60_000; + const poll = waitOpts?.pollMs ?? 250; + const since = waitOpts?.since; + const start = Date.now(); + while (Date.now() - start < wTimeout) { + if (pty.exited) { + throw new Error( + `claude exited (code=${pty.exitCode}) before any pattern matched. ` + + `Last visible:\n${stripAnsi(pty.buffer).slice(-2000)}`, + ); } + const visible = since !== undefined ? stripAnsi(pty.buffer.slice(since)) : stripAnsi(pty.buffer); + for (let i = 0; i < patterns.length; i++) { + const p = patterns[i]!; + const matchIdx = typeof p === 'string' ? visible.indexOf(p) : visible.search(p); + if (matchIdx >= 0) { + return { matched: p, index: matchIdx }; + } + } + await Bun.sleep(poll); } + throw new Error( + `Timed out after ${wTimeout}ms waiting for any of: ${patterns + .map((p) => (typeof p === 'string' ? JSON.stringify(p) : p.source)) + .join(', ')}\nLast visible (since=${since ?? 'all'}):\n${ + since !== undefined ? stripAnsi(pty.buffer.slice(since)).slice(-2000) : stripAnsi(pty.buffer).slice(-2000) + }`, + ); +} - type Key = Parameters[0]; - function sendKey(key: Key): void { - const map: Record = { - Enter: '\r', - Up: '\x1b[A', - Down: '\x1b[B', - Esc: '\x1b', - Tab: '\t', - ShiftTab: '\x1b[Z', - CtrlC: '\x03', - }; - send(map[key] ?? ''); +/** SIGINT, then SIGKILL, each bounded by a shared 3s cleanup deadline; then recorders and viewport. */ +async function closePty(pty: PtyProcess, wallTimer: ReturnType, + trust: ReturnType): Promise { + pty.closing = true; + notifyOutput(pty); + const cleanupDeadline = Math.min(pty.wallDeadline, performance.now() + 3_000); + const cleanupTimer = setTimeout(() => pty.screenAbort.abort(new Error('PTY cleanup deadline exceeded.')), + Math.max(0, cleanupDeadline - performance.now())); + clearTimeout(trust.stop); + clearInterval(trust.watcher); + for (const timer of trust.inputTimers) clearTimeout(timer); + try { + for (const [signal, timeout] of [['SIGINT', 2000], ['SIGKILL', 1000]] as const) { + if (pty.exited) break; + try { + pty.proc.kill?.(signal); + } catch { + /* ignore */ + } + let deadline!: ReturnType; + try { + await Promise.race([pty.exitedPromise, new Promise((resolve) => { + deadline = setTimeout(resolve, Math.max(0, Math.min(timeout, cleanupDeadline - performance.now()))); + })]); + } finally { + clearTimeout(deadline); + } + } + disposeRecorders(pty.recorders); + await disposeScreen(pty); + if (pty.screenFailure) throw pty.screenFailure; + } finally { + clearTimeout(cleanupTimer); + clearTimeout(wallTimer); } +} +function sessionHandle(pty: PtyProcess, launch: LaunchSetup, wallTimer: ReturnType, + trust: ReturnType): ClaudePtySession { + const { screen } = pty; + const { pendingExit, pendingQuestion, pendingArtifact, pendingFiles } = pty.recorders; + const send = (data: string) => writeTerminal(pty, data); let lastMark = 0; - function mark(): number { - lastMark = buffer.length; - return lastMark; - } - function visibleSince(marker?: number): string { - const offset = marker ?? lastMark; - return stripAnsi(buffer.slice(offset)); - } - - async function waitForOutput(since: number, timeoutMs: number): Promise { - if (buffer.length > since || exited || closing) return; - await new Promise((resolve) => { - const done = () => { - clearTimeout(timer); - outputWaiters.delete(done); - resolve(); - }; - const timer = setTimeout(done, timeoutMs); - outputWaiters.add(done); - }); - } - - async function waitForAny( - patterns: Array, - waitOpts?: { timeoutMs?: number; pollMs?: number; since?: number }, - ): Promise<{ matched: RegExp | string; index: number }> { - const wTimeout = waitOpts?.timeoutMs ?? 60_000; - const poll = waitOpts?.pollMs ?? 250; - const since = waitOpts?.since; - const start = Date.now(); - while (Date.now() - start < wTimeout) { - if (exited) { - throw new Error( - `claude exited (code=${exitCodeCaptured}) before any pattern matched. ` + - `Last visible:\n${stripAnsi(buffer).slice(-2000)}`, - ); - } - const visible = since !== undefined ? stripAnsi(buffer.slice(since)) : stripAnsi(buffer); - for (let i = 0; i < patterns.length; i++) { - const p = patterns[i]!; - const matchIdx = typeof p === 'string' ? visible.indexOf(p) : visible.search(p); - if (matchIdx >= 0) { - return { matched: p, index: matchIdx }; - } - } - await Bun.sleep(poll); - } - throw new Error( - `Timed out after ${wTimeout}ms waiting for any of: ${patterns - .map((p) => (typeof p === 'string' ? JSON.stringify(p) : p.source)) - .join(', ')}\nLast visible (since=${since ?? 'all'}):\n${ - since !== undefined ? stripAnsi(buffer.slice(since)).slice(-2000) : stripAnsi(buffer).slice(-2000) - }`, - ); - } - - async function waitFor( - pattern: RegExp | string, - waitOpts?: { timeoutMs?: number; pollMs?: number; since?: number }, - ): Promise { - await waitForAny([pattern], waitOpts); - } - let closePromise: Promise | undefined; - function close(): Promise { - return closePromise ??= closeOnce(); - } - async function closeOnce(): Promise { - closing = true; - notifyOutput(); - const cleanupDeadline = Math.min(wallDeadline, performance.now() + 3_000); - const cleanupTimer = setTimeout(() => screenAbort.abort(new Error('PTY cleanup deadline exceeded.')), - Math.max(0, cleanupDeadline - performance.now())); - clearTimeout(trustWatcherStop); - clearInterval(trustWatcher); - for (const timer of trustInputTimers) clearTimeout(timer); - try { - for (const [signal, timeout] of [['SIGINT', 2000], ['SIGKILL', 1000]] as const) { - if (exited) break; - try { - proc.kill?.(signal); - } catch { - /* ignore */ - } - let deadline!: ReturnType; - try { - await Promise.race([exitedPromise, new Promise((resolve) => { - deadline = setTimeout(resolve, Math.max(0, Math.min(timeout, cleanupDeadline - performance.now()))); - })]); - } finally { - clearTimeout(deadline); - } - } - pendingFiles.forEach(({ recorder }) => recorder.dispose()); - pendingExit?.dispose(); - pendingQuestion?.dispose(); pendingArtifact?.dispose(); - await disposeScreen(); - if (screenFailure) throw screenFailure; - } finally { - clearTimeout(cleanupTimer); - clearTimeout(wallTimer); - } - } - + const readable = () => { + if (!screen) throw new Error('PTY screen observation was not enabled for this session.'); + if (pty.screenFailure) throw new Error('PTY screen observation failed.', { cause: pty.screenFailure }); + return screen; + }; + const waitForAny = (patterns: Array, waitOpts?: { timeoutMs?: number; pollMs?: number; since?: number }) => + waitForAnyOutput(pty, patterns, waitOpts); return { send, - sendKey, - rawOutput: () => buffer, - visibleText: () => stripAnsi(buffer), - currentScreen: async (deadlineAt?: number) => { - if (!screen) throw new Error('PTY screen observation was not enabled for this session.'); - if (screenFailure) throw new Error('PTY screen observation failed.', { cause: screenFailure }); - return screen.read(deadlineAt); + sendKey: key => { + const map: Record = { + Enter: '\r', + Up: '\x1b[A', + Down: '\x1b[B', + Esc: '\x1b', + Tab: '\t', + ShiftTab: '\x1b[Z', + CtrlC: '\x03', + }; + send(map[key] ?? ''); }, + rawOutput: () => pty.buffer, + visibleText: () => stripAnsi(pty.buffer), + currentScreen: async (deadlineAt?: number) => readable().read(deadlineAt), currentScreenFrame: async (deadlineAt?: number) => { - if (!screen) throw new Error('PTY screen observation was not enabled for this session.'); - if (screenFailure) throw new Error('PTY screen observation failed.', { cause: screenFailure }); - const frame = await screen.readFrame(deadlineAt); + const frame = await readable().readFrame(deadlineAt); return {text: frame.text, rawEnd: frame.inputOffset, styledText: frame.styledText}; }, - mark, - waitForOutput, - visibleSince, + mark: () => (lastMark = pty.buffer.length), + waitForOutput: async (since: number, timeoutMs: number) => { + if (pty.buffer.length > since || pty.exited || pty.closing) return; + await new Promise((resolve) => { + const done = () => { + clearTimeout(timer); + pty.outputWaiters.delete(done); + resolve(); + }; + const timer = setTimeout(done, timeoutMs); + pty.outputWaiters.add(done); + }); + }, + visibleSince: (marker?: number) => stripAnsi(pty.buffer.slice(marker ?? lastMark)), waitForAny, - waitFor, - pid: () => proc.pid as number | undefined, - exited: () => exited, - exitCode: () => exitCodeCaptured, - hermeticConfigDir: hermetic ? childEnv.CLAUDE_CONFIG_DIR ?? null : null, - hermeticSkillStateRoot, + waitFor: async (pattern, waitOpts) => { await waitForAny([pattern], waitOpts); }, + pid: () => pty.proc.pid as number | undefined, + exited: () => pty.exited, + exitCode: () => pty.exitCode, + hermeticConfigDir: launch.hermetic ? launch.childEnv.CLAUDE_CONFIG_DIR ?? null : null, + hermeticSkillStateRoot: launch.hermeticSkillStateRoot, pendingPlanReadyFile: pendingExit?.file, pendingQuestionFile: pendingQuestion?.file, pendingAutoplanArtifactFile: pendingArtifact?.file, - autoplanArtifactStateRoot: pendingArtifact ? autoplanArtifactStateRoot : undefined, - autoplanEngTestPlanStateRoot: pendingArtifact ? autoplanEngTestPlanStateRoot : undefined, + autoplanArtifactStateRoot: pendingArtifact ? launch.autoplanArtifactStateRoot : undefined, + autoplanEngTestPlanStateRoot: pendingArtifact ? launch.autoplanEngTestPlanStateRoot : undefined, startAutoplanArtifactEditApproval: pendingArtifact?.startEditApproval, pendingFilePermissionFiles: pendingFiles.map(({ expected, recorder }) => ({ expected, file: recorder.file })), - close, + close: () => closePromise ??= closePty(pty, wallTimer, trust), }; }