Skip to content

feat(app-server): Codex-compatible app-server protocol surface - #3782

Draft
Yeachan-Heo wants to merge 139 commits into
devfrom
feat/codex-app-server-protocol
Draft

feat(app-server): Codex-compatible app-server protocol surface#3782
Yeachan-Heo wants to merge 139 commits into
devfrom
feat/codex-app-server-protocol

Conversation

@Yeachan-Heo

Copy link
Copy Markdown
Owner

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

  • Protocol authority — pinned two-profile (stable + experimental) Codex schema vendored with provenance, a parity gate, direction-specific validators, and a drift check (scripts/check-codex-app-server-main-drift.ts, scripts/verify-codex-app-server-parity.ts).
  • Transport & router — framing, error envelopes (no jsonrpc, no data key, malformed JSON dropped+logged), auth, listen, connection state, direction-split dispatch with the golden code set.
  • Thread runtime — admission, session projection, one child process per loaded thread, a production child bridge, transactional thread/start, turn/start, durable turn projection and resume.
  • Items & approvals — the item family plus an awaitable approval reverse-bridge, including permission-gated patch approval reachable in the shipped assembly.
  • MCP surfacesmcpServerStatus/list, tool call, resource read, reload and OAuth login wired against the live MCPManager, projected into JSON-clean wire DTOs before outbound validation.
  • SDK prerequisites — bus prompt-retention repair, projection control operations, broker endpoint authority tuple, authority-gated offline test model provider hook.
  • Anti-self-certification — an obligations manifest with five non-supersedable gates (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.
  • Triageissues/*.md per residual defect with repro, root cause, owner and disposition.

Verification

  • Live obligations verifier: oracle-stable, oracle-experimental, spawned-cli-blackbox, trace-replay, real-t3all VERIFIED, exit 0, sharing one exact-tree hash.
  • G2 spawned-CLI JSON-RPC blackbox 3/3, G3b real-client T3 3/3, signal-shutdown 1/1.
  • tsc --noEmit clean; biome clean.
  • Every story carries a completion receipt; the run closes with a final-aggregate receipt and a terminal-critic OKAY (zero blockers) at HEAD.

Known limitations (documented, not hidden)

TOCTOU on the approval path, ast_edit scope, 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/** and packages/coding-agent/src/runtime-mcp/**.

Yeachan-Heo added 30 commits July 30, 2026 06:37
…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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +489 to +499
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));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +215 to +220
exec.timeoutTimer = setTimeout(() => {
if (!exec.settled) {
exec.timedOut = true;
try {
exec.child?.kill("SIGTERM");
exec.pty?.kill();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment on lines +304 to +306
// unix:// — same WebSocket-over-Unix semantics.
if (config.mode.kind === "unix" && config.mode.path) {
const socketPath = config.mode.path;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +177 to +179
const src = join(source, file);
if (!existsSync(src)) throw new Error(`Generated artifact not found: ${file}`);
copyFileSync(src, join(outDir, file));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +197 to +200
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 });

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Yeachan-Heo added 3 commits August 3, 2026 16:17
…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.
@Yeachan-Heo
Yeachan-Heo force-pushed the feat/codex-app-server-protocol branch from c29f7ee to a9576b2 Compare August 3, 2026 08:37
…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.
@Yeachan-Heo
Yeachan-Heo marked this pull request as draft August 3, 2026 08:42

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +150 to +151
process.on("SIGINT", onSignal);
process.on("SIGTERM", onSignal);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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.";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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) },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@Yeachan-Heo

Copy link
Copy Markdown
Owner Author

Signed exact-head hostile disposition — REQUEST_CHANGES

Reviewed head: 700c3fe6d306131985df2868a0eb24d002b4b444
PR API base: dev@1d74b45ee4627951ece3c920c8e61280a80933c2
Merge base: 9d0a1c3ecf656dd1b5d1a2b8e787be15c93863e3
Live dev: 732856b3ccb3fade6e9fbc17908a4fbca5a7682f
State: draft, mergeable=false, mergeable_state=dirty; live dev has 25 commits not present in this head.

CI gate

Exact-head run 30798391179 is terminal red: 3 failure / 6 success / 9 skipped. The root plan job says this head must contain base 1d74b45e…; the evidence-producer failure is a cascade from the missing plan artifact. The product test matrix therefore did not execute on this head.

Blocking review evidence

The head has not moved since the current Codex review attached 15 exact-head findings: 9 P1 and 6 P2. Those include same-connection command follow-up deadlock, command timeout without hard-kill, process and watcher teardown leaks, loaded-thread deletion without runtime teardown, compiled generation subcommands reopening unavailable source paths, filesystem contract violations, and mandatory changelog/import/prompt/type-policy failures.

Independent hostile review found additional terminal-lifecycle blockers:

  1. HIGH — concurrent turn admission is not reserved before async override setup (thread-runtime/turn-controller.ts:646-751). Two connections can both pass the idle check while model/policy setup awaits; the later start can overwrite the active slot and corrupt admission/cleanup ownership.
  2. HIGH — notification delivery can prevent terminal durability (turn-controller.ts:947-1003). #finish emits reducer notifications before appending the terminal projection; a disconnect/rejecting subscriber can leave an accepted child-terminal turn active/recovery-required forever.
  3. HIGH — assistant fallback is not turn-correlated (thread-runtime/production-child.ts:386-397). A cached assistant message survives agent_failed and can be injected into a later message-less agent_end, attributing one turn's output to another.
  4. HIGH — approval-policy compensation can fail silently (production-child.ts:438-482). After permission_mode is changed to allow, a later setup error can swallow restoration failure before a restore closure reaches the controller, allowing retained-child policy drift.

The separate claim that stdio request responses lack a newline was rejected: serializeResult/serializeError already append LF for stdio.

Required disposition path

  • Reconcile/rebase onto current dev so the exact head contains the base and the comparison is reviewable.
  • Repair or explicitly retire every current-head P1/P2 thread plus the lifecycle blockers above; add the required packages/coding-agent/CHANGELOG.md entry and satisfy repository import/prompt/type contracts.
  • Obtain fresh terminal exact-head CI and a new hostile review. Existing obligation receipts and current review evidence do not survive a head change.

This is a comment-only owner-authored PR disposition; no self-review approval/request event, code mutation, push, merge, CI control, build, or release action was performed.

Verdict: REQUEST_CHANGES

Signed-off-by: Yeachan-Heo owner@gajae-code.com

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