[russellb]

An OpenShell observed-hook GitHub installation preview could release its Codex Harness and serve authenticated WebSocket requests while its revision remained incomplete. Native Codex returns HTTP 400 for ordinary HTTP requests after the held-launch endpoint closes; the readiness checks expected 404.

Both observeHeldRepositoryGitPreview and enrollHarness now require the recorded release acknowledgment and a successful authenticated WebSocket handshake. A receipt alone or a serving socket alone does not admit the Harness. Enrollment does not repeat an acknowledged release.

The regression uses a real loopback HTTP/WebSocket server and the production transport client, with controlled gateway metadata and release state. It covers both refusal cases and successful enrollment. The owning provisioning flow is updated.

Verification

Final source head: bed2f33b6adef58b58d7fdf501388086023882ef, merged with main at 6678e567282a09d2f0fa79046e7aae821c22ee20. The conflict resolution preserves the October 24 ServiceAccount and October 23 readiness changelog entries, along with the readiness paragraph.

• Focused regression: 1 passed, 0 failed, 0 skipped. It failed against the original readiness implementation before the fix.
• At the refreshed head, the installed ESLint, full-workspace Prettier check, TypeScript build with test types, workspace-boundary check, flow-document validator, and git diff --check passed. The checks used installed executables directly because pnpm tried to reconcile the stale dependency graph; no dependencies were installed or reconciled.
• Before the base refresh, fix commit 45023eaf3109f4c8dd2914cc4e09f00a8546b25f was exercised through the regular Agent workflow on disposable local Kubernetes with OpenShell 0.1.5 and a real model. The Agent became ready, completed a model turn, cloned a disposable repository, and pushed an empty commit. Independent GitHub ref readback confirmed the commit; durable audit recorded the receive-pack.

Outstanding validation

Local validation gaps are recorded below; hosted CI is still running:

• Before the October 26 merge refresh, the full startup integration file reported 111 passed, 4 failed, 1 skipped. The failures are two wrapper-refusal exit-status assertions and their parent tests. These assertion failures were also reproduced on unmodified main at 725e2371e before the base refresh. The skipped native-initializer runtime case requires OCC_TEST_RUNTIME_IMAGE.
• Documentation checks cannot start because the installed docs dependency graph lacks @sindresorhus/slugify. No dependencies were reconciled during verification. The provisioning flow remains one coherent workflow; its conservative whitespace count is 2,482 words, below the 2,500-word limit, and its dedicated validator passes.

The live result proves the explicitly selected request-hook-observed / github-installation preview. It does not qualify protected git-write or exact-dispatch/response custody.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
🔒 Security Review	✅ Completed 2026-10-23T19:03:09.522097Z	bbce40a	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[clawsweeper]

Codex review: needs maintainer review before merge.

What this changes

This PR verifies released OpenShell preview readiness through the existing authenticated WebSocket client instead of requiring HTTP 404.

Example: An acknowledged Codex preview on OpenShell 0.1.5 returns HTTP 400 for ordinary requests and accepts its authenticated WebSocket handshake.

• Before: The revision remains incomplete because both readiness paths require HTTP 404.
• After: Both paths recognize the released Harness without sending another release.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	Focused, source-supported repair with useful transport-backed coverage and passing head checks; directly inspectable shipped-workflow evidence remains limited.
Proof confidence	🦐 gold shrimp (3/6)	This maintainer-authored PR is outside the ordinary contributor proof gate; its real-transport regression supports the changed readiness conditions, while the reported Kubernetes model-turn and Git-push run is narrative evidence without attached live output.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A released, serving Codex preview remains incomplete because ordinary HTTP requests return 400.
Reason: The maintainer-authored repair restores the intended preview lifecycle through existing owners without adding configuration, public APIs, or broader sender guarantees.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: current main still rejects a released preview that returns HTTP 400, and the focused fix preserves acknowledgment and authentication checks without introducing an actionable defect.

Priority: P2
Reviewed head: bbce40a53afd6910f0f1b0409b9a0b76fd86d41c

Before merge

None.

Findings

None.

Tests

• Missing end-to-end proof: The body reports a Kubernetes Agent workflow at the earlier fix commit, but supplies no linked live output establishing that workflow at the final head.

Agent review details

How this fits together

Kubernetes Compute uses the OpenShell Sandbox Driver to observe and enroll a dedicated Harness, combining its revision-bound release acknowledgment with service observations before completing preparation.

flowchart TB
  Compute[Kubernetes Compute] --> Driver[OpenShell Sandbox Driver]
  Receipt[Recorded release acknowledgment] --> Driver
  Driver --> Client[Existing authenticated transport]
  Client --> Harness[Codex Harness]
  Harness --> Client
  Driver --> Readiness[Preview readiness]

Loading

Technical review

Best possible solution:

Use the existing authenticated WebSocket check alongside the recorded release acknowledgment in both preview readiness paths.

Do we have a high-confidence way to reproduce the issue?

Current-main source deterministically rejects HTTP 400 despite an acknowledged release; the PR reports a failing-before/passing-after regression, but no target code was run during this review.

Is this the best way to solve the issue?

The repair is appropriately scoped: it reuses the same authenticated readiness mechanism already used by harnessStatus and retains the release acknowledgment rather than treating an HTTP status as readiness.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 98785b43abf9.

Provenance checked

• Preview observation and enrollment keeps the original intent (3115679: The original implementation required a recorded authenticated release acknowledgment so disappearance or refusal of the launch endpoint could not authorize release.)
• Provisioning flow overview and readiness documentation keeps the original intent (docs: clarify Agent revisions, versions, and limits #2958: The documentation cleanup preserved support limits while clarifying deployment terminology and staying within the length budget; the current edit retains the removed overview boundaries in the execution trace and adds the changed readiness behavior.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

• Introduced change: The pinned introduction changes only the two readiness conditions, their provisioning flow, and one transport-backed regression test; the verified test merge contains the same three paths. (apps/controller/src/drivers/sandbox/openshell.ts:3338, bbce40a53afd)
• Current-main failure remains: Current main requires HTTP 404 together with previewReleased in both observation and enrollment, so HTTP 400 cannot complete either path. (apps/controller/src/drivers/sandbox/openshell.ts:3338, 98785b43abf9)
• Existing transport and release boundaries: The reused client sends the bearer token and validates the WebSocket accept key; previewReleased rejects blocked entries, and enrollment retains exact plan and Sandbox checks. (apps/controller/src/drivers/sandbox/openshell-service-transport.ts:267, bbce40a53afd)
• Meaningful regression coverage: The added case uses real loopback HTTP/WebSocket connections through the production client, rejects a socket without acknowledgment and acknowledgment without a successful handshake, then verifies both successful paths without another release. (tests/integration/sandbox-driver-startup.test.mjs:4922, bbce40a53afd)
• Original intent and profile scope: Credential Proxy Implementation #2487 established acknowledgment-based release safety; fix(openshell): refuse git-write delivery the bundled sender cannot enforce #3236 explicitly preserved the separate v6 observed-hook preview while refusing unqualified protected git-write. (apps/controller/src/drivers/sandbox/openshell.ts:3313, 31156790238d)
• Checks on reviewed head: CI Required and Image Runtime Startup 2 passed for the reviewed head; the latter includes the existing startup integration file. Downloading its detailed log was blocked by the artifact host allowlist, so no log-level result is claimed. (scripts/ci/test-suites/images-runtime-startup-2.json:12, bbce40a53afd)

Review metrics

Metric	Value	Why it matters
Focused implementation and regression coverage	Production +14/-4 lines; tests +115/-0 lines	The production change reuses an existing transport; the larger test covers two independent refusal conditions and successful observation and enrollment.

Labels

Label changes:

• add P2: A specifically selected OpenShell observed-hook preview can remain incomplete despite a serving Harness; the affected scope is limited.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

• P2: A specifically selected OpenShell observed-hook preview can remain incomplete despite a serving Harness; the affected scope is limited.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 23, 2026, 3:15 PM ET / 19:15 UTC.

[freeqaz]

Free's clanker here (an AI agent working on Free's behalf). I reviewed this against current main (98785b43a; head bbce40a53, merges cleanly).

Verdict: correct and safe to land as written. No blocking issues. One pre-existing gap next to this change is worth a follow-up, plus three nits.

What I checked

• Admission still needs both the ACK and authentication. Both paths gate on previewReleased(revision.id) first, and that is in-memory state that only releasePreview sets after the launcher's own {enrolled, startupId} ACK. The handshake then has to return 101 with the matching Sec-WebSocket-Accept for the transport bearer. A receipt alone, or a serving socket alone, leaves the Harness pending/refused. Unreleased previews skip the handshake entirely (&& short-circuits). The old 404 && released shortcut is gone, so this is strictly tighter than main.
• No repeated release. In-process, after the ACK the launcher answers 410 (then Codex answers 400), so enrollHarness takes the non-200 branch and returns before releasePreview. releasePreview also refuses when preview.release is already set. Across a controller restart, preview routes are never adopted (openshell-git-runtime.ts blocks them as worker_replaced), so the revision fails closed with REPOSITORY_ROUTE_BLOCKED and never re-releases. That is unchanged by this PR.
• An honest serving Harness does not get stuck. If Codex is not listening yet right after the ACK, observeHeldRepositoryGitPreview returns undefined, enroll returns false, and the next pass retries. Transport failures keep the existing OpenShellServiceTransportError class that the GET already threw on these paths. The error text reuses the existing Cannot verify the observed-hook launcher's release.
• Test. The new test passes at the head. With main's openshell.ts swapped in, it fails (undefined vs 'installed'). I ran four mutants (drop the ACK check or the handshake, in each of observeHeldRepositoryGitPreview and enrollHarness) and the test kills all four. CI Required and First Agent Smoke are green on the head.
• Overlap. Merging this does not add any new conflict in its three files with the other open PRs that touch them (Start the API before enabling workers on an existing installation #3253, perf(openshell): end the Sandbox startup wait once a new Codex Harness answers #3054, feat(occdev): wire the OpenShell Git route into occ dev up with an installed acceptance #3045, fix(openshell): fail Pod-label refusals permanently and check the allowlist #2643, feat(runtime): apply Gateway tool policy to dedicated OCE deployments #2633, Propose execution-scoped Strands history policy above Credential Proxy #2533, Add OCE policy simulation and tester for network and plugin actions #2445, fix(test): close the openshell gateway forward before the outage proof stops the gateway #3346). Their existing conflicts with main are unchanged.

Follow-up (pre-existing, not introduced here)

• openshell.ts:3338 / :3407: a released preview whose Harness then holds a startup failure never reports it. The launcher runs before login and the first model probe (harness-egress-enrollment.ts:3). If the probe fails after the release, the held-failure server answers both the enrollment GET and the upgrade with 503 (runtime-entrypoints.ts:198). So observeHeldRepositoryGitPreview returns undefined and enroll returns false. incomplete() then reads no failure either, because privateStatusReadback skips provider-fenced OpenShell Harnesses (kubernetes/index.ts:10435), and harnessStatus (which does read the held failure) is never reached. The revision waits until its readiness deadline and fails without the held code (for example MODEL_PROBE_FAILED). Main does the same today (503 !== 404). Possible fix: when previewReleased is true and the handshake fails, read the runtime status path (or reuse harnessStatus) and let a held runtimeFailure through to the normal status read. That could be done here or in a separate PR.

Nits

1. openshell.ts:3644 says the Driver handshakes first "so a serving app-server never receives a plain request". After a release, both preview paths now send a plain GET to the serving Codex on every pass before they handshake. When previewReleased is already true, consider handshaking first and skipping the GET.
2. In one pass, enroll handshakes in observeHeldRepositoryGitPreview and again in enrollHarness. If only the second one fails (a Codex blip in between), the pass throws a configuration failure instead of staying pending. This is low-probability, and a pending result would be a little gentler.
3. Docs: the new sentence says "even when HTTP returns 400", but the code accepts any non-200 answer, so "any non-200 answer (native Codex returns 400)" would match it. The Overview trim also drops facts that don't appear elsewhere in this flow ("without Agent Secret permission", the development workspace-node scoped token). Unless they are covered in another doc, consider keeping one short clause.

This is harness-admission logic in the Sandbox driver, so I'm leaving the merge to a maintainer.

@freeqaz-openai flagging this one for your review.

[freeqaz-openai]

GitHub now reports a conflict in this branch. When updating it against main, please keep both changelog entries in docs/flows/openshell-sandbox-provisioning.md (the October 24 ServiceAccount entry and the October 23 readiness entry), along with the readiness paragraph.

The loopback regression also caught the opposite failure from the HTTP 400 case. With an acknowledged release on main at 893ff2a, the unpatched driver reported installed after an HTTP 404 even though the authenticated WebSocket handshake was refused. With this PR’s readiness change applied to that same revision, the 404 case stayed pending and enrollment was refused. The HTTP 400 case with an accepted handshake reported installed and enrolled successfully.

[russellb]

@freeqaz-openai ready for another review