[chrisohearn]

Problem

Round four, ledger r4/w6-identity-events: Codex threads inherit unrelated host T3 sign-ins, while event diagnostics hide missing pins, overlapping watches and closed bindings. Notify on live sends also misclassifies newly created or queued runs as pruned history (item 26b).

Changes and evidence

• 25. Provider identity. Global t3_omarchy is a shared, unattributable sign-in; retired t3_macbook times out. Read configured MCP names offline from root mcp_servers in CODEX_HOME/config.toml (otherwise the user's .codex) and root launch config overrides. Ignore unsupported profile MCP tables; preserve profile arguments on launch. Cache per instance until config path, mtime, size or launch args change. Disable only configured t3_omarchy/t3_macbook entries; absent names never introduce incomplete transports. No codex mcp list process or auth/network probe on thread startup. Local inspection is bounded to 250 ms; failure skips isolation and logs a warning rather than blocking a thread. Preserve other servers and the existing per-thread t3-code and wait-only t3-code-direct injections, both authenticated as the thread. No global config edits or cross-sign-in pin sharing.
• 26 — Missing pin. The caller and pinned inbox owner were different sign-ins. Notify without the caller's own pin returns a top-level warning naming only the caller's short sign-in ID and the count of pinned sign-ins. Record a durable no_pinned_inbox alert visible in caller-scoped bridge_status.relayAlerts. Never borrow another sign-in's inbox.
• 26b. Newly created and queued runs. Four live sends failed with source_history_pruned because binding searched only run.updated, which appears after run.created and can be delayed behind a busy run. Include both event types. When no lifecycle event remains, return source_history_pruned if the run is terminal or its requested time predates the oldest retained application event. A retained older sentinel therefore cannot conceal a finished run's pruned history. Empty history and equal/newer timestamps alone do not prove pruning for an active or queued run. Preserve cursor rewind so a queued send's whole lifecycle remains in range.
• 27 — Subscription identity. Read-only inspection found 46 rows and 46 distinct principal/thread/event/callback identities. The apparent three-per-event duplicates use different callbacks. Existing canonical URL identity and transactional upsert already reuse identical/equivalent subscriptions; add a concurrent regression rather than rewrite them. Caller-scoped subscriptionDuplicates reports active thread/event overlap groups with subscription IDs and callback hashes, not URLs or secrets. No subscription cleanup.
• 28. Binding lifecycle. Thirteen zero expiries were terminal markers. Fork migration 16, McpRelayDiagnostics, separates nullable expiry from nullable closed_at, preserves finite expiries/cursors/errors, and migrates legacy zero rows to closed state without reviving them. The merged integration retains 13, WorkLedgerHygiene, 14, McpRequestLogConnection from feat(mcp): freeze a thread in one call and list the agents that call this server #90, and 15, ScheduledRunHistoryIndex from fix(server): close stuck run items, explain worktree failures, add scheduled-task history #86, without a collision. bridge_status.relayBindings shows ISO timestamps or null. Completion and authorization-denial closures set both closed_at and expires_at to now, preventing rollback to older builds from reviving them. Collect closed/expired bindings after 24 hours; preserve active unbounded and recently closed bindings.

Notify delivery, retry/coalescing timing, webhook payloads and pin ownership are otherwise unchanged. No service restart/deploy, live DB writes or subscription deletion. The coordinator owns independent review, migration allocation, merge, release and live relay acceptance.

Known limits

• Provider discovery's app-server probe and codex exec text generation still load the global entries. This change isolates the app-server used by T3 provider threads only.
• Project-level .codex/config.toml definitions of these names are not covered by offline user-home inspection.
• Inspection failure deliberately skips isolation with a warning instead of preventing thread startup.

Verification

Current head: 503c11584e832a3d2ab88d8c5b9230f018ad2b78, including integration 3791db6ba6a0ce4868a5c012ec41135628c3214a. Immediately before pushing, a fresh fetch confirmed that integration's highest fork migration was 15. Relay diagnostics uses 16 in the filename, registry and test import; the manifest is contiguous through 16. The coordinator's delta review of parent c56bd6d had no blocking findings and explicitly required no further code review. New-head hosted CI remains the full gate; earlier green checks are not proof of this repaired head.

• CI repair on 503c115: the integration merge left duplicate launchResult.warning schema fields, producing TS1117. Remove only the extra declaration; preserve the existing schema field and runtime warning mappings. Server-only typecheck now exits 0 (previously exit 2), and 77/77 tests pass across two focused files, toolkits/project/handlers.test.ts and McpEvents.test.ts. Both commands ran serially under the shared lock; Vitest used --maxWorkers=2 --no-file-parallelism. git diff --check passes. No notify timing/delivery or migration changes. Hosted CI on 146efc failed, including its one targeted retry; that head is not accepted.
• 10/10 focused tests passed after this integration merge and renumber to 16 across ForkMigrations.test.ts and BridgeStatusService.test.ts, serialized under the shared lock with --maxWorkers=2 --no-file-parallelism. This covers contiguous migration/file registration and the resolved bridge-status overlap, preserving both relay diagnostics and scheduled-run reliability. Staged and unstaged diff checks pass.
• 81 tests passed across five focused files on parent c56bd6d after the prior rebase and final repairs, before the integration merge and migration renumber. This includes all six original 26b cases: idle/busy sends before the first update and missing-event controls for empty, newer, equal and genuinely pruned history. The busy case verifies the binding cursor covers run.created, running and completed events. Five added terminal-state cases reject absent lifecycle history despite an older retained sentinel; active and queued controls still bind.
• Two actual adapter regressions passed on the prior candidate: app-server lifetime and authenticated per-thread MCP injection. Launch spawns exactly one app-server process, not an MCP-listing process. These were not rerun for this final P3-only delta; hosted CI is the current-head full gate.
• All current test runs use the shared heavy lock, --maxWorkers=2 --no-file-parallelism. No full local suites; hosted CI is the full gate under the coordinator's post-OOM resource rule. Broad commit hooks were not run under that rule.
• Red proof before 26b's fix: idle/busy binding plus empty/newer-history cases failed with failed notify; genuine-pruned control passed. The repaired six-case set passes.
• Offline tests cover ignored profile MCP tables, home selection, root config overrides, cache invalidation, absent transports, malformed/unreadable config and stalled IO. The public Codex configuration schema at https://developers.openai.com/codex/config-schema.json defines root mcp_servers, omits it from ConfigProfile, and sets that profile's additionalProperties to false. This verifies that profile MCP tables are not supported; the installed CLI reports codex-cli 0.167.2. Completion/denial tests verify rollback-safe expiry, including an initially unbounded binding.
• Six RED cases reproduced the final two defects before repair: five terminal runs bound despite absent lifecycle history, and a profile table introduced unsupported server names. All six pass with the fixes. Focused formatting of the four repaired files and git diff --check also pass.
• The temporary-home runtime transcript returned only the configured disabling override for an unreachable MCP URL, without spawning Codex. The global config's before/after SHA-256 was identical. The coordinator's independent review of the earlier head also verified byte-identical global config.

Read-only post-deploy acceptance

Run from the merged checkout. Do not launch notify probes before Jarvis's separate relay acceptance. Absence of a suitable new process or live run is unverified, not a pass.

25: Inspect only the disabling flags of newly started T3 Codex app-servers, never headers or config values:

node - <<'NODE'
const fs = require('node:fs');
for (const pid of fs.readdirSync('/proc').filter(value => /^\d+$/.test(value))) {
  try {
    const args = fs.readFileSync('/proc/' + pid + '/cmdline', 'utf8').split('\0');
    if (!args.includes('app-server') || !args[0].includes('codex')) continue;
    console.log({pid, omarchyDisabled: args.includes('mcp_servers.t3_omarchy.enabled=false'), macbookDisabled: args.includes('mcp_servers.t3_macbook.enabled=false')});
  } catch {}
}
NODE

Both booleans must be true where the corresponding home entries exist. Per-thread identity/injection is covered by the adapter regression and remains in codexThreadRuntimeParams.

26–28: Call bridge_status under the original outside sign-in; inspect alerts, overlap reports and explicit binding expiry/closure. Inspect persisted diagnostics without printing callbacks or tokens:

sqlite3 -readonly ~/.t3/userdata/statev2.sqlite <<'SQL'
SELECT substr(principal,1,8) AS sign_in, reason, day FROM mcp_event_relay_alerts ORDER BY day DESC;
SELECT substr(principal,1,8) AS sign_in, thread_id, name, COUNT(*) AS watches, COUNT(DISTINCT callback_url) AS callbacks FROM mcp_event_subscriptions WHERE active=1 AND expires_at > unixepoch()*1000 GROUP BY principal,thread_id,name HAVING COUNT(*)>1;
SELECT substr(principal,1,8) AS sign_in, thread_id, run_id, expires_at, closed_at FROM mcp_event_relay_bindings;
SELECT COUNT(*) AS stale_bindings FROM mcp_event_relay_bindings WHERE closed_at <= (unixepoch()-86400)*1000 OR expires_at <= (unixepoch()-86400)*1000;
SELECT COUNT(*) AS inconsistent_finite_closures FROM mcp_event_relay_bindings WHERE closed_at IS NOT NULL AND expires_at IS NOT NULL AND expires_at != closed_at;
SQL

Missing-pin alerts appear only after that caller attempts notify. Overlap groups do not authorize deleting distinct callbacks. After a normal worker drain, stale binding count is zero. Active null expiry stays active; migrated terminal rows stay closed until collected. Newly closed bindings have equal closure and expiry; migrated zero rows have null expiry and are excluded from the finite-closure check.

26b: For existing runs, inspect retained lifecycle starts without launching anything:

sqlite3 -readonly ~/.t3/userdata/statev2.sqlite <<'SQL'
SELECT b.thread_id, b.run_id, b.scan_cursor, b.closed_at,
       MIN(e.sequence) AS first_retained_run_sequence
FROM mcp_event_relay_bindings AS b
LEFT JOIN orchestration_events AS e
  ON e.application_event_version = 2 AND e.aggregate_kind = 'thread'
 AND e.stream_id = b.thread_id AND e.event_type IN ('run.created','run.updated')
 AND json_extract(e.payload_json, '$.id') = b.run_id
GROUP BY b.id;
SQL

A cursor advances after drains, so this is diagnostic, not proof of its original value. The deterministic busy-send regression proves initial cursor coverage of the whole lifecycle. After separate relay acceptance permits normal live notify use, new idle/queued sends with retained run.created must bind rather than report source_history_pruned. A terminal run without retained lifecycle events must report source_history_pruned, including when older sentinel events remain.

Re-run only isolated regressions (temporary test state):

flock -w 1800 /home/cohearn/.cache/t3-r4/heavy.lock vp test run apps/server/src/provider/CodexMcpIsolation.test.ts apps/server/src/provider/codexLaunchArgs.test.ts apps/server/src/mcp/McpEvents.test.ts apps/server/src/mcp/BridgeStatusService.test.ts apps/server/src/persistence/ForkMigrations.test.ts --maxWorkers=2 --no-file-parallelism

Implemented with gpt-6.1-sol in Codex CLI.

[chatgpt-codex-connector]

Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	❌ Failed 2026-11-13T02:57:15.476598Z We couldn’t complete this review. Comment @codex review to try again.	7b4ca3d	PR opened

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[chrisohearn]

Independent Claude Opus review of 7b4ca3d (review of record)

Checked: full diff, the head source, CI run 43355653180 (green at 7b4ca3d), the live DB read-only, and codex-cli 0.167.2 run against a temp CODEX_HOME whose config.toml is a symlink to a copy of ~/.codex/config.toml.

P1: none.

P2

• apps/server/src/persistence/ForkMigrations.ts:34: open PR #85 also registers fork id 13 (013_WorkLedgerHygiene.ts). Under FORK.md, whichever merges second must renumber to 14 and rerun its migration checks.
• apps/server/src/provider/CodexMcpIsolation.ts:18-54: codex mcp list --json does network I/O for auth_status. It takes about 115 ms online but 5.1 s with no network (unshare -rn). It runs on every session open and fails closed at 10 s, so a slow or blackholed MCP host or DNS could delay or fail every Codex thread start. Consider reading names without auth probes (e.g. config/read), or caching per config mtime.

P3

• apps/server/src/mcp/McpEventRelay.ts:258,343: closing now sets closed_at and leaves a future expires_at. If the update guard rolls back to a pre-#87 build, which scans expires_at > now, it would rescan bindings closed after migration (dedupe mostly absorbs this). Also setting expires_at = now on close would make rollback inert.
• apps/server/src/mcp/McpEventRelay.ts:410-415: the warning lists the 8-char prefix of every pinned principal. Session ids are UUIDv4, not bearer secrets, and only 8 chars appear, so (b) passes. It is still a small cross-sign-in disclosure; "N other sign-ins have pins" would avoid it.
• apps/server/src/mcp/McpEventRelay.ts:519 with :467: once GC removes a closed binding, re-notifying the same run more than 24 h later can re-bind it and relay the outcome again. Delivered outbox rows are also collected at 24 h. Unlikely in practice.
• apps/server/src/provider/CodexProvider.ts:537 (probe) and apps/server/src/textGeneration/CodexTextGeneration.ts:303 (codex exec) are not isolated. This is outside item 25's thread scope. Separately, project-scoped .codex/config.toml entries are not seen, because the listing runs without the thread cwd.

Checks

• (a) Pass. Only the exact names t3_omarchy and t3_macbook get -c mcp_servers.<n>.enabled=false, appended after the user's launch args; absent names are never overridden and t3-code/t3-code-direct are untouched. With those same overrides, codex mcp list shows just those two disabled and the other 11 enabled; the app-server run itself showed only that it starts with them. Running mcp list and app-server left the symlink target and the real ~/.codex/config.toml byte-identical (sha256). The PR adds no file writes.
• (b) Pass, apart from the P3 above. The warning appears only when notify is requested, and an alert-insert failure is caught in ThreadNotifications, so launch and send still succeed.
• (c) Pass. Identity is sha256(principal, canonical URL href, name, threadId) with a per-id lock (pre-existing). Different principals or callbacks can never share an id. The overlap report is per principal, lists only active subscriptions, and shows hashed callbackGroups.
• (d) Pass. Nothing else in sqlite_master references the table. Legacy 0 becomes closed_at=now with NULL expiry, finite expiry is kept, and no row is revived. Live DB: 14 rows close, 1 live row is kept, 0 orphans. The migration is fork-only (ForkMigrations/, t3_fork_migrations). It is not idempotent on its own; it relies on the migrator recording the id. GC matches only rows whose closed_at or non-null expires_at is at least 24 h past, so live bindings are never deleted.
• (e) Pass. For every row the code writes, the scan predicate matches the old expires_at > now. Dispatch, backoff, nextAttempt and outbox dedupe are unchanged.

Verdict: no blocking findings

[chrisohearn]

Independent delta review of 3cadf83 (review of record, follows 7b4ca3d review)

Scope: head 3cadf83, rechecked right before posting. git range-diff shows 2b31071 is 7b4ca3d rebased onto 7836910 with no change, so the delta is commit 3cadf83 only. Diff and source only; no tests run. CI was still running when I reviewed.

Earlier findings

• P2 codex mcp list probe: resolved. No subprocess starts. The code parses CODEX_HOME/config.toml (otherwise $HOME/.codex) with smol-toml and caches per instanceId, keyed by path, mtime, size and launch args. Each call still runs a local exists and stat. The whole step is capped at 250 ms, and any error or timeout logs a warning and returns unisolated args, so session open never blocks. The code only reads and stats; nothing writes ~/.codex/config.toml.
• P2 fork migration 13: now real. #85 merged as ff68541 with [13, "WorkLedgerHygiene"], and this PR is CONFLICTING in ForkMigrations.ts. Before merge, rebase, renumber McpRelayDiagnostics to 14 and rerun the migration checks (FORK.md). Do not keep both at id 13.
• P3 close leaves a future expires_at: resolved at both close sites (McpEventRelay.ts:259 on denial, :345 on finish). I checked every expires_at reader. collect already requires closed_at IS NULL, GC already ORs closed_at so its timing is unchanged, and status only displays the value.
• P3 warning lists other sign-ins: resolved. It shows the caller's prefix plus count(*), and the test checks that the other principal's prefix is absent.
• P3 known limits: noted in the PR body (probe and codex exec not isolated, project .codex not covered, isolation skipped on failure).

26b: pass, with one P3 below.

• Cursor and busy queued run: MIN over run.created/run.updated gives run.created - 1, which covers the whole lifecycle. A busy queued run now binds, and the test checks that cursor, no relay while queued and one delivery on completion.
• For a normal send, prepared.cursor already precedes run.created, so the rewind only changes notify on an existing run.
• No duplicates: the binding insert is ON CONFLICT DO NOTHING on hash(principal, thread, run, inbox), and the outbox id is hash(principal, inbox, source_key).
• No pre-run events: collect requires a matching runId, except an opted-in thread-level provider-session.detached, which has the same pre-existing behavior.
  Item 4: pass. dispatch, retry and coalescing are untouched. The only timing change is the isolation cap (10 s fail-closed became 250 ms fail-open), which is the requested fix.
  New: P1 none. P2 none beyond the migration renumber.
• P3 McpEventRelay.ts:458-467: the "oldest retained event" is not a prune watermark. ProjectionMaintenance always keeps thread.created, run.updated(running) and the newest row of each stream, so on a pruned DB oldest is ancient and the check almost never fires. A terminal run that never started and whose events were pruned now returns bound silently, with cursor = latest, no scanCursor < floor alert, and a wait until session expiry. Before, it returned source_history_pruned. Suggestion: no run events plus a terminal run.status means pruned; otherwise bind at prepared.cursor.
• P3 CodexMcpIsolation.ts:58 merges profiles.<p>.mcp_servers. I could not confirm that Codex honors profile-scoped MCP servers. If it does not, an entry defined only in a profile emits enabled=false for a name with no transport. That cannot happen today, because the live config has no profiles.

Merge prerequisite (coordinator-owned): rebase and renumber the migration to 014.
Verdict: no blocking findings

[chrisohearn]

Independent delta review of c56bd6d (review of record, follows 3cadf83 review)
Method: git range-diff 7836910e1c..3cadf83 7eddc7fc51..c56bd6d in a blobless clone. Read-only, no tests run.

1. McpEventRelay bind: if the run exists and has no run.created/run.updated events, isTerminalRunStatus (completed, failed, cancelled, interrupted, rolled_back) now returns source_history_pruned before the retained-history sentinel check. A new it.effect.each test covers all five statuses with an older retained sentinel and asserts no binding row. Queued and live runs still go through the unchanged requestedAt-vs-oldest-event check, and the 26b matrix (empty/newer/same/pruned) now pins status queued. OK.
2. CodexMcpIsolation: dropped --profile/-p, profile= and profiles.* handling. Only top-level mcp_servers and -c mcp_servers.* overrides count now. The test checks that --profile travel, -c profile=travel and a -c profiles.work.mcp_servers... override all leave the home servers unchanged. OK.
3. Migration: renamed to 014_McpRelayDiagnostics.ts, imported as ForkMigration0014 and registered as [14, "McpRelayDiagnostics", ...]. ForkMigrations.test.ts imports the 014 path. 013_WorkLedgerHygiene keeps its file, its registration and the WorkLedger.test import. OK.
4. Range-diff: commit 2 (81cab48) is identical. Commit 1 differs only in the migration renumber and its rebase context. Commit 3 touches only the 4 files above. Nothing else changed.
   Nit (not blocking): the missing-history matrix has no explicit running case. That path is unchanged, so this is coverage only.
   Verdict: no blocking findings