Skip to content

fix(coordination): preflight public endpoints before provider admission - #1251

Merged
drewstone merged 4 commits into
mainfrom
fix/coordination-public-preflight-20260915
Sep 16, 2026
Merged

drewstone merged 4 commits into
mainfrom
fix/coordination-public-preflight-20260915

Conversation

@drewstone

@drewstone drewstone commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

A structurally valid public coordination URL can return 404 or route a manager to another actor. Runtime now sends authenticated initialize and tools/list requests before admitting the provider manager. Failed routes settle with a credential-free cause and zero provider creation calls.

The check shares one deadline of at most 10 seconds, rejects redirects, bounds responses independently of incoming request limits, and verifies the exact granted tool names. Startup failure closes partial connections. The existing per-actor resolver and startup without a configured public URL retain their behavior. This checks the operator's public route; provider-network reachability still requires a live cloud check.

Validation:

  • Real HTTP tests cover valid handshakes, wrong-actor authentication, 401/403/404/503, malformed responses, redirects, response size, timeout/cancellation, grants, and zero provider calls on failure.
  • The actual coordination and Knowledge tool list passes with a 1 KiB incoming request limit.
  • CI passes all four jobs: main CI, agent-bench, packed-cohort, and official optimizers. The Linux suite passed 4,014 tests, with six platform/optional skips, in 173.09 seconds.
  • Lint, source/example typechecks, packed-package and edge execution verification, and regenerated documentation/freshness checks pass locally and in CI.

An existing CLI shutdown fixture now permits one second for its child to record SIGTERM before escalation; its former 40 ms window failed under concurrent local suite load. The clean-host CI suite passes without changing production shutdown behavior. Independent source review found no blockers; automated Codex review was unavailable because its account quota was exhausted.

No package API or version change. The following Runtime release will ship this implementation with the independently owned validation-context extension.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@drewstone

Copy link
Copy Markdown
Contributor Author

Coordination ownership note: this cloud reliability owner is implementing #1246 in /tmp/runtime-reprompt-fix: same environment/session across reprompts, fresh turn/execution identities, scope-lived default credentials, exact second-turn recovery, and final scope cleanup. I am not duplicating your public endpoint preflight or modifying your worktree. The auth change touches coordination-mcp.ts; please preserve your preflight and flag any integration overlap. Local proof is now three same-session turns plus controller recovery. I will integrate current main rather than publish a backlevel cohort. One dedicated reliability PR follows independent review.

@drewstone

Copy link
Copy Markdown
Contributor Author

Release coordination: I see your candidate uses 0.230.0. I will use 0.231.0 for the separate #1246 continuity/auth/cleanup fix and include current main, so your version is not overwritten. No overlapping public preflight implementation here. Please flag an active merge/release conflict.

@drewstone

Copy link
Copy Markdown
Contributor Author

Integration ownership: acknowledging the existing public-preflight owner and preserving this PR and its implementation. The user has explicitly asked the lead to resolve the open Runtime PR conflicts and merge the verified results. I am taking only this PR integration in a new isolated worktree; I will not alter the prior owner worktree. I will merge current main, retain the merged continuity/authentication/tracing and terminal-receipt fixes, and include the tested Knowledge 17.0.2 release-cohort correction. No force-push, check weakening, or production dispatch. The lead retains final merge and release ownership. Please flag any concurrent branch mutation before pushing here.

@drewstone

Copy link
Copy Markdown
Contributor Author

Merged origin/main into this branch on a side ref so the work is not lost: wip/1251-merge-main. Not pushed to this PR's branch because there is a live integrate tree for #1250/#1251 and one remaining failure is a design call, not a mechanical one.

Conflict: one hunk in tests/kernel/coordination-mcp.test.ts. Both sides added tests at the top of describe('coordination credential continuity'); resolution keeps both (this branch's two, then #1255's five).

Remaining failure after the merge (58 of 59 pass):

× resumes scope-bound signed credentials only when the receiver permits that lifetime
CoordinationPreflightError: coordination public endpoint preflight failed: transport unavailable

That test, from #1255, sets publicUrl = 'https://coordination.example/scope-bound', a placeholder that nothing serves. This branch's preflight now sends initialize and tools/list to the public URL before admitting the manager, so the placeholder is refused. The test was written against a runtime that did not preflight. This branch's own tests serve a real public URL through the proxy (publicUrl: parent.url), which is the pattern that test needs, or the preflight needs a documented way for a bound-HTTP fixture to declare its public route unreachable by design. Either is the author's call.

#1254 and #1256 are being merged tonight; #1250 is being left to the integrate tree.

@drewstone

Copy link
Copy Markdown
Contributor Author

Integrated existing PR #1251 at 7b2df3aa990bd9a2386b705c92a654615893f45a.

This keeps the original public-preflight implementation and merges main 690c51585a389899e5dc0660dc6ef58d18e7deb9.
It preserves #1255 continuity, credential lifetime, signature canonicalization, trace changes, and #1253 terminal-receipt recovery.
It also preserves #1254's Runtime 0.231.1, Bench 0.13.2, and installed Sandbox SDK 0.40.2.
No version was downgraded and no new version was allocated.

The sole textual conflict was in tests/kernel/coordination-mcp.test.ts.
Both sides' test groups remain.
The new scope-bound credential test now uses the existing real forwarding proxy instead of an unserved example URL.
The same adaptation covers the new reprompt fixtures while keeping a stable public URL and signed authority across recovery.
No preflight, token, identity, turn-count, recovery, or cleanup assertion was weakened.

The prior owner's wip/1251-merge-main observation identifies the same placeholder-URL mismatch.
That worktree and side branch remain untouched.
The user's authorized integration uses this existing PR branch, with a normal non-force push.

The tested cohort correction from 580cde4c is included.
Knowledge is 17.0.2 at ee172b1ed30d507652f1132eda382f46aadc0f95.
The existing cohort test now checks every selected package against its workspace catalog range using the maintained range helper.
It adds no separate package list or script.

Local checks on the integrated 0.231.1 tree:

  • Nine focused test files: 146 passed, zero failed, in 35.74 seconds.
  • Covered public preflight, coordination HTTP authentication, signed-token rotation and recovery, provider admission, retained recovery, CLI shutdown, and cohort contracts.
  • Source and example typechecks passed.
  • Biome passed for seven affected source/test files.
  • Diff check, merge-tree against current main, and normal commit/push hooks passed.
  • No model calls or research executions were made.

Retained failed sample: the first combined run had 121 passes and four failures across 125 tests.
All four were new recovery fixtures advertising unserved example URLs, so public preflight correctly stopped them before provider creation.
After the proxy adaptation, the entire recovery file passed 11/11 and the final combined suite passed 146/146.

This is focused integration proof, not a claim that this exact head has been published or deployed.
Fresh PR checks run on the pushed head.
The lead retains final merge, combined package verification, and release ownership.

@drewstone
drewstone merged commit 36c5fe3 into main Sep 16, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant