fix(codex): seat a role from the app-server's loaded threads, not rollout files - #583
Merged
Conversation
…lout files A seat was recorded only when exactly one of the 40 newest rollouts had a session_meta cwd matching the project. On a project worked in more than once that count is never one, so the seat was never written -- and every layer of the monitor path requires it, so the bridge could not arm in that session or in any later one. Rollout files cannot say which thread is the current session; they carry the whole history of the cwd. The app-server can, but only as a bare id list, and it keeps a thread loaded after its window is gone. Subtracting the threads a role already sits in leaves the session not yet seated. A unique remainder is recorded; zero or several record nothing, so the bias toward a fresh boot over waking someone else's conversation is unchanged. The rollout scan remains the fallback for a session with no app-server to ask, where it resolves the same single-rollout case it always did. The bridge also rewrites the seat once armed: whatever seeded it was an inference, the thread the app-server resumed is not.
…er seated `delivery.sh status` printed "not running" whenever the pidfile was missing, which reads as a process that died. The far more common cause is that the role has no seat, and no layer says so: the SessionStart hook exits 0, the launcher re-execs, the dispatcher's output goes to /dev/null. The only visible symptom was this one misleading line. A recorded seat now yields "not running" with the seat printed, so the wording means what it says. Without one, the loaded-thread count decides the message, because it is what determines whether the next session can seed a seat at all: none loaded, exactly one, or several with no way to tell them apart -- the last carries the app-server recreation that unblocks it, and the warning that it stops the project's other bridges too.
…seat a seated role Three ways the previous commit could still seat a wrong thread. An ambiguous probe fell through to the rollout scan. "The app-server answered and this session cannot be told apart" is the strongest fail-closed statement available, and a rollout that happens to be unique was overruling it -- which reintroduces the original defect as a fallback. Probe availability is now tracked apart from the candidate count, and only an app-server that could not be reached at all may fall through. The inference did not check whether the target role was already seated. The subtraction removes every recorded thread including that role's own, so a seated role running it again would find its own gone and adopt whatever was left, replacing a correct seat with a stranger's thread. Inference now refuses a seated role; CODEX_THREAD_ID, which is not an inference, still re-seats one -- that is how the bridge writes back after arming. The status diagnostic probed once per seatless identity with the bridge's own timeouts, so a stale port held by something that accepts and never answers could stall `status` for tens of seconds per identity. It now probes at most once per run, with a timeout sized for a status line. The seat write-back also moves after armWatch, where the comment already claimed it was.
…every path The guard sat inside the app-server branch, so it did not cover the rollout scan -- which is precisely the path left in place for sessions with no app-server. A seated role in such a session could still have its seat replaced by whatever rollout happened to be unique in the cwd. The rule is not a property of the subtraction. Any inference can pick the wrong thread; the subtraction does it by removing this role's own thread and taking what is left, the rollout scan by taking whatever the cwd happens to offer. What makes CODEX_THREAD_ID different is not that it is more accurate, it is that it is not an inference at all -- the session exports it, or the bridge passes the thread the app-server confirmed. So the guard moves to where the inference begins, ahead of both paths, and the comment says which side of that line each source is on.
fujibee
added a commit
that referenced
this pull request
Aug 1, 2026
…ames A numeric prefix of a real port is itself a valid port, so no reader-side check can tell a half-written file from a good one. 5296 read while 52962 is being written names a DIFFERENT app-server — on this machine, plausibly another project's, which would answer and let its thread be seated here. That is the exact outcome #583 exists to prevent, so the partial state has to be unobservable rather than filtered: the writer publishes through the existing atomic helper. The reader additionally requires a port a TCP stack could have handed out. Digits alone never established that, and the bound limits what anything other than the writer could leave behind. Also: the unseated count now returns nothing when it cannot be worked out instead of 0, which had asserted "another role holds it" on a failed mktemp. Reported by review of #591.
fujibee
added a commit
that referenced
this pull request
Aug 1, 2026
…le is absent (#591) * fix(codex): resolve the app-server from the port file when the variable is absent #583 seats a role from the app-server's loaded threads, but the probe was gated on AGMSG_CODEX_BRIDGE_APP_SERVER. Under codex 0.146 --remote an agent's shell_command runs inside the app-server process, and codex-monitor.sh cannot export into that context: the URL does not exist until the banner has been parsed, which is after the server is running. So the fix never ran on the path it was written for. The port file carries the same string. codex-monitor.sh writes the port, builds ws://127.0.0.1:$PORT from it, and hands that to both the variable and --remote — reading the file reconstructs the value byte for byte rather than adding a second way to reach the server. "No URL" still means "could not ask", never "asked and got nothing", so the rollout fallback keeps its fail-closed shape. Also: delivery.sh status called a missing seat unexpected whenever one thread was loaded. A thread seats one role, so when that thread is already seated elsewhere the missing seat is correct. The count is now compared against the seated set, from the list the probe already fetched. * fix(codex): publish the port file atomically, and bound the port it names A numeric prefix of a real port is itself a valid port, so no reader-side check can tell a half-written file from a good one. 5296 read while 52962 is being written names a DIFFERENT app-server — on this machine, plausibly another project's, which would answer and let its thread be seated here. That is the exact outcome #583 exists to prevent, so the partial state has to be unobservable rather than filtered: the writer publishes through the existing atomic helper. The reader additionally requires a port a TCP stack could have handed out. Digits alone never established that, and the bound limits what anything other than the writer could leave behind. Also: the unseated count now returns nothing when it cannot be worked out instead of 0, which had asserted "another role holds it" on a failed mktemp. Reported by review of #591.
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
1 participant
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.
Closes #579.
A codex role's seat was recorded only when exactly one of the 40 newest rollouts
had a
session_metacwd matching the project. On a project worked in more thanonce that count is never one, so the seat was never written — and every layer of
the monitor path requires it, so the bridge could not arm in that session or any
later one. The reporter measured 17 matching rollouts; a machine here has 44.
Why not the three directions the report offers
Measured against a machine with the same shape (44 rollouts sharing one project
cwd, 6 among the 40 newest).
Newest-matching-rollout fallback. The two functions that pick a rollout do
not sort by the same key:
codex-record-session.shsorts by filename (creationtime),
agmsg_resolve_codex_threadby mtime (last write). A resume appends tothe existing rollout rather than creating one, so creation order systematically
misses the thread in use. At measurement time the two disagreed — filename-newest
was last written 2.7 days earlier, mtime-newest 2 minutes earlier.
Seat with what
agmsg_resolve_codex_threadresolved. Reconstructed at all 77historical session starts across two projects: correct up to 2s of hook delay,
first wrong at 3s, shortest observed gap to another session's write 2.6s. The
decisive part is that its retry does not cover this — it returns the first match
in mtime order and only sleeps when there are zero matches, and a project with
history always has matches. Today a wrong resolution fails the seat comparison
and delivery stops; writing it into the seat turns the same value into a resume
that wakes the wrong conversation. Harmless becomes harmful.
--thread loaded. Recommended internally, then measured and dropped.thread/loaded/listreturned 3 ids while 2 Codex windows were running — theapp-server keeps a thread loaded after its window closes — and its order is
ascending by thread id, which for UUIDv7 is creation order, so
ids[0]is theoldest loaded thread: on that machine, another running agent's live session.
What this does
Keep the rule — exactly one candidate or record nothing — and change what is
counted. Instead of rollouts matching the cwd, count the app-server's loaded
threads minus the threads a role already sits in. Stale loaded entries are
precisely the seated ones, so the remainder is the session not yet seated. A
unique remainder is recorded; anything else records nothing, leaving the bias
toward a fresh boot over a resume mis-fire unchanged.
The rollout scan stays as the fallback where there is no app-server to ask, and
the bridge rewrites the seat once armed, so the stored value is one the
app-server confirmed rather than one anything inferred.
delivery.sh statusalso stops reporting "not running" for a role that simplyhas no seat, and in the ambiguous case says how many threads are loaded and what
recreates the app-server.
Known limit
The subtraction needs at least one seat to subtract. An app-server already
holding several threads with no seat ever written — the state an upgrade lands
in — stays ambiguous until it is recreated. After one seat exists it is
consistent from then on. Recorded in #579 rather than left implicit.
Verification
--print-loaded-threadswas run against a live app-server, and the whole pathwas exercised by driving a real Codex TUI through
codex-monitor.shunder a pty(the loaded set is 1 for a fresh session; adding a second role sequentially
leaves exactly one unclaimed id).
Mutation evidence, each breaking only what it should:
agmsg_role_session_recorded_uuids