fix(server): new Codex home entries no longer disable shadow-home providers#32058

🟠 High Drivers/CodexHomeLayout.ts:301

A failed rollback leaves the effective home partially restored, but setup still treats it as usable and continues starting Codex. The restore handler logs and swallows failures from removing created links and renaming backups back, then the outer handler returns false; propagate rollback failures so setup cannot proceed with entries stranded in the backup directory.

Valid. This is fixed in ee400bff2f.

When a replacement fails and its restore also fails, the shadow home can hold a SQLite base without its WAL, or a base next to the shared WAL link. Continuing setup would start Codex on that state. restore now collects every remove and rename failure. If there are any, it logs them along with the original cause and fails setup with a CodexShadowHomeFileSystemError for the first failure (remove or rename). For a rename, its path is where the entry was left in the backup.

A single failure still restores, logs a warning and continues, because the group is back to its original state. The next setup finishes the move: the base is still a regular file, so the group is resolved again.

There is a new test, fails setup instead of starting Codex on a SQLite family it could not restore. It fails on the previous head because setup succeeded there. It passes now and checks that the WAL is still intact at the reported backup path.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

🟠 High Drivers/CodexHomeLayout.ts:352

Cancelling setup after a sidecar has been renamed leaves the SQLite family split between the shadow home and its backup. Interruption bypasses this Effect.catch, so restore never runs; make the mutation and rollback interruption-safe, for example with an interruption finalizer.

No longer relevant as of 95b84be92d397e8a0b64820991a765258523d4fe

Valid. This is fixed in 95b84be92d.

The move-and-link step and its restore now run under Effect.uninterruptible. The step is a few local renames and symlinks for a single group, so an interruption waits until the group is either fully replaced or restored.

There is a new test, finishes replacing a SQLite family when setup is interrupted mid-move. It blocks the base rename after the WAL has already moved, interrupts the setup fiber with interruptUnsafe, and then releases the rename. On the previous head the base stayed a regular file beside a moved WAL. Now both entries end up linked.

Sorry, I'm unable to act on this request because you do not have permissions within this repository.

External image Minor · Functional Correctness · Quick win

Handle non-SQLite inspection failures per entry.

ensureSymlink calls readLinkState directly for non-SQLite entries. A non-NotFound inspection error becomes CodexShadowHomeFileSystemError and rejects the outer Effect.forEach, so later entries are not processed. Add the same log-and-skip handling used for SQLite inspection failures, and cover it with a non-SQLite readLink failure test.

Declining for this PR: this behavior already exists on main and isn't part of #5817.

On main, any readlink error other than ENOENT/EINVAL on a non-SQLite entry already fails setup, and this PR doesn't change that path. The issue is about regular-file conflicts, which come back as EINVAL and are now handled. The SQLite sibling reads are recoverable only because this PR adds them: they read files the shared home may not even have, so they shouldn't add a new way to fail. Making every unexpected filesystem error non-fatal is a separate change, and it would need its own reasoning about when a broken shadow home should still start Codex.

@jakeleventhal You’re right. Unexpected non-SQLite readLink errors already fail setup, and changing that behavior is outside this PR’s scope. I withdraw the request.

Would you like a follow-up issue to evaluate that behavior?

Review thread resolved.

_{You are interacting with an AI system.}

External image Minor · Functional Correctness · Quick win

Handle atomically replaced shared entries without repeated backup runs.

Codex 0.167.0 publishes cloud-config-bundle-cache.json through a temporary file and persist, so it can replace the shadow-home symlink with a real file. The next materializeCodexShadowHome call then moves that newer file to a new backup run and links the older shared copy. Repeated updates can accumulate unbounded backup runs and keep newer shadow data out of the active home. Define an idempotent reconciliation and retention policy, and add a regression that performs an atomic replacement between setup calls. installation_id is not an example of this path because Codex writes through its existing file handle.

This is real, but the fix belongs in the private/shared classification, which this PR deliberately leaves unchanged. Declining here.

cloud-config-bundle-cache.json holds per-account data (account ids and workspace policy), as reported in #5817. Sharing it is wrong whether or not Codex replaces it atomically. The right fix is to treat it like auth.json and models_cache.json. Triage on #5817 put that boundary under maintainer review because it touches auth, so the PR description flags it, along with installation_id and memories vs memories_v2, instead of changing it.

Until then, the behavior is safe and bounded. Each startup that finds a newer shadow copy moves it into a timestamped backup and logs the path, so nothing is lost. The cost is one small file per restart. Before this PR, the same state disabled the provider. A retention policy that deletes old backups would conflict with the requirement to never delete shadow data.

@jakeleventhal Understood. I accept leaving private/shared classification to maintainer review under #5817. My request for backup cleanup was inappropriate given the requirement to preserve shadow data.

The supplied code preserves the conflicting copy and logs its backup path. Total backup storage is not bounded if Codex repeatedly replaces the symlink, but that does not require a retention change in this PR.

The behavior remains a known limitation, not a completed fix. Would you like a separate follow-up issue, or should classification remain tracked under #5817?

_{You are interacting with an AI system.}

Keep it under #5817. The PR description lists the entries that need a classification decision, so a separate issue would only split the discussion.

@jakeleventhal Understood. The private/shared classification decision stays under #5817. I will not create a separate issue or request a retention change in this PR.

_{You are interacting with an AI system.}

External image Minor · Functional Correctness · Quick win

Preserve all replacement and restoration failures in the returned error.

When replacement fails and restoration also fails, the returned error contains only the first restoration failure. The original replacement failure and any additional restoration failures remain only in logs. The caller-visible error therefore cannot identify every stranded member or the original replacement cause.

Return a structured error that preserves the original replacement failure, all restoration failures, and each affected backup and destination path.

Source: Learnings

Declining. A structured multi-failure error would not give the user anything they can't already find.

• All members of one group go into the same backup folder, so the returned error's path already points to the folder holding every stranded file.
• The logError call just before it records the original replacement cause and every restore failure, with their paths, at the moment of failure.
• The only consumer is the provider's one-line "failed to create instance" status. A list of errors would be flattened back into one string there.
• This path needs a replacement failure and then a restore failure on the same group, and the next setup re-resolves that group anyway.

@jakeleventhal Understood. The existing logError records the replacement cause and all restoration failures before setup stops. I did not establish a concrete need to duplicate those diagnostics in a structured return error. I withdraw this request.

Review thread resolved.

_{You are interacting with an AI system.}