refactor(adapters): centralize runtime adapter authority - Part 1 - #1721
Merged
Conversation
Rebased onto current dev with the reviewed public evidence trust core, consumer durability recovery, sparse-array JCS hardening, and required Windows publisher-key ACL hardening.
Rebase the reviewed CL-10 operator/community layer onto the current public-evidence core. This squashes the child history onto cl10-public-core while preserving the exact conflict-free GitHub merge tree, including the final review fixes.
The sidebar button is the fix; the models tab is where the belief that something
is wrong actually forms. Someone comparing the picker against this list is
already looking at this page, so the recovery belongs here too.
A banner appears only when the proxy reports a running app-server older than the
catalog. Every other verdict renders nothing: telling a user "we could not tell"
on a page about models is noise, and a locked-down host where enumeration fails
would otherwise show a permanent warning.
The reading is fetched once on mount and on invalidation, never on a timer.
Enumeration shells out to ps, procfs, or PowerShell CIM, and this workspace
already gates its own catalog poll on tab activity for that reason.
Cross-surface invalidation is an epoch rather than a shared controller: App bumps
a counter when a restart settles anywhere, Models takes it as a prop and re-reads.
The backend is already single-flight, so what was missing was invalidation, not
mutual exclusion — without it a restart from the sidebar left the banner on screen.
Two defects the earlier string-matching tests could not see are now covered by
real DOM tests: a page-head restart did not refresh staleness at all, and the
settled callback fired after unmount. The hook also lost its timeout translation
in a rewrite, so a timeout showed the transport's hardcoded English.
Naming note: this file already binds catalogState to the /api/catalog resource,
so the new value is appServerState.
Verification: gui bun test 846 pass / 0 fail; lint, lint:i18n, build green; root
typecheck and privacy:scan green. Live proxy on a fresh build returned
{"state":"fresh","runningCount":4} for the authenticated GET, 401 unauthenticated,
and 403 for an admin token from a foreign origin.
restartCodexAppServers sent process.kill(pid, "SIGTERM") on every platform. On Windows that is not a graceful signal — it is an unconditional terminate of one process, and it leaves the process tree behind. The repository already knew this and already had the right ladder for the proxy in process-control.ts; app-servers never got it. That gap matters most exactly where this feature is most needed. Windows has no Ctrl+Q, so users close the Codex window and the app-server keeps running in the background holding a catalog snapshot — the stale picker this whole unit exists to fix. Windows now uses taskkill /PID <pid> /T /F, resolved from a trusted system directory rather than PATH, with a fallback to the old process.kill so the new path can never be worse than the one it replaces. Unix keeps SIGTERM only: there the signal really is graceful, and following it with SIGKILL would ask a harsher consent than a restart click gives. Survivors are reported instead. The asymmetry is recorded in the function comment and the phase doc so a later reader does not "fix" it into symmetry. Tests drive each branch through injected io: Windows exec receives /PID /T /F and no signal follows, a throwing taskkill falls back to SIGTERM, and Linux and macOS stay SIGTERM-only with no exec and no SIGKILL.
The CLI parity sweep reads GUI sources for /api paths, so naming the catalog route inside a comment made it look like an endpoint the GUI calls with no CLI mirror. The comment only needed to explain why the new value is appServerState rather than catalogState; it did not need to spell the route.
…y fixture (#1714 guard)
…ance fixture (#1714 guard)
Contributor
✅ READY
UI screenshot waived by the |
Wibias
added
the
gui-screenshot-waived
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack
Scope
Fresh reimplementation of the architecture slice from #1623 on current
dev.resolveAdapter()constructs through that registryrunTurntransport and gated native-file fallback are distinctstructure/10_adapter-registry.mdDeliberately not included
Verification
The registry contract was added test-first. The red run failed only because
src/adapters/registry.tsdid not yet exist. On the implementation tree, TypeScript/gates and all four Linux test shards passed before the branch was squashed to one commit.The PR remains draft while exact-head post-squash CI finishes.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation