fix(server): Codex shadow homes share thread writer locks with the main home#26286

External image Major · Data Integrity & Integration · Heavy lift

Do not replace a directory that contains active writer locks.

If a Codex process still holds a lock in the shadow-local thread-writer-locks, a later materialization reaches ensureSymlink and recursively removes that directory. The process can retain its open lock file while a new writer locks a different file in the shared directory. Both writers can then write the same rollout. Check for active lock owners and fail safely or coordinate the migration before removing the directory. Codex’s writer-lock implementation holds open lock files for writer ownership. (github.com)

✅ Confirmed as addressed by @t3dotgg

Fixed in 9d895cd. The risk was real: app-servers from a killed server can still run during the next setup (apps/server/src/process/agentScope.ts reaps them only at first spawn). Also, Codex's WriterLockGuard::drop deletes <id>.lock by path, so after the swap it would delete the new writer's lock through the symlink.

Codex deletes each thread lock on release (codex-rs/rollout/src/writer_lock.rs). So ensureSymlink now keeps a local thread-writer-locks directory while it holds any file other than .coordination.lock. It logs a warning, and a later setup finishes the repair after the locks are gone. The new test covers both steps and fails without the guard.

@t3dotgg The supplied code now returns before recursive removal when a thread .lock file remains, excluding .coordination.lock. A later setup can replace the directory after those locks are gone. This addresses the reported active-lock deletion path.

I have not run the regression test.

✅ Review thread resolved.

_{You are interacting with an AI system.}

🟠 High Drivers/CodexHomeLayout.ts:279

Setup succeeds with a local thread-writer-locks directory while sessions remains shared, so shadow-home and main-home writers use different lock namespaces and can concurrently write the same rollout. This branch treats any remaining lock as a warning; fail setup until the locks can be safely migrated instead.

Keeping the warning. Failing setup here would stop the repair from ever finishing.

A <thread>.lock left in the local directory is often stale, for example from an app-server that was killed with SIGKILL. Codex removes stale locks only when a process on that home takes its first writer lock (remove_stale_thread_locks in codex-rs/rollout/src/writer_lock.rs). If setup fails, no shadow app-server starts, so the stale lock stays and the provider stays disabled. A disabled provider is the bug this PR fixes.

With the warning, the shadow home keeps its own lock directory until the locks are gone. That is the same split as before this PR, minus the conflict error. Then a later setup links it (see the shares local thread writer locks once no writer still holds one test). A clean shutdown leaves no thread locks, so in that case the link is made on the first setup.

External image Major · Data Integrity & Integration · Heavy lift

Check coordination ownership before removing the local directory.

If .coordination.lock is held but no thread lock file is present, this condition permits removal of the local directory. Codex can hold that lock through rollout publication without a thread lock file. On systems that allow removal of an open lock file, a new writer can then use the shared coordination lock while the earlier publication remains in progress. Coordinate the migration with Codex’s lock protocol, or defer it while the local coordination lock is held. This is a separate case from the previously reported removal of a directory containing an active thread lock. (github.com)

Declining. To find out whether .coordination.lock is held, we would have to take Codex's flock, and Node has no API for that. A held coordination lock and a free one look the same as files. This is the same check-then-remove race as the scan in https://github.com/pingdotgg/t3code/pull/26286#discussion_r4396981225.

The remove runs only during the one-time repair of a shadow home that was created before this PR. Once the link exists, ensureSymlink never takes this branch again. Before this PR, the same home had a lock namespace split from the main home and no shared serialization at all. We accept the remaining migration risk on purpose.

@t3dotgg Understood. Once the shared symlink exists, ensureSymlink skips the local-directory replacement branch. The coordination-lock race remains a migration risk that you explicitly accept, not a claim that migration is race-free. I won’t press for an additional lock check.

_{You are interacting with an AI system.}

External image Major · Data Integrity & Integration · Heavy lift

Do not complete provider setup with an isolated writer-lock directory.

If a live writer leaves a thread lock in the shadow home, this branch preserves the directory but returns success. CodexDriver.create then continues with that shadow home. A new Codex writer can use its local lock namespace while a writer using the shared home locks the same thread through the shared namespace. Fail setup or prevent new writers until the migration is safe. Codex acquires thread ownership through lock files under its home directory. (raw.githubusercontent.com)

Declining, same reason as https://github.com/pingdotgg/t3code/pull/26286#discussion_r4396973814. Failing setup leaves the shadow provider disabled for good when a lock is stale. A SIGKILLed app-server leaves its <thread>.lock behind, and Codex removes stale locks only when a process on that home takes its first writer lock (remove_stale_thread_locks). A failed setup starts no such process, so nothing ever clears the lock.

Keeping the local directory gives the same split namespace as before this PR, minus the conflict error. A later setup then finishes the repair.

@t3dotgg Understood. I withdraw the setup-failure recommendation. It would prevent Codex from starting the process needed to clear stale locks.

The split lock namespace remains a limitation during migration, not a new regression from this branch. Preserving the local directory lets Codex clear stale locks and allows a later setup to complete the migration. No change requested.

✅ Review thread resolved.

_{You are interacting with an AI system.}

🟠 High Drivers/CodexHomeLayout.ts:263

A shadow-home writer can acquire a thread lock after this scan reports none but before remove(link) runs, so setup deletes a live writer's lock and lets another process acquire a separate lock for the same thread. Codex serializes lock changes with .coordination.lock, but this migration does not acquire it; ensure shadow writers are stopped or synchronize with that lock before removing the directory.

Declining. To sync with .coordination.lock, we would have to take Codex's flock, and Node has no portable API for it. Stopping every shadow writer first would mean failing setup, and that leaves the provider stuck behind stale locks (see https://github.com/pingdotgg/t3code/pull/26286#discussion_r4396973814).

The scan-then-remove window is real. It exists only during the one-time repair of a shadow home created before this PR. T3's own app-servers left from a killed server have stdin closed, so they get no new resume requests. We accept this as a limited migration risk. Before this PR, those writers were not serialized with the main home at all.