test: add isolated branch stack browser harness - #431
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds a fail-closed isolated branch stack launcher with run-scoped state, manifests, process ownership checks, and structured logging. It wires manifest-authoritative Playwright E2E validation and evidence publication, updates A3/A4 consumers, and adds governance workflow and lifecycle documentation updates. ChangesIsolated branch stack verification
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant AgentGovernance
participant IsolatedLauncher
participant GovernanceBackend
participant CoordinatorBackend
participant Playwright
participant EvidenceWriter
AgentGovernance->>IsolatedLauncher: start isolated stack
IsolatedLauncher->>GovernanceBackend: launch with run-scoped state
IsolatedLauncher->>CoordinatorBackend: launch after governance health
IsolatedLauncher-->>Playwright: write stack manifest
Playwright->>CoordinatorBackend: probe health and verify ownership
Playwright->>EvidenceWriter: publish same-run evidence
AgentGovernance->>IsolatedLauncher: run machine tests and stop checks
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (2)
artifacts/e2e/isolated-branch-stack-browser-e2e/p1-consumer-20260730-030207201-c15f2e25/stack-manifest.json (1)
35-56: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueManifest fields not modeled by the consumer type.
stopped_atandprocessesare emitted by the launcher but absent fromIsolatedStackManifestinweb-viewer-sample/e2e/support/isolated-stack.ts. The loader ignores extras so nothing breaks today, but the schema drift means consumers can't reason about lifecycle state. Consider adding both fields (optional) to the type.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@artifacts/e2e/isolated-branch-stack-browser-e2e/p1-consumer-20260730-030207201-c15f2e25/stack-manifest.json` around lines 35 - 56, Update the IsolatedStackManifest type in isolated-stack.ts to include optional stopped_at and processes fields matching the launcher manifest schema. Model processes with the existing process-entry shape used by the consumer, preserving compatibility with manifests where either field is absent.web-viewer-sample/e2e/a4-closeout.spec.ts (1)
143-175: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHoist the deterministic preflight out of
beforeEach.The nested loop issues up to
orderedJobs.length × 8real governance search POSTs, and it repeats for all six tests (3 tests × 2 viewports) even though the resolved(jobId, searchQuery)pair is deterministic for a fixed head/backend pair. Resolve it once (module-scoped memo or abeforeAll-style cached promise) and reuse it; the per-test budget is 180s and this fixture currently dominates it.♻️ Sketch: memoize the preflight result
+let preflight: Promise<{ jobId: string; searchQuery: string }> | undefined; + test.beforeEach(async ({ page, request }, testInfo) => { - jobId = ""; - searchQuery = ""; - // ... nested preflight loop ... + preflight ??= resolveDeterministicPreflight(request); + ({ jobId, searchQuery } = await preflight); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-viewer-sample/e2e/a4-closeout.spec.ts` around lines 143 - 175, Move the deterministic preflight loop currently executed in beforeEach into a module-scoped memoized resolver or beforeAll-style cached promise. Cache the resolved jobId and searchQuery for each fixed head/backend pair, reuse that result across all tests and viewports, and preserve the existing diagnostics and requireReal failure behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@artifacts/e2e/isolated-branch-stack-browser-e2e/p1-consumer-20260730-030207201-c15f2e25/stack-manifest.json`:
- Around line 19-24: Update the manifest artifact for run
p1-consumer-20260730-030207201-c15f2e25 so its known-gap evidence matches the
recorded run: either replace it with a passing-ready run that exposes the
coordinator-listed ifc_ready_job_id, or revise the gap text to describe the
actual backend-unreadable, early-stop failure. Do not claim the isolated
Playwright configuration is blocked by a known gap without supporting readiness
evidence.
In `@docs/plans/NOW.md`:
- Line 42: Remove the blank line within the affected blockquote in NOW.md so the
entire progress note remains one contiguous blockquote and passes markdownlint
MD028.
In `@docs/superpowers/plans/2026-07-29-isolated-branch-stack-browser-e2e.md`:
- Line 57: Replace the bare completion output with the repository’s structured
logger: update
docs/superpowers/plans/2026-07-29-isolated-branch-stack-browser-e2e.md at line
57 to prescribe StructLog.psm1 logging, and update
scripts/tests/test-isolated-branch-stack.ps1 at line 361 to import and use the
same logger for the completion event.
- Around line 1-7: Update the document header near “Isolated Branch Stack
Browser E2E Implementation Plan” to explicitly label the document as a working
note and state that implementation is the runtime/API source of truth. Preserve
the existing plan content while satisfying the required docs classification and
authority boundary.
- Around line 2006-2007: Add the missing trailing comma after the
deployment-listeners-after.json entry in the Task 10 PowerShell path array,
preserving the existing evidence-manifest.json entry and ensuring the snippet
parses correctly.
In
`@openspec/changes/isolated-branch-stack-browser-e2e/specs/isolated-branch-stack-verification/spec.md`:
- Around line 34-36: Update Stop-IsolatedBackends and its process-record flow to
carry the resolved backend role/port for each PID, then revalidate the live
listener’s port against that expected resolved backend port immediately before
stopping. Preserve the existing PID, entrypoint, and creation-identity checks,
and fail closed without stopping when the port or any authorization field does
not match.
In `@openspec/changes/isolated-branch-stack-browser-e2e/tasks.md`:
- Line 28: Complete task 3.7 by implementing harness labeling and evidence
eligibility based on the VITE_VIEWER_HARNESS build/query flags. Ensure harness
runs are excluded from real coordinator authority evidence, then rerun the
relevant test until GREEN and execute typecheck, affected Vitest, and applicable
browser E2E checks. Update the task with the preserved RED/GREEN commands and
results and mark it complete.
- Line 41: Correct task 5.4’s status and wording to match the available
evidence: it currently documents a pre-browser failure caused by the absence of
a downloaded IFC-ready job, not a failed require-real browser run. Either
broaden task 5.4 to explicitly cover this failure mode or mark it incomplete,
avoiding the current checked status unless the required evidence exists.
In `@openspec/lifecycle-ledger.json`:
- Around line 1420-1425: Update the current_slice field in the lifecycle ledger
to accurately reflect the still-open launcher validation, harness
implementation, governance checks, browser evidence, and PR-body tasks listed in
tasks.md, rather than stating that only PR-only closeout remains. Keep the
ledger’s completed and total task counts consistent with the actual task status.
In `@scripts/dev/start-isolated-branch-stack.ps1`:
- Around line 195-197: Update the cleanup loop over the verified processes to
attempt every Stop-Process call even when an earlier stop throws, while
collecting any failures instead of aborting immediately. Persist a recoverable
partial-stop state in the manifest when only some stops succeed, so retries can
continue stopping surviving backends without failing ownership validation for
already-stopped PIDs. Add a failure-injection test covering a later stop failure
and verifying the manifest and retry behavior.
- Around line 338-342: Update the isolated-stack startup flow around the
preflight handling and backend launch so it acquires an exclusive run
reservation before invoking any backend start. Replace reliance on the
non-atomic preflight Test-Path check with the reservation, and retain that
reservation through manifest finalization; release it only after successful
manifest persistence or after the catch rollback completes, including cleanup
when startup fails.
- Around line 138-143: Update StartProcessFn and its coordinator launch call to
preserve argument boundaries when paths contain spaces: construct one argument
string with escaped inner quotes around the tsx entrypoint and index path before
passing it to -ArgumentList, rather than passing a raw string array. Add a
regression test covering coordinator startup from a worktree path containing
spaces.
In `@web-viewer-sample/e2e/a3-federated-session-chain.spec.ts`:
- Around line 51-59: Remove the mutating federated-set POST probe from
beforeEach and replace it with an existing read-only coordinator/governance
readiness route. Update the associated response validation to match that route,
while preserving the readiness failure handling and avoiding creation of any
federated_model_sets row.
In `@web-viewer-sample/e2e/support/isolated-stack.ts`:
- Around line 58-70: Harden URL parsing for request listeners so malformed or
non-HTTP(S) URLs cannot escape as uncaught exceptions: in
web-viewer-sample/e2e/support/isolated-stack.ts lines 58-70, update
createForbiddenRequestGuard().observe to safely parse and ignore unparsable or
non-HTTP(S) URLs before checking RESERVED_PORTS; in
web-viewer-sample/e2e/a4-closeout.spec.ts lines 100-104, apply the same guarded
parsing before reading pathname, or reuse the hardened helper for the
generic-route check.
---
Nitpick comments:
In
`@artifacts/e2e/isolated-branch-stack-browser-e2e/p1-consumer-20260730-030207201-c15f2e25/stack-manifest.json`:
- Around line 35-56: Update the IsolatedStackManifest type in isolated-stack.ts
to include optional stopped_at and processes fields matching the launcher
manifest schema. Model processes with the existing process-entry shape used by
the consumer, preserving compatibility with manifests where either field is
absent.
In `@web-viewer-sample/e2e/a4-closeout.spec.ts`:
- Around line 143-175: Move the deterministic preflight loop currently executed
in beforeEach into a module-scoped memoized resolver or beforeAll-style cached
promise. Cache the resolved jobId and searchQuery for each fixed head/backend
pair, reuse that result across all tests and viewports, and preserve the
existing diagnostics and requireReal failure behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0c1abfc9-8545-4ca3-b9ff-bda84d8d4898
📒 Files selected for processing (26)
.github/workflows/agent-governance.ymlartifacts/e2e/isolated-branch-stack-browser-e2e/p1-consumer-20260730-030207201-c15f2e25/consumer-result.jsonartifacts/e2e/isolated-branch-stack-browser-e2e/p1-consumer-20260730-030207201-c15f2e25/deployment-listeners-after.jsonartifacts/e2e/isolated-branch-stack-browser-e2e/p1-consumer-20260730-030207201-c15f2e25/deployment-listeners-before.jsonartifacts/e2e/isolated-branch-stack-browser-e2e/p1-consumer-20260730-030207201-c15f2e25/deployment-listeners-during.jsonartifacts/e2e/isolated-branch-stack-browser-e2e/p1-consumer-20260730-030207201-c15f2e25/stack-manifest.jsondocs/agents/product-operability-and-script-contract.mddocs/plans/NOW.mddocs/superpowers/plans/2026-07-29-isolated-branch-stack-browser-e2e.mdopenspec/changes/isolated-branch-stack-browser-e2e/design.mdopenspec/changes/isolated-branch-stack-browser-e2e/proposal.mdopenspec/changes/isolated-branch-stack-browser-e2e/specs/isolated-branch-stack-verification/spec.mdopenspec/changes/isolated-branch-stack-browser-e2e/tasks.mdopenspec/lifecycle-ledger.jsonscripts/SCRIPT_CONTRACT.mdscripts/dev/start-isolated-branch-stack.ps1scripts/script-registry.jsonscripts/tests/test-agent-governance-check.ps1scripts/tests/test-isolated-branch-stack.ps1web-viewer-sample/e2e/a3-federated-session-chain.spec.tsweb-viewer-sample/e2e/a4-closeout.spec.tsweb-viewer-sample/e2e/support/isolated-stack-global-setup.tsweb-viewer-sample/e2e/support/isolated-stack.test.tsweb-viewer-sample/e2e/support/isolated-stack.tsweb-viewer-sample/playwright.config.tsweb-viewer-sample/vitest.config.ts
There was a problem hiding this comment.
Pull request overview
This PR adds a fail‑closed, repo‑owned harness for running an isolated governance/coordinator "branch stack" so that unmerged branches can produce CPU/browser operability evidence without touching the single test‑deployment ports. It introduces a PowerShell launcher with per‑run identity, offset/port validation, process‑lineage ownership gates, and an immutable per‑run manifest; a TypeScript support module that binds Playwright to the manifest and writes typed browser evidence atomically; and it rewrites the A3/A4 Playwright specs to require‑real mode. It also wires a new machine check into the agent-governance workflow and records the first A4 consumer run honestly as a pre‑browser known gap (no A4 browser completion is claimed). This slots into the repo's existing OpenSpec governance and design/runtime evidence contracts.
Changes:
- New launcher
scripts/dev/start-isolated-branch-stack.ps1(start|stop|status) with reserved‑port disjointness, offset0..4domain, ownership‑verified stop, and atomic manifest writes; registered in script registry +SCRIPT_CONTRACT.mdand covered bytest-isolated-branch-stack.ps1(now run inagent-governance.yml). - New
web-viewer-sample/e2e/support/isolated-stack.ts(+ tests + global setup) validating manifest identity/ports/head SHA and writing evidence manifests;playwright.config.ts/vitest.config.tsupdated to consume it, and A3/A4 specs converted to require‑real (no skip‑to‑green). - OpenSpec artifacts updated (proposal/design/spec/tasks/ledger/NOW.md, 21/31 tasks) plus committed consumer evidence recording the A4 downloaded‑job known gap.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
scripts/dev/start-isolated-branch-stack.ps1 |
New backend‑only launcher: port resolution, ownership gates, atomic manifest, rollback. |
scripts/tests/test-isolated-branch-stack.ps1 |
Machine test for launcher behavior, registry/doc/workflow wiring. |
scripts/tests/test-agent-governance-check.ps1 |
Asserts the workflow runs the new isolated‑stack test. |
scripts/script-registry.json / scripts/SCRIPT_CONTRACT.md |
Registers launcher as non‑canonical branch adapter. |
.github/workflows/agent-governance.yml |
Adds isolated‑stack machine test step. |
web-viewer-sample/e2e/support/isolated-stack.ts |
Manifest validation, reserved‑port guard, atomic evidence writer. |
web-viewer-sample/e2e/support/isolated-stack.test.ts |
Vitest coverage for the helper. |
web-viewer-sample/e2e/support/isolated-stack-global-setup.ts |
Pre‑spec coordinator/viewer probes. |
web-viewer-sample/playwright.config.ts / vitest.config.ts |
Manifest‑authoritative wiring; includes support tests. |
web-viewer-sample/e2e/a4-closeout.spec.ts / a3-federated-session-chain.spec.ts |
Rewritten to require‑real mode using the helper. |
openspec/**, docs/plans/NOW.md, docs/agents/product-operability-and-script-contract.md |
Contract/spec/tasks/ledger updates for the change. |
artifacts/e2e/.../*.json |
Committed consumer/manifest/listener evidence (known‑gap run). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 678abc1562
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed69a72d4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
scripts/dev/start-isolated-branch-stack.ps1 (2)
120-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnapproved verbs
Acquire-/Release-.PSScriptAnalyzer
PSUseApprovedVerbsflags both; approved equivalents areLock-/Unlock-orNew-/Remove-. Worth renaming now (with the test call sites) since the analyzer wasn't runnable in this PR.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/dev/start-isolated-branch-stack.ps1` around lines 120 - 149, Rename Acquire-IsolatedStackReservations and Release-IsolatedStackReservations to approved-verb equivalents, preferably Lock-IsolatedStackReservations and Unlock-IsolatedStackReservations, and update every corresponding test and call-site reference. Preserve the existing reservation creation and cleanup behavior.
104-111: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winEmbedded-quote escaping is wrong and untested. .NET replacement strings expand only
$-substitutions, so'$1$1\\"'emits two backslashes plus a bare";CommandLineToArgvWthen sees a literal backslash and a quote toggle, breaking the argument boundary. The gap survives because the only regression covers a spaced path with no quotes.
scripts/dev/start-isolated-branch-stack.ps1#L104-L111: change the replacement to$1$1\"so each inner quote is escaped with exactly one backslash.scripts/tests/test-isolated-branch-stack.ps1#L41-L46: add a case with an argument containing an embedded"(and a trailing backslash) and assert the exact expected command line.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/dev/start-isolated-branch-stack.ps1` around lines 104 - 111, The embedded-quote replacement in ConvertTo-IsolatedWindowsArgumentLine is over-escaping backslashes; update it to emit exactly one backslash before each inner quote. In scripts/dev/start-isolated-branch-stack.ps1 lines 104-111, make that replacement change. In scripts/tests/test-isolated-branch-stack.ps1 lines 41-46, add coverage for an argument containing an embedded quote and trailing backslash, asserting the exact generated command line.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@openspec/changes/isolated-branch-stack-browser-e2e/specs/isolated-branch-stack-verification/spec.md`:
- Around line 66-72: Update the wording in the partial-stop recovery scenario so
“listener時” becomes “listener 時,” preserving all surrounding behavior and
requirements unchanged.
In `@openspec/changes/isolated-branch-stack-browser-e2e/tasks.md`:
- Line 50: Update the completion status in tasks.md so it does not claim “Full
completion claimed” while tasks 2.5, 3.7, 4.3, 5.2, and 5.3 remain unresolved.
Mark the change as partial/known-gap, or complete or formally delegate every
outstanding task before retaining the full-completion claim; archive tasks.md
only after all tasks are checked or an accepted successor inherits them.
- Line 49: Update checklist item 6.4 to require recording the stable repository
verification gate, npm run verify, in the completion evidence. If that command
is intentionally not run, document an explicit exception alongside the existing
targeted checks, while preserving the current deploy dry-run, diff, secret scan,
and status requirements.
---
Nitpick comments:
In `@scripts/dev/start-isolated-branch-stack.ps1`:
- Around line 120-149: Rename Acquire-IsolatedStackReservations and
Release-IsolatedStackReservations to approved-verb equivalents, preferably
Lock-IsolatedStackReservations and Unlock-IsolatedStackReservations, and update
every corresponding test and call-site reference. Preserve the existing
reservation creation and cleanup behavior.
- Around line 104-111: The embedded-quote replacement in
ConvertTo-IsolatedWindowsArgumentLine is over-escaping backslashes; update it to
emit exactly one backslash before each inner quote. In
scripts/dev/start-isolated-branch-stack.ps1 lines 104-111, make that replacement
change. In scripts/tests/test-isolated-branch-stack.ps1 lines 41-46, add
coverage for an argument containing an embedded quote and trailing backslash,
asserting the exact generated command line.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 947dc9dd-2570-4ae3-890a-ff1a7d08fc0c
📒 Files selected for processing (19)
artifacts/e2e/isolated-branch-stack-browser-e2e/review-repair-b4d3c55/consumer-result.jsonartifacts/e2e/isolated-branch-stack-browser-e2e/review-repair-b4d3c55/deployment-listeners-after.jsonartifacts/e2e/isolated-branch-stack-browser-e2e/review-repair-b4d3c55/deployment-listeners-before.jsonartifacts/e2e/isolated-branch-stack-browser-e2e/review-repair-b4d3c55/deployment-listeners-during.jsonartifacts/e2e/isolated-branch-stack-browser-e2e/review-repair-b4d3c55/stack-manifest.jsondocs/plans/NOW.mddocs/superpowers/plans/2026-07-29-isolated-branch-stack-browser-e2e.mdopenspec/changes/isolated-branch-stack-browser-e2e/design.mdopenspec/changes/isolated-branch-stack-browser-e2e/specs/isolated-branch-stack-verification/spec.mdopenspec/changes/isolated-branch-stack-browser-e2e/tasks.mdopenspec/lifecycle-ledger.jsonscripts/dev/start-isolated-branch-stack.ps1scripts/tests/test-isolated-branch-stack.ps1web-viewer-sample/e2e/a3-federated-session-chain.spec.tsweb-viewer-sample/e2e/a4-closeout.spec.tsweb-viewer-sample/e2e/support/isolated-stack-global-setup.tsweb-viewer-sample/e2e/support/isolated-stack.test.tsweb-viewer-sample/e2e/support/isolated-stack.tsweb-viewer-sample/playwright.config.ts
💤 Files with no reviewable changes (1)
- web-viewer-sample/e2e/support/isolated-stack-global-setup.ts
🚧 Files skipped from review as they are similar to previous changes (8)
- docs/plans/NOW.md
- openspec/lifecycle-ledger.json
- web-viewer-sample/playwright.config.ts
- web-viewer-sample/e2e/support/isolated-stack.test.ts
- web-viewer-sample/e2e/a4-closeout.spec.ts
- docs/superpowers/plans/2026-07-29-isolated-branch-stack-browser-e2e.md
- web-viewer-sample/e2e/a3-federated-session-chain.spec.ts
- openspec/changes/isolated-branch-stack-browser-e2e/design.md
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aef24b2dea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c4c759a274
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d6ee9c78e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ed-branch-stack-browser-e2e
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63ba35c8ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2eeab7e2d4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cb27856c42
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9ed319ae81
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1fdb6bfc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ($Role -eq 'governance') { | ||
| return @{ |
There was a problem hiding this comment.
When a downloaded IFC-ready job is available, the A4 preflight still cannot reach governance: the child wrapper removes inherited A4_INTERNAL_CONTEXT_TOKEN, but neither environment returned by New-IsolatedBackendEnvironment supplies a replacement. forwardTrustedA4 therefore returns 503 a4_trusted_context_unavailable for every /for-ifc-ready/ search before contacting governance, so tasks 5.2/5.3 remain impossible even after the documented fixture gap is fixed; generate one run-local token and inject the same value into both backend environments.
AGENTS.md reference: AGENTS.md:L43-L43
Useful? React with 👍 / 👎.
| $rollbackResults = @(Stop-IsolatedBackends -Processes @($started) -IdentityLookup $IdentityLookup -ProcessHandleLookup $ProcessHandleLookup -StopProcessFn $StopProcessFn -MissingProcessFn $rollbackMissingProcessFn -AllowMissing) | ||
| $rollbackFailures = @($rollbackResults | Where-Object { $_.status -in @('not_owned','stop_failed') }) |
There was a problem hiding this comment.
Fresh evidence beyond the earlier normal-stop finding is that the startup catch path still treats Stop-IsolatedBackends returning stopped as a complete rollback without calling Wait-IsolatedBackendTermination. If health checking or manifest publication fails after a backend spawns, the default stop callback waits only for the wrapper process; a descendant listener can remain briefly or indefinitely, yet this path releases the reservations and writes no recovery manifest, leaving the isolated port occupied with no ownership-gated retry path.
Useful? React with 👍 / 👎.
| "scripts/tests/test-isolated-branch-stack.ps1" | ||
| ], | ||
| "subject_commit": "deb5af552022c3ee171e3174f59c9f1e3dfb5936", | ||
| "subject_commit": "72fcf5f85ee1460b42b5f57a0709634bbf816c02", |
There was a problem hiding this comment.
Fresh evidence specific to reviewed commit 8786735 is that the squash made this subject_commit non-ancestral: git merge-base --is-ancestor 72fcf5f 8786735 fails, as does the shared 6fc759d subject now assigned to the other ledger rows. collectSourceObservations explicitly rejects every non-ancestor subject with subject_not_ancestor, so the required full machine-truth verifier cannot validate this checkout even when those otherwise-unreachable objects happen to exist locally; update the ledger and NOW projection to snapshots that are ancestors of the squashed head.
AGENTS.md reference: openspec/AGENTS.md:L32-L32
Useful? React with 👍 / 👎.
Summary
:49103, coordinator:8005, and viewer:5180.origin/main8f2ff8b4cb840f1258cc36690f8a13f5f9783a9dat merge commitcdd1f2f0449bc0cb61546bd836d7218256d5b0e1.storage/root and requires realpath equality, regular files, and physical containment.Change ID:
isolated-branch-stack-browser-e2eAI Coding Governance
openspec/changes/isolated-branch-stack-browser-e2e/p5-20260730-163713failed closed 6/6 before navigation because no downloaded IFC-ready job existed; the new code subject was not followed by a real browser/runtime smoke and claims no success evidenceorigin/main8f2ff8b4cb840f1258cc36690f8a13f5f9783a9dc1fdb6bfc9ff58b3821e5e168b278fc4e49e9b500170c8dfb58b011e87f3d84252bafd21d7045afdcdd1f2f0449bc0cb61546bd836d7218256d5b0e172fcf5f85ee1460b42b5f57a0709634bbf816c02c1fdb6bfc9ff58b3821e5e168b278fc4e49e9b50(subject_commit=72fcf5f…,current_slice=482, tasks30/33)Frontend Verification
#semantic-search; prior require-real navigation was held by the missing downloaded-job preflighta4-refresh-sourcesanda4-run; not clicked because the prior real-fixture preflight failed before navigationstorage/with A3e2e-a3/arch.usdcandstr.usdcregular-file containment contract; no successful runtime fixture was available:8005:GET /health,GET /api/governance/search/llm-status, andGET /api/external/ifc-ready?limit=100E2E_REQUIRE_REAL=1with the manifest-owned Playwright A3/A4 projects; fresh real execution remains a known gapartifacts/e2e/isolated-branch-stack-browser-e2e/p5-20260730-163713/playwright-output/; no success screenshot PNG or trace is claimeddocs/plans/design-system-reference.manifest.jsonunchangedartifacts/e2e/design-system-visual-result.jsonfor headc1fdb6bfc9ff58b3821e5e168b278fc4e49e9b50; pending fresh CI*-actual.pngand*-diff.pngoutputs; pending fresh head and not substituted by old artifactsDeploy Path Verification
scripts/deploy.ps1 -DryRunpassed on final codescripts/dev/pwsh -NoProfile -NonInteractive -File scripts/deploy.ps1 -DryRun— passedpwsh -NoProfile -NonInteractive -File scripts/tests/test-isolated-branch-stack.ps1— passedValidation
Passed on the final code subject after the latest-main merge:
git diff --check.npm run typecheck.npm run verifyinweb-viewer-sample: 78 files / 965 tests, production build, and struct-log 23/23.current_slicelength 482.0170c8d; no full user-facing completion is claimed.OpenSpec State
Merge Boundary