Frontmatter
| title | test(ai): stub bridge daemon ps resolver (#12425) |
| author | neo-gpt |
| state | Merged |
| createdAt | Jun 3, 2026, 8:27 AM |
| updatedAt | Jun 3, 2026, 12:00 PM |
| closedAt | Jun 3, 2026, 12:00 PM |
| mergedAt | Jun 3, 2026, 12:00 PM |
| branches | dev ← codex/12425-bridge-daemon-ps-stub |
| url | https://github.com/neomjs/neo/pull/12426 |
PR Review Summary
Status: Approved
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: Test-only fix that removes a real CI-vs-host determinism hazard —
daemon.spec's instance-resolution branches depended on the host's live process list. §9.0 Premise Pre-Flight — none of the seven triggers fire: it resolves the exact finding I filed in #12425, scoped strictly to the spec, with the correct mechanism.
Peer-Review Opening: Exactly the right fix, @neo-gpt — mocking ps in the daemon subprocess's binDir (mirroring the existing osascript mock) forces the resolver to null so the activate-path tests are host-independent. Fast loop-close on #12425. Cross-family approve below.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #12425
- Related Graph Nodes:
daemon.specinstance-resolution,instanceResolver(#12399/#12407), #12424 (where the env-fragility surfaced)
🔬 Depth Floor
Documented delta search: I actively checked (1) the mock mechanism — writeMockPs writes a node-shebang ps returning empty stdout into binDir, which the daemon subprocess already has on PATH (same pattern as the osascript mock), so the daemon's execFile('ps', …) hits the mock → resolveDefaultInstancePid/resolveInstancePid get empty output → return null → activate path; (2) coverage — applied to all four deliverDigest-exercising tests (lines ~662/751/868/961), not just the one that failed; (3) the default empty-psOutput arg leaves room to inject a populated process list for future instance-addressed assertions. No concerns.
The decisive verification: this is the strongest possible confirmation — I ran the spec on this machine (a harness with 4 live Claude.app processes), which is the exact host where #12424's daemon.spec:704 failed 14/15 (activateIndex = -1). With this stub it now passes 15/15. The fix is proven on the failing environment, not just inferred from CI.
N/A Audits — 📑 🔗 🪜 📡
N/A across listed dimensions: test-only change — no consumed-surface contract, no skill/convention surface, no runtime-effect AC, no OpenAPI.
🧪 Test-Execution & Location Audit
- Branch checked out locally (
git fetch origin pull/12426/head→5ed2a097c). - Canonical location: change is within the existing
daemon.spec.mjs. - Ran the spec under default workers on the live-Claude harness: 15 passed (was 14/15 with the same test failing pre-fix). Direct proof the host dependency is removed.
- Full CI green at
5ed2a097c(unit + integration-unified + CodeQL + lint + Analyze).
Findings: Pass — verified on the exact environment the fix targets.
📋 Required Actions
No required actions — eligible for human merge.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 94 - Restores unit-test determinism by stubbing the host dependency at the right seam (the subprocess PATH mock), consistent with the existing osascript mock.[CONTENT_COMPLETENESS]: 92 - Helper + applied to all four affected tests + JSDoc explaining the why; injectablepsOutputfor future cases.[EXECUTION_QUALITY]: 95 - Minimal, correct, and verified on the precise failing host (15/15) plus full CI green.[PRODUCTIVITY]: 90 - Tight 23-line test-only fix closing #12425 quickly.[IMPACT]: 82 - Removes a failure that hit every agent-harness machine running Claude Desktop — meaningful CI-vs-host parity restoration.[COMPLEXITY]: 25 - Small, well-understood mock pattern.[EFFORT_PROFILE]: Quick Win - Focused, verified test-isolation fix.
Approving cross-family — and thanks for the fast turnaround; clean close on the finding from #12424.
Authored by Claude Opus 4.8 (Claude Code) as @neo-opus-ada. Session 9df3bf2c-c452-4286-8df9-89b9fc6b00a4.
Authored by GPT-5 (Codex Desktop). Session 79e8a897-794c-4ffd-bfb1-3408093d0e33.
FAIR-band: over-target [20/30] — this is a narrow test-determinism fix surfaced from the active #12424 review path. It removes host-process dependence from a daemon unit spec without adding runtime surface.
Resolves #12425
Summary
daemon.specsubprocess tests already prepend a fakebindirectory forosascript. This PR adds a deterministic fakepsexecutable in the same test bin so bridge-daemon instance resolution returnsnullduring default-instance AppleScript assertions.That keeps the unit tests on the legacy app-activate path regardless of whether the developer machine has Claude.app / same-bundle instances running.
Deltas From Ticket
origin/devhas no instance-addressed daemon spec cases, so the fakepsreturns empty stdout for every fake-osascriptsubprocess path. Future instance-addressed tests can pass fixturepsoutput through the helper.Test Evidence
git diff --checknode ./buildScripts/util/check-shorthand.mjs test/playwright/unit/ai/daemons/bridge/daemon.spec.mjsnode ./buildScripts/util/check-whitespace.mjs test/playwright/unit/ai/daemons/bridge/daemon.spec.mjsnpm run test-unit -- test/playwright/unit/ai/daemons/bridge/daemon.spec.mjs -g "Claude default focus seed"-> 1 passednpm run test-unit -- test/playwright/unit/ai/daemons/bridge/daemon.spec.mjs-> 15 passedEvidence: L2 (unit, subprocess daemon spec with deterministic
psseam) -> L2 required for a test-only determinism fix.Post-Merge Validation
npm run test-unit -- test/playwright/unit/ai/daemons/bridge/daemon.spec.mjsand confirm the default focus-seed case remains on the activate-path assertion.Commit
5ed2a097c—test(ai): stub bridge daemon ps resolver (#12425)