391 Commits

Author	SHA1	Message	Date
Michał Pierzchała	016577e6bc	docs: remove superseded architecture proposals and design prototypes (#1478 P7) (#1580) ... P7 cleanup per #1478's ratified defer decision: delete the daemon-modularity proposal, module-interface-principles (durable kernel folded into CONTEXT.md), the pre-package Maestro debt map, and the daemon-boundary prototypes with their package scripts; refresh CONTEXT.md's R10 bullet to post-arc state. Also reconciles the interaction façade symbol pin with #1570's wait symbols, which had left check:layering red on main via #1570 × #1574 branch-race skew. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>	2026-08-04 11:14:51 +02:00
Michał Pierzchała	83322a3f2f	test(layering): pin exact façade symbols for all workspace packages (#1574) ... * test(layering): pin exact façade symbols for all workspace packages #1555 added the repo's first exact exported-symbol gate, pinning @agent-device/ad-replay's named export list. Every other workspace package was still covered only by the exports-subpath locks, which prove which files a package exposes but say nothing about what those files name — so any façade could grow a symbol silently. Pin all 29 exported subpaths across the remaining 8 packages: ad-script, contracts (14), kernel (8), maestro, provider-limrun, provider-webdriver, replay-test, and xml. The lists are the honest current surface, untrimmed — contracts/interaction alone names 140 symbols, and pinning the real number is what makes the next widening visible. The table is checked in both directions, so a new package or subpath that nobody pinned fails rather than being silently skipped. Pinning contracts needed the export-discovery helper widened: 13 of its 14 façades are bare `export * from '../x.ts'` barrels, and readNamedExports throws on those by design, because given only a source string the contributed set is genuinely unknowable. Given the FILE it is not, so readFacadeExports resolves the relative re-export chain and enumerates it. Resolution stays narrow — a package-specifier star still throws (that would mean re-entering another package's exports map, the unbounded widening the gate exists to refuse), cycles are visit-guarded, and a default export still throws through a barrel. Helper unit tests cover the shapes the merged AST scan handles but left unpinned: `export { default as x }` (the form between the two rejection rules — named, so reported, never `default`), a local `export { … }` list with no `from`, and multi-declarator `export const a = 1, b = 2`. Plant-verified per package rather than asserted: a stray export on ad-script, one two files deep behind contracts' `export *` chain, and an unpinned new subpath on xml each failed with a named diff; each reverted to green. Gates: check:layering (63 tests, up from 53) / typecheck / lint / format:check — green. * fix(layering): model real `export *` semantics; split the pinned table out Addresses both P1 findings on #1574. P1 — `readFacadeExports` did not model `export *` façade semantics. It unioned every child name and threw on every child default. Both are wrong: - Per GetExportedNames, a star export excludes the child's `default`, so a private `export default` in a leaf is not reachable through the barrel and does not widen the façade. It is now passed over rather than rejected; the previous test codified that false positive and is replaced. A default on the ENTRY file is still a real default export of the façade and still throws. - Per ResolveExport, a name two star sources resolve differently is `ambiguous` — importing it is a SyntaxError, so it is not part of the surface at all. Unioning would pin a symbol no consumer can import; ambiguity now throws and names both origins. Origins are tracked by declaring module rather than by path taken, so a diamond (two barrels reaching one declaration) resolves normally, and an explicit export shadows a star-provided name of the same name as the spec's own precedence does. Both counterfactuals are tested alongside the two rejection cases. P1 — module size. The 885-line generated FACADE_SYMBOLS table moves to a focused sibling, scripts/layering/facade-symbols.ts, leaving the behavioral tests at 642 lines (from 1,455) so the test file stays one bounded read per AGENTS.md. Gates: check:layering (66 tests, up from 63) / typecheck / lint / format:check — green. Contracts plant re-verified under the corrected semantics: a stray two files deep behind the `export *` chain still fails with a named diff, and reverts to green. * fix(layering): resolve re-export identity transitively; extract facade-exports Addresses both P1 findings on the second review round. P1 — named re-export identity stopped at the immediate source. Given `a` re-exporting `x` from `b`, `c` re-exporting `x` from `a`, and a façade starring both, ESM resolves ONE binding (b's `x`), but the walker identified the two paths as `b#x` and `a#x` and falsely rejected the façade as ambiguous. Reproduced before fixing. Origins now resolve through the chain to the binding a name ultimately names, by asking the child's own already-resolved map instead of synthesizing an identity from the specifier. A package specifier keeps a stable synthetic identity (it is not a file this gate reads), and a cycle in progress falls back to the immediate source. Two tests, counterfactual-verified against each other: the chain diamond now resolves to one name (confirmed failing with the old immediate-source identity, passing with the fix), and a same-depth chain whose branches bottom out in two genuinely distinct declarations still throws — so the fix cannot be satisfied by simply collapsing every duplicate. P1 — context-safety extraction was incomplete. Façade export enumeration moves to scripts/layering/facade-exports.ts (219 lines) with its own facade-exports.test.ts (245), registered in check:layering. package-boundaries.ts drops to 338 from 528 and its test file to 450 from 642: the boundary rules answer "may this file import that one?", this module answers "what does this façade name?". Every layering file is now under the 500-line tripwire except the generated symbol table, which the rule exempts. Gates: check:layering (68 tests, up from 66) / typecheck / lint / format:check — green. Contracts plant re-verified after the split. * fix(layering): filter `default` at the star, not at its source The reported P1 does not reproduce: intermediate `export { default } from './x.ts'` links are reported by oxc as kind `Name` with the name `default`, not kind `Default`, so they already resolve transitively; and for a terminal `export default <decl>`, the fallback identity `${child}#default` is exactly the canonical binding, so both paths agree. The exact five-module scenario from the review returns ['x']. That behavior is now pinned by a test so it cannot silently regress. Investigating it did surface a real spec violation in the opposite direction. Because a re-exported `default` is a named entry, it landed in the module's map and was then copied wholesale by star enumeration, so `export * from './mid.ts'` reported `default` as part of the surface — a name `GetExportedNames` explicitly skips, and which oxc itself labels `AllButDefault` on the star's own import. `default` is now filtered at the star rather than at the source. That placement is the point: the name has to stay in the module's map so a later `export { default as x }` can resolve its binding, while never being reachable through a star. Filtering at the source would have broken identity resolution — the very thing the review round before this one fixed. A façade entry re-exporting a default under the name `default` is now rejected too. It carries a default export exactly as `export default …` does; only the parse shape differs, and only the declared form was being caught. Three tests: the star filter (counterfactual-verified — removing the filter fails it — with a sibling name proving the module is still read), entry-level rejection, and the two-paths-to-one-default-binding case from the review. Gates: check:layering (71 tests, up from 68) / typecheck / lint / format:check — green. Contracts plant re-verified. --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-04 10:34:57 +02:00
Michał Pierzchała	178a36c419	fix: prevent publishing unresolved workspace imports (#1577)	2026-08-03 22:34:15 +02:00
Michał Pierzchała	761317deb7	refactor(daemon): extract native .ad replay to packages/ad-replay (#1478 P5) (#1555) ... * refactor(replay): move the dependency-free engine leaves into packages/ad-replay Stage A of the #1478 P5 extraction: vars, plan-digest (+canonical-json, sole consumer), the target-identity classification core, report-action, and suggestion-ranking move verbatim; imports updated. The package facade temporarily re-exports the moved symbols so root consumers keep compiling; a later stage narrows it to inspectAdReplay/runAdReplay only. * chore(layering): register packages/ad-replay in the workspace and DAG * refactor(replay): define the three-operation replay selector port with dual adapters (#1478 P5) * refactor(daemon): route replay handlers through the selector port (#1478 P5) * refactor(replay): split target verification into engine policy and daemon authority (#1478 P5) * refactor(replay): move the .ad step loop behind inspectAdReplay/runAdReplay (#1478 P5) * refactor(replay): lock the ad-replay façade to its real consumers (#1478 P5) * test(replay): prove shared-id demotion on both selector-port adapters (#1555 review) * fix(replay): restore invalid replayBackend rejection on the native path (#1555 review) * refactor(replay): move shared .ad vocabulary to its owner, packages/ad-script (#1555 review) * refactor(replay): neutral step/run outcomes and digest/resume behind inspectAdReplay (#1555 review) P1 "do not smuggle daemon wire failures through a generic": drop the TResponse generic from AdReplayStepRuntime/runAdReplay. executeStep and handleActionFailure now return neutral tagged AdReplayStepOutcome/ AdReplayStepFailure values (kind/message/artifactPaths only); runAdReplay returns a neutral completed/failed AdReplayRunOutcome. The engine never holds or returns a DaemonResponse. The daemon adapter (createAdReplayStepRuntime, session-replay-runtime.ts) keeps its real wire response in a local side-map as it builds each neutral outcome, and runReplayScriptFile reads it back once runAdReplay reports which step failed, so the final response is byte-identical to before this split. P1 "parsing/planning/digest/resume must also occur behind runAdReplay": relocate computeReplayPlanDigest's call site and the --from/--plan-digest resume-point math (resolveReplayEntryIndex) behind inspectAdReplay's manifest as planDigest and a resolveEntryIndex closure. Neither is a new top-level export -- inspectAdReplay/runAdReplay stay the only two. Timing is preserved exactly (still called eagerly in prepareReplayPlan, before prepareReplaySession's coordinator-mutating side effects) since moving resume validation to run inside runAdReplay itself would let a rejected --from request mutate coordinator/session state first -- a real ordering hazard, not just a cosmetic one. computeReplayPlanDigest/ReplayPlanDigestMetadata/resolveReplayEntryIndex leave the ad-replay façade; request-router-repair-expired.test.ts and prepareReplayPlan read the digest/resume result off the manifest instead. * refactor(replay): relocate classifyTargetBindingMatch and pin the ad-replay façade (#1555 review) P1 "complete the binding façade instead of documenting deviations": classifyTargetBindingMatch never had a real consumer reachable through inspectAdReplay/runAdReplay -- both its callers (the daemon's record-time self-check in session-target-evidence.ts and its replay-time classification wrapper in session-replay-target-classification.ts) are daemon files that imported it directly. It interprets TargetAnnotationV1 evidence semantics shared beyond the engine, so it moves to packages/ad-script alongside target-annotation-identity.ts (new target-annotation-classification.ts + its test), and both daemon call sites now import it from there instead of @agent-device/ad-replay. One deviation remains and is reported rather than papered over per the review's own instruction: the four target-verification policy functions (planPreDispatchTargetVerification, planPostResolutionTargetVerification, deriveReplayTargetGuardMismatchEvidence, deriveWaitLandmarkMismatchEvidence) and the ReplaySelectorPort type family stay exported. Their sole caller, session-replay-target-verification.ts, interleaves these pure decisions with daemon-only async work (capture, SessionStore, coordinator/resume stamping, wire shaping) that must stay outside the engine by design; moving their call sites to live only behind runAdReplay would require restructuring that whole orchestration into new fine-grained AdReplayStepRuntime capabilities, which is out of scope for this pass. See packages/ad-replay/src/index.ts's header comment for the full reasoning. P1 "add the reviewer-required exact exported-symbol gate": adds readNamedExports (scripts/layering/package-boundaries.ts), a small parser over a façade's `export { .. } from`, `export type { .. } from`, and direct-declaration forms, and pins @agent-device/ad-replay's exact 21-symbol export list in package-boundaries.test.ts. Plant-verified: a stray `export const` addition failed the assertion; removed it and the gate went green again. * refactor(replay): drive target verification from the engine step loop (#1555 review) Moves the verify-then-dispatch decision flow into packages/ad-replay's step loop so the four target-verification policy functions (plan{PostResolution,PreDispatch}TargetVerification, derive{ReplayTargetGuardMismatch,WaitLandmark}MismatchEvidence) become engine-private and leave the ad-replay façade. The daemon (session-replay-target-verification.ts) shrinks to the narrow AdReplayStepRuntime capabilities the engine drives: routing (beginTargetVerification), capture (captureObservation), classification (classifyTarget), dispatch (dispatchStep), and wire-building (buildRecordedUnverifiableFailure, buildTargetBindingFailure, buildPostDispatchTargetBindingFailure). Wire output and replay-compat stay byte-identical; the exact-symbol façade gate is updated to the shrunken export list. * refactor(daemon): decompose the replay adapter's two over-threshold functions (#1555) * refactor(replay): fold #1554's keep-session terminal-lifecycle policy into the ad-replay engine Rebasing p5/extract-ad-replay onto main pulled in #1554's --keep-session feature, which had grown its own daemon-side terminal-close-suppression predicate (session-replay-terminal-lifecycle.ts's resolveSuppressedTerminalCloseIndex/countExecutedReplayActions) independently of this branch's own engine-side one (step-loop.ts's isRepairArmedTerminalCloseAction). Both are the same decision family — replay --keep-session and an active --save-script repair now share ONE structural resolution (resolveSuppressedTerminalCloseIndex, generalized to "terminal among EXECUTABLE actions" rather than the old physical-last-index check) and one suppression check inside runAdReplay, gated on keepSession OR runtime.isRepairArmed(). AdReplayRunRequest grew a keepSession field; the neutral 'replayed' count in AdReplayRunOutcome is now computed inline in the loop instead of the daemon's old actions.length - entryIndex approximation. requireLiveSessionForKeepSession (the --keep-session live-session postcondition) stays daemon-side, inlined into session-replay-runtime.ts, since it inspects SessionStore state the engine never sees. The daemon-only session-replay-terminal-lifecycle.ts this arrived with is deleted entirely — its isExecutableReplayAction was a duplicate of the engine's own. runReplayScriptFile's Maestro-format routing (including the new --keep-session Maestro rejection) was extracted into routeMaestroReplay to keep the function under fallow's complexity threshold after re-threading keepSession through it. Added packages/ad-replay/src/internal/__tests__/step-loop.test.ts covering the unified suppression decision (both keepSession and repair-armed) directly against runAdReplay, including the terminal-among-executable-actions case with a trailing nested replay marker. The daemon-level integration tests (6 tests in session-replay-terminal-lifecycle.test.ts, exercising the same behavior through runReplayScriptFile) and the SDK provider-scenario test (active-session-script-publication.test.ts) needed no changes and pass unmodified. * refactor(daemon): decompose session-replay-runtime.ts into three modules (#1555) Splits the ~1096-line replay runtime into cohesive pieces, keeping session-replay-runtime.ts as thin orchestration (~240 LOC): - session-replay-runtime-engine-adapter.ts: the AdReplayStepRuntime adapter (createAdReplayStepRuntime, the build*Failure capability implementations, and the lastResponse/lastObservation side-map mechanics), extracted verbatim. - session-replay-runtime-plan.ts: extended with the plan-side helpers (validateReplayBackendFlag, inspectReplayPlanManifest, resolveReplayPlanEntryIndex, prepareReplayPlan, routeMaestroReplay) alongside the buildReplayMetadataFlags helper already there — buildReplayMetadataFlags is now module-private since its one caller moved into the same file. Also introduces ReplayScriptFileParams, named here (instead of derived via Parameters<typeof runReplayScriptFile>) so routeMaestroReplay can reference the shape without importing back from session-replay-runtime.ts. - session-replay-runtime-session.ts (new): session preparation (prepareReplaySession and its coordinator arming/repair-preflight helpers), extracted verbatim. Coordinator ownership is unchanged: createReplayCoordinator is still constructed only in session-replay-runtime.ts, matching replay-coordinator-ownership.test.ts's allowlist as-is — every extracted module receives the already-constructed ReplayCoordinator as a parameter. Pure move; no behavior change. * test(replay): cover pre-step artifact ordering and resume-before-mutation (#1555) Two invariants found during the P5 decomposition pass now have direct counterfactual-verified coverage: - packages/ad-replay/src/internal/__tests__/step-loop.test.ts: a post-dispatch target-binding mismatch (dispatchWithGuard) must report the accumulated PRE-step artifact snapshot it was called with, never the artifacts the failed dispatch itself produced. Verified red by swapping the buildPostDispatchTargetBindingFailure call to outcome.artifactPaths. - src/daemon/handlers/__tests__/session-replay-runtime-plan.test.ts: a rejected --from/--plan-digest resume must never reach prepareReplaySession's coordinator-mutating writes (the R2 ordering invariant) — a pre-armed repair transaction and corrective-resume watermark are asserted byte-for-byte unchanged after rejection. Verified red by calling prepareReplaySession before honoring the plan-validation rejection. * fix(ad-replay): enforce the exact two-entrypoint facade (#1555 review P1) packages/ad-replay/src/index.ts now exports exactly two value symbols, inspectAdReplay and runAdReplay, and zero types — formatReplaySuccessMessage (presentation) moves beside its one caller in session-replay-runtime.ts, and every type a root daemon file needs is derived structurally off the two entrypoints in the one new src/daemon/ad-replay-facade-types.ts module instead of being named off the façade. scripts/layering/package-boundaries.ts's readNamedExports is rewritten on oxc-parser's own static-export table instead of a regex, so it can no longer silently miss a widening export form: a bare `export *` re-export or an `export default` now throws (an un-enumerable, and therefore un-pinnable, export), while `export * as ns` and every other enumerable form is still counted. The pinned exact-symbol assertion in package-boundaries.test.ts is narrowed to ['inspectAdReplay', 'runAdReplay']. * fix(ad-replay): translate wire failures before the engine boundary (#1555 review P1) AdReplayDispatchOutcome's guard-mismatch/landmark-mismatch variants carried a generic `details: Record<string, unknown> | undefined` bag straight off the wire response — a daemon wire projection crossing into the engine even though the outcome itself was already a neutral type. The daemon adapter (session-replay-runtime-engine-adapter.ts) now narrows that bag into the typed AdReplayGuardMismatchEvidence/AdReplayLandmarkMismatchEvidence shapes (observed identity, expected/observed structural denotation, ancestry entries, match count) before returning the outcome; the unknown-parsing readers move there with the wire-reading responsibility they always were. target-verification.ts's deriveReplayTargetGuardMismatchEvidence/ deriveWaitLandmarkMismatchEvidence now consume only the typed values — no `unknown`-valued record type remains on any engine-crossing signature. * fix(ad-replay): move variable semantics/planning behind runAdReplay (#1555 review P1) The daemon assembled the `${VAR}` scope (buildPreparedReplayScope) and interpolated actions at two independent call sites: dispatch's own (invokeReplayAction) and target verification's separate one (resolveTargetVerificationEntry) — duplicated orchestration the P5 design assigns to the engine. runAdReplay's request now carries the raw scope INPUTS (varSources: plain builtins/file/shell/cli-env data, plus actionLines/actionSourcePaths/ resolvedPath for interpolation-error location) instead of a built scope; the engine builds the scope and resolves each action exactly once per step, handing the RESOLVED action to dispatchStep/beginTargetVerification while every other capability still receives the ORIGINAL recorded action (a target-binding divergence reports the recorded selector, never an expanded ${VAR}). This is the one resolution site now — session-replay-action-runtime.ts's invokeReplayAction and session-replay-target-verification.ts's resolveTargetVerificationEntry no longer hold a scope or call resolveReplayAction themselves. Scrub-value collection (collectReplayScrubbableVarValues, for divergence-report redaction) is kept single-sourced in the engine too: it's computed from the engine's own live scope and threaded to each build-failure/handleActionFailure capability as an explicit scrubVars argument, rather than the daemon recomputing it from a second scope object (which would have gone stale, since expandedBuiltinNames tracking now only happens engine-side). The Maestro replay path's own daemon-side vars usage is unrelated (a different engine) and is out of scope here. * fix(ad-script): make ${VAR} interpolation a linear scanner CodeQL flagged the interpolation regex's fallback group as js/polynomial-redos once vars.ts moved into packages/ (library-input classification): every ${NAME:- prefix of an unclosed input rescanned to end-of-string, quadratic overall — 1,857 ms measured on 20k repetitions of '${A:-['. Replaced with a single-pass scanner; failed fallback scans emit their span verbatim and resume after it (escape-pair alignment is identical from every candidate start inside the span, so no later candidate can terminate where the failed scan could not). Equivalence: 200k-trial differential fuzz against the retired regex over the adversarial alphabet, zero mismatches; both adversarial shapes now resolve in 1-2 ms. * refactor(ad-replay): typed façade replaces the zero-type rule (#1555 structural-quality review) Reverses the exact-two-value zero-type export shape #1555's second review pass established: it forced every root type derivation through one shim (src/daemon/ad-replay-facade-types.ts) and left four daemon-side twin types (TargetVerificationEntry, TargetClassificationOutcome, TargetBindingFailureEvidence, ReplayVerifiedTargetGuard) plus a toDaemonEvidence copy translator shadowing the engine's own shapes. packages/ad-replay/src/index.ts now exports inspectAdReplay/runAdReplay (unchanged, still the only two values) plus the neutral vocabulary their signatures are built from, by name — following packages/maestro's façade precedent. The exact-symbol gate in scripts/layering/package-boundaries.test.ts is widened to pin the full sorted list (values + types). The four daemon twins are deleted; session-replay-target-verification.ts and session-replay-runtime-engine-adapter.ts now use the engine's own AdReplayVerificationEntry/AdReplayTargetClassification/ AdReplayTargetBindingEvidence/AdReplayVerifiedTargetGuard directly. TargetBindingDivergenceBuilt's array fields are now readonly-compatible, so toDaemonEvidence's copy is gone — evidence flows through unchanged. * fix(ad-replay): honor the selector port's own contract in the parse gate target-verification.ts's planPreDispatchTargetVerification used resolveRecordedTarget (operation 2, resolve) over an empty node tree purely to read its parse-invalid reason — a resolve call standing in for a parse call, even though readSelectorExpression (operation 1, parse) exists to answer exactly that question and was already unused inside the engine. Replaced with port.readSelectorExpression('ordinary', [token]). The mapping is not 'invalid' -> skip: production's 'ordinary'/'wait' grammars only ever record a boundary once it has already parsed, so a single malformed token can only come back 'not-applicable' there ('invalid' is unreachable from this call site on the production adapter). Both non-'expression' outcomes map to skip, matching the historical behavior (a single parse-invalid reason covered both cases). platform dropped from the function's params — it was only ever threaded to the resolve call this replaces. Added a contract-suite cell pinning the exact (diverging) discriminant each adapter reports for a selector-shaped-but-malformed bare token, and why the divergence is harmless for the one real consumer. * refactor(ad-replay): split step-loop.ts and shrink the daemon adapter (#1555 structural-quality review) step-loop.ts (810 LOC) splits three ways, following packages/maestro's own precedent: - internal/runtime-port-types.ts: the AdReplayStepRuntime boundary vocabulary (all the neutral types the engine/daemon exchange). - internal/verify-dispatch.ts: verifyAndDispatchStep + its dispatchNoGuard/ dispatchWithGuard helpers. - internal/step-loop.ts: runAdReplay itself plus the terminal-close/ executable-action structural logic (isExecutableReplayAction, resolveSuppressedTerminalCloseIndex). packages/ad-replay/src/index.ts's type exports now source from runtime-port-types.ts. step-loop.test.ts's AdReplayStepRuntime import moves to the new path (no assertion changes). src/daemon/handlers/session-replay-runtime-engine-adapter.ts (553 LOC after item 1's twin removal) shrinks to 294 via two further extractions: - session-replay-dispatch-narrowing.ts: the wire `details` bag -> typed evidence narrowing and dispatch-failure classification. - session-replay-runtime-step-support.ts: ReplayStepContext (moved here to avoid a cycle with the adapter, which re-exports it by name) plus the failure-wrapping/diagnostics-support helpers. Final LOC: adapter 294, dispatch-narrowing 148, step-support 153, step-loop 225, verify-dispatch 246, runtime-port-types 374. * test(ad-replay): package-local tests for resume.ts/target-verification.ts + terminal-lifecycle test rename resume.test.ts covers resolveReplayEntryIndex directly (previously only exercised transitively through the daemon's session-replay-runtime-plan tests): no --from/--plan-digest, the paired-flags requirement, in-range --from, out-of-range rejection, stale-digest rejection, the authorized empty-tail boundary (actionCount + 1) gated on a matching watermark, and the unperformed-record-and-heal growth check. Counterfactual run and restored: widening describeOutOfRangeResumeFrom's bound turns the out-of-range/ empty-tail-without-watermark assertions red (2 failures observed). target-verification.test.ts covers all four engine policy functions directly: the two plan* pre-capture gates and the two derive* post-dispatch evidence builders, including item 2's own new decision surface (a fake ReplaySelectorPort proving both non-'expression' readSelectorExpression outcomes map to skip). Counterfactual run and restored: narrowing the check to the literal `'invalid' -> skip` reading turns the 'not-applicable' case red (reports recorded-unverifiable instead of skip). session-replay-terminal-lifecycle.test.ts renamed to session-replay-runtime-keep-session.test.ts: its production module (session-replay-terminal-lifecycle.ts) was already deleted by the #1554 fold-in, and its six cases drive the full runReplayScriptFile round trip against a real SessionStore (including daemon-only postconditions the engine's step loop never reaches) rather than testing engine policy through the façade in isolation — the engine's own terminal-close-suppression decision already has direct, cheaper coverage in step-loop.test.ts. No assertion changes; both files' header comments cross-reference the split. * refactor(ad-replay): compute scrub values once per step, one name end to end collectReplayScrubbableVarValues(scope) was called fresh at 5 separate return points inside one verifyAndDispatchStep invocation plus once more in handleActionFailure — always the same result, since nothing between them mutates scope. step-loop.ts's runAdReplay now computes scrubVars ONCE per step, right after resolveReplayAction (the one call that can grow the scope's expanded-builtins set), and threads it as a plain readonly AdReplayScrubValue[] value; verify-dispatch.ts no longer imports ReplayVarScope or collectReplayScrubbableVarValues at all. "One name" end to end: the daemon's TargetBindingDivergenceContext.scrubVars and withReplayFailureDiagnostics's scrubVars param used a separately-derived ReturnType<typeof collectReplayScrubbableVarValues> (mutable array) instead of the engine's own AdReplayScrubValue, requiring a [...scrubVars] copy at every daemon call site to satisfy the mutable-array type. Both now use readonly AdReplayScrubValue[]/readonly ReplayVarScrubEntry[] (structurally identical, already readonly-safe downstream — scrubReplayVarValues and createReplayDivergenceSanitizer already accepted readonly arrays), so the four [...scrubVars] copies in session-replay-runtime-engine-adapter.ts are gone. * fix(daemon): make lastObservation genuinely per-step, not per-run createAdReplayStepRuntime's lastObservation closure lives for the whole replay run (one factory call covers every step), but was never reset between steps. Every current buildTargetBindingFailure call site happens to be preceded by this same step's own captureObservation, so the `lastObservation ?? { reason: 'observation-missing' }` fallback could never actually fire — but if it ever did (a future call path reaching buildTargetBindingFailure without capturing first), it would silently attach the PREVIOUS step's screen instead of reporting the missing-capture condition the fallback message claims. armStep runs exactly once per step, before any of that step's other capabilities (verified against step-loop.ts's runAdReplay loop order) — the natural per-step boundary. It now clears lastObservation first. No behavior change on any reachable path today (full daemon + ad-replay suite: 1766/1766 green); an unrelated device-claim-prune contention flake was observed once and did not reproduce on isolated or full-suite reruns. * docs(ad-replay): fix decayed review-changelog comments naming defunct symbols Four comments named symbols/paths that no longer exist, left behind by earlier review passes describing PR history rather than the current constraint: - session-replay-runtime-step-support.ts / session-replay-runtime.ts (2 sites): referenced a function called executeStep, which was never reintroduced under that name after the P5 split — the actual mechanism is the runtime's dispatch/build-failure capabilities recording into the lastResponse side-map. - session-replay-runtime.ts: referenced an engine collectArtifactPaths capability that does not exist — artifactPaths is a daemon-side Set the adapter mutates via collectReplayActionArtifactPaths. - packages/ad-replay/src/internal/selector-port.ts: pointed at ./testing/in-memory-selector-port.ts, the in-memory adapter's pre-stage-D location — it has lived at src/__tests__/test-utils/in-memory-replay-selector-port.ts since. - session-replay-repair-hint.ts / session-replay-runtime-step-support.ts (2 sites): named target-identity.ts, which does not exist (the real file is target-identity-node.ts); the second site additionally mislabeled classifyReplayTarget as engine-side when it is daemon-side (session-replay-target-classification.ts). Comment-only; no behavior change. * refactor(ad-script): move declaredScriptPlatform to its natural shared owner packages/ad-replay/src/internal/inspect.ts's declaredScriptPlatform and src/daemon/replay-device-selection.ts's readScriptReplaySelection each kept their own copy of the same "platform declared before the first open" scan over runtime/open actions — .ad script semantics, not engine or daemon policy, needed independently by ad-replay's plan-digest precedence and the daemon's device-selection platform resolution. Verified this was a genuine duplicate (not the single-sourced state I initially reported): readScriptReplaySelection's platform-tracking loop computes the identical result via a differently-shaped traversal fused with its own app-target scan. resolveDeclaredScriptPlatform now lives in packages/ad-script (its natural owner: the one package both ad-replay and the daemon already depend on, avoiding the R11 issue that justified the original duplication). The daemon's app-target scan stays its own separate pass; fusing it back into the shared function would smuggle a daemon-only concern into ad-script for no measurable cost (the actions array is small, and the shared function already stops at the same point the app-target scan needs to look). * docs(ad-replay): fix package.json description to match the current façade Described "target-identity, variable substitution, plan-digest, and report primitives" — the wide pre-#1555-review façade shape. Vars/identity/report vocabulary moved to ad-script/daemon across the P5 and #1555 review passes; the package now exports exactly inspectAdReplay/runAdReplay plus the neutral AdReplayStepRuntime vocabulary. Description updated to match. * refactor(daemon): fold the step-support fragment back into the engine adapter A simplicity audit judged session-replay-runtime-step-support.ts a size-target fragment, not a concern boundary: four unrelated concerns, one consumer, and a header comment admitting it existed to satisfy the <300 LOC metric. Folded back; the previously-exported helpers are module-private again; the adapter's honest size is renegotiated from the plan metric (dispatch-narrowing stays extracted — it has one nameable job).	2026-08-03 17:25:13 +02:00
Michał Pierzchała	2e74b789fd	feat: verify device cloud connections (#1564) ... * feat: verify device cloud connections * refactor: unify connect provider adapters * refactor: separate connect verification facts * fix: tighten connect provider verification * fix: use neutral cloud connection wording * perf: deduplicate local affected checks * refactor: simplify affected check runner * refactor: derive connect workflow from verification	2026-08-03 16:47:57 +02:00
Michał Pierzchała	2c2df031ff	feat: keep replay session active on request (#1554) ... * feat: keep replay session active on request * test: cover replay keep-session provider route * fix: make replay session handoff reliable * refactor(daemon): extract the replay terminal-lifecycle policy module (#1554 review) session-replay-runtime.ts was already over the 500-line extract-before-adding-behavior tripwire before this PR; the keep-session/repair terminal-close decision, its live-session postcondition, and the dispatched-action count pushed it further past budget. Move that policy into a focused session-replay-terminal-lifecycle.ts (isExecutableReplayAction, resolveSuppressedTerminalCloseIndex, countExecutedReplayActions, requireLiveSessionForKeepSession) so the runtime file stays orchestration-only, and mirror its PR-added unit tests into session-replay-terminal-lifecycle.test.ts. Pure extraction: no assertions changed.	2026-08-02 18:35:13 +02:00
Michał Pierzchała	60400d04b7	feat(mutation): add target-annotation-serde + snapshot-occlusion kernels (#1553) ... * feat(mutation): add target-annotation-serde + snapshot-occlusion kernels Both are pure decision kernels the lane's own membership rule covers (target-annotation-serde: parse/validate/normalize the .ad comment-line codec, zero I/O; snapshot-occlusion: pure covered/not-covered decision where a wrong answer silently blocks or mis-allows a tap) but were excluded from KERNEL_MODULES. Fixing the harness's packages/*/src blind spot was required, not optional: test-scope.ts, ownership.ts, and vitest.mutation.config.ts all hardcoded `src/` as the only place a kernel's tests could live. target-annotation-serde's own tests live under packages/ad-script/src/internal/__tests__/, so without this fix the module would score 0% from day one — not from weak tests, but because its test file was silently invisible to the lane. Widened the same three places, plus mutation-affected.yml's path filter and isTestFile/ownedTestFiles in ownership.ts, to also recognize packages/*/src/**/*.test.ts (mirroring vitest.config.ts's own unit-core project include list). Triaged every surviving mutant from the initial run: real coverage gaps got a new/adjusted test (kill-with-test), everything else is documented equivalent with an inline comment at the mutation site explaining the invariant that makes it unobservable (redundant early-returns, JSON.stringify dropping undefined-valued keys, Number.isFinite/isSafeInteger's total-function safety, caller-enforced positiveRect/candidate invariants, etc). Baseline recorded from the actual measured run, not inherited or guessed: 94.03% (315/335) and 89.74% (175/195). * style: run the formatter over the four files the gate flagged	2026-08-02 11:36:43 +02:00
Karthik Varma	92b22229e6	feat(cloud-webdriver): BrowserStack device-feature capabilities, and fix cloud orientation (#1544) ... * feat(cloud-webdriver): support BrowserStack device-feature capabilities Adds the eight BrowserStack "device feature" session capabilities that had no representation in agent-device: deviceOrientation, geoLocation, timezone, language, locale, networkProfile, customNetwork, and resignApp. These are vendor capabilities, so they are emitted inside `bstack:options` rather than at the top level. BrowserStack's YAML config lists them unnested and its SDK relocates them; agent-device talks to the hub directly, so it nests them itself. A single spec table drives both the flag reader and the capability builder, so adding a capability is a table row rather than a branch in each. A structural test asserts every field owns exactly one row, since a field the table forgets would parse off the CLI, ride the profile, and then be silently dropped before the hub ever saw it. Rejects combinations the provider cannot act on unambiguously: an unknown orientation is caught at the flag boundary instead of being forwarded to a hub that accepts and then ignores it, --provider-no-resign-app is refused on Android, and a named network profile cannot be combined with a custom network shape. Also fixes a latent shallow-merge bug in buildBrowserStackCapabilities: a caller supplying its own `bstack:options` replaced the whole object and silently dropped the project, build, and session labels. It is now merged per key. * fix(cloud-webdriver): rotate via WebDriver orientation endpoints `setOrientation` on the cloud WebDriver path sent `mobile: rotate`, which is not a driver command at all. UiAutomator2's own error enumerates its extensions and `rotate` is absent from the list, so `agent-device orientation` was a hard failure on every hosted provider. It also forwarded agent-device's four-way rotation vocabulary verbatim ("landscape-left", "portrait-upside-down"), where the protocol accepts only uppercase PORTRAIT/LANDSCAPE. Every other platform has a translation layer; this path was the only one without one. Now two transports, ordered by backend. `POST /rotation` takes exact four-way degrees and leads on Android, since it is the only endpoint that can express upside-down and left-versus-right. `POST /orientation` is two-way and leads on XCUITest, which rejects `/rotation`. Each falls back to the other, because only BrowserStack's UiAutomator2 is verified and a provider whose driver disagrees should degrade rather than hard-fail. Verified live against BrowserStack App Automate: POST /rotation {"x":0,"y":0,"z":0} -> 200 {"value":"ROTATION_0"} The rotation-to-surface-index mapping moves to contracts/device-rotation.ts and the existing adb path now reads from it, so the local and hosted mappings cannot drift apart. Note this rotates the current display, not persistent device rotation, so an activity that does not pin its own orientation may still need rotating once it is in the foreground. The capability was declared "partial" without the transport existing, and no test covered setOrientation on the cloud path; only adb and the Apple runner were covered. Both gaps are now closed. * fix(cloud-webdriver): narrow orientation fallback and gate provider-owned flags Addresses review on #1544. The orientation fallback caught every error, so a timeout, an auth rejection, a dead session or a provider 5xx on the first transport was swallowed and retried against the second. When that one also failed the caller got "rejected both endpoints" with the real cause discarded. Fallback is now keyed on structured unsupported-endpoint signals only — HTTP 404/405, or a W3C `unknown command` / `unknown method` code — matching the repo rule of keying on typed details rather than message text. Everything else rethrows unchanged. Device-feature capabilities are BrowserStack-owned, but the flags were accepted by any cloud provider, persisted into the generated profile, and then silently dropped at session creation. `connect aws-device-farm` now rejects them with a typed error naming each offending flag, raised before the provider's own required-argument checks so the caller is told what is unsupported rather than what else is missing. Ownership is modelled on the capability spec table, so a new capability inherits the guard without a second list to maintain. Adds provider-backed orientation scenarios driven through public daemon dispatch against the fake WebDriver provider: the four-way endpoint on the happy path, the documented collapse onto the two-way endpoint when the driver does not implement `/rotation`, and a provider 5xx that must surface without consulting the second transport. The fake server's route handling became a table in the process — it had grown to ten branches in one function. * fix(cloud-webdriver): read W3C error codes before status, enforce ownership at the runtime boundary Addresses the second review pass on #1544. The fallback classifier returned on any 404/405 before consulting the W3C error code, so an HTTP 404 carrying `invalid session id` was masked as a missing route and retried against the second transport. The structured code now takes precedence whenever the driver sent one; bare status is consulted only when no code exists. Two cases pin it: a 404 `invalid session id` and a 405 `timeout` must both surface rather than fall through. Provider ownership was enforced only in the CLI profile builder, which the typed client and hand-authored remote-config profiles bypass entirely — both reach session preparation without passing through `connect`, so the capabilities were accepted and then dropped. The check now lives on the capability-ownership module and runs inside AWS Device Farm's `prepareSession`, with the CLI builder calling the same helper instead of its own copy. Covered by a scenario that drives the runtime boundary directly and asserts the rejection happens before any provider session is created.	2026-08-02 08:17:49 +02:00
Michał Pierzchała	b9509fe006	refactor: extract the .ad script codec into packages/ad-script (#1478) (#1536) ... * refactor: extract the .ad script codec into packages/ad-script Moves the mutually-coupled .ad read/write codec (script.ts, script-utils.ts, script-formatting.ts, open-script.ts) plus the target-v1 annotation SERDE slice of target-identity.ts into a new private leaf package, @agent-device/ad-script, exporting only `.`. This is option 1 from the P5 scoping dossier on #1478: the codec is shared by the daemon's session-script publication writer, the future replay engine, the CLI's `replay export`, and Maestro's failure-label formatting, so it can no longer live in root src/ once packages/ad-replay lands (R11 forbids a package reaching into root src), and a second export subpath or writer-half duplication are both ruled out by existing gates/tests. target-identity.ts keeps only the record/replay-shared classification core (classifyTargetBindingMatch, local-identity/ancestry-prefix matching), importing its shared types from the new package. Every real consumer (re-derived by grep, not the dossier's list alone) is rewired to @agent-device/ad-script. Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com> * refactor: trim the ad-script façade to real consumers, lock the one-export boundary - packages/ad-script/src/index.ts: drop parseReplaySeriesFlags, formatTargetAnnotationCommentLine, parseTargetAnnotationCommentLine, TargetAnnotationLineParseResult, and TargetRect from the public façade — none has a consumer outside the package (re-swept every remaining export by grep; everything else kept has at least one real external importer). The functions/types stay exported from their declaring internal modules for the package's own internal use (script.ts, script-formatting.ts). - scripts/layering/package-boundaries.test.ts: add the parallel R11 assertions "the real tree parses, declares, and passes R11" already makes for maestro/provider-webdriver/provider-limrun/xml — ad-script exports exactly `.`, depends on exactly contracts+kernel, and is declared in root package.json — plus ad-script entries in the deep-resolution rejection coverage. Verified the lock catches a regression: temporarily added a fake `./codec` export to packages/ad-script/package.json and confirmed both the export-key-list assertion and the deep-resolution-rejection assertion fail; removed the plant and reconfirmed green. Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com> * fix: remove polynomial-redos ambiguity from the target-v1 annotation line regex CodeQL js/polynomial-redos flagged TARGET_ANNOTATION_LINE_RE (packages/ad-script/src/internal/target-annotation-serde.ts): the payload group's `\s+(.*)` let `\s+` and the unconstrained `.*` both match whitespace, so a run of separator whitespace that ultimately fails to complete the match has many `\s+`/`.*` splits to backtrack through before concluding failure. Anchor the payload group on `\S` (the exact complement of `\s`), so the mandatory `\s+` separator and the payload's first character can never overlap — the split point becomes unique and no backtracking is possible. Behavior-preserving: the only caller (parseTargetAnnotationCommentLine) always matches against an already-.trim()-ed line, whose last character (whenever the tag matches at all) is never whitespace — so a payload section `\S.*` would reject (content that is entirely whitespace) can never reach this regex through the real call path. Verified against the frozen replay-compat corpus and the full serde/parser test suites, unmodified. Added a regression test with the exact adversarial shape CodeQL/the reviewer cited (many tab pairs after the version digits), asserting sub-second parse. Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com> * test(ad-script): pin the annotation-line pattern's linear rejection directly The entry-point adversarial case matched greedily even with the retired regex (trim strips edge whitespace and per-line input carries no newline), so it proved nothing about the pattern. The regression surface is the pattern itself: an interior tab run with an x-newline tail fails the match, which the retired form re-split quadratically (3.7s at 100k tabs) and the \S anchor rejects in one attempt. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-01 20:21:38 +02:00
Michał Pierzchała	ef66dcdf25	refactor(daemon): serialize replay transactions behind a locked coordinator (#1478 P4b) (#1535) ... * refactor(daemon): serialize replay transactions behind a locked coordinator Adds session-replay-coordinator.ts, a ReplayCoordinator scoped to one locked native .ad replay request, and routes every repair-transaction write session-replay-runtime.ts and session-replay-resume.ts perform through it: arm, demote-for-rerun, mark-complete, hold-on-divergence stamping, the pendingRecordAndHeal corrective watermark (set + clear), and reap-tombstone clearing. Neither file imports session-replay-transaction.ts (P4a's ReplaySessionTransaction) or writes session.pendingRecordAndHeal directly anymore. Adds a minimal immutable ReplaySessionView (repairBoundary, pendingRecordAndHeal) so the three readers this slice touches (preflightReplayAgainstActiveRepair, isRepairArmedTerminalClose, the entry-index resolution in prepareReplayPlan) stop taking mutable SessionState. Close-time sequencing (session-close.ts's platform-close receipt, session-close-script.ts's commit/abort) stays a direct ReplaySessionTransaction caller by design: commit/abort happen at teardown, ordered against platform close and lease release, not during a replay request. Updates the R7 session-state ownership registry: pendingRecordAndHeal moves from session-replay-resume.ts to session-replay-coordinator.ts. The daemon-modularity baseline (writer-owned fields / owner claims) is unchanged. Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com> * docs: state the coordinator constraint, not the migration Co-Authored-By: Claude <noreply@anthropic.com> * fix(daemon): thread one bound resume-stamper instead of a second coordinator buildAndPersistReplayDivergenceResume (session-replay-resume.ts) constructed a SECOND ReplayCoordinator from a bare SessionStore + session name, reachable from both divergence paths (session-replay-target-verification.ts and the action-failure chain through session-replay-runtime-failure.ts / session-replay-divergence.ts). That let a lower handler manufacture repair authority by naming a session instead of using the request's own locked coordinator. Adds ReplayResumeStamper: a narrow capability bound to the coordinator runReplayScriptFile already created, exposing only sessionExists() and stampCorrectiveWatermark(). Threads it through ReplayStepContext and the failure-wrapper params into both chains. buildAndPersistReplayDivergenceResume now takes the stamper and holds no SessionStore or coordinator-construction ability at all. Adds src/daemon/__tests__/replay-coordinator-ownership.test.ts, an oxc-parser AST structural test (same approach as scripts/layering/session-state.ts) asserting: createReplayCoordinator has exactly one production call site (session-replay-runtime.ts); none of the five divergence-chain files import the coordinator factory or session-replay-transaction.ts; session-replay-resume.ts holds no session-store.ts import at all; and the other four hold SessionStore only as a type. Verified the test fails on a planted violation of each of the two structurally-distinct invariants (coordinator-construction, SessionStore value-import) and passes once removed. Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-01 13:51:02 +02:00
Michał Pierzchała	67f3d09d95	refactor(daemon): session script publication behind one capability (#1478 P4a) (#1532) ... * refactor(daemon): add the tagged script-publication aggregate First step of P4a. Nine co-resident optional SessionState fields encode two lifecycles plus a shared output target, with nothing in the shape saying the lifecycles are disjoint — so readers re-derived that from field combinations and writers had to remember which siblings to clear. The aggregate makes both invariants structural: a session publishes nothing, authors ordinarily, or is under repair; and force lives inside the target, so retargeting replaces the authorization along with the path. Three corrections after an adversarial review of the first draft: - The target is a default|explicit union, not a mandatory path. A bare 'open --save-script' arms with no path and lets the writer resolve a daemon-owned destination at write time, and force can be granted before any path exists. Eagerly materializing a default path would have silently changed retarget semantics, because today's check requires a previously persisted path — so 'open --save-script --force' then 'close --save-script=out.ad' is not currently a retarget and the grant survives. That behavior is preserved here and flagged in the docblock as a probable #1258 gap; tightening it is a product change and belongs in its own commit. - The repair status relation is not linear. A failed commit followed by 'replay --from' demotes complete back to armed, so demoteRepairToArmed exists and deliberately RETAINS the close receipt: the platform close already succeeded for that operation identity, and dropping it would re-dispatch a close on retry — which is also how a migrator ends up reaching for the caller-computed platformCloseSucceeded boolean the brief forbids. - The receipt doc no longer claims it is set only at close-succeeded and later, since the demotion path makes {armed, receipt set} reachable. Still to come in this PR: both projections, and the writer migration. Note the brief's seven-file writer inventory omits session-open.ts, which holds the only two writers of the authoring armed/aborted states. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * refactor(daemon): migrate script publication onto the tagged aggregate (#1478 P4a) The eight co-resident SessionState fields (scriptRecordingState, saveScriptPath, saveScriptForce, saveScriptBoundary, saveScriptComplete, saveScriptCommitted, repairPlatformCloseReceipt, repairSourcePath) are gone; SessionState.scriptPublication holds the aggregate, and every writer migrated in this commit — no shadow state. Two daemon-private projections own the writes, enforced by the R7 ownership gate: - session-replay-transaction.ts (ReplaySessionTransaction): repair arm/demote/ complete/abort, close receipts, and the uncommitted/boundary/sourcePath reads that idle-reap, tombstones, divergence-hold, and the recorder's exclusion key off. - session-script-publication-capability.ts (SessionScriptPublication): authoring arm on open, the recorded --save-script flag ingress, active publication, the published transitions, and the effective per-target force decision (#1258). The writer keeps the commit transition so idempotence stays colocated with the atomic publish. Failure/retry transitions pinned as the brief requires: platform-close failure leaves state unchanged (no receipt, retry re-dispatches); publication failure retains target+force+receipt (same-identity retry skips close dispatch); committed and aborted are explicit terminal states that drop the receipt. Design decisions resolved: - Force retention across a default->explicit retarget is preserved as-is and still flagged in resolveScriptTarget's docblock as a probable #1258 gap; tightening it stays a separate product change. - The never-armed 'close --save-script' whole-log publication folds into the authoring lifecycle (armed at the recorded close, published in the same request) instead of a fourth variant: every close path that reaches the write deletes the session, so the transient armed state cannot leak into 'session save-script' eligibility, whose not-armed-before-this-journey rejection is untouched. One real bug caught by the migrated tests and fixed in resolveScriptTarget: a bare (pathless) re-arm collapsed an already-materialized explicit target back to the daemon default, wiping the healed-sibling path on every per-step repair re-arm and defeating the persisted-force preflight bypass. A bare re-arm now keeps the previous target and only adds a live force grant. R7 rows consolidated to one scriptPublication entry (three owners) and the recordSession row narrowed; the R10 baseline drops to 22 writer-owned fields / 28 owner claims so the consolidation cannot regrow. Gates: typecheck, lint, format, layering clean; 624 files / 5220 tests pass (two known contention-flake timeouts reproduce only under full-suite load and pass in isolation). Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com> * refactor(daemon): satisfy the Fallow gate by extracting decisions, not suppressing - scriptPublicationTarget is module-private; both public target reads (scriptTargetPath/scriptTargetForce) go through it and nothing else did. - validatePublicationEligibility splits into a pure ineligibility classifier and an error table, so the four rejections read as one decision each. - prepareSaveScriptSession hands its two arm-time rejections (authoring re-arm, EEXIST preflight) to rejectSaveScriptArming and keeps only the demote-and-arm flow. - The repair-record-exclusion provider scenario extracts its three phases (arm-and-hold, exclusion contrast, healed-script contract) into named helpers; the test body is the journey again. Refs #1478 Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-01 11:10:09 +02:00
Michał Pierzchała	cbe1a57094	refactor(replay-test): extract packages/replay-test (#1478 P3b) (#1525) ... * refactor(replay-test): source the manifest device vocabulary from the kernel `session-test-types.ts` reached `ReplayScriptMetadata['platform']` and `['target']` through `replay/script.ts` — the native `.ad` engine. A format-neutral scheduler must not name an engine module, and P5 relocates that engine into `packages/ad-replay` regardless, so the import had to go before the scheduler can move. Both members already resolve to neutral kernel types (`Exclude<PlatformSelector, 'web'>` and `DeviceTarget` from `@agent-device/kernel/device`), so this re-sources them directly and the manifest shape is unchanged. Only the import direction differs. First increment of P3b; the scheduler still has request-global, engine and daemon imports to port before the physical move. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * refactor(replay-test): inject the progress sink instead of reading a request global The scheduler called `emitRequestProgress` in eight places, which reads a sink out of a request-global `AsyncLocalStorage`. That is ambient authority a format-neutral scheduler cannot hold once it lives in `packages/replay-test`, and #1505 recorded it as a shrink-only R10 entry. The host now injects the capability through the existing `ReplayTestRuntimeDependencies` seam established in P3a, so no new seam is invented. `session-replay.ts` supplies `emitProgress: emitRequestProgress`; `src/request/progress.ts` keeps the sink and its AsyncLocalStorage binding for every other caller. The port is deliberately narrower than `RequestProgressSink`: it accepts only `ReplayTestSuiteProgressEvent | ReplayTestProgressEvent`, so the scheduler is not handed the ability to emit `CommandProgressEvent`. Authority narrows again one hop down: `runReplayTestAttempt` spread the whole dependency bag but uses three of its members and never publishes progress, so it now takes `Pick<..., 'runReplay' | 'cleanupSession' | 'finalizeAttempt'>`. That is why no runtime test fixture needed changing — the attempt runtime never gained the capability in the first place. Also drops the last two `replay/script.ts` type references from `session-test-runtime.ts`, so the engine import is gone from that file too. Reporter contract preserved: `session-test-reporter-values.test.ts` and `session-test-reporter-values-maestro.test.ts` both pass unmodified (27 tests green across the five scheduler suites). Typecheck clean. Remaining scheduler boundary for P3b: `request/cancel.ts`, `replay/format.ts`, `replay/script.ts` in discovery, `session-store.ts`, `daemon/types.ts`, `replay-source-discovery.ts`, `core/dispatch*`, `utils/diagnostics.ts`. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * refactor(replay-test): ask the host whether the suite is canceled The scheduler called `isRequestCanceled(requestId)` in five places. That both reaches a request-global registry and forces the scheduler to name a daemon request id as the cancellation key — neither survives the move into `packages/replay-test`. The host now binds the predicate to its own request and passes `isCanceled: () => boolean`. The scheduler asks a question it is entitled to ask and learns nothing about how cancellation is tracked. `shouldStopReplayTestExecution` takes the capability rather than a request id, so no scheduler function threads a daemon identifier for this purpose any more. `session-test-attempt.ts` and `session-test.ts` no longer import `request/cancel.ts` at all. It remains in `session-test-runtime.ts`, which does something different — `registerRequestAbort`, `markRequestCanceled` and the parent-abort relay are cancellation *binding*, which the brief assigns to the daemon adapter, so that split is its own step. Behavior preserved: both pinned reporter characterizations pass unmodified, 32/33 across the five scheduler suites. The one failure is the pre-existing P2/#1506 discovery-ordering regression, unrelated and untouched here. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * refactor(replay-test): drop the dead request-tracking call from attempt ids `buildReplayTestAttemptRequestId` wrapped its template in `resolveRequestTrackingId`, pulling `request/cancel.ts` into the scheduler. That wrapper substitutes a generated id only when its first argument is an empty string. The template here always contains `:test:`, so it is never empty and the wrapper always returned it unchanged — the call is unreachable in this path. Probed all three shapes (explicit request id, suite-id fallback with a shard, and degenerate empty inputs); every one returns the template verbatim. Removing it takes `request/cancel.ts` out of discovery without altering a single produced id. The scheduler mints attempt identity itself, which is what the brief asks for. Evidence the ids are byte-identical: the pinned reporter characterizations assert exact session strings such as `default:test:suite-reporter:1-02-retry:attempt-1` and pass unmodified — 30 tests green across the reporter, suite and discovery suites. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * refactor(replay-test): move cancellation binding and diagnostics to the host `session-test-runtime.ts` held the last two request-globals in the scheduler: `request/cancel.ts` (registerRequestAbort, markRequestCanceled, clearRequestCanceled, plus the parent-abort relay) and `utils/diagnostics.ts`. These are different in kind from the earlier ports. The brief gives the daemon adapter the job of mapping an attempt id to daemon request identifiers and *binding cancellation*, while timeout policy stays scheduler-owned. So the scheduler now receives a per-attempt capability with exactly two verbs — `cancel()` on timeout and `release()` when the attempt settles — and every registry interaction, including `relayReplayTestAbortFromParent`, moved to `session-replay.ts` next to the rest of the adapter. Diagnostics became a narrow publish capability for the same reason: `emitDiagnostic` reads a request-global scope. The level vocabulary is spelled out at the seam rather than imported, so nothing engine- or daemon-shaped crosses it. The runtime fixtures drive the real exported host binding rather than a stub. They assert cancellation through `isRequestCanceled`, and a stubbed binding would have kept those assertions passing while proving nothing. 24 tests green across the runtime, suite and both reporter characterizations, which pass unmodified. Typecheck, lint and oxfmt clean. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * refactor(replay-test): split discovery into host inspection and scheduler policy discoverReplayTestEntries expanded paths, read every file, and called both engines — readReplayScriptMetadata for .ad, inspectMaestroFlow for Maestro — plus resolveReplayFormat to choose between them. Four imports a format-neutral scheduler cannot hold. Inspection is now the host's discoverSources capability. What stays in the scheduler is the genuinely neutral half: which sources a --platform filter runs, which it skips and with what message, and the empty-suite error. The manifest carries exactly the four fields the scheduler consumes (platform, target, retries, timeoutMs) plus the reporter's title, per the brief's instruction not to add more without a demonstrated call site. The platform tag is what removes the last format leak. The filter used to ask resolveReplayFormat(...) === 'maestro' to decide whether a missing platform was disqualifying. It now reads a tag: caller-bound means the invocation supplies the platform, unspecified means the source declared none. Maestro is what caller-bound looks like from the scheduler's side, and the format cannot be recovered from it. Discovery tests drive the real inspection capability, writing actual .ad and Maestro sources — a stubbed host half would have kept them green while proving nothing about the composition they exist to pin. 35 tests green across discovery, suite, runtime and both reporter characterizations, which pass unmodified. The Maestro one is the direct check that titles still flow, since they now arrive via the manifest. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * refactor(replay-test): build attempt ids from named segments; trim comments Review feedback on the attempt-id builder: the comment explained a deletion that git already records, and it sat above an opaque template literal. The id is now a segment list joined on ':', so its shape is readable without prose. Output is byte-identical — the reporter characterizations assert exact session and attempt strings and pass unmodified. Applied the same standard to four other docblocks in this PR that narrated what the code used to do rather than what it does. The durable 'why' stays: which side of the seam owns what, and why the vocabulary is neutral. The migration history goes, since git carries it and these docblocks will outlive the migration. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * refactor(replay-test): move shard device binding to the host buildReplayTestShardPlan called listDeviceInventory to discover what to shard across, and buildReplayTestShardFlags constructed daemon CommandFlags for the nested request. Inventory enumeration, allowlists, simulator set paths, explicit --device selectors and the too-few-devices error are host concerns; what is scheduler-owned is deciding how many shards exist and which entries each one runs. The scheduler now receives resolved shard targets through a capability. The target is neutral: id and name for session labels and progress metadata, plus platform and target, which are already kernel vocabulary. DeviceInfo no longer crosses into scheduling. One behavior note: an explicit --device selector could in principle name a web target, which is not a shardable device. That is now rejected with INVALID_ARGS rather than widening the neutral platform vocabulary to carry something the scheduler can never run. Implicit selection already filtered to mobile. 919 of 920 handler tests pass. The one failure, session-test-runner.test.ts 'binds each replay script to its declared platform metadata', fails identically on clean origin/main in this container and is unrelated: directory discovery walks with opendirSync/readSync and directory results are deduped but not sorted, while glob results are sorted, so suite order is filesystem-dependent. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * refactor(replay-test): extract packages/replay-test behind a façade Completes the P3b extraction. The scheduler, attempt runtime, discovery policy, sharding distribution, artifacts and neutral types now live in packages/replay-test/src/internal/, with one package-root export. The façade takes a neutral ReplayTestSuiteRequest and returns a tagged ReplayTestSuiteOutcome. DaemonRequest, DaemonResponse and CommandFlags no longer reach the scheduler; the adapter translates flags and meta in, and the outcome back to a daemon response. Eight flags were read by the scheduler and each became a field it owns. Host work moved to daemon adapters: source inspection (both engines and format routing), shard device binding and shard-flag parsing, and artifacts-dir home expansion, which is why the package can resolve paths without SessionStore. The one remaining shared concern was the timing trace: the host writes video lifecycle events into the same trace the scheduler owns. Rather than export a writer from the façade, each attempt hands the host an appendTimingEvent closure, so the trace format stays private and the authority is scoped to that attempt. Tests mirror the topology. Discovery tests split along the seam they now cross: ordering, traversal and routing are pinned host-side against real files, filtering policy is pinned in the package against fake sources. The runtime tests assert the scheduler's cancellation obligation (cancel once on timeout, always release) against a recording binding, and a new daemon test pins the adapter's half — registry entries, the parent-abort relay, and detach on release — so that coverage moved rather than disappeared. R10 retargeted to packages/replay-test/src/ and the zone ranked alongside maestro. R11 confirms zero root-src imports from the package. 914 of 915 handler and package tests pass. The one failure, session-test-runner 'binds each replay script to its declared platform metadata', fails identically on clean main here: directory discovery walks with opendirSync and dedupes without sorting, while globs sort, so suite order is filesystem-dependent. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * refactor(daemon): simplify replay-test request translation Fallow flagged toReplayTestSuiteRequest at 14 cyclomatic in 18 lines. The branches were self-inflicted: every req.flags?.x is one, and each optional field was written as a conditional spread to avoid setting an undefined key. exactOptionalPropertyTypes is not enabled, so assigning undefined to an optional field is equivalent and the spreads bought nothing. Destructuring flags once and extracting two flag readers removes most of the rest. One correctness note on the simplification itself: the first version used `artifactsDir && expandHome(...)`, which returns '' for an empty-string flag where the previous code called expandHome(''). Replaced with an explicit undefined check so the empty-string path is unchanged. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * test(live): share the replay test-suite harness across iOS and Android Both live journeys invoked the public test command and then re-derived the same value-contract assertions by hand — suite totals, per-script status, replay counts, non-empty JUnit. Those are claims about the published suite result and are identical on every platform, and they had already drifted: iOS iterated with readReplayCommands inline, Android cast data.tests at the call site. The shared helper owns exactly that boundary. It takes the caller's runStep rather than binding a context type, so it is not a platform-configured runner and cannot template a platform's journey. Everything a platform genuinely differs on stays with the caller: which scripts run, the retry policy (iOS 2, Android none — itself a claim worth keeping), which commands each script exercises, and the behavioral evidence. Both callers keep every verify* call they had. 67 lines removed, 15 added. Residual risk: this container has no iOS or Android devices, so the live suites could not be executed here. Typecheck and lint pass; the harness needs a run on real targets before the claim that behavior is unchanged is evidence rather than inference. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * test: pin directory enumeration in the platform-binding suite test The test wrote two scripts into a temp directory and assumed discovery would return them in creation order. Directory expansion deliberately preserves filesystem order to match Maestro — only glob expansion sorts, and 'preserves Maestro directory filesystem order' pins that with a mocked opendirSync. So the ordering contract is correct; this test's assumption about enumeration was not. It passes on CI, where small directories usually enumerate in creation order, and fails on filesystems that do not — identically on clean main, where the platform-to-script binding appears reversed. Pinning enumeration the way the discovery tests already do keeps the subject intact (each script binds to ITS declared platform, and session numbering follows discovery order) without depending on the host filesystem. The fs import became a default import because vi.spyOn cannot redefine an ESM namespace export. 915 of 915 handler and package tests now pass here. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * test: scope the enumeration spy and restore it in a finally The spy I added restored only on the happy path and asserted on its argument inside the mock implementation. Either would misfire for anything else sharing the worker: an assertion thrown from inside fs, or a leaked global opendirSync, surfaces as a worker crash with no failed test rather than a readable failure. It now delegates to the real implementation for any directory but this suite's own, and restores in a finally. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * fix(replay-test): put package tests where they are actually run Review found the moved package tests were neither executed nor typechecked. They sat under packages/replay-test/test/, but vitest's unit-core lane includes packages/*/src/**/*.test.ts, and neither the root nor the package tsconfig covers a top-level test directory. A plain unit-core run discovered zero files under the package. That is why they looked green: my earlier runs passed those paths explicitly on the command line, which masked that the default run skipped them. The count is the proof — 550 files/4741 tests before, 553/4753 now, and the delta is exactly the three files and twelve tests that were being skipped. The runtime test also imported runReplayTestAttempt from the package specifier, which the facade does not export. It would have failed the moment it was discovered. It now imports internally, like the rest of the internal tests. Also removed replayTestAttemptFailure from the facade: zero consumers outside the package, so exporting it widened the boundary for nothing. P3 asks for a one-function facade. 553 test files and 4753 tests pass; lint and the layering guard are clean. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * style: format the facade after removing the unused export A scripted edit removed the export line but left a stray blank line; oxfmt was not re-run on that file afterward, so Lint & Format caught what pnpm lint alone does not. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * fix(replay-test): typecheck the package and fix a type-only import Review found the moved package tests were transpiled by vitest but never typechecked: the root typecheck script builds six packages via tsc -b and packages/replay-test was not among them, so its tsconfig was never used. That hid a real TS2459. session-test-runtime.test.ts imported ReplayTestAttemptOutcome from ../session-test-runtime.ts, which imports that type but does not re-export it. It now imports from ../session-test-types.ts, where the type is defined. Adding the package to the tsc -b list closes the gap. Verified empirically rather than assumed: planting a string-to-number error in a package test makes typecheck fail, and removing it makes it pass. This is the second finding of the same shape on this PR — first the tests were not discovered by vitest, now they were not covered by typecheck — so the gate was confirmed to reach the files rather than trusted to. 12 package tests pass, lint, format and the layering guard are clean, and typecheck is clean with the package included. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-31 20:16:18 +02:00
Michał Pierzchała	da93191201	refactor: move Limrun provider behind package facade (#1518) ... * refactor: move Limrun provider behind package facade * fix: preserve Limrun public provider types * fix: tighten Limrun provider facade boundaries * test: harden Limrun compatibility coverage * fix: narrow Limrun public type exports * fix: narrow Limrun provider exports	2026-07-31 15:35:49 +02:00
Michał Pierzchała	b125435989	refactor: extract WebDriver provider package (#1504) ... * refactor: extract webdriver provider package * refactor: consolidate shared XML codec	2026-07-31 09:10:04 +02:00
Michał Pierzchała	a3ab69a110	refactor(replay-test): neutralize the values crossing the scheduler seam (#1478 P3, part 1) (#1509) ... * refactor(replay-test): neutralize the values crossing the scheduler seam #1478 P3, part 1 of 2. Prepares the replay-test extraction by removing every non-neutral value that crosses the scheduler seam, in place under `src/`, so the physical move to `packages/replay-test` is a file move rather than a redesign. `DaemonResponse` no longer crosses the seam. `session-test-types.ts` typed `runReplay`/`finalizeAttempt` as returning a daemon response and the scheduler read `.error.code`, `.error.details`, and `.data.replayed/.healed/.warnings/ .snapshotDiagnostics` off it throughout. That is invisible to R10 today only because `checkDaemonTypesImporters` skips `src/daemon/`; once the files live in a package they become external `daemon/types.ts` importers, which the ratchet only lets shrink. Attempts now resolve as tagged `ReplayTestAttemptOutcome` values carrying exactly what the scheduler consumes, including an `infrastructure` tag — classifying an environmental failure needs platform boot-diagnostic vocabulary the scheduler must not import, so the host decides and the scheduler reads the verdict. `session-test-outcome.ts` is the one place a daemon response becomes an outcome. Step events get a narrow per-attempt port. They were emitted from `session-replay-runtime.ts` and `session-replay-maestro-observer.ts`, both reading a request-global `AsyncLocalStorage` seeded per attempt. The scheduler now hands each attempt an `onStep` sink, threaded the way `tracePath` already is; both engines call it and `withReplayTestActionProgress`/`readReplayTestActionProgress` are gone. A direct `replay` simply has no sink. ADR 0012 divergence becomes a neutral leaf. `src/replay/divergence.ts` depended only on kernel contracts and redaction, yet Maestro constructs divergences too and CLI/MCP both render them, so P5 could not have moved it into `packages/ad-replay`. It is now `@agent-device/contracts/divergence`; the renderer's output text is unchanged. The progress wire vocabulary moves to `@agent-device/contracts/progress`. It is serialized by `request-progress-protocol.ts` and reconstructed by the CLI reporter path, so it belongs below both; `src/request/progress.ts` keeps only the sink and its AsyncLocalStorage binding. Together these clear all four of replay-test's recorded R10 migration imports, so the rule now enforces unconditionally for that module. Behavior is unchanged. The shipped reporter contract — export spellings, object/factory loading, hook names, timing/order, value fields, the synchronous live-hook rule, awaited suite completion, error handling, exit codes — is untouched, and `session-test-reporter-values.test.ts` passes unmodified. The `--shard-all` `total`/`runnable` asymmetry is preserved as characterized. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * test(replay-test): pin the Maestro reporter step path against the onStep port Review finding on #1509: the native `.ad` reporter ratchet exercises only one of the two `onStep` forwarding chains, so deleting a link in the Maestro chain would silently stop `onTestStep` for every `test --maestro` run while every existing reporter test stayed green. Same defect class as the dropped diagnosticId/logPath (#1501) and the dropped reporter `hint` (#1505). Maestro is one of P3's two required real adapters and its chain shares no links with the native one below `runReplayScriptFile`: scheduler sink -> runReplayScriptFile -> runTypedMaestroReplayFile -> createMaestroReplayObserver({ onStep }) -> actionStarted -> onStep Adds a Maestro scenario driving `test --maestro` through the real session handler and the real reporter registry. It asserts the step payload the engine produces (`stepIndex`/`stepTotal`/`stepCommand`/`stepValue`, including that a value-less command stays value-less) together with the attempt/session identity the scheduler supplies, since that half of the event came from request-global AsyncLocalStorage before P3. A second case drives a retry so step events must carry attempt-1's session and then attempt-2's. The flow `name` also pins the reporter `title`, a value only the Maestro path can produce. New file rather than an addition to session-test-reporter-values.test.ts: that file is the pinned characterization and must keep passing unmodified, and Maestro needs its own vi.mock of core/dispatch for device resolution. Counterfactual run, both links, each restored after: - dropping `onStep` from createMaestroReplayObserver in session-replay-maestro-runtime.ts - dropping the emitMaestroStep call from actionStarted in session-replay-maestro-observer.ts Each dropped both onTestStep events ("expected [ 'onSuiteStart', 'onTestStart', …(2) ] to deeply equal [ 'onSuiteStart', 'onTestStart', …(4) ]") and failed both new cases, while session-test-reporter-values.test.ts passed all 4 — exactly the hole the reviewer identified. Test-only; no production change. Bundle output is byte-identical to b339c640f. Refs #1478 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-31 08:48:20 +02:00
Michał Pierzchała	1fc9169188	refactor(daemon): consolidate session-script test factories and drop saveScriptDefaultedHealedPath (#1508) ... Preparatory slice for P4a (#1478). No aggregate, no transaction type, no publication-writer migration — those land separately. Two things: 1. Name the session-script session states in the shared test factories (`makeAuthoringSession`, `makeRepairArmedSession`, `makeRepairCompleteSession`) and route 40 inline session literals across 11 test files through them. The `saveScript*` fields are not independent — recording without a boundary is ordinary authoring, a boundary without `saveScriptComplete` is an ARMED-but-uncommittable repair, and only the COMPLETE combination publishes — so re-deriving the combination per test buried the distinction each test was actually about. Pure refactor: the factories write today's fields and no assertion was weakened. 2. Delete `saveScriptDefaultedHealedPath`. It had zero production readers: the writer's refuse-on-exist guard has been uniform since #1235, so the flag was written in three places and never consulted. Removing it takes its R7 owner row with it and lowers the R10 baseline from 30/42 to 29/40 (the ratchet fails on a drop too, so this cannot be deferred). Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 Co-authored-by: Claude <noreply@anthropic.com>	2026-07-31 07:24:22 +02:00
Michał Pierzchała	0e51007b04	refactor: isolate maestro engine package (#1506) ... * refactor: isolate maestro engine package * perf: deepen maestro facade boundaries	2026-07-30 20:58:20 +02:00
Michał Pierzchała	352428d37a	test: characterize replay-test reporter contract and extend R10 (#1505) ... * test: characterize replay-test reporter contract and extend R10 P3 of #1478 moves the replay-test scheduler into `packages/replay-test` and makes attempt identity scheduler-owned. Before production code moves, pin what a shipped custom reporter actually observes today, and close the import boundary the extraction has to end up satisfying. Reporter values (all pinned as shipped, none proposed): - the `RequestProgressEvent` -> reporter-value projection field by field, including key presence for absent optionals and the dropped `command` events; - `session` provenance across the seam: the start value is always `attempt-1` (it is built before any attempt runs), step values track the running attempt through the per-attempt AsyncLocalStorage context, and result values carry the attempt that produced them, so a retried case reports three different sessions to one reporter; - the shard-scoped session prefix and device identity a sharded run reports; - module export spelling precedence, the six optional hook names, the live-hook vs final-hook error asymmetry, and exit-code recommendation semantics. Late-timeout finalization/cleanup: - finalization always runs before cleanup, and the timing trace records that order (`finalize_start/stop` then `cleanup_start/stop`); - a replay settling inside the 2s grace window cleans up once and is not marked `timeout_cleanup_pending`, and the raced TIMEOUT response still wins; - a replay that misses the window defers its second cleanup until the abandoned replay settles, and that late cleanup's failure is swallowed. R10 now rejects replay-test imports from `src/request/**` and engine internals (`src/replay/`, `src/compat/`, `src/maestro/`, `src/ad-replay/`), with the four imports that exist today recorded as shrink-only migration entries so the rule enforces immediately and the extraction must delete them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 * test(daemon): assert reporter hint on the real onTestResult value The failing-suite characterization supplied `hint` as input but never asserted it on the hook value, so dropping `hint: error.hint` from the scheduler path left all reporter tests green. Assert it on the real reporter value so the ratchet catches a shipped field going missing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RXQLYV7etZx3gcXsUsrQJ8 --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-30 19:05:39 +02:00
Michał Pierzchała	0ee2a86129	refactor: extract contracts workspace package (#1499) ... * refactor: extract contracts workspace package * fix: preserve screenshot diff result contract * test: stabilize Android keyboard smoke	2026-07-30 17:07:46 +02:00
Michał Pierzchała	cd9a7ce41b	test(android): add comprehensive emulator E2E coverage (#1482) ... * test(android): add catalog emulator smoke coverage * test(android): use stable snapshot diff mutation * test(android): assert actual back destination * test(android): separate keyboard and fill IMEs * fix(ci): keep Android timing report in one shell * refactor(test): simplify simulator e2e coverage * test(android): assert stable diff landmarks * fix(android): release snapshot helper gracefully * fix(android): fully release snapshot helper runtime * test(android): report coverage classifications * fix(android): stabilize accessibility root capture * fix(android): bound UiAutomation connection * ci: upload worktree daemon diagnostics * fix(android): cancel stalled wait captures * fix(android): bound helper fallback lifecycle * fix(android): harden emulator e2e lifecycle * fix: align e2e changes with kernel package * test(android): prove alert helper reuse directly * fix(android): cancel stalled settle captures * fix(android): separate helper retirement budgets	2026-07-30 15:10:21 +02:00
Michał Pierzchała	76453add71	refactor: pnpm workspace + @agent-device/kernel pilot (#1490 W0) (#1494) ... * refactor: pnpm workspace + @agent-device/kernel pilot (#1490 W0) Extend the workspace with packages/* and move the kernel behind an enforced public API: packages/kernel with nine consumer-earned subpath exports (errors, device, snapshot, contracts, collections, rect, redaction, daemon-error, bounds — the last absorbed from utils as Rect vocabulary). Every kernel import repo-wide becomes the @agent-device/kernel/<sub> specifier; kernel tests move to src/__tests__/kernel/ and exercise the package surface. The root declares the package in devDependencies (workspace:*), tsdown bundles it (noExternal) so the published artifact and its runtime dependency manifest are unchanged. Gate rewiring in the same change, per the W0 brief: - R1 kernel-sink retires (physically subsumed); new R11 package-boundaries guards no-root-back-imports, relative tunnelling past exports maps, undeclared workspace deps, and non-exported subpaths, with runtime resolution pins via import.meta.resolve. - resolveImportEdges and mutation ownership follow workspace specifiers through exports maps, keeping R4 cycle checks, depgraph, and derived test ownership connected across the seam (kernel-errors still owns 495 tests). listSourceFiles includes packages/*/src. - kernel becomes an unranked zone; mutation registry, stryker mutate globs, and the mutation-affected workflow path filter move to packages/kernel/src/errors.ts. - check:affected gains packages/ ownership (manifests fail open); vitest and coverage include packages/*/src; fallow ignores packages/** (its resolver cannot follow workspace specifiers). - The affected-selector CI job installs dependencies: its closure now crosses workspace specifiers, and the R8 relative exception is unsafe for production src files (Node ESM does not realpath, so dual specifier/relative loads would instantiate modules twice). The R8 zero-dep set is pinned empty with that rationale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: address W0 review — mutation sandbox, exports-map resolution, tsc -b Review findings on #1494, all five: 1. contracts-schema-public.test.ts reads the kernel source at its packages/ path (fs access invisible to the codemod and typecheck). 2. Mutation lane: Stryker sandboxes the tree but pnpm's node_modules symlink resolves @agent-device/* back to the real repo, so mutants in the sandbox never load and vitest.related finds no tests. vitest.mutation.config.ts now aliases each EXPORTED specifier to its source (derived from exports maps, never a wildcard), keeping resolution inside the mutated tree. Validated: kernel-errors module runs end to end (dry run 3,984 tests, mutants killed, exit 0). 3. Layering/depgraph resolve workspace specifiers through the exports-derived map (workspaceSpecifierTargets) instead of reconstructing paths, so '.'-facade packages resolve; the positional fallback remains only for map-less fixtures (P0 pin). 4. Per-package project references implemented: packages/kernel is composite (emitDeclarationOnly -> dist-types, gitignored), the root references it, and typecheck becomes tsc -b — probed to catch type errors on both sides under TypeScript 7 native. 5. R11's relative-route exception now requires membership in an actual R8 zero-dep job closure (zeroDepClosureFiles walks entries), not mere scripts/ placement — closing the dual-instantiation bypass. Also from review discussion: daemon-error moves out of the kernel package to src/client/ — its consumers (cli, client facade) rehydrate wire DaemonErrors client-side; the daemon only produces them. Kernel drops to 8 exported subpaths before any of them ship. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * refactor: one exports-map reader for mutation alias and ownership Fallow flagged workspaceExportAliases (cognitive 15, CRAP 90). The manifest-reading logic already exists as workspaceSpecifierTargets in scripts/layering/package-boundaries.ts, so both the Stryker sandbox alias table and the mutation ownership walker now consume it instead of carrying near-clones. Behavior unchanged; mutation suite 45/45 and changed-code fallow green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: composite kernel without a root references edge FreeRange runs plain `tsc -p tsconfig.json`, and a root `references` entry makes non-build-mode TypeScript demand the referenced project's built declarations (TS6305) — a standing "build first" tax on every plain -p consumer (fr, editors). Keep the per-package composite project and build it in typecheck (`tsc -b packages/kernel` before the root and examples/sdk passes), but drop the root references edge: root consumption resolves through exports to source, identical to runtime and to the bundler. Probed: plain -p green with no prebuilt output; kernel-side type errors still caught by its own build. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix: R11 uses the layering parser; mutation config is a fallow entry Review blockers on #1494: - R11's private single-quote regex could miss a double-quoted or re-export route into packages/*/src. specifierSites now delegates to the layering model's parseImports (both quote styles, side-effect imports, re-exports, dynamic imports), with direct regressions for each formerly-invisible form. - vitest.mutation.config.ts becomes a declared fallow entry instead of a tolerated unused-file finding: the full-repo audit now reports it reachable (unused files 2 -> 1; the remainder predates this PR). FreeRange clean-checkout evidence: with packages/kernel/dist-types and every *.tsbuildinfo deleted, `pnpm check:freerange` reports 0 findings on this head — the TS6305 topology died with the root references edge in the previous commit; check:freerange has no build precondition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-30 12:12:46 +02:00
Michał Pierzchała	adcbdda8f0	perf: speed up unit tests and streamline checks (#1488) ... * perf: speed up unit tests and streamline checks * fix: validate canonical packaging workflows	2026-07-29 18:53:10 +02:00
Michał Pierzchała	2316fd32c5	test: pin daemon modularity migration contracts (#1487) ... * test: pin daemon modularity migration contracts * refactor: tighten daemon modularity ratchets * test: pin reporter live hook isolation	2026-07-29 18:05:00 +02:00
devin-ai-integration[bot]	885c1486bb	test(ci): single-retry policy for enumerated contention-flaky files (timeouts only) (#1448) ... * test(ci): single-retry policy for enumerated contention-flaky files * fix: satisfy fallow * test(ci): read failures through a lane reporter so timeouts stay distinguishable * test(ci): cover the lane reporter and drop its duplicated boilerplate * chore(fallow): own the retry lane's tool-loaded export seams * test(ci): block retries on non-test failures and classify timeouts structurally * test(ci): decide retry eligibility from runner metadata and route gate verdicts through blockers * refactor(ci): name the retry policy's rules in code instead of comments * test(ci): mark runner-aborted timeouts inside the runner instead of inferring them * test(ci): make timeout provenance a per-run secret, not a writable flag Cover direct task.meta mutation in the real child-Vitest fixture gate. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(ci): retry the failed files in the first run's project and coverage modes Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix: drop deleted repo-health file from the retry list after #1480 Rebase onto main post-#1480: the SkillGym/repo-health descope deleted scripts/repo-health/run.test.ts, whose CONTENTION_RETRY_FILES entry would now fail this PR's own missing-file check, and inlined the slow-test budgets into the reporter, resolving the budgets-module import. Envelope comments now point at scripts/lib/lane-envelope.ts instead of the closed #1430. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>	2026-07-29 12:53:33 +02:00
Michał Pierzchała	d5eb785e6f	docs: propose daemon module boundaries (#1451) ... * docs: propose daemon module boundaries * docs: preserve repair close retry state in proposal * docs: align session boundary probe with handoff	2026-07-29 12:51:57 +02:00
Michał Pierzchała	53e4be5f86	Remove SkillGym suite and repo-health snapshot infrastructure (#1480) ... * chore: drop SkillGym and the repo-health aggregator (#1412 descope) Remove the SkillGym harness (test/skillgym/), its check-affected lane, package scripts, and devDependency — the help-conformance bench is now the single non-gating small-model oracle. skills/ markdown classifies as docs in the affected-check selector instead of failing open. Remove scripts/repo-health: its only gating assertion duplicated the Layering Guard job, its case-count metric imported the deleted SkillGym suite, and its sole planned consumer (#1424 / PR #1477) was closed with the Track C descope on #1412. Verified: check-affected node --test suites, oxfmt, oxlint, tsc, check:layering, fallow audit vs origin/main, and the full unit suite (unit-core + subprocess-stub) all pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * fix(scripts): fold slow-test budgets into the reporter for production-exports The Fallow production-exports gate flagged all three budget exports: their in-file consumer (SLOW_TEST_RATCHET) and the repo-health entry point that kept the module reachable were both removed in the descope, leaving the config-loaded reporter as the only consumer — invisible to --production analysis. The data-only module's second consumer is gone, so per the boundaries-are-earned norm the constants move into the reporter instead of gaining a suppression. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep * docs: align skills/ format policy and purge last SkillGym mention Address both P2 review findings on #1480: the testing-matrix row and the selector's formatGate both still claimed oxfmt covers skills/, while selectChecks classifies skills/*.md docs-only (oxfmt ignores **/*.md, so the claim was a no-op even before). The matrix now states the docs-only policy and formatGate drops the dead underSkills fact. The merged examples/README.md index (from #1469) loses its skillgym mention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FUv7bvbWNryuXgSBuqTtep --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-29 11:45:22 +02:00
Michał Pierzchała	255deb6c28	ci: fold single-grep jobs into steps, call named pnpm scripts (#1465) ... * ci: fold single-grep jobs into steps, call named pnpm scripts - Merge ios-runner-swift-compat and no-test-di-seams (each just checkout + one rg assertion) into steps of a new static-checks job, keeping each step's own failure message. Removes two job-scheduling/ checkout overheads and two PR status-check lines. - Replace the layering-guard job's inlined copies of check:layering and depgraph:test with the named pnpm scripts, removing the silent-drift risk between the workflow and package.json. - Fix the same drift in conformance-regenerate.yml, which inlined maestro:conformance:regenerate byte-for-byte. - Leave affected-selector's inline node invocation as-is: R8's zero-dep closure check (scripts/layering/zero-dep-jobs.ts) finds a job's entry scripts by matching literal paths in the run: block, so switching to `pnpm check:affected:test` would zero out its entries and make R8 fail closed. Documented inline why this one stays inlined. - Leave publish-mcp-registry.yml's sync-mcp-metadata --check alone: that job never runs the setup-node-pnpm action, so pnpm isn't provisioned there at all. Refs #1462 * ci: teach R8 to resolve pnpm script names, drop affected-selector's inline copy R8's zero-dep-job entry scan matched literal script paths in a run: block, so a bare `pnpm <script>` invocation found zero entries and R8 failed closed — the reason affected-selector kept an inline node command instead of calling pnpm check:affected:test (#1462). zeroDepJobs now also resolves a pnpm script name against package.json and scans the resolved command for entry paths, so affected-selector can call the named script like every other job. Also replaced the other workflows' inlined copies of named package.json scripts (test:replay:*, perf, perf:android, maestro:conformance:differential, check:mcp-metadata, size) with their pnpm names, keeping each job's CI-specific trailing flags — found via a repo-wide sweep for any run: block whose text duplicates a scripts entry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L67kDSTAJwaoLCwANEJFRM * fix: revert publish-mcp-registry pnpm regression, recurse R8 alias resolution publish-mcp-registry.yml's job only provisions Node via actions/setup-node, never the repo's setup-node-pnpm action, so pnpm is never installed there — the earlier sweep's `pnpm check:mcp-metadata` would have broken the release path. Reverted to the direct node invocation with a comment explaining why, matching the PR's own stated rationale for leaving it alone. zeroDepJobs' pnpm-alias resolution only expanded one level: a resolved script that itself invoked another named pnpm script had its entries silently dropped from R8's closure. resolveRunEntries now recurses through chained aliases with a per-chain visited set, so a nested alias's entries are found and a cycle stops re-expanding a repeated name instead of recursing forever. Added coverage for both the chained and cyclic cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L67kDSTAJwaoLCwANEJFRM --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-28 20:56:19 +02:00
devin-ai-integration[bot]	6544e9a0c5	obs: repo-health snapshot command aggregating existing analyzers into one JSON (#1471) ... * obs: add repo-health snapshot command aggregating existing analyzers into one JSON Adds `pnpm repo-health [--json]`, an offline command that aggregates the signals this repo already computes into one deterministic JSON snapshot by reusing each analyzer (depgraph, layering ratchets, coverage json-summary, size-report, fallow baselines, slow-test ratchet, bench/skillgym registries) rather than reimplementing any metric. Carries mandatory v1 provenance (schemaVersion, commit, per-analyzer content hashes, input provenance) so #1424 can persist history. Component metrics (instability/abstractness/main-sequence distance) are observatory-only. The only gating behaviour is the depgraph-vs-layering R6 consistency assertion, already wired into the Layering Guard job via scripts/depgraph/model.test.ts. Closes #1423 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * repo-health: hash read artifacts and flag staleness in provenance Coverage and size are artifacts repo-health reads but does not produce, so they carry no producing-commit stamp. Previously provenance recorded only their paths and the current HEAD, so a snapshot taken after a source edit without rerunning those producers would pair prior metrics with the new SHA — #1424 would persist a false commit-indexed history entry. Now each read artifact is bound via artifactProvenance() to its content hash (so history keys on the bytes the metrics came from, not a commit they may predate) and an explicit `stale` flag (true when a production source file is newer than the artifact). The coverage/size analyzer-config hashes are added to provenance.tool, and the human summary marks stale artifacts. Adds a pure stale-artifact regression test. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * repo-health: bind artifact freshness to the whole producer input set The stale check derived freshness only from listSourceFiles() (src/*.ts) and applied it to both coverage and size. Coverage also depends on tests and vitest config; size-report.mjs reads package.json and runs npm pack — so a change to a non-src producer input could leave an old artifact marked stale:false while provenance recorded the current HEAD, a commit the metrics predate. Each read artifact now declares its full producer input set (PRODUCER_INPUTS) and is flagged stale when ANY tracked input in that set is newer than the artifact (via git ls-files mtimes). Freshness that cannot be proven — an empty observable input set — is reported stale, never falsely fresh. The input set is recorded as provenance.inputs.*.producerInputs. Adds isArtifactStale() with a regression covering a non-src input newer than the artifact and the unprovable case. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * repo-health: prove artifact freshness from a stamped producer commit; route git through runCmdSync Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-07-28 20:07:45 +02:00
Michał Pierzchała	d6d2e09529	test(kernel): cover all six errors.ts exports, kernel-errors mutation 55.19% → 86.26% (#1475) ... * test(kernel): cover all six errors.ts exports with table-driven cases (#1456) Extends errors.test.ts beyond normalizeError/toAppErrorCode: - AppError construction and NormalizedError/AppError instanceof contract - asAppError identity/wrap/duck-typing behavior (cross-realm errors, non-Error throws, fallback code precedence) - isAgentDeviceError real-instance-only contract - retriableForErrorCode and defaultHintForCode, table-driven over KNOWN_APP_ERROR_CODES (the module's own source of truth) rather than a hand-copied code subset - normalizeAgentDeviceError delegation to normalizeError - a cross-cutting composition test asserting the retriable/hint contract request-router.ts's enrichDaemonError and the CLI's printHumanError actually rely on No production changes; mutation re-run and PR to follow. * test(kernel): pin maybeEnrichCommandFailedMessage gating and regex boundaries (#1456) Adds targeted cases for branches the table-driven/contract tests didn't reach: the COMMAND_FAILED/processExitError/string-stderr gating in maybeEnrichCommandFailedMessage, GENERIC_EXIT_MESSAGE's `-?\d+$` boundary (negative code, trailing text, non-numeric code, whitespace in the tool token), and STDERR_NOISE_PREFIX's zero-or-more-whitespace semantics for both the adb/xcrun/simctl and "error:" groups. * test(kernel): close remaining mutation gaps in firstStderrLine/detail helpers (#1456) Each addition here was verified against a hand-applied mutation (matching Stryker's actual survivor diffs) before landing, confirming it fails on the mutant and passes on the original: - firstStderrLine: line-trim-before-prefix-match (a real skip-pattern line ahead of the leading-whitespace one is required, since redactDiagnosticData already .trim()s the whole stderr string once before firstStderrLine ever sees it), and the >200-char truncation branch (both sides of the boundary: 200 exact vs 201+). - stringDetail/booleanDetail: reject a wrong-typed diagnosticId/logPath/ hint/retriable instead of surfacing it. - stripDiagnosticMeta: details drops to undefined once stripping the known meta keys leaves nothing behind. * style(kernel): apply oxfmt to errors.test.ts Fixes the Lint & Format CI failure on PR #1456 — format:check flagged this file, oxfmt --write resolves it with no behavior change. * test(kernel): split errors.test.ts into focused sibling suites (#1456) Per review feedback: the growing errors.test.ts hit 601 lines, past AGENTS.md's extract-before-500 threshold (tests are not exempt). Splits along the reviewer's requested seams, moving tests verbatim with no assertion changes: - errors-message.test.ts (234 lines) — stderr/message normalization: maybeEnrichCommandFailedMessage, firstStderrLine, the GENERIC_EXIT_MESSAGE/STDERR_NOISE_PREFIX regex boundaries. - errors-metadata.test.ts (157 lines) — hint/diagnosticId/logPath/ retriable/supportedOn lifting and stripping, divergence passthrough, the stringDetail/booleanDetail/stripDiagnosticMeta type-guard contracts, and the normalizeError/retriableForErrorCode/ defaultHintForCode cross-cutting contract. - errors-code-policy.test.ts (228 lines) — AppError construction, asAppError/isAgentDeviceError duck-typing contract, toAppErrorCode, the retriableForErrorCode/defaultHintForCode table-driven policy tests, and normalizeAgentDeviceError. Also repoints scripts/mutation/ownership.test.ts's two references to the deleted errors.test.ts path at errors-code-policy.test.ts — the mutation ownership deriver reads the file from disk to follow its import graph, so a stale path silently resolves to "owns nothing" instead of failing loudly. --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-28 20:06:03 +02:00
devin-ai-integration[bot]	e0b8463ef8	feat(scripts): blast-radius query — dependents, owning gates, live-coverage owners (#1425) (#1470) ... * feat(scripts): blast-radius query over the depgraph model (#1425) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(scripts): keep --limit bound to its value in depgraph affected Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-07-28 18:20:39 +02:00
Michał Pierzchała	ba1a5efbc6	refactor(layering): declare R1-R3 as a policy table, and test them (#1449) ... R1-R3 were three hand-written predicate functions. Each was short, but each buried its boundary in control flow: you had to read the early-returns to learn that R3 tolerates dynamic imports, or that R1 opens exactly one door. They are now data in scripts/layering/zone-policy.ts -- which zones a boundary governs, which import kinds it tolerates, which path prefixes are its declared seam -- walked by one small evaluator. A fourth zone boundary becomes a table entry rather than a fourth predicate to keep consistent with the other three. R1 is deliberately two entries rather than one with a special case, because "kernel may import contracts type-only" and "kernel may import nothing else at all" are two statements, and writing them separately is what makes the single open door visible. The refactor exposed a real gap: checkLayeringRules had NO unit test. The only thing exercising R1-R3 was the real tree, which is clean, so a rule that had silently stopped matching would have looked exactly like a rule being obeyed. zone-policy.test.ts now asserts each boundary fires and each documented exemption holds, including that src/daemon/client/ is excluded from the daemon seam. Verified end-to-end by injecting one violation per rule plus an exempt file: the gate reports 4 zone-policy violations (R3 twice, catching the daemon/client case), ignores the type-only and dynamic edges, and exits 1. Also records in docs/dependency-graph-findings.md the result of spiking eslint-plugin-boundaries under oxlint's jsPlugins, so nobody repeats it. It does work -- jsPlugins loads npm ESLint plugins with no ESLint install, and R1-R3 are all expressible once you know importKind: "value" and settings["boundaries/dependency-nodes"]. Not adopted: no ratchet mechanism (the thing that took R6 from 61 to 7 incrementally), it cannot express R4-R9 so the architecture would be defined in two places, it misreads inline `{ type Foo }` specifiers as value imports (one false positive on providers/limrun/android.ts), message interpolation renders empty under the current selector syntax, and it costs 230 transitive packages on an API documented as alpha. No behaviour change: 932 source files, R6 = 7, R7 = 41 fields, R8 clean, R9 = 102, all identical to before. Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur Co-authored-by: Claude <noreply@anthropic.com>	2026-07-28 17:40:55 +02:00
Michał Pierzchała	a0aa02579b	build(android): unify the 4 helper build/package scripts behind one parameterized pair (#1466) ... * build(android): unify snapshot/ime helper build+package scripts Replace the four ~75%-duplicated shell scripts with one parameterized build script and one parameterized package script, mirroring scripts/build-xcuitest-apple.sh's env-var-driven pattern. The helper is selected via AGENT_DEVICE_ANDROID_HELPER or a first positional arg; per-helper differences (HELPER_DIR/PACKAGE_NAME, snapshot's test-compile+run step, ime's aapt2 resource-compile step, and the manifest JSON fields) live in small case blocks. All package.json entry points keep their names and output paths. Verified byte-level equivalence between main and this branch: identical unzip -l listings, identical classes.dex SHA-256 for both helpers, and identical manifest fields (only the per-signing-run sha256 differs). Fixes #1461 * chore: drop stale android/multitouch-helper .gitignore entries The multitouch helper was consolidated away in #1281; these two lines were never cleaned up.	2026-07-28 17:39:35 +02:00
devin-ai-integration[bot]	7402a40bac	test: enumerate error-code recovery quizzes in a unit-lane gate (#1445) ... * test: enumerate error-code recovery quizzes in a unit-lane gate Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: mark recovery quizzes structurally and derive retriability from the enumeration Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-07-28 15:46:27 +02:00
devin-ai-integration[bot]	61f8696d28	test(ci): gate PRs on changed-line coverage (#1418) (#1447) ... * test(ci): gate PRs on changed-line coverage (#1418) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor: split coverage-changed model into small helpers for fallow Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(ci): simplify coverage-changed reporting and CLI surface Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-07-28 14:25:10 +02:00
devin-ai-integration[bot]	edca35d122	chore(deps): Renovate config, packageManager-derived pnpm in CI, repo-wide format (#1444) ... * chore(deps): add Renovate config and enforce packageManager pnpm version in CI Refs #1422 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: bump pnpm to 11.17.0 and format the whole repo with oxfmt format/format:check drop their hand-maintained path list: oxfmt already skips node_modules and honors .gitignore, so the only exclusion list is .oxfmtrc.json ignorePatterns. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(mutation): accept either quote style in the affected-lane path filter Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore(deps): keep fixture-app runtime deps as individual Renovate PRs Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-07-28 13:50:39 +02:00
devin-ai-integration[bot]	006c4cadc9	test: nightly parser fuzz lane — parser input fails as typed AppErrors, never hangs (#1414) (#1438) ... * test: nightly parser fuzz lane with typed-AppError invariant (#1414) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): run envelope, artifact promotion, and harness self-check tests (#1414) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): shared scheduled-lane envelope on every terminal path, watchdog after ready (#1414) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): envelope for malformed options; add scheduled-lane health consumer (#1414, #1430) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(lanes): actions:read scope, terminal error envelope, first-due grace (#1414, #1430) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(lanes): anchor first-run grace to schedule registration, use exec helper in tests (#1414, #1430) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(lanes): portable POSIX pickaxe pattern for schedule registration (#1414, #1430) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(fuzz): fast-check generators over the shared hazard list, drop the bundled lane-health work (#1414) - Strip scripts/scheduled-lane/* and scheduled-lane-health.yml: that watcher is #1430's own deliverable and collides with PR #1439's implementation of the same lane. What this lane owes (a per-run envelope) moves into scripts/fuzz/envelope.ts. - Rebase onto #1437 and rebuild the generator layer on fast-check: cases come from arbitraries sharing SELECTOR_VALUE_HAZARDS with the property suite, and counterexamples are shrunk, so a failure names a minimal input plus fast-check's seed/path instead of a 20k-char random string. - Route harness.test.ts into the serialized subprocess-stub project. - Drop the AGENT_DEVICE_FUZZ_STARTUP_DELAY_MS test seam: the ready handshake is now proven by a case budget far below real worker startup. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): replay the regression corpus through the worker watchdog (#1414) A promoted hang case used to wedge the unit job until the CI timeout, because corpus replay called checkCase in-process. It now goes through the same worker-backed watchdog the nightly lane uses, so such a case fails against a 5s per-case budget; the file moves to the serialized subprocess-stub project with the rest of the worker-driven fuzz tests. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): let the watchdog outlive vitest's default case timeout (#1414) A wedged parser was surfacing as a bare 'Test timed out in 5000ms' instead of the named hang: failure that says which input wedged, because the file's vitest timeout was shorter than the watchdog budget times the number of replayed cases. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(fuzz): complete drift provenance in the lane envelope (#1414) configHash now covers every input that decides what a seed generates (generate.ts and the shared property arbitraries, not just the arbitraries/targets/invariant), and tool records fast-check's installed version. A generation-loop edit or a fast-check upgrade previously changed the case set while the envelope looked unchanged. A test recomputes the hash with each input omitted so a future omission fails. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-07-28 11:29:25 +02:00
devin-ai-integration[bot]	8cce0ef6b8	test: ratchet mutation score over enumerated decision kernels (#1441) ... * test: ratchet mutation score over enumerated decision kernels Adds a Stryker (vitest runner) mutation lane scoped to the decision kernels, a per-module baseline with tool/config provenance, and a ratchet that only lets scores rise. Non-gating until two consecutive stable weekly sweeps. Refs #1415 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * chore: declare the mutation test-scope seam for production-export analysis Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: own kernel tests in the mutation registry and ship the #1430 lane envelope - restore bench:help-conformance, broken by a formatting-path edit - kernel test files select their module on PRs (registry `tests` + workflow paths), asserted to reach the kernel through the import graph - every mutation run writes the standard scheduled-lane artifact envelope - move src/utils/__tests__/errors.test.ts beside its source per the mirror rule Refs #1415, #1430 Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: derive kernel test ownership and land the scheduled-lane health monitor Ownership of a kernel's tests is now computed from the static import graph (scripts/mutation/ownership.ts) instead of a hand-listed set, so a test that reaches a kernel indirectly -- src/__tests__/daemon-error.test.ts through src/daemon.ts -- selects that kernel on a PR. The PR lane triggers on every src test and shards the derived modules, keeping wall clock at one module. The lane envelope (#1430) is now written on every exit path with the stage it reached, so a crash before any mutant runs is distinguishable from a lane that never ran. Adds the derived cadence monitor (scripts/lane-health, daily workflow): scheduled lanes are enumerated from .github/workflows/ and reported dark, failing, or never-run against their own cron cadence. * fix: merge only Stryker reports from a shard directory The shard artifacts now carry the lane envelope beside mutation.json, and the merge globbed every .json under the download path, so the ratchet job fed the envelope to the report parser and died after the mutants had already run. * fix(mutation): fail on incomplete shard sets and envelope pre-run failures Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(mutation): downgrade a passing envelope when a later lane step fails Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * refactor(mutation): shard by registry, defer the PR lane, drop the bundled watcher Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: describe registry sharding and the deferred PR mutation lane Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * fix(mutation): make the pre-graduation tooling exception select real mutants Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test(mutation): give the worktree fixture commits their own identity Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-07-28 10:07:14 +02:00
devin-ai-integration[bot]	d747ef6230	test: frozen replay-compat corpus with expected verdicts (#1417) (#1436) ... * test: frozen replay-compat corpus with expected verdicts (#1417) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: pin replay-compat corpus bytes to released blobs and assert via parseReplayInput Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: lock replay-compat provenance kind by corpus area and verify it in CI Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: describe corpus provenance-kind lock and CI job Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: prune replay-compat corpus to minimal witnesses per shipped form Reviewer feedback on #1436: the mechanism earns its place, the dataset did not. Drop the 30 corpus entries whose bytes repeat a syntactic form or a migration refusal another entry already witnesses (platform twins and adjacent-release re-recordings), leaving 22 deliberate entries; make note required and state per entry which form or refusal it is the sole witness of. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: address corpus review nits (typed coverage list, cap rationale, derived-citation note) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: split corpus rule — form from the release, verdict from today's parser Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> * docs: format corpus README emphasis markers Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Michał Pierzchała <thymikee@gmail.com> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-07-28 10:06:00 +02:00
Andrew Barnes	4c02b6ad2b	fix(cli): reject excess positionals (#1433) ... * fix(cli): reject excess positionals * fix: preserve get ref labels in arity checks --------- Co-authored-by: Andrew Barnes <169967362+Bortlesboat@users.noreply.github.com> Co-authored-by: Michał Pierzchała <thymikee@gmail.com>	2026-07-27 17:23:30 +02:00
Michał Pierzchała	fcaa6c995c	refactor(contracts): declare shared vocabulary below its consumers, ratchet what remains (#1435) ... * refactor(contracts): declare the public API vocabulary below its consumers The layering gate's largest remaining cluster was 28 type-only inversions from a single edge: `commands/` declaring itself in terms of `client/client-types.ts`. R2 forbids the reverse import, so a shape both surfaces need has to sit below both. The command/device vocabulary — connection config, the device and session views, and every per-command Options/Result — now lives in `contracts/client-api.ts`; `client/client-types.ts` keeps the `AgentDeviceClient` facade and re-exports the rest through one wildcard. R6 total: 42 -> 18. No new inversion in any pair. The published surface is unchanged, and that is verified rather than asserted: the built `index.d.ts` exports the same 216 type names as main, byte-identical. Eight shapes deliberately did NOT move, because each is stated in terms of a HIGHER-ranked zone: `ScrollOptions` (ScrollInputDirection, commands/), the four navigation Options plus `AgentDeviceCommandClient` (navigation-projection, commands/), and the two Metro result aliases (metro/). Declaring those in contracts/ would trade 28 commands->client edges for contracts->commands and contracts->metro ones — the foundation depending on the layers above it, worse in kind even though fewer in number. This is measured, not assumed: moving the whole file to contracts/ first took the gate from 42 to 48, which is how the floor was found. Two keystone moves made the other 84 movable: - `RemoteConnectionProfileFields` joined its sibling `CloudProviderProfileFields` in contracts/remote-config-fields.ts. It was the root of the base chain (AgentDeviceClientConfig -> AgentDeviceRequestOverrides -> DeviceCommandBaseOptions -> every per-command Options), so one rank-4 declaration was pinning ~80 shapes up with it. - `DaemonBatchStep` moved to contracts/batch-step.ts. Its `runtime` field was written `DaemonRequest['runtime']`, dragging the whole daemon request type in to say `SessionRuntimeHints` — the same type, three zones lower. `CompanionTunnelScope`/`MetroBridgeScope` also moved to contracts/, since the vocabulary needs the scope shape and it sat next to client-local env-var names. Six pass-through re-exports in client-types.ts are suppressed per-name with the reason inline: they exist only to publish contracts/kernel types through the package entrypoint wildcard, every internal consumer imports them from the declaring module, so "no consumer" is correct and not actionable — deleting them would remove names from the public types. `pnpm check` green, 4488 unit tests. Findings doc records the sequencing for the last 5: the upstream declarations have to come down before the shapes that need them can. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * docs: drop the graph viewer, keep the query that replaces it The rendered dependency-graph viewer is not being merged (PR #1409 closed). It cost ~2200 lines plus a Fallow exemption for a 920-line canvas renderer, and nobody — human or agent — reached a conclusion from the picture. Every finding in this document came from short queries against the gate's own model. This file pointed at the `claude/depgraph-viewer` branch for the tooling, which would have dangled once that branch is deleted. Replaced with the thing that was actually load-bearing: a throwaway probe script, inlined, that re-derives the numbers from `scripts/layering/model.ts` and nothing else. Verified verbatim — it reproduces TYPE_INVERSION_BASELINE exactly, which is also the check that tells you whether either side has gone stale. Two numbers in the summary table were stale, describing an intermediate state rather than what shipped: R6 said "35 across 4" (actually 18 across 5 after the vocabulary move) and ranked coverage said "729 of 894" (actually 888 of 901). Both corrected, along with the file/edge counts in the header. Also notes the deduplication detail that makes the query agree with the gate: each file pair counts once, so a raw edge count reads higher. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * refactor(contracts): move the four keystones that pinned the rest of the inversions R6 type-only spine inversions: 18 -> 7, and every one of the 7 that remains is a deliberate architectural position rather than a misplaced declaration. Four keystones moved to contracts/, each of which was pinning a much larger set: - `CommandFlags` (was core/dispatch-context.ts). One rank-2 declaration holding the daemon's request type and every recorded action above it. Its last non-contracts dependency was `DaemonBatchStep`, already moved in 3fdbfe0. - `SessionAction` (was daemon/types.ts). replay/ (6 modules) and compat/maestro/ read and write session scripts; declaring the shape inside the daemon made both depend on the server to describe a file format neither asks it to produce. The daemon still owns the recording — only the shape moved. - `TargetAnnotationV1` shape (was replay/target-identity.ts). ADR 0012 target evidence, written by 8 daemon modules and read by commands/; the parsing and classification logic stays in replay/. - `ScrollInputDirection` and the Metro prepare/reload result payloads, which unblocked `ScrollOptions` and `MetroPrepareResult`/`MetroReloadResult`. `DaemonRequest` also split into the three shapes it had been conflating: the kernel WIRE shape (`flags?: Record<string, unknown>`, because a process boundary cannot enforce a vocabulary), the new `contracts/command-request.ts` `CommandRequest` (wire shape with flags typed — what a command surface needs), and the daemon's own refinement (+ `internal?: DaemonRequestInternal`, carrying SessionState callbacks and the admitted lease). core/command-descriptor/ had been importing the third to read `command`, `positionals` and `flags`. Two things deliberately NOT moved, because moving them would add coupling rather than remove it, and the baseline now argues both: - `DaemonCommandDescriptor`/`DaemonCommandRoute` — the route type is `keyof typeof DAEMON_ROUTE_HANDLERS`, derived from what the server implements. Moving it down means re-declaring route names in contracts plus a gate to prove the handler map still covers them. ADR 0003/0008 own that boundary. - `AgentDeviceClient` — used as an opaque handle by 4 files. The facade is built from commands/'s own NAVIGATION_COMMAND_PROJECTIONS, so this is a genuine zone-level cycle; breaking it is a design call about where that registry belongs. R5 is zero here: nothing imports the client at runtime, only its type. Also records the largest structural finding, which R6 does not measure: cycles by edge kind are 1 (value only), 87 (value + type-only), 1 (value + dynamic), 213 (all). At runtime the graph is a clean DAG; the 87-file type-level cluster means no one of those files' types can be read in isolation. Hubs are runtime-contract.ts, commands/runtime-types.ts, backend.ts, commands/runtime-common.ts. Not attempted here — it is a different and much larger change. `pnpm check` green, 4488 unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * feat(layering): ratchet type-cycle growth (R9), and rule out a narrower client port R9: the largest strongly-connected component over value + type-only edges may not grow. R4 keeps the VALUE graph acyclic, so every cycle counted here is created by type-only imports - free at runtime, invisible to R5/R6, and the largest single obstacle to reading a subsystem in isolation: inside a component of 102 files, no file has a self-contained slice. Baseline set to 102, which is what THIS branch achieves - main carries 107 and the boundary moves here bring it to 102. An earlier revision baselined 87, measured against an older main; after rebasing onto f19864e the real figure was 102 and the new rule fired on its own stale baseline. Worth stating because the failure looked like a regression and was not: attribution showed main at 107 and this branch reducing it, which is the check working rather than complaining. Growth-only, deliberately unlike R6. Reducing 102 is a real refactor rather than a file move, so a hard equality would turn every unrelated improvement into a baseline edit. A shrunk tree is reported in the success line instead of failing. Verified at the new baseline by adding one type-only import that closes a loop and watching 102 become 108 and the gate reject it. The refactor itself is still not attempted. Hubs by in-component dependents are runtime-contract.ts, commands/runtime-types.ts, backend.ts, commands/runtime-common.ts; a pass starts there. Separately, investigated the narrower-port idea for the 4 remaining -> client inversions and it does not work. Measured first: files NAMING AgentDeviceClient (the inversions) 4 files CALLING client methods 26 distinct facade namespaces reached 13 The narrowness is an artifact of where the type is named, not of what is used. Making those four generic over the client type pushes the concrete type into the 26 implementations, turning 4 inversions into up to 26. A port spanning 13 namespaces is the whole facade, so it would either duplicate the public API shape - a second source of truth for it - or derive from the facade and carry the same dependency. So the four are the minimum number of naming sites rather than an accident: they are the choke point. Recorded as a position with the numbers behind it. The remaining option is the question underneath it - whether NAVIGATION_COMMAND_PROJECTIONS belongs in commands/ - and that is a design decision about the command surface, not a dependency cleanup. pnpm check green, 4535 unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * fix(layering): test R9, specify its floor, and drop two duplications Adversarial self-review of #1435 found four things worth fixing. R9 shipped with no unit test. Every other rule in this gate has one (R5 back-edges, R6 inversions, R7 session state, R8 zero-dep closures); R9's only verification was a manual injection CI cannot repeat. Added tests for the three distinctions it depends on: a type-only loop counts, a dynamic-only loop does not, a value loop still does. Writing that test immediately found an undocumented edge case, which is the argument for it. largestTypeCycleSize returns 1 for an acyclic graph that has non-dynamic edges but 0 when every edge is dynamic, because only edge-participating files enter the walk. Immaterial to a growth ratchet, but an inconsistent floor nobody had written down. Now specified in the doc comment and pinned by the test, so 0 and 1 cannot later be read as a meaningful difference. largestTypeCycleMembers was exported with no consumer - speculative API, and scripts/layering is in Fallow's ignorePatterns so nothing would have flagged it. Same pattern review caught on the previous head with MaestroRuntimeFlags and TargetRect. Made module-private. ResolvedMetroKind was declared twice after the Metro payload move: exported from contracts/metro.ts and still private in metro/client-metro.ts. client-metro.ts now imports it. The gate computed the SCC twice per run, once in the rule and once for the success line. Computed once and threaded, so the two can no longer disagree. Also re-verified the claim this PR rests on, with a stronger check than the one in the body: comparing DECLARATION names in index.d.ts counts inlined internals, and by that measure this branch appears to lose five names (PrepareMetroRuntimeResult, ReloadMetroResult, ResolvedMetroKind, SCROLL_INPUT_DIRECTIONS, ScrollInputDirection). All five are declared-but-not-exported helpers. The real surface - exported names across all eleven published entrypoints - is 69 on both sides, identical. Also proved DaemonRequest structurally equal to its pre-split shape with a type-level assertion rather than by reasoning, and confirmed SessionAction, CommandFlags and TargetAnnotationV1 moved byte-identically. pnpm check green, 4535 unit tests, 24 layering tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * refactor(contracts): one file per command family, one name per request Addresses review on #1435. contracts/client-api.ts was 1,064 LOC and grouped session, app, interaction, replay, observability and recording contracts together, so it answered no one question and crossed the >1,000-LOC architecture-debt tripwire in AGENTS.md:124. Split it into 14 domain-family files by the command families that already exist (client-connection, client-device-view, client-session, client-lease, client-app, client-capture, client-target, client-gesture, client-selector-read, client-replay, client-observability, client-settings, client-system, client-request); the four Metro client shapes went into the existing contracts/metro.ts so one file answers the Metro question. Largest resulting file is 137 LOC. client/client-types.ts re-exports one wildcard per family, so the published import path is unchanged. Published surface verified unchanged against main two ways: the exported-name set of all 11 published entrypoints is identical (70 names), and every declaration in the built index.d.ts is byte-identical after normalization -- 0 names added, 0 shapes changed. index.d.ts got smaller (1,726 -> 1,682 lines): 10 declarations main duplicated into it now resolve through a shared chunk. Also, from re-examining the two findings the review flagged as blind spots: - CommandRequest was a third name for "a request" that no consumer needed. Every core/command-descriptor/ use read only command/positionals/flags, in two spellings (the full type and a Pick of it). Replaced by contracts/dispatched-command.ts DispatchedCommand -- those three fields and nothing else, with command/positionals Picked from the wire type so they cannot drift. daemon/types.ts DaemonRequest now extends the wire shape directly. Two request shapes again, at two ranks. - The 7 remaining R6 inversions each get a mechanical reason rather than an appeal to an ADR: the 4 AgentDeviceClient edges are a real zone-level cycle (client-types.ts imports ProjectedNavigationCommandClient from commands/), and no narrower port exists (26 call sites across 13 namespaces); the 2 DaemonCommandDescriptor edges are unavoidable because that shape is stated in terms of the server-private DaemonRequest; the 1 DaemonCommandRoute edge is unavoidable because the type is computed from the daemon's handler table. Cleanups found on the way: three doc comments this branch had orphaned from their declarations (SettleCommandOptions, RecordControlOptions, ReloadMetroResult -- the last had drifted onto an unrelated type it misdescribed) are reattached; intra-contracts imports normalized from '../contracts/x.ts' to './x.ts', which is what the duplicate-import lint caught; and stale references to the deleted file removed from the docs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-27 17:22:39 +02:00
Michał Pierzchała	e8b779cb32	fix(daemon): keep close-time script-save failures from leaking the session/device claim (#1392) ... * fix(daemon): keep close-time script-save failures from leaking the session/device claim A close-time script write (implicit from `open --save-script`, or this close's own `--save-script`) that refuses to publish (e.g. a no-clobber target-exists AppError) threw uncaught out of `handleCloseCommand`, skipping lease release, device-claim release, and `sessionStore.delete` entirely — while the `close` action had already been recorded with no rollback. Live-repro'd over the real CLI against an Android emulator: this single gap explained both symptoms split out of #1384 into #1391 — a lingering `DEVICE_IN_USE` claim after a failed `close`, and a published `.ad` rewritten with duplicated trailing `close` lines when the same close was retried (each attempt re-recorded a `close` action on top of the one never rolled back from the prior failure). Catch the write failure, roll back the just-recorded `close` action (mirroring the existing repair-armed commit-failure pattern), and let teardown (lease release, device-claim clear, session delete) complete regardless — exactly as an ordinary platform-close failure already doesn't block them. The failure is still surfaced to the caller, but after teardown, with a corrected hint: retrying the same close is no longer meaningful since the session is now gone. Fixes #1391 * refactor(daemon): shrink handleCloseCommand/runSessionCloseTeardown under fallow's complexity gate CI's fallow code-quality check flagged handleCloseCommand (126 lines, 19 cyclomatic / 16 cognitive) and runSessionCloseTeardown (73 lines) as exceeding the large-function/high-complexity thresholds after the prior commit's fix. Extract runCloseTeardownAndRelease (teardown + lease release + claim clear + delete + ordered error surfacing) and buildCloseSuccessResponse (final response shaping) out of handleCloseCommand, and finalizeOrdinaryCloseScript out of runSessionCloseTeardown. No behavior change — same control flow, split into named, independently-readable steps; fallow now reports 0 complexity findings for this diff. * fix(daemon): preserve the write error's structured details in the close-time save failure Review feedback on #1392 (thymikee): toOrdinaryCloseSaveScriptFailure rebuilt the AppError from only the original message, dropping its machine-readable details.reason ("script_target_exists"), details.path, and cause. A caller dispatching on those fields (or reading the CLI's --json error.details) lost them even though the underlying write failure carried them. Preserve the original error's details/cause, overriding only the close-specific hint and retriable:false. Extends the #1391 regression test to assert the routed close response still carries reason/path. * refactor(daemon): drop the vestigial close-time rollback, add router-level #1391 coverage Review feedback on #1392 (thymikee), P2 items: - The close-time save-script failure's session.actions rollback (finalizeOrdinaryCloseScript) was left over from an earlier design where a failed save could keep the session alive for retry. It never does now — runCloseTeardownAndRelease always tears the session down regardless of the outcome — so there is no surviving session for a later write to duplicate the close action on. Drop the rollback; the durable events.ndjson entry (which the rollback never touched anyway) and the in-memory action now agree, both accurately recording that the close happened. - Add a request-router-level regression (request-router-typed-error.test.ts, alongside the existing repair-close BLOCKER 2 test it mirrors) proving the normalized JSON error shape a real client sees: top-level retriable:false, details.reason/path preserved, and the session torn down — not just that handleCloseCommand throws the right AppError when called directly. * test(daemon): assert the durable close event survives a failed close-time save Review feedback on #1392 (thymikee), final P2 item: the previous commit removed the actions rollback because there's no surviving session to duplicate the close action on, but nothing actually asserted the durable events.ndjson action.recorded:close event stays put. Flush and read it back so a future rollback or event-order change can't silently recreate the in-memory/durable mismatch the removed rollback used to paper over asymmetrically. * test(daemon): assert the retained session's in-memory close action, not just the durable event Review feedback on #1392 (thymikee): the durable-event assertion alone doesn't catch a reintroduced session.actions.length = actionsBeforeClose rollback, because that event is queued (and durable) before the write even attempts — a regression there would leave the assertion passing while silently reintroducing the in-memory/durable mismatch. Retain the session object past handleCloseCommand (store.delete only drops the map entry, not the object a local variable still points at) and assert its actions array contains exactly one close entry, matching the durable event count. Verified by temporarily reintroducing the old rollback locally: this assertion fails (0 !== 1) where the prior durable-only check did not, then reverted. * refactor(daemon): model repair close retry as receipt * refactor(daemon): merge blockingError to state, not explain, the save-script exclusion Following up on the comment-trimming pass already on this branch: the device-claim condition (!platformCloseError && !cleanupAggregate) and the two-line throw sequence right below it both needed a paragraph explaining why saveScriptError is excluded from one but not the other. Merge platformCloseError and cleanupAggregate into a single named blockingError — its name now states the exclusion the comment used to argue for, and the throw sequence collapses from two ifs to one. Trimmed the remaining long docblocks in this file the same way: state what's non-obvious in 1-3 lines instead of re-deriving it in prose. * refactor(daemon): clarify close script finalization	2026-07-27 16:04:38 +02:00
Michał Pierzchała	2d1d70613f	feat(bench): renderer-pinned samples, topic-coverage gate, error-recovery quizzes; trim skillgym to agentic checks (#1411) ... * feat(bench): renderer-pinned samples, topic-coverage gate, error quizzes; trim skillgym to agentic checks The help conformance bench's quoted CLI output is now sourced from scripts/help-conformance-sample-outputs.mjs, and every sample is rebuilt through the real production renderers (settle output formatters, printHumanError, formatSnapshotText, refMutationAdmissionResponse) by scripts/__tests__/help-conformance-sample-outputs.test.ts — a rendering or message change fails deterministically instead of leaving the bench grading against output the CLI no longer prints. This retires the fabricated recoverable-failure envelope (production never throws a textual settle timeout; that case is replaced by a real DEVICE_IN_USE recovery quiz). Bench cases move to scripts/help-conformance-cases.mjs and are enumerated against the help-topic registry: helpTopicIds() is exported from cli-help, and scripts/__tests__/help-conformance-topic-coverage.test.ts fails when a help topic has neither a bench case nor an explicit waiver. New case families: error-envelope recovery quizzes (device-in-use, stale pinned ref, ambiguous find match, app-not-installed) pinned to real error text, topic coverage for tv/web/react-native/debugging/workflow, and a metamorphic twin of the settled-diff quiz. The skillgym smoke suite shrinks from 119 cases to the 5 that measure what only an agentic runner can show: skill routing plus output interpretation with a proven local CLI help probe (local-cli-help-policy). Its embedded samples now import the same pinned constants, replacing hand-transcribed output that had already drifted from the renderer. Knowledge checks belong to the bench; live fixture behavior belongs to the iOS simulator e2e suite. * review: drive error samples through the real producers; enforce local-help on the routing smoke The DEVICE_IN_USE, AMBIGUOUS_MATCH, and APP_NOT_INSTALLED parity tests no longer hand-author the producer message before rendering: each drives the actual producer — buildDeviceInUseBySessionError (extracted in session-open.ts and called by the handler), buildAmbiguousMatchError (now exported from find.ts), and buildAppNotInstalledError (extracted in app-resolution.ts and thrown by the resolver). Because each factory is exported from its producer file and called by the production path, dropping the production call would make it test-only and fail check:production-exports — the wiring is gate-enforced, not conventional. open-and-snapshot now sets requireLocalCliHelp and allowOnlyLocalCliHelpCommands, so the 'skill plus local help' claim is observed rather than assumed; without them the case can pass on model prior alone.	2026-07-27 14:24:36 +02:00
Michał Pierzchała	f19864e486	feat(scripts): dependency-graph report over the layering gate's model (#1410) ... * feat(scripts): dependency-graph report over the layering gate's model Reports what the layering gate deliberately does not enforce, as JSON plus a short summary. No renderer: the productive artifact is the JSON. pnpm depgraph # -> .tmp/depgraph/graph.json + summary pnpm depgraph:test Dependency graph: 898 files, 4627 edges, 25 zones value-import cycles (R4): 0 type-only/dynamic cycles (not gate-rejected): 8 spine back-edges (R5): 0 type-only spine inversions (R6): 42 transitively redundant value edges: 1338 The two numbers worth having are the ones CI cannot give you. Transitively redundant value edges — where the target is still reachable at distance >= 2, so the direct import changes nothing about what the module can see — need a real reachability pass, not a grep. And cycle detection over type-only and dynamic edges covers the loops R4 excludes by design. Both are candidate lists, never work lists; at ~1300 the redundancy set is a place to look. It reuses scripts/layering/model.ts, the same module check.ts uses in CI, so the file set, zone partition, edge kinds and cycle definition are the enforced ones. A second extractor would describe a graph nobody gates. Consequence worth having: its R6 count reproduces TYPE_INVERSION_BASELINE, so a mismatch means one of the two is stale. This is the analysis half of a viewer that was built and dropped. The render cost ~2200 lines and needed a Fallow exemption for a 920-line canvas file, and nobody read it. Everything here clears the repo's bar with NO exemption — scripts/depgraph is deliberately absent from ignorePatterns, unlike scripts/layering, scripts/perf and scripts/maestro-conformance. Getting there meant fixing rather than suppressing: extracted `valueSuccessors` (the value-edge adjacency was built identically in two places — a real clone), split `buildGraph` into four named aggregation steps, split `reachableBeyondDirectEdge` out of `markRedundantEdges`, extracted `compareZoneEdges`/`crossedZonePair`, extracted `edgeKindCode`/`edgeFlags` from a nested ternary scoring CRAP 42, and deleted `fileGroup` plus the `group` node field once the cluster layout went. Two additive exports on scripts/layering/model.ts: `zoneRank` and `targetDagZone` (previously module-private). The gate's behaviour is unchanged. `pnpm check` green, 4488 unit tests, 5 model tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * ci(layering): assert the depgraph report reproduces the gate's baseline The report reads the same model as the gate, so its inversion count must equal TYPE_INVERSION_BASELINE. That agreement was previously a nice property nobody checked; the Layering Guard job now runs scripts/depgraph/model.test.ts, so the two cannot be green independently. Verified by bumping a baseline entry by one and confirming the job fails with a message naming the fix. The count feeding the check is computed by `typeInversionsByPair`, which applies the gate's rule — once per FILE pair, over the raw resolved edges — rather than reading the collapsed edge list. That matters: `collapseEdges` keeps one edge per pair with the strongest kind winning, and `dynamic` outranks `type`, so a module imported both lazily and for its types would collapse to `dynamic` and drop out of the count. No such pair exists today (measured: 0 of 42 inverting pairs), but a number wired into a CI equality check must not be able to drift for a reason unrelated to layering. Stated honestly in the README and the test: this is a cross-check of the report's extraction and the baseline against the real tree, not two independent algorithms. The gate remains the authority — if they disagree, the baseline or the tree is wrong, never the test. TYPE_INVERSION_BASELINE is now exported for this purpose. Not done here, deliberately: the ~1338 transitively redundant value edges are a candidate for a loose growth-only ratchet later. They are a candidate list, not a work list, and a hard count would be noise. `pnpm check` green, 4488 unit tests, 6 depgraph model tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * fix(depgraph): make the source reviewable, and stop overclaiming removability Four review findings, all of them real. P1 - the implementation was binary. scripts/depgraph/model.ts contained two raw NUL bytes used as map-key delimiters, so Git classified a ~346-line file as binary and hid its entire diff behind `- -`. Replaced with a unicode escape: identical at runtime, textual on disk. I had seen the symptom repeatedly - every grep on that file printed "binary file matches" - and worked around it with python instead of asking why, which is how it survived to review. Guarded repo-wide rather than for this one file: a new test asserts no tracked .ts under src/ or scripts/ contains a raw NUL, verified by reintroducing one and watching it fail. Nothing else would catch a recurrence, and the failure mode is silent - the code works, the review does not. P1 - "transitively redundant" claimed removability it cannot support. Module reachability does not carry bindings: if `a` imports `{ c }` while `b` only re-exports it as `{ c as b }`, the path a -> b -> c exists and deleting a -> c still breaks `a`. The fixture in model.test.ts is exactly that shape and its comment said "removable". Reachability also says nothing about when a module's side effects run. Renamed throughout to what it measures - `transitivelyReachable`, `markTransitivelyReachableEdges`, and a summary line reading "value edges whose target is also reachable at distance >= 2 (reachability only - not a removability claim)". The caveats and the counterexample are now stated in the marker function, the fixture comment and the README, and symbol-level analysis is named as what deciding any individual edge would actually require. P2 - build.ts had no coverage. Every test exercised model.ts, so the CLI could break its output path, wire shape or summary silently. Added three subprocess tests: default path plus summary-agrees-with-payload, `--out` honoured and valid JSON written, and a trailing `--out` falling back rather than crashing (pinned so it is a decision, not an accident). `pnpm depgraph:test` now runs inside `check:tooling`, so `pnpm check` covers it. P2 - README was wrong three ways: it queried `.tmp/depgraph/index.json` after the output moved to `graph.json` (the documented command failed as written), it derived inversions from collapsed `zoneEdges`, which can undercount, and it claimed both that the report runs in CI and that nothing here runs in CI. The query now reads `typeInversions` and was run verbatim; the CI sentence names exactly which single test runs and states that nothing else gates a merge. pnpm check green, 4488 unit tests, 10 depgraph tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-27 13:59:53 +02:00
Michał Pierzchała	ab913c9720	feat: strengthen agent help benchmarks (#1404) ... * feat: strengthen agent help benchmarks * fix: harden help benchmark review findings * fix: close help benchmark validation bypasses * fix: make selector scoring quote-insensitive	2026-07-27 10:17:46 +02:00
Michał Pierzchała	56b72c5cf7	refactor(boundaries): put shared contracts below their consumers, gate the result (#1405) ... * refactor(boundaries): move shared contracts below their consumers Acts on the depgraph findings: type-only edges are invisible to R5, so vocabulary that everything depends on had drifted above the zones that use it. - contracts/: the four platform-plugin facet tags (LogBackend, RecordingBackendTag, PerfMetricsSamplerTag, PlatformGatedProviderResolverKey) now live beside the plugin contract itself, which also moves out of core/; NetworkEntry moves next to the command surface that renders it; and the click-button, recording-export-quality, interactor-types and runner-lease-context vocabularies move down out of core/. - (root) drops from 29 files to 13: the internal *-contract/output/annotation modules move into contracts/, kernel/ (daemon-error, observability-redaction beside kernel/redaction), core/ (batch-policy, an ADR 0008 projection), commands/ (cli-command-aliases) and remote/ (upload-progress, upload-stream). What remains is entrypoints and the composition roots that R2 requires to sit outside the spine. - utils/ joins the ranked spine at rank 1 after its only two upward files move to the zones they were reaching for (cli/resolve-cli-options, cli-schema/cli-config), putting ~336 value edges under the gate. - Internal imports that routed types through the client-types re-export hub now name their real source. Type-only spine inversions drop from 61 to 35; the remainder is two clusters (client/client-types.ts and the ADR 0003 daemon facet). No behaviour change: 4470 unit tests and the layering gate pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * style: merge the duplicate contract imports the tag moves created Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * refactor(imports): name the declaring module, share find's argument rules Two follow-ups from re-measuring the graph after the boundary moves. 1. 89 type imports across 79 files routed through a re-export hub in another zone: `CliFlags` reached through commands/cli-grammar/flag-types.ts (52) when it is declared in contracts/cli-flags.ts, the replay suite result types reached through daemon/types.ts when they are declared in contracts/replay.ts, the doctor types through a daemon handler module, and so on. Each hop invented a cross-zone edge the architecture never asked for — including every apparent replay -> daemon and utils -> commands dependency. They now name the module that declares them. Within-zone hops are left alone; those are a local style choice, not a boundary claim. 2. `find`'s three positional/flag checks existed in both daemon entry points with hand-repeated messages, and the copy in dispatchFindReadOnlyViaRuntime was unreachable — its only caller validates first. Both now call checkFindArgs in selectors/find.ts, beside parseFindArgs and isReadOnlyFindAction, for the reason that module's own comment already gives: so the two paths cannot disagree. The refusal is returned rather than thrown, because the two mechanisms are not observationally identical in the session event log. Type-only spine inversions: 61 -> 35. 4470 unit tests and every gate pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * feat(layering): ratchet type-only spine inversions (R6) R5 ignores type-only edges by design — they cost nothing at runtime and do not affect cold start — so nothing was watching the direction they point. Ranking them the same way found 61 inversions, including contracts/ and utils/ declared in terms of rank-4 zones. 26 are fixed by the preceding commits; R6 pins the rest per zone pair so they can only shrink, and a new pair fails outright rather than being added to the baseline. The two remaining clusters each need their own change, and the baseline says so: the per-command Options/Result vocabulary declared inside the public Node-client surface, and the ADR 0003 daemon facet shape that core's descriptor registry composes. Both ratchet directions are covered: growth fails, and shrinking without lowering the number fails too, so the baseline cannot quietly stop describing the tree. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * docs: record the import-graph findings behind this refactor A dated snapshot, not a normative document: when it disagrees with scripts/layering/, the gate wins. The graph tool that produced it lives on the claude/depgraph-viewer branch, deliberately out of this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * refactor(selectors): state the shared selector argument rules once R2 (commands-floor) forbids the daemon from importing commands/, and that is the right call: commands/ is the client-side surface — its only consumers are cli/, cli-schema/, mcp/, client/ and the composition roots — while the daemon is the executor on the other side of the wire. ADR 0008 protects exactly that seam. Relaxing R2 would let the executor depend on a client projection and pull CLI grammar and output formatting into the daemon's bundle. But the rule does force duplication: the daemon must validate independently because it accepts requests from any client, so 10 refusal messages existed in both zones. The only place a shared rule can live is below both, and selectors/ already held the parsers (splitIsSelectorArgs, splitSelectorFromArgs, isSupportedPredicate) and even the `is` predicate message — just not the checks that use them. Three drifts had already appeared in the `is` predicate rule alone: - commands/interaction/selectors.ts re-implemented the predicate list as an inlined seven-way `!==` chain while importing the message and hint from selectors/predicates.ts, so adding a predicate to the shared list would not have reached the CLI grammar. - That inlined chain compared the raw token, so the CLI rejected `is TEXT ...` while the daemon it hands the command to accepts it. The CLI now matches the executor; this is an intentional alignment, not an accident. - isCommand raised the same refusal without IS_PREDICATE_USAGE_HINT, so whether an agent got recovery guidance depended on which layer noticed first — the failure mode ADR 0010's audit calls out. checkIsPredicate, checkIsArgs, checkGetFormat, checkElementTargetArgs and checkWaitText now hold those rules, each beside the parser it wraps, and report a refusal rather than choosing how to raise it: the daemon returns a response, the command surface throws. Those mechanisms are not interchangeable — they write different session events — so the shared check stays out of that decision. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * feat(daemon): give ADR 0014's ref frame one transition, pin SessionState owners `SessionStore.get()` returns the live record out of a private Map and `set()` re-puts the same reference, so every `session.<field> = …` in the daemon is a durable write to store-owned state: 57 of them across 17 files, against 26 `set()` calls that are therefore ceremonial. Nothing at the store boundary can check what those writes are supposed to keep true. Measuring which module writes which field showed the problem is narrower than the raw count suggests — 16 of 27 fields already have exactly one writer. The sharp case is ADR 0014's ref frame: `refFrameState`, `refFrameScope`, `refFrameTree` and `refFrameGeneration` must move together or the frame is incoherent (an `active` state with a stale tree resolves refs against a namespace nobody authorized), yet complete issuance wrote them in ref-frame.ts and partial issuance wrote the same four in session-snapshot.ts. ref-frame.ts's own header claims to be "the single owner of the frame's transitions", and session-snapshot.ts documented itself as the exception. Both forms now go through `activateRefFrame`; they differ only in scope. `recordSession` deliberately moves alone in two paths (recording without arming a publication), so the save-script cluster gets no invented abstraction — it gets ownership instead. R7 records every field's owner and stops the set from growing quietly: a new SessionState field must declare one, a foreign write fails naming the owner to call, and an owner that stops writing must be removed so the table cannot drift into fiction. Field names are read out of the `SessionState` declaration, so a daemon module with an unrelated local named `session` — a provider or runner session — cannot trip it. 4475 unit tests and every gate pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * docs: record the reference semantics and refresh the findings SessionStore.get/set now document that the record is handed out live, since that is the fact behind R7. The findings snapshot picks up the resolved R2 question, the ref-frame consolidation and the two new gate scopes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * refactor(boundaries): rank every satellite zone, extract the provider port Second-order effect of the earlier rounds. With `utils` on the spine and `(root)` emptied of shared contracts, the eleven zones that were unranked "because ranking them would invent an order the architecture had not committed to" turned out to have a consistent rank already — the order was there, unasserted. Solving the constraint system showed one blocker: `utils/remote-config.ts` projected a remote-config profile into `CliFlags` while reaching up into `remote/`, and its only three consumers were in `cli/`. It moves there as `cli/remote-config-flags.ts`, and every satellite zone joins the spine. Ranked coverage goes from 730/895 files to 882/895. Only `(root)` stays out, and now for one stated reason: R2 forbids `daemon/` from importing `commands/`, so the files that wire them compose the spine from above. Ranking them exposed 22 type-only inversions R6 had never been able to see, and they were concentrated rather than scattered: - The device-provider port. `providers/` and `cloud-webdriver/` implement what the daemon calls, so both sides name `DeviceLease`, `LeaseLifecycleProvider`, `LeaseLifecycleContext` and `DeviceInventoryProvider` — now declared in contracts/device-provider.ts, below both. The adapters also imported the daemon's NARROWED `DaemonRequest` while only ever reading `req.flags`; they now name the public one from kernel/contracts. - `MetroPrepareKind` and the remote-config profile field groups move to contracts/ for the same reason: the command surface validates them and contracts/cli-flags.ts is composed from them. Two clusters remain, ratcheted with their reasons in TYPE_INVERSION_BASELINE: the client-types vocabulary, and `SessionAction`, which needs `CommandFlags` and `DaemonBatchStep` to move with it. Also fixes two things CI caught: the eight type re-exports my earlier import redirection orphaned (none published through any src/sdk/* entrypoint, so no public surface changes) and `isSupportedPredicate`, now module-private since `checkIsPredicate` is the admission API. `fallow-baselines/health.json` is keyed by path, so the moved cli-config entry moves with the file rather than being regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * fix(selectors): use the admitted predicate, not the raw option Review finding. `isCommand` called `checkIsPredicate` and then kept reading `options.predicate` for the capture policy, the `exists` branch, `evaluateIsPredicate`, the failure message and the returned result. Admission normalizes case, so an upper-case predicate was let past the gate and then evaluated against lower-case branches: `EXISTS` skipped its own branch and fell through to the generic path, and the result echoed the raw token. I widened admission at that surface without threading the normalized value through it — the CLI-grammar surface in the same change does use the admitted value. Every decision after admission now reads it. Two tests, both verified to fail without the fix: - a production-route regression driving `device.selectors.is` with `EXISTS`/`TEXT`, plus one pinning that an unknown predicate is still refused WITH the ADR 0010 usage hint; - a surface parity gate (selectors/__tests__/is-argument-surface-parity.test.ts) in the repo's existing parity style, asserting the daemon and CLI-grammar surfaces reach the same verdict and hand the same normalized predicate downstream across an input table. A helper-only test cannot catch a surface that admits correctly and then discards the result, which is what happened here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * docs: name the pre-push gate, and the formatter's path allowlist Both misses in this PR's review were process, not judgement, and the docs pointed the wrong way for both. AGENTS.md said "prefer the aggregate package.json scripts" without naming which aggregate, and CONTRIBUTING listed `pnpm test` and the targeted checks but never `pnpm check`. `check:tooling` looks like the gate and is a subset of it: it stops before the Fallow audit, so the dead exports this PR introduced passed a clean `check:tooling` and failed CI. Both files now name `pnpm check`, say what it covers, and say what it cannot (the device matrix). The same gap produced a second mistake twice: `oxfmt <path>` reformats whatever you point it at, while the repo's `format` script is an allowlist that excludes `scripts/` and every `.md`. One run reformatted 50 unrelated script files into a commit; the next nearly did it to AGENTS.md. AGENTS.md now says to run `pnpm format`, never `oxfmt <path>`. It also records the rule that cost a CI cycle: Fallow's baselines are keyed by path, so a renamed file needs its baseline entry moved, not the baselines regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * revert: undo stray formatter output across docs and scripts Three separate `oxfmt <path>` runs in this branch reformatted files the repo's `format` script deliberately excludes: 55 files under scripts/maestro-conformance plus scripts/perf, sync-mcp-metadata and the slow-test reporter, and 12 markdown files including six ADRs and docs/agents/. All of it was whitespace, quote style and markdown table padding — no content — but it inflated the diff a reviewer has to read and would have rewritten prose ownership across files this change has no business touching. All 70 are back to their origin/main content, so the diff outside src/ is now exactly this change's scope: three docs, scripts/layering, the Fallow baseline, and five provider integration tests. The rule this violated is now in AGENTS.md: run `pnpm format`, never `oxfmt <path>`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * style: reformat two provider tests with the repo's pinned oxfmt `pnpm format:check` failed in CI on the two files whose imports I merged by hand. The repo pins oxfmt 0.42.0 as a devDependency and both `format` scripts invoke `./node_modules/oxfmt/bin/oxfmt`; I had reformatted with `npx oxfmt`, which resolved 0.60.0, and the two versions disagree about wrapping a 100-column import. This is the rule AGENTS.md already states — run `pnpm format`, never oxfmt directly — so there is nothing to add to the docs, only to do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * fix(ci): install deps for the layering guard, and gate the zero-dep contract The Layering Guard job failed with ERR_MODULE_NOT_FOUND on `oxc-parser`. The job ran with `install-deps: false` — no `pnpm install`, so no `node_modules` — and R7 had started parsing the daemon with oxc-parser instead of matching assignment operators with a regex. `pnpm check:layering` passed on every local run, because locally `node_modules` is always there. The job now installs dependencies. The alternative was to put R7 back on a regex, which cannot see `??=` or a computed `session[key] =` write, so it would trade a correct rule for a fast job. That leaves the interesting part: the zero-dep contract is real for the jobs that keep it, and it is invisible to every local run, which is the worst combination a constraint can have. R8 makes it checkable. It reads the zero-dep job list out of `.github/workflows/` rather than restating it — declaring a job zero-dep is what puts it under the rule — walks each job's entry scripts and their whole relative-import closure, and requires every specifier to be a Node builtin or another repo file. A zero-dep job whose entry scripts the scan cannot identify fails too, so the rule cannot be escaped by changing how the job invokes them. Specifiers come from oxc-parser's module record, not a line scan. The closures include `--test` files, and a test about imports naturally embeds import syntax in a fixture string; the line scanner reported two such phantom violations in model.test.ts before the switch, which is how a gate stops being trusted. Verified by re-running the real gate against three injected regressions: the layering job back on `install-deps: false` (reproduces the exact CI failure, pointing at session-state.ts:24), a package import added to the still-zero-dep affected-selector closure, and a zero-dep job whose run step names no script. Also corrects the CONTEXT.md spine paragraph, which still described the satellite zones as deliberately unranked after they had all joined the ranked spine. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * fix(layering): make R7 exhaustive, and follow session records through aliases Review finding: `SESSION_STATE_FIELD_OWNERS` covered 27 of `SessionState`'s 42 fields and nothing asserted parity, so a new field could be added and pass the gate by being invisible to it. R7's advertised claim — "every SessionState write is inside its declared owner" — was broader than what it checked. Investigating that turned up a second, larger gap the finding did not name: the scan only recognized a binding literally named `session`. The daemon names these records by role, so `nextSession`, `provisionalSession`, `completedSession`, `preRunSession` and `preEntrySession` were all invisible — and three of those writes were genuine violations R7 existed to catch: src/daemon/snapshot-runtime.ts:256 nextSession.snapshotScopeSource src/daemon/snapshot-runtime.ts:265 nextSession.snapshotGeneration src/daemon/handlers/session-replay-runtime.ts:707 preEntrySession.pendingRecordAndHeal The first two are the #1076 versioned-ref invariant: the generation advances exactly when the stored tree is replaced. That rule lived in `setSessionSnapshot` and had acquired a second statement of itself in snapshot-runtime.ts, whose own comment admitted the bypass. It now goes through `setSnapshotLineage` in the owning module. The third clears a watermark stamped by session-replay-resume.ts; `clearPendingRecordAndHealWatermark` puts the clear beside the stamp. Gate changes: - Binding detection accepts aliases, paired with the existing declared-field filter so an unrelated `…Session` local only registers if it also writes a field SessionState owns — where the remedy is the same anyway. - `fieldClassificationDrift` asserts parity in all three directions: unclassified, in-both, and naming a field SessionState no longer declares. - `STORE_OWNED_SESSION_STATE_FIELDS` classifies the 11 fields the store establishes at construction. It is a positive claim, so a direct write to one fails and names both remedies. - Four fields the widened scan made visible (`lease`, `deviceClaim`, `appName`, `saveScriptComplete`) got real owners. `nextSnapshotGeneration` is now module-private: replacing its only external call site orphaned the export, which `pnpm check` caught via Fallow. Verified against three injected regressions: a new SessionState field with no direct write (the reviewer's exact scenario), a foreign write through an alias binding, and a direct write to a store-established field. All three rejected. `pnpm check` green, 4486 unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur * docs(daemon): correct the snapshot-lineage claim, and pin the real contract Device verification of the snapshot-lineage route found that a ref pinned before a `diff` keeps resolving with no pinned-ref warning. That is the designed ADR 0014 behaviour, not a regression — the comment describing it was wrong, and I propagated it. `main`'s comment in snapshot-runtime.ts said a diff "leaves client refs pinned to the previous generation, which is exactly what the pinned warning diagnoses". The counter and the authorization epoch are different clocks: - `diff` passes `issuesRefsToClient: false`, so it never reactivates the frame; - `resolveRefStalenessWarning` compares a pin against the frame EPOCH, not the observation counter, and its own comment says why — a capture that bumped the counter must not make a valid pin from the issuing frame look stale. So advancing the counter is not the same as invalidating client refs, and the observable the comment promised does not exist. I carried the sentence into `setSnapshotLineage`'s doc when the transition moved, and then into a hardware verification request, which cost a reviewer a device run against a false claim. `setSnapshotLineage` itself is unchanged and was a pure move: same expressions, same inputs as the inline assignments it replaced, so this route behaves exactly as it does on main. A comment that contradicts the code should be an assertion instead, so the contract is now pinned in session-snapshot.test.ts: the diff advances the counter, preserves the epoch, leaves the pre-diff pin resolving without a warning, and still warns for a pin from a different frame. Verified to fail when the epoch comparison is swapped for the counter. A second test covers the keep-current branch, which had no coverage. `pnpm check` green, 4488 unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bfu8HofkhybiAm5LECfqur --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-27 08:08:34 +02:00
Michał Pierzchała	1a76344685	docs: restructure AGENTS.md and CONTEXT.md for progressive disclosure (#1402) ... * docs: restructure AGENTS.md and CONTEXT.md for progressive disclosure Apply the Claude 5 context-engineering guidance to the repo's agent docs: keep the always-loaded file to gotchas and invariants, and move situational guidance one hop away behind a routing table. AGENTS.md 315 -> 229 lines. Cut generic agent-behavior boilerplate, three-way duplication (Common Mistakes restated Hard Rules; Finding Source Owners restated the registry section), and facts visible from the repo itself. Kept verbatim: the expensive-lessons principles, enforcement gates, Hard Rules, and environment traps. Split out docs/agents/{cli-flags,pull-requests,device-verification}.md and folded the Testing Matrix into docs/agents/testing.md, reframed around pnpm check:affected so the prose stops duplicating the selector. CONTEXT.md keeps all 50 terms, now grouped under a section index so a task loads one section instead of the whole glossary. * fix(check-affected): move the selector-owning sentinel to the Testing Matrix The Testing Matrix moved from AGENTS.md to docs/agents/testing.md, but the affected-check selector still treated only AGENTS.md as selector-owning. A later matrix edit would have been classified as inert docs and skipped the fail-open, so the selector could keep deriving gates from a spec that had changed underneath it. Move the sentinel with the prose, as a named SELECTOR_OWNING_DOCS set so the next move is one line, and fix the two in-code comments plus the testing.md paragraph that still pointed at the AGENTS.md matrix. * docs: restore two rules dropped by the AGENTS.md split Review caught two repo-specific rules that did not survive the move. Both are prose without any backticked identifier, so the identifier-diff used to verify the split could not see them. - "Test through public interfaces; do not add unrelated production exports solely to enable tests" returns next to the behavioral-tests rule in docs/agents/testing.md, with the reason it exists. - The guidance-ownership rule (decide whether new guidance/schema/metadata belongs to the command surface, CLI grammar, CLI help, MCP projection, or daemon runtime) returns to the always-loaded Docs & skills section, since it governs all command-surface work and not just the flag case. Also point the ADR routing row at docs/adr/README.md, which is already the "read when you touch…" index, rather than at the bare directory.	2026-07-25 12:13:09 +02:00
Michał Pierzchała	877e68fe30	fix(cli): compact stale device status (#1388) ... * fix(cli): compact stale device status * fix(cli): quote stale status selectors	2026-07-25 11:44:22 +02:00
Michał Pierzchała	5507a08b9c	feat: parameterize sensitive recorded inputs (#1369) ... * feat: parameterize recorded inputs * fix: harden parameterized replay recording * fix: sanitize parameterized fill echoes * fix: scrub embedded parameterized fill echoes * fix: make recorded fill scrubbing idempotent * fix: replay parameterized coordinate fills * test: align parameterized publication landmark	2026-07-24 20:37:29 +02:00
Michał Pierzchała	d45190613c	fix: report bundle sizes with two decimal MB precision (#1382)	2026-07-24 08:17:42 +02:00
Michał Pierzchała	d237bc555d	chore: remove verified dead code and migration scaffolding (~700 LOC) (#1367) ... * chore: remove verified dead code and migration scaffolding Multi-agent audit of accumulated waste, every finding adversarially verified against call sites, git history, and the published surface before removal. Net -710 lines. - delete src/core/platform-descriptor/ (superseded ADR-0009 migration scaffold; parity tests now assert an inline table) - remove test-only seams: registry introspection exports, CommandFacet.extraDaemonWriters, MaestroEngineOptions.timing - remove dead flexibility: backend capability allow-list, screenshot-diff maxRegions, CloudWebDriverSupportLevel 'partial', clearFirst on the TS+Swift runner wire contract - remove dead deprecated surface: --session-locked / --session-lock-conflicts aliases (hard migration error now points at --session-lock), replay export --format single-value enum, unused Lease*Payload contract types, runtime-layer rotate duplicate - collapse pass-throughs/duplication: withRetry adapter, default-cloud-artifact-provider, connect-profile client-id hashing (3x sha256 impls -> one helper, byte-identical output), shared scripts walker, cloneValue -> structuredClone, fill-diagnostics moved into android/ BREAKING CHANGE: --session-locked and --session-lock-conflicts now fail with a migration error pointing at --session-lock; replay export --format is removed (Maestro was the only value); Lease*Payload types are dropped from the ./contracts subpath. * chore: satisfy fallow gates tightened by #1363/#1364 after rebase - drop the consumer-less AndroidFillVerificationNode re-export - reuse requireSnapshotSession in resolveSnapshotForRef instead of inlining the same authorized-frame resolution (fallow clone group); the helper's return type now guarantees the session it already throws for * chore: address review — keep cloud-webdriver partial capability metadata The partial/supported/unsupported levels and their notes are part of the lease-response capability contract for genuinely limited operations (Appium page-source snapshots, upload-then-install), not dead scaffolding. Restore them and the asserting tests unchanged from main. Also add the missing CHANGELOG entry for the Lease*Payload type removal from agent-device/contracts.	2026-07-23 07:36:52 +02:00