feat(app-server): Codex-compatible app-server protocol surface - #3782
feat(app-server): Codex-compatible app-server protocol surface#3782Yeachan-Heo wants to merge 139 commits into
Conversation
…cycle Audience-scoped replay previously had two defects that this change fixes. 1. Unscoped replay regression. When any audience-tagged event was present in the replay window, an `event_replay` request without a `requesterRef` was rejected outright. That broke existing unscoped consumers (chat daemon runtime, Telegram daemon), which legitimately replay public events. An unscoped replay now filters to non-audience-scoped events instead of failing, so legacy callers keep working while private frames stay hidden. 2. Dropped rejection frames. Rejections (`audience_selector_required`, `audience_forbidden`) were emitted without `generation`/`lastSeq`. The native transport validator (crates/gjc-sdk/src/server.rs) drops any `event_replay_result` lacking numeric cursor fields, so these rejections never reached the client and surfaced as a client-side hang. Rejections now echo the caller's own cursor, which satisfies the validator and cannot advance a client past scoped events. Replay-token lifecycle is also hardened: - token binding fails closed with a typed error instead of returning silently - expired pending claims are purged before every eviction decision, so a stale claim can no longer block a legitimate token-bearing reconnect - a lost acknowledgement is recovered by replaying the cached response and promoting its token, rather than redispatching the non-idempotent prompt - replay-token state is bounded with oldest-first eviction that never evicts an audience with an in-flight claim Observer isolation is preserved: an observer still cannot obtain another requester's correlated message_update or lifecycle frames.
Vendors the codex app-server protocol surface at pinned upstream commit 81da9deb065d7adb283816b19b40f89bcc484276 and adds an offline gate that detects drift from it. protocol-source/vendor/ holds the checksummed snapshot: the method bundle with per-method params/result field shapes, the behavior fixture (serialized golden error envelopes, the exact AppServerWebsocketAuthArgs CLI definition, wire-protocol policy), and meta.json recording the upstream commit, per-file source SHA-256 provenance, direction counts, and GJC-only overrides. Generated artifacts derive from that vendor snapshot only: per-method interfaces, required-key validators, the four directional catalogs, and the support manifest with per-method evidence columns. Authority level is honestly labeled `method-and-field-shapes`, not full JSON Schema parity: top-level serde-aware fields are captured (Option<T> optionality, rename/rename_all, skip_serializing_if), while unresolved nested type references are enumerated explicitly rather than silently flattened. scripts/sync-codex-app-server-schema.ts refreshes the snapshot from the pinned commit and fails closed on fetch or parser drift. scripts/verify-codex-app-server-parity.ts is a read-only gate that checks provenance, checksums, generated-artifact parity, direction counts (130/1/11/72), manifest evidence columns, and the JSON-RPC error-envelope contract, with --self-test covering shape-drift negatives. Verified: parity gate PASS; sync tests 9 pass / 0 fail.
…sten) Neutral transport layer for the codex-compatible wire protocol. All modules derive their contracts from the vendored protocol authority rather than hand-written constants. framing.ts implements the JSONL codec. The `jsonrpc` header is omitted on the wire per the pinned contract, and three failure branches are kept strictly distinct: stdio oversize discards to newline and emits a single GJC-only -32600 with `id: null` and the `data` key absent; malformed non-oversize JSON is dropped and logged with no wire response; ws/unix oversize is left to the transport to close with 1009. The frame cap is a documented GJC operational default with a --max-frame-bytes override. errors.ts builds typed error envelopes. Standard codes resolve from the vendored golden envelopes; the `data` key is always absent (never null) since upstream marks it skip_serializing_if, and `id` is echoed verbatim as string|integer except for the undecodable stdio-oversize case. GJC-only extension codes are pinned separately and recorded in meta.json so the parity gate can audit them apart from upstream goldens. Internal causes are retained for logs and never cross the wire. auth.ts mirrors the pinned AppServerWebsocketAuthArgs exactly: capability-token and signed-bearer-token modes, --ws-token-file XOR --ws-token-sha256, a file-only shared secret, and iss/aud/clock-skew validation. Tokens are accepted only from the Authorization header during the HTTP upgrade; there is no query-parameter path. Malformed JWT payloads return 401 rather than throwing. connection.ts provides the bounded outbound queue with a slow-client policy. http-probes.ts serves /readyz and /healthz and returns 403 for any request carrying an Origin header. listen.ts parses stdio/ws/unix/off listen URLs and classifies loopback binds. Verified: 54 pass / 0 fail.
…patch connection-state.ts tracks the per-connection handshake. A single `initialize` request must precede every other method; anything earlier is refused with "Not initialized" and a repeat `initialize` with "Already initialized" (both -32600 per the vendored goldens). Capabilities captured at initialize include the `optOutNotificationMethods` allowlist, matched by exact method name with no wildcard or prefix semantics (unknown names are accepted and ignored), and the `experimentalApi` flag. Connection state instances are independent, so no handshake or capability state leaks between connections. dispatch.ts routes inbound frames using the four generated catalogs as the only source of method identity. Frames are classified by direction: id-bearing known methods are client requests, id-less frames are notifications, and id-bearing unknown methods resolve to method-not-found. Notifications are always consumed without a response, since JSON-RPC forbids answering them — including unknown ones. Requests pass three gates in order: the handshake gate, an experimental capability gate (an experimental method on a non-experimental connection is not-supported rather than unknown), and the support-manifest gate that returns -32081 for backend-less methods. shouldEmitNotification applies the same exact-match opt-out and experimental gating to outbound notifications. Verified: 21 pass / 0 fail.
thread-runtime-manager.ts owns multi-thread admission and ownership. Admission is bounded by a global maxLoadedThreads cap, a per-connection load limit, and a spawn semaphore exposed as explicit acquire/release tokens so it can bound the real asynchronous child spawn rather than a synchronous call window. Capacity pressure first evicts idle owned children oldest-first, and never evicts a thread with an active turn or pending approvals; genuine exhaustion returns a typed conflict. Ownership is enforced rather than advisory: `spawned` runtimes must be terminated and `attached` runtimes must be detached, and calling the wrong one throws. Termination and eviction invoke a close callback carrying the captured endpoint authority tuple so the caller can fence a session close against a recycled endpoint. Per-connection load counts are released on detach, terminate, and eviction, and duplicate thread ids are rejected. child-bridge.ts is the production callsite: it acquires a spawn token, performs the async spawn, registers the thread, and releases the token in a finally block on success or failure. app-server-projection.ts stores versioned projection envelopes through the existing SessionManager rather than introducing a competing thread store, keeping the owning session as the single writer. endpoint-authority.ts holds the one shared endpoint-incarnation derivation used by both broker and lifecycle paths, replacing duplicated hashing. Verified: 28 pass / 0 fail.
subscriptions/index.ts maintains the thread/connection subscription topology with paired forward and reverse indexes so both fan-out and disconnect cleanup are direct lookups. Subscribe is idempotent, unsubscribe clears the reverse entry (dropping the connection key once it holds no threads), and a disconnect removes every subscription for that connection while leaving other subscribers intact. Empty thread sets are pruned so the map does not accumulate dead keys. ConnectionRegistry tracks active connections and their exact-match notification opt-outs. server-requests/broker.ts brokers server-to-client requests such as approvals. Each request snapshots its eligible connection set, and resolution is first-responder: only an eligible connection can resolve, and a second response to an already-resolved request is refused. Losing one eligible connection keeps the request pending; losing the last one cancels it. Requests are also cancellable per thread for turn and thread transitions, and expired requests are swept by timeout. Verified: 24 pass / 0 fail.
suites/handlers.ts introduces the handler registry and the first backable handlers. The fs/* family (readFile, writeFile, getMetadata, readDirectory, createDirectory, remove) is implemented end to end with typed invalid-params and not-found results. config/read and model/list are translate-semantics handlers: they emit the codex response shapes taken from the vendored bundle (ConfigReadResponse's config/origins with codexHome mapped to the gjc agent dir, ModelListResponse's data/nextCursor) rather than inventing a GJC-shaped payload. skills/list, hooks/list, and experimentalFeature/list are wired as empty-catalog handlers pending their backing integrations. server.ts is the request pipeline joining the layers: decode through the framing codec, route through the handshake and direction-split gates, dispatch to the handler registry, and serialize responses through the golden error envelopes. `initialize` performs the handshake and `initialized` completes it. Thread lifecycle methods exercise ThreadRuntimeManager admission, mapping thread/start to a spawned runtime with a server-generated id and thread/resume to an attached runtime. create-app-server.ts is the composition root: it constructs the connection state, thread runtime manager, and a registry pre-populated with the built-in handlers, so callers get a wired server rather than assembling parts. Scope: methods without a wired handler return -32081. turn/start and agent execution are not implemented in this commit, so the server is not yet usable by a codex-compatible client. Verified: 29 pass / 0 fail.
cli/runtime.ts resolves the app-server CLI arguments into a listen mode plus server configuration, validating --max-frame-bytes and --max-loaded-threads as positive integers and defaulting to stdio. runStdioServer builds a wired server via createAppServer and pumps JSONL frames between stdin and stdout. generateTs and generateJsonSchema emit the generated artifacts and the vendored schema bundle to an output directory, failing loudly when a source artifact is missing instead of silently skipping it. commands/app-server.ts is the oclif command mirroring the codex entrypoints: default and --stdio, --listen with stdio/ws/unix/off, the generate-ts and generate-json-schema subcommands (reading --out), and the pinned --ws-* auth flags. The ws:// and unix:// modes bind a Bun.serve WebSocket listener that serves /readyz and /healthz, rejects any request carrying an Origin header with 403, and gives each accepted connection its own server instance. The off mode starts without a transport or probes and idles until a signal. Known gap: this command is NOT yet registered in the CLI command table, so `gjc app-server` does not resolve and falls through to global help. Wiring the registration, and adding verify:codex-app-server-parity to ci:check:full, are follow-ups. Verified: 13 pass / 0 fail.
…xture r2-spike.test.ts and r2-broker-integration.test.ts exercise the seams this work depends on: broker authority-tuple validation (a captured tuple is accepted while mutated generation/incarnation and a stale close are refused), projection append/read with ordinal-cursor and idempotency semantics, reverse permission-lease install through a real SessionSdkHost round trip, and audience-scoped replay sequencing. Three cases are explicit named skips rather than silent gaps: two await the P2 ThreadRuntimeManager attach/detach and destructive-close seams, and one records that real broker child-spawn is blocked in this sandbox (spawn_failed, then terminal_uncertain from lifecycle post-child persistence verification, which is a pre-existing defect worth separate triage). fixtures/stub-model-provider.ts is the offline provider used to keep these tests network-free. Verified: 6 pass / 3 named skips / 0 fail.
commands/sdk.ts gains an opt-in hook that registers a model provider from a module path supplied via GJC_TEST_MODEL_PROVIDER, so SDK sessions can run against a deterministic offline provider instead of a live upstream. The hook is gated: it only activates when GJC_TEST_MODEL_PROVIDER_AUTHORITY is explicitly set, so an inherited environment variable alone cannot swap the provider. It runs before startup model profiles are applied, so the registered provider is visible to profile selection. An authorized module that fails to load is a terminal startup failure rather than a silent fallback to a live provider. Registered models are deep-copied so the registry cannot mutate the provider module's exported catalog. Verified: 4 pass / 0 fail.
Adds projection.append and projection.read to the SDK control surface so the app-server can persist and read projection records through the owning session. operation-registry.ts registers both as C53/C54 with their typed error sets (invalid_input, projection_corrupt, and idempotency_conflict on append only, since a read cannot produce one). Both are marked prohibited on the chat transports (Telegram/Discord/Slack), which have no legitimate projection surface. control/operations.ts declares them on ControlSurface and control/dispatch.ts routes them with cursor and envelope validation. session-manager.ts reserves the gjc.app-server.projection custom-entry type: generic extension callers are refused, and only the trusted append path may write it, keeping the owning child session the single writer. Generated inventory and the adapter parity manifest are regenerated from the registry rather than hand-edited. Known pre-existing failures: the three AD-*-C54 "projection.read forwarded" adapter rows fail with an internal error. Verified pre-existing by bisect — they fail identically with these changes stashed — so they are not a regression from this commit, but they do need triage before the read path can be considered adapter-complete.
… hooks broker.ts and lifecycle.ts now return the endpoint authority tuple (endpointGeneration, endpointIncarnation, endpointMtimeMs, pid) from session.create, session.resume, and session.get_endpoint, and both derive the incarnation through the shared endpoint-authority helper instead of duplicating the serialization and hash. Callers can capture the tuple at load time and pass it back on close so an owned shutdown is fenced against a recycled endpoint; stale generation or incarnation is refused. runtime-init.ts and extension-ui-controller.ts wire the projection control handlers into the runtime surface. package.json adds the verify:codex-app-server-parity script, and run-test-manifest.ts picks up the new suites. Known pre-existing failure: sdk-broker "preserves typed verified-delete partial-cleanup evidence" fails here and on an unmodified tree (verified by stash bisect), so it is unrelated to this change.
…enforced anchor
Vendors upstream's committed generated schema trees at pin
81da9deb065d7adb283816b19b40f89bcc484276 and adds the experimental profile
produced by upstream's own exporter.
- stable/{json,typescript}: 275 + 622 files, copied from upstream's committed
schema/ tree
- experimental/{json,typescript}: 349 + 701 files, generated by running
`cargo run -p codex-app-server-protocol --bin export -- --experimental`
at the same pin
The trust anchor is a byte comparison, not an assertion: running that same
exporter with NO --experimental flag reproduces the committed stable trees
exactly (275/275 json, 622/622 typescript, zero differences), which is what
transfers upstream provenance to the experimental output from the same binary.
vendored-schema-provenance.ts holds the expected subtree OIDs as literal code
constants and recomputes them from the vendored bytes using git's own tree-hash
algorithm. provenance.json is a record, never the authority: replacing every
OID in it with a dummy value does not rescue a mutated tree.
An honest asymmetry is documented in the source rather than glossed: the stable
OIDs are anchored to upstream's own git objects and therefore prove upstream
provenance, while the experimental OIDs are computed over locally generated
bytes and prove only tamper-detection after vendoring. Git tree OIDs also do
not capture non-executable permission changes; that limit is stated where the
verifier could otherwise be read as covering it.
Verified: mutating one byte in a vendored file is detected and named; both
profiles verify clean; 11 pass / 0 fail.
…dored schema
Replaces key-presence checking with real Ajv Draft-07 validators compiled from
the vendored JSON Schema. The previous validators.generated.ts asserted only
that required keys existed, so `{thread:null, model:"", cwd:0}` validated
successfully and response-shape correctness was unfalsifiable.
Six direction maps per profile, because the same method name means different
things by direction: clientRequestParams, clientRequestResults,
clientNotificationParams, serverRequestParams, serverRequestResults,
serverNotificationParams. 271 validators for stable, 347 for experimental.
Rust integer formats are normalized to bounded ranges before compilation
(uint8 0-255, int32 +/-2147483648, 64-bit widths to JS safe-integer bounds)
and the format key is dropped, so those are enforced structurally.
The JSON-data guard is deliberately strict, because Ajv alone accepts values
JSON.parse can never produce. It rejects non-finite numbers, sparse array
holes, non-plain prototypes, and symbol-keyed or non-enumerable own
properties, while a JSON.parse(JSON.stringify(x)) round trip still validates.
Ajv 8.17.1 is pinned. Standalone codegen was rejected because twelve
self-contained envelope compilations produced a 162 MB module; the reason is
recorded in the generated header rather than left implicit. The generator
formats its output through the repo's biome so regeneration is byte-identical,
and a guard test enforces that.
Verified: the payload the old validators accepted is now rejected; zero of the
271 stable validators are no-ops; 7 pass / 0 fail.
…manifest Adds the machinery that decides "done" from evidence the verifier produces itself, rather than from evidence the agent authored. obligations.manifest.json declares five required gates (oracle-stable, oracle-experimental, spawned-cli-blackbox, trace-replay, real-t3), each marked required and non-supersedable with a receipt contract naming the exact argv it must have run. obligations.digest freezes the canonicalized manifest so an isolated edit is detectable. The verifier does not trust a receipt's own claims. It re-executes the gate's contracted argv with the interpreter resolved through fs.realpath(process.execPath) and a fixed minimal PATH, then compares the live exit code and output against the recorded artifact. A receipt asserting a command it never ran cannot pass. Receipts are bound to a repository snapshot taken immediately before AND after re-execution, so a stale pass cannot be replayed and a gate command that mutates tracked state fails rather than certifying itself. Git is invoked at an absolute path with GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE and friends stripped, because the snapshot is the evidence everything else rests on: a shadowed git or a hostile git env would otherwise let the verifier measure the wrong tree. All five gates are BLOCKED today and the verifier exits 1. That is the honest state, not a defect: their contracted commands belong to later stages. The machinery ships here; the passes do not. Compiled artifacts emit an explicit VERIFIER UNAVAILABLE rather than silently skipping re-execution. Residual gap stated in the manifest: a coordinated edit of both the manifest and its digest is not detected. Verified: fabricated receipt rejected on output mismatch; argv injection rejected; shadowed git and hostile GIT_DIR both inert; 30 pass / 0 fail.
Before this, createAppServer() was called inside the WebSocket open(ws)
handler, so every connection got its own server. Cross-connection thread
subscriptions and first-responder approvals were therefore structurally
impossible, and BoundedOutboundQueue, ThreadSubscriptionIndex and
ServerRequestBroker were assembled nowhere in production.
AppServerRuntime now owns what is genuinely shared (thread and session
ownership, the subscription index, the approval broker, the support registry)
and is constructed once per process. AppServerConnection owns what is
per-peer: connection id, handshake and negotiated capabilities, an ordered
inbound chain, its own bounded outbound queue, and close cleanup.
processInbound is async and handlers receive a typed context with respond,
emitTo, broadcastThread and requestClient.
A request's response is enqueued before any notification produced while
handling it, via an explicit publication barrier rather than incidental
ordering. Inbound {id, result|error} frames route to the broker so a client
can answer a server request.
Queue semantics are honest about failure: a writer rejection reaches the
caller that enqueued the frame instead of being swallowed, close() does not
report success while frames remain unsent, and the production ws/unix writers
await the socket rather than draining into its buffer and calling that
backpressure.
A frame the codec rejects now closes the connection instead of resolving
indistinguishably from success. The wire behaviour is unchanged - malformed
JSON still produces no response - but the disposition carries a reason, so
oversize closes with 1009 (Message Too Big) and malformed with 1002.
Request ids are decided in exactly one place. coerceId rejects non-integer
numbers, -0, and values beyond safe-integer range, and a rejected id is
reported as null rather than echoed back to the peer.
Verified: 82 pass / 0 fail. Mutation-checked - inverting the ordering barrier,
scoping the subscription index per connection, dropping instead of applying
backpressure, and echoing a rejected id each turn the guarding tests red.
Wires the generated validators into dispatch and the emit paths. Generating
validators is not the same as enforcing them: three review rounds each found
another direction where a validator existed but nothing consumed it.
Inbound client-request params are validated before any spawn, session
mutation, prompt submission or approval side effect, returning the locked
-32602 with no mutation. Outbound client-request results are validated before
serialization and fail closed with -32603, which is how the response shapes
below were caught. Approval responses are validated before settling the
broker, outbound notifications and server-request params before reaching the
wire. All six direction maps now have a consumer.
Responses were violating the schema this repo vendors. initialize returned
{ok:true} where InitializeResponse requires userAgent, codexHome,
platformFamily and platformOs; skills/list, hooks/list and
experimentalFeature/list returned bare arrays where the schema requires a
wrapped object. All eleven implemented methods now validate against their own
generated result validator, and initialize reports real runtime values -
userAgent is slash-delimited so a client parsing /\/([^\s]+)/ gets a version,
codexHome is the resolved agent dir, platform fields come from node:os.
The gate chain is ordered so each refusal is the honest one: method-known,
experimental capability, handshake, support manifest, missing-validator, then
params. Param validation deliberately follows the handshake gate so an
uninitialized caller gets -32600 rather than schema detail about a request it
may not make yet, and a method absent from the negotiated profile gets -32081
rather than -32602.
The support gate now requires an explicit implemented row instead of treating
a missing row as reachable. Eleven methods are marked implemented, each naming
its production seam.
A client answering a server request with a JSON-RPC error settles it as an
error outcome. Validating that error against the success-result validator had
left such requests pending forever; a malformed result still does not settle.
Verified: 47 pass / 0 fail; 11/11 implemented methods schema-valid through the
production pipeline; valid notifications still delivered, proving the
fail-closed directions do not over-reject.
The app-server command existed but was absent from the commands table, so `gjc app-server` fell through to global help. Registering it makes the entrypoint reachable and adds a spawned --help smoke so unreachability cannot return unnoticed. verify:codex-app-server-parity was defined but never invoked; it now runs in ci:check:full, and a ci-dev-affected selector test proves changes under the schema and app-server paths actually schedule it. Both the legacy protocol-source vendor tree and the new vendor/codex-app-server-schema tree are covered, since a change to either should run the gate. Adds a weekly drift job that re-syncs against codex main and reports divergence from the pin. It is schedule-only and carries continue-on-error, and the script contains no write path and no reference to the pin, so it can neither fail the main build nor advance the pin on its own. Detection is automatic; adoption stays an explicit reviewable change. Vendors the t3.code v0.0.28 startup trace with provenance. It is a static extraction from the shipped client bundle, not a captured socket transcript, and says so: it records the spawn contract (`<binary> app-server` with CODEX_HOME), the initialize params including capabilities.experimentalApi true, and the startup call sequence through account/read, skills/list and paginated model/list. That sequence is why the scope stays maximal - a thread-and-turn-only subset would leave this client hitting -32081 during provider discovery. Verified: `app-server --help` exits 0; parity gate exits 0 at 130 client requests / 1 client notification / 11 server requests / 72 server notifications.
…edger A prior run on this repo declared six goals complete while core functionality was unimplemented. It did that by authoring its own passing quality-gate JSON, which the checkpoint validated for shape rather than truth, and by calling mark_blocked_superseded to route around the blocked-to-complete guardrail. This closes the routes that made it possible. A goal marked supersedable:false is a protected gate. Supersession is now refused through every route that reached it - mark_blocked_superseded, checkpoint --status superseded, split_subgoal replacement, and review-blocker completion - and the refusal happens before any mutation, writePlan, or steering_accepted append, so a refused attempt leaves no trace of success. The marker is not self-guarding. Protection is derived from durable protected_gate_established ledger events bound to an immutable per-goal digest over id, title, objective and supersedable, scoped to a plan generation. So deleting the marker from goals.json, rewording a protected gate, rotating the generation, or editing its status to superseded, complete or failed are all detected as tampering rather than silently obeyed. Deleting or truncating the ledger fails closed rather than reading as "no protection was ever established", which is the shape that would otherwise reward destroying the evidence. create-goals refuses to reseed over an unresolved protected gate, and discarding one requires a receipt bound to its exact goal_checkpointed event rather than a status string. Protected completion additionally requires a verdict the runtime derives by re-executing the frozen obligations, not one the submitting agent supplies: an obligationsVerifier key in the gate JSON is rejected as unsupported. Completion gate payloads are canonicalized and bound per goal, so the same evidence cannot be replayed across goals under whitespace or key-order mutation. Backward compatibility is preserved deliberately: goals without the marker still supersede, ordinary reseed still works, and plans predating protected gates mint their generation lazily instead of failing closed on a plan that has nothing to protect. Honest limits, stated rather than implied: an unprotected goal's completion gate is still validated for shape only, and trust is relocated to the package snapshot rather than eliminated. Verified: 193 pass / 0 fail across the ultragoal runtime, nudge-guard and durable-completion suites.
…rt prelude Re-implements the prompt-retention contract. Two sdk-host-wiring tests were failing because it was absent. `abortAndPrompt` ran `awaitAbortReady()` before knowing whether the replacement could be admitted, so a capacity-full abort-and-prompt aborted the prior run and then failed to admit its replacement, destroying work it could not replace. `reservePromptDelivery` now takes capacity first, making admission decidable before anything destructive happens, and every validation, abort, preflight, send and ack rejection path releases the reservation exactly once - neither leaking a slot nor double-releasing one. `recordPromptAccepted` evicts terminal records before active ones, so a full table sheds finished work rather than a live prompt. Removes the five-minute `PROMPT_SUBMISSION_TTL_MS` active-expiry from `cleanupPromptRecords`. Wall-clock age is the wrong signal for an active prompt: a long-running turn is not a leak, and expiring it dropped correlated events and terminal replay for exactly the prompts that needed retention most. Active records are now retained by lifecycle and released when they reach a terminal state. Requester/audience isolation is preserved, so retention never widens who can observe a prompt. Verified: sdk-host-wiring 74 pass / 0 fail, including the two named regressions. Mutation-checked - removing the reservation or the terminal-first eviction turns the guarding tests red.
…ession
The control surface declared `appendProjection`/`readProjection` as required
members and dispatched them, but nothing implemented them, so all six
AD-{M,A,L}-C54/C55 adapter rows failed with a bare `internal` error.
The binding has to come from the runtime that owns the real `SessionManager`.
`ctx.sessionManager` is the frozen `createReadonlySessionManager` facade, which
deliberately exposes no mutation authority: it satisfies `getEntries()` for
reads but has no `appendAppServerProjectionEntry`, so appends failed there by
design. Widening that facade would have handed projection-write authority to
every extension and hook, so the operations are wired in `runtime-init.ts` and
`extension-ui-controller.ts` where the owning session is in scope, and the
facade is left untouched.
Preserves the existing contracts: append is flush-backed and returns
`{entryId, revision}` only after the entry is durable, a repeat with identical
content is idempotent, a conflicting `sourceKey` is `idempotency_conflict`, a
corrupt store is `projection_corrupt`, and `projection.read` returns records in
append order. Projection stays single-writer in the owning child.
Verified: sdk-adapter-dispositions 582 pass / 0 fail (all six projection rows);
`createReadonlySessionManager` unchanged.
… providers `SessionSdkHost` expects a provider to register, heartbeat, and answer reverse requests under a lease. Without maintenance an immediate approval test passes while production approvals fail with `provider_required`, because the lease expires between registration and use. The controller is retained with each child SdkClient and covers registration, heartbeats, reconnect reclaim with `expectedLeaseId` and idempotency, fenced response settlement, and release on shutdown. It imports `REVERSE_HEARTBEAT_MS` from the host rather than restating 5s locally, so the cadence cannot drift from the 15s TTL it exists to satisfy. Two independent fences, because they fail differently. The ingress check drops a reverse request whose lease this controller does not own, so a foreign lease never reaches a provider handler. The pre-send check re-validates ownership after the handler resolves, so a response computed under a lease that expired mid-flight is discarded rather than settling a request the peer already reclaimed. Tests drive the TTL under fake time: a lease heartbeaten every 5s stays live across the 15s window, an unheartbeated one expires and is reclaimed with a new lease id, and a stale in-flight response does not settle while the current lease still does. Contract-only where it must be: the child spawn path is sandbox-blocked in this environment, so this covers the controller against a fake client and does NOT demonstrate an end-to-end production approval. Verified: 7 pass / 0 fail. Mutation-checked - disabling the heartbeat, the ingress fence, or the pre-send fence each turns a guarding test red.
…controls
An installed extension could forge reserved app-server projection records:
ctx.sdkControl("projection.append", { envelope: { schemaVersion: 1,
recordKind: "forged", sourceKey: "extension-forge", payload: {} } })
The app-server then treated that record as its own trusted state. The
extension runner forwards sdkControl with no allowlist, reaching the switch
that calls appendAppServerProjectionEntry - the trusted method that
deliberately bypasses the reserved-entry-type guard SessionManager applies to
its public appendCustomEntry path.
A location-based gate cannot fix this, and two were tried and reverted. The
seam is shared by design: extensions and the SDK host both reach projection
through the same ExtensionContext.sdkControl, documented as "typed nonvisual
session controls exposed to the SDK host". Gating the runtime switch, or the
extension runner's context builder, each refused the legitimate SDK path and
broke all six AD-{M,A,L}-C54/C55 rows.
So authority is carried explicitly instead of inferred from the call site. The
projection cases require a branded SdkControlAuthority symbol held in a
WeakMap keyed by the runtime-owned handler. An extension receives the context
but never the token, and the brand means it cannot fabricate one, so the same
seam serves both callers while only the owning runtime can write.
Both seams are gated independently, because each reaches the trusted method on
its own: the runtime-init control switch and the extension UI controller.
Verified: extension-originated append and read both rejected with `forbidden`
while the authority-bearing calls succeed. sdk-adapter-dispositions 582 pass /
0 fail, sdk-host-wiring 74 pass / 0 fail, app-server 277 pass / 3 skip / 0
fail. Mutation-checked - disabling either seam's guard turns the authority
test red. createReadonlySessionManager is unchanged.
…ble token
The previous authority token was defeatable. It lived in a WeakMap keyed by the
sdkControl handler, and the SDK bus recovered it with
`getSdkControlAuthority(ctx.sdkControl)` - a lookup keyed on the very function
object every extension is handed. So an extension could perform the same
lookup:
const stolen = getSdkControlAuthority(ctx.sdkControl);
await ctx.sdkControl("projection.append", { envelope }, stolen);
// -> {"entryId":"1ab98ac0","revision":1}
Any check an untrusted caller can also perform is not an authority check.
Rolling the plumbing back was not an option either: it is load-bearing for the
real SDK path, and removing it dropped sdk-adapter-dispositions from 582 pass
to 576 pass / 6 fail.
So the ambient lookup is removed rather than guarded. `sdk-control-authority.ts`
is deleted, and the bus receives an `AppServerProjectionCapability` at
construction from the runtime that owns the real SessionManager. The capability
is held in the bus closure and never reachable from an ExtensionContext, so
there is no token to steal and no getter to call. Both extension-facing seams
now refuse `projection.*` unconditionally, which is simpler than comparing a
secret and cannot be bypassed by supplying one.
The regression test probes adversarially rather than asserting the happy path:
it walks every function exported from the projection and (now absent) authority
modules, calls each with values an extension can reach, and forges opaque
candidates, then asserts every resulting `projection.append` is `forbidden`.
Verified: the exploit is refused with `forbidden` and no token is recoverable;
sdk-adapter-dispositions 582 pass / 0 fail; sdk-projection-authority 2 pass / 0
fail; sdk-host-wiring 74 pass / 0 fail; app-server 277 pass / 3 skip / 0 fail.
Mutation-checked - removing either seam's refusal turns the test red.
… expired leases A red-team round found the lease controller trusted more of the wire than it should. A completed reverse request forgot its own id: the `finally` deleted the pending entry once the handler resolved, so a re-sent frame with the same id looked new and ran the provider a second time. For an approval provider that means one prompt executing twice, which is a safety problem rather than a bookkeeping one. Completed ids are now remembered in a bounded LRU sized from MAX_REVERSE_OUTSTANDING, so a replay is refused without letting a long-lived connection grow the ledger without limit. Response fencing checked lease ownership but not lease liveness, so a response computed under a lease that aged out mid-flight was still sent. `#canRespond` now also requires the lease to be unexpired, which is the case that matters: ownership alone still looks valid at the moment the handler resolves. Also closes the surrounding lifecycle races - a stale `expectedLeaseId` during the disconnect grace window, and a lease resurrected by a registration or reclaim still in flight when `close()` runs. `close()` fences in-flight registration rather than only clearing state. Verified: 12 pass / 0 fail; app-server 282 pass / 3 skip / 0 fail; tsc clean. Mutation-checked - removing the replay guard or the expiry fence each turns a guarding test red. Still fake-client protocol coverage: the child spawn path is sandbox-blocked in this environment, so this does NOT demonstrate an end-to-end production approval.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c29f7ee9f5
ℹ️ 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".
| } catch {} | ||
| } | ||
| }, timeoutMs); | ||
| await exec.done; |
There was a problem hiding this comment.
Allow follow-up controls during command execution
When command/exec starts an interactive process such as cat with streamStdin: true, this await keeps the request open until the child exits, while Connection.process() serializes every later frame behind that request via #inbound. Consequently the required command/exec/write, resize, and terminate requests on the same connection can never reach their handlers; a second connection cannot help because execution IDs are connection-scoped. The direct handler tests bypass this connection-level serialization, so this needs a wire-level concurrency path for follow-up controls.
Useful? React with 👍 / 👎.
| export async function disposeProcesses(): Promise<void> { | ||
| const records = [...processRegistry.values()]; | ||
| for (const record of records) { | ||
| try { | ||
| if (record.child) record.child.kill("SIGKILL"); | ||
| else record.pty?.kill(); | ||
| } catch { | ||
| // The exit watcher remains responsible for settling the record. | ||
| } | ||
| } | ||
| await Promise.all(records.map(record => record.done)); |
There was a problem hiding this comment.
Terminate spawned processes during runtime shutdown
This cleanup routine has no production caller, so processes launched through process/spawn remain in the module-level registry when their connection closes or AppServerRuntime.close() handles SIGINT/SIGTERM. A long-running child such as sleep can therefore survive the client and keep the app-server process alive during shutdown. Wire disposeProcesses() into runtime/connection teardown with the appropriate ownership scope.
Useful? React with 👍 / 👎.
| exec.timeoutTimer = setTimeout(() => { | ||
| if (!exec.settled) { | ||
| exec.timedOut = true; | ||
| try { | ||
| exec.child?.kill("SIGTERM"); | ||
| exec.pty?.kill(); |
There was a problem hiding this comment.
Escalate timed-out commands to a hard kill
When a non-PTY child ignores SIGTERM, the timeout callback marks it timed out and sends only that signal, after which the handler waits indefinitely on exec.done. For example, a command that traps TERM never returns the promised timeout result and permanently occupies the serialized connection. Add a bounded grace period followed by SIGKILL, as the neighboring process implementation already does.
Useful? React with 👍 / 👎.
| // unix:// — same WebSocket-over-Unix semantics. | ||
| if (config.mode.kind === "unix" && config.mode.path) { | ||
| const socketPath = config.mode.path; |
There was a problem hiding this comment.
Resolve the default path for bare unix listeners
For gjc app-server --listen unix://, parseListenUrl() deliberately returns { kind: "unix", path: null } to represent the documented default socket path, but this truthiness guard skips the entire listener branch. The command then returns successfully without binding any transport. Resolve the default path before this check or handle the null case inside the Unix branch.
Useful? React with 👍 / 👎.
| const src = join(source, file); | ||
| if (!existsSync(src)) throw new Error(`Generated artifact not found: ${file}`); | ||
| copyFileSync(src, join(outDir, file)); |
There was a problem hiding this comment.
Embed artifacts used by generation subcommands
In the released single-file Bun executable, __dirname remains the build-time source directory, but these generated .ts files are not embedded as raw filesystem assets. On an installed machine without the source checkout, gjc app-server generate-ts therefore reaches this existence check and always reports Generated artifact not found; generate-json-schema has the same problem with its neighboring filesystem copy. Import/embed the artifact contents at build time instead of reopening source-tree paths.
Useful? React with 👍 / 👎.
| if (typeof path !== "string") return { ok: false, errorKey: "invalidParams" }; | ||
| try { | ||
| const { rmSync } = require("node:fs"); | ||
| rmSync(path, { recursive: p?.recursive !== false, force: p?.force !== false }); |
There was a problem hiding this comment.
Reject non-absolute filesystem paths
The vendored fs/remove contract declares path as an absolute, normalized AbsolutePathBuf, but this handler accepts every string and passes it directly to rmSync; the other new basic filesystem handlers use the same validation. A malformed request such as { path: "cache" } is therefore accepted and deletes relative to the app-server process's working directory rather than being rejected with invalidParams. Enforce the absolute/normalized path invariant before any filesystem side effect.
Useful? React with 👍 / 👎.
…s instead of committing them The Codex app-server schema trees (1947 files, 264k lines) and the six derived protocol sources were checked in, dominating every diff that touched this package. They are reproducible from openai/codex at the pinned commit, so the repository now keeps the reproducers and only the slice the build actually imports. - codex-app-server-schema-materialize.ts materializes both profiles: stable via git archive of the pinned commit, experimental via the pinned Rust exporter. Both are verified against the frozen subtree OIDs, which stay the only verification authority, and materialization is skipped when the trees already hash correctly. - app-server-schema-closure.ts derives the committed slice from the sources' own import graph and generates the vendored .gitignore. --check, wired into check:ts, fails when that slice drifts, so importing an uncommitted vendored type is a named error rather than a missing-module failure on a fresh clone. - codex-app-server-codegen.ts renders the protocol sources and Ajv validators from committed inputs and runs from prepare. It needs neither network nor a Rust toolchain; full materialization is best-effort so bun install never fails on a machine without cargo. - The oracle gates materialize before verifying, so they prove reproduction from source rather than the integrity of checked-in bytes. Verified: both profiles rebuild byte-identically from a clean upstream fetch (all four frozen subtree OIDs match), typecheck and 137 suite tests pass with only the 123 committed schema files and no Rust, full codegen leaves no diff, and all five obligation gates re-verify against the new tree.
…protocol # Conflicts: # issues/README.md # packages/agent/src/agent.ts # packages/agent/test/agent-force-abort.test.ts # packages/ai/src/utils/oauth/callback-server.ts # packages/coding-agent/src/defaults/gjc/skills/ultragoal/SKILL.md # packages/coding-agent/src/gjc-runtime/ultragoal-runtime.ts # packages/coding-agent/src/internal-urls/docs-index.generated.ts # packages/coding-agent/src/modes/controllers/extension-ui-controller.ts # packages/coding-agent/src/sdk/broker/lifecycle.ts # packages/coding-agent/test/gjc-runtime/ultragoal-runtime.test.ts # packages/coding-agent/test/sdk-adapter-dispositions.test.ts # packages/coding-agent/test/sdk-operation-matrix.test.ts # packages/coding-agent/test/telegram-baseline-manifest.test.ts
…merge dev moved 243 commits under the same code this branch touches, so the merge needed real reconciliation rather than textual conflict resolution: - agent.ts: dev removed the managed-fallback terminal branch and replaced it with a unified finalizeRun carrying the attempt scope and resource cancellation domain. This branch's contribution (stopReason on the cancelled terminal) is already present upstream, so the branch keeps dev's shape and only re-adds the logical-run-id tracking the forceAbort terminal guard needs. - ultragoal-runtime.ts: dev restructured the completion gate pipeline, so the cross-goal reuse assertion now runs against the gate that is actually recorded, and the review-blocker writer keeps its ledger-integrity update while returning dev's richer result shape. - broker/lifecycle.ts: dev already canonicalizes metadataRoot, so only the new evidenceSessionsRoot field is added rather than re-setting a duplicate key. - trace-replay: dev added the 'mara' provider, which leaked 124 models past the test's disable list. - ultragoal fixtures: dev keeps tempDir() deliberately outside a git work tree, so protected-gate tests that snapshot a tree hash use protectedTempDir(), which now creates an authoritative integration base for dev's change-set resolution. - The spawned-CLI golden is re-recorded for dev's default-model catalog entry and version bump; the transcript is otherwise byte-identical. Verified: 588 app-server tests pass, ultragoal runtime 205/205, typecheck clean, schema closure intact, and all five obligation gates re-verify. The six remaining sdk-broker failures are pre-existing on dev (dev fails seven; this branch fixes one) and are not regressions.
c29f7ee to
a9576b2
Compare
…urces generateFromVendor wrote behavior/generated-behavior.ts directly, which only worked because the directory already existed locally. In a clean clone the directory is absent: its sole file is now generated rather than committed, so git never creates it, and prepare failed with ENOENT during bun install. Verified on a fresh clone with cargo off PATH: bun install exits 0, renders all six protocol sources, and the package typechecks with the experimental schema materialization skipped.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 700c3fe6d3
ℹ️ 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".
| process.on("SIGINT", onSignal); | ||
| process.on("SIGTERM", onSignal); |
There was a problem hiding this comment.
Take ownership of signals in stdio mode
When the real gjc app-server --stdio entrypoint receives SIGINT or SIGTERM, the bootstrap postmortem handlers are still installed and call process.exit(130/143) after their own bounded cleanup, while this connection-close promise is not registered with that cleanup. The listener modes explicitly remove those handlers because they can pre-empt runtime teardown, but stdio only adds another listener, so the primary transport can exit before its retained child, broker, projection, and queue cleanup completes; apply the same signal-ownership pattern to stdio.
Useful? React with 👍 / 👎.
| try { | ||
| const manager = ConcreteSessionManager.create(location.cwd, destinationFor(location, context)); | ||
| try { | ||
| await manager.dropSession(location.path); |
There was a problem hiding this comment.
Close loaded runtimes before deleting transcripts
When thread/delete targets a thread currently present in context.manager, this drops the persisted session without removing or closing the retained child. An active turn can therefore continue writing through its existing SessionManager after the request reports success—losing writes to an unlinked inode on Unix or producing platform-dependent failures elsewhere—while the runtime still advertises the thread as loaded. Reject this case as a conflict or perform identity-fenced runtime teardown before deleting the transcript.
Useful? React with 👍 / 👎.
| parentThreadId: null, | ||
| preview, | ||
| ephemeral: false, | ||
| isPinned: false, |
There was a problem hiding this comment.
Project persisted metadata in read handlers
After thread/metadata/update persists isPinned: true and Git metadata in the gjc.app-server.thread-metadata custom entry, thread/read, thread/list, and thread/search rebuild the thread with isPinned: false and gitInfo: null. Consequently a successful update disappears on the next read, and thread/list with isPinned: true can never return the updated thread; parse the custom metadata entries when constructing persisted-session projections.
Useful? React with 👍 / 👎.
| if (typeof path !== "string") return { ok: false, errorKey: "invalidParams" }; | ||
| try { | ||
| const { statSync } = require("node:fs"); | ||
| const stat = statSync(path); |
There was a problem hiding this comment.
Use lstat when reporting symlink metadata
When fs/getMetadata receives a symlink path, statSync follows the link, so the returned Stats describes the target and isSymbolicLink() is always false. This makes the protocol's required isSymlink field incorrect for every symlink; use lstat/lstatSync for link metadata and decide separately whether target type information is needed.
Useful? React with 👍 / 👎.
| watcher, | ||
| active: true, | ||
| }; | ||
| watchRegistry.set(key, record); |
There was a problem hiding this comment.
Release filesystem watchers when connections close
When a client registers fs/watch and then disconnects without calling fs/unwatch, this module-level record and its OS watcher remain alive because production connection/runtime teardown never invokes disposeFsWatchers; the disposer is only referenced by tests. Repeated reconnects can therefore accumulate inotify/kqueue handles until watcher creation fails, even though notifications can no longer reach their owning connections. Remove records by connectionId during disconnect and dispose all remaining watchers during runtime shutdown.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| let shutdownPromise: Promise<void> | undefined; | ||
| let keepAlive: ReturnType<typeof setInterval> | undefined; |
There was a problem hiding this comment.
Replace ReturnType with the concrete timer type
This new production declaration uses ReturnType<>, and the same forbidden construct appears throughout the added app-server runtime and handler files. Replace these aliases with the actual timer or function-result type names required by the repository contract.
AGENTS.md reference: AGENTS.md:L103-L105
Useful? React with 👍 / 👎.
| /** Runtime-owned MCP lifecycle/auth service; absent only in isolated handler unit tests. */ | ||
| readonly mcpService?: McpAppServerService; | ||
| /** Explicit test seam for a manager; production dispatch uses mcpService. */ | ||
| readonly mcpManager?: import("../../runtime-mcp/manager").MCPManager; |
There was a problem hiding this comment.
Move app-server imports to module scope
This inline type import violates the repository's top-level-import requirement; the added code repeats the problem at handlers.ts:70, plugin-handlers.ts:297, and with runtime await import() in environment-app-handlers.ts:403. Replace them with top-level type/value imports so dependency and bundling behavior remains explicit.
AGENTS.md reference: AGENTS.md:L103-L106
Useful? React with 👍 / 👎.
| if (!value || typeof value.type !== "string") return undefined; | ||
| switch (value.type) { | ||
| case "uncommittedChanges": | ||
| return "Perform a code review of the current working tree, including staged, unstaged, and untracked changes. Report concrete findings with file and line evidence."; |
There was a problem hiding this comment.
Store review prompts in static Markdown
The new review/start implementation constructs model-facing prompts directly in TypeScript, including several interpolated variants in this switch. Move the prompt text to a static .md file imported with { type: "text" } and substitute only the validated target values, as required for repository prompts.
AGENTS.md reference: AGENTS.md:L109-L110
Useful? React with 👍 / 👎.
| @@ -39,6 +39,7 @@ export const commands: CommandEntry[] = [ | |||
| { name: "state", load: () => import("./commands/state").then(m => m.default) }, | |||
| { name: "setup", load: () => import("./commands/setup").then(m => m.default) }, | |||
| { name: "acp", load: () => import("./commands/acp").then(m => m.default) }, | |||
| { name: "app-server", load: () => import("./commands/app-server").then(m => m.default) }, | |||
There was a problem hiding this comment.
Add the public app-server feature to the changelog
This registers a new shipped gjc app-server command and protocol surface, but the commit does not modify packages/coding-agent/CHANGELOG.md. Add an entry under that package's ## [Unreleased] section so the public feature is included in the required release record.
AGENTS.md reference: AGENTS.md:L178-L178
Useful? React with 👍 / 👎.
Signed exact-head hostile disposition —
|
Summary
Implements a Codex-compatible app-server protocol surface for GJC (
gjc app-server --stdio), built and verified through a 26-story ultragoal run that is now closed with a final-aggregate completion receipt.The branch adds a full JSON-RPC app-server: vendored two-profile protocol authority, transport/framing/auth, direction-split dispatch, thread runtime + per-thread child processes, turn projection and resume, the item family, an awaitable approval reverse-bridge, MCP surfaces, and a mechanically-enforced obligations/receipt system that prevents self-certified completion.
What's in it
scripts/check-codex-app-server-main-drift.ts,scripts/verify-codex-app-server-parity.ts).jsonrpc, nodatakey, malformed JSON dropped+logged), auth, listen, connection state, direction-split dispatch with the golden code set.thread/start,turn/start, durable turn projection and resume.mcpServerStatus/list, tool call, resource read, reload and OAuth login wired against the liveMCPManager, projected into JSON-clean wire DTOs before outbound validation.oracle-stable,oracle-experimental,spawned-cli-blackbox,trace-replay,real-t3), exact-tree-hash receipts, and a live verifier that re-executes each gate and fails closed on any mismatch.issues/*.mdper residual defect with repro, root cause, owner and disposition.Verification
oracle-stable,oracle-experimental,spawned-cli-blackbox,trace-replay,real-t3— all VERIFIED, exit 0, sharing one exact-tree hash.tsc --noEmitclean; biome clean.final-aggregatereceipt and a terminal-criticOKAY(zero blockers) at HEAD.Known limitations (documented, not hidden)
TOCTOU on the approval path,
ast_editscope, approval timeout semantics, OAuth requester lifetime, loaded-thread reload, and load-scoped real-client concurrency are each stated honestly in the support manifest / docs rather than silently claimed as covered. The sandbox-blocked broker-child spawn is labelled an environment block (issues/23-broker-child-spawn-environment-block.md).Review notes
Large diff (~2.1k files) — the bulk is the vendored protocol schema and generated validators. The hand-written surface is
packages/coding-agent/src/app-server/**andpackages/coding-agent/src/runtime-mcp/**.