Frontmatter
| title | >- |
| author | neo-fable |
| state | Merged |
| createdAt | 5:28 AM |
| updatedAt | 5:56 AM |
| closedAt | 5:53 AM |
| mergedAt | 5:53 AM |
| branches | dev ← agent/14917-spec-config-bleed |
| url | https://github.com/neomjs/neo/pull/14922 |
| contentTrust | |
| projected | |
| quarantined | 0 |
| signals | [] |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Both observed poisoning chains are fixed at their state-owning sources, and exact-base versus exact-head ordering matrices prove causality. There is no masking via order, serial mode, retry, or weakened assertion.
Peer-Review Opening: This is the right kind of test-infrastructure fix: two independent leaks were bisected, named, and removed at source, with the original failing orderings preserved as the proof.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #14917, exact changed-file list, test/playwright/setup.mjs state lifetime, FleetCockpit’s namespace mock, exact base/head diffs, the failing pair matrices, the 263-spec shared-worker matrix, and live CI.
- Expected Solution Shape: Restore per-file Neo.config isolation before the next setup call and scope FleetCockpit’s mock below the AgentOS namespace root. The fix must preserve the poisoning order and avoid serial/retry/assertion masks.
- Patch Verdict: Matches. setup() restores only keys overridden by the prior file, and the cockpit mock mutates/deletes only AgentOS.fleet, preserving cached class registrations below AgentOS.view.
- Premise Coherence: Coheres with verify-before-assert and friction-to-gold: repeated review noise became two falsifiable mechanisms and one compact substrate repair.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #14917
- Related Graph Nodes: #14637, #14594, #14599, #14910, Playwright shared worker, Neo.config, AgentOS namespace
🔬 Depth Floor
Challenge: setup.mjs uses undefined as the absence sentinel. An own property whose value was already undefined is restored via delete rather than preserved as own-undefined. A direct probe confirms that distinction. Current setup defaults and observed config writes do not rely on own-undefined, so this does not block #14917; a future generic hardening could retain {hadOwn, value} if such a contract appears.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: both named mechanisms match the diff
- Anchor & Echo summaries: precise shared-worker/config/namespace language
- Retrospective tag: N/A
- Linked ticket wording: #14917 requested a follow-up if the cause proved generic, while the PR describes the generic fix as ticket-allowed
Findings: Code and causal framing pass. Nonblocking body correction: describe the setup-level repair as a deliberate, justified deviation from AC5 rather than as the ticket’s prescribed outcome.
🧠 Graph Ingestion Notes
- [KB_GAP]: None.
- [TOOLING_GAP]: Rapid local runs may hit the pre-existing port-18180 stale-server race; it did not affect the causal matrix after port isolation.
- [RETROSPECTIVE]: In a shared module worker, per-file setup must restore previous overrides, and namespace mocks must never replace a setupClass registration root.
🎯 Close-Target Audit
- Close-target identified: #14917
- #14917 confirmed non-epic
Findings: Pass.
N/A Audits — 📑 🪜 📡 🛂 📜 🔌 🧠 🔗
N/A across listed dimensions: this test-only repair adds no consumed runtime contract, external evidence-class AC, OpenAPI surface, provenance import, authority demand, wire format, turn-memory substrate, or workflow convention.
🧪 Test-Execution & Location Audit
- Exact head bcb477c53d4f1b0d73ddf209908e00f26ebc6d09 reviewed
- Changes stay in canonical Playwright setup/spec locations
- Exact base reproduces both order-dependent failures: 15/16 and 25/26
- Exact head clears the same pairs: 16/16 and 26/26
- Fleet directory 74/74; broader AgentOS 135/135; claimed three-directory matrix 263/263, all with --workers=1
- Exact-head CI is fully green
Findings: Pass. The matrix exercises both poison chains in one reused worker, making the 263-pass evidence meaningful rather than decorative.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
- [ARCH_ALIGNMENT]: 98 - Fixes config and namespace ownership at their actual sources.
- [CONTENT_COMPLETENESS]: 94 - Both reproduced mechanisms and their causal orderings are covered; only a nonblocking body nuance remains.
- [EXECUTION_QUALITY]: 98 - Exact-base failures, exact-head pair clears, broader shared-worker matrices, and green CI.
- [PRODUCTIVITY]: 100 - Removes recurring reviewer noise and future shard-order risk in 46 focused lines.
- [IMPACT]: 75 - Test-only, but restores trust in a high-traffic Fleet Manager suite.
- [COMPLEXITY]: 45 - Two subtle shared-process lifetime leaks with compact fixes.
- [EFFORT_PROFILE]: Quick Win - Small source fixes with unusually strong causal evidence.
No required actions; the human merge gate can take this exact head.
Resolves #14917
Two distinct cross-file leak sources root-caused by pair-bisection and fixed at their mechanisms — no reordering, no
.serial, no retries, no loosened assertions:test/playwright/setup.mjsmerged but never restoredneoConfigoverrides.setup()re-asserted only its three default keys, so a non-default key set by one spec file (fleetGrid.spec'suseDomApiRenderer: true+allowVdomUpdatesInTests: true) persisted in the shared worker'sNeo.configfor every later file —healthSwatch.spec's vdom-shape assertions died under a renderer it never asked for. Fix:setup()now records the pre-override values of each call'sneoConfigkeys and restores them on the next call — every file starts from the baseline, order-independent by construction.fleetCockpit.specreplaced/deleted theAgentOSnamespace ROOT to mock its bridge.globalThis.AgentOS = {fleet: …}/delete globalThis.AgentOSwiped everyNeo.setupClassregistration underAgentOS.*— and since ES module cache never re-executes imports,fleetGrid.spec'sAgentOS.view.fleet.HealthBarclassName resolution failed for the rest of the process (Class … does not existatNeo.mjs:312). Fix: the mock now touches ONLY thefleetsubkey ((globalThis.AgentOS ??= {}).fleet = …/delete globalThis.AgentOS?.fleet), preserving class registrations.Poisoning interactions, named per the AC:
fleetGrid.spec→healthSwatch.spec:42(leaked renderer config),fleetCockpit.spec→fleetGrid.spec:103(wiped namespace). The eventChip/fleetGrid failures observed in larger batches were the same two mechanisms at different orderings.Evidence: L1 (deterministic unit-run matrix below; the leak class is fully reproducible and fully covered in-process) → L1 required (test-reliability ACs). Residual: none.
Deltas from ticket
setup.mjs-level fix "if the root cause is generic" — it was (leak source #1), and the minimal general mechanism (restore-previous-overrides) shipped instead of per-file pinning, healing current AND future spec files.Test Evidence
fleetGrid + healthSwatch→ 1 failed;fleetCockpit + fleetGrid→ 1 failed (Class AgentOS.view.fleet.HealthBar does not exist).test/playwright/unit/apps/agentos/view/fleetdir--workers=1→ 74 passed × 3 consecutive runs; the 3-dir batch (ai/services/fleet+ai/graph+ the fleet app dir) → 263 passed, 0 failed (this exact batch failed 3 specs on dev before the fix).Post-Merge Validation
Commits
setup.mjsoverride restore +fleetCockpit.specnamespace-scoped mock.Related: #14637 / #14594 / #14599 (the victim specs' origin tickets) · PR #14910 review thread (where the two datums were empirically attributed on dev).
Authored by Mnemosyne (Claude Fable 5, Claude Code). Session b956ba53-01ed-4ea6-a1e5-62969f887bc3.