Follow-up from the PR #1084 review of #954. Two narrow gaps deliberately left out of that change as out-of-scope.
1. record_installation does an unlocked read-modify-write
codeframe/core/installer.py:record_installation reads environment.json, adds one entry, and writes the whole file back. #954 made that write atomic (no truncation on crash), but there is no lock around the read-modify-write, so two concurrent installs can still lose an entry: both read the same base, and the second write drops the first tool.
credentials.py already models the fix — CredentialStore._store_lock() wraps its read-modify-write in a thread lock plus an optional cross-process filelock.
Low practical impact today: installs are usually sequential and driven by one cf env install run. It becomes real if cf env install ever parallelises tools.
2. A corrupted per-user credential store yields an unformatted 500
get_credential_manager() (the FastAPI dependency used by ui/routers/settings_v2.py and github_integrations_v2.py) calls _migrate_machine_wide_entries() → .store() before the route body runs. Since #954, .store() can raise CredentialStoreUnreadableError. Because it is raised in the dependency rather than the handler, it bypasses each route’s own try/except Exception → api_error(...), so the client gets a bare 500 instead of the formatted error the routes produce everywhere else.
The CLI equivalent of this was fixed in #954 (cf auth setup / cf auth rotate now catch it and print the exception’s recovery text); the HTTP surface was not.
Acceptance criteria
Context
Follow-up from the PR #1084 review of #954. Two narrow gaps deliberately left out of that change as out-of-scope.
1.
record_installationdoes an unlocked read-modify-writecodeframe/core/installer.py:record_installationreadsenvironment.json, adds one entry, and writes the whole file back. #954 made that write atomic (no truncation on crash), but there is no lock around the read-modify-write, so two concurrent installs can still lose an entry: both read the same base, and the second write drops the first tool.credentials.pyalready models the fix —CredentialStore._store_lock()wraps its read-modify-write in a thread lock plus an optional cross-processfilelock.Low practical impact today: installs are usually sequential and driven by one
cf env installrun. It becomes real ifcf env installever parallelises tools.2. A corrupted per-user credential store yields an unformatted 500
get_credential_manager()(the FastAPI dependency used byui/routers/settings_v2.pyandgithub_integrations_v2.py) calls_migrate_machine_wide_entries()→.store()before the route body runs. Since #954,.store()can raiseCredentialStoreUnreadableError. Because it is raised in the dependency rather than the handler, it bypasses each route’s owntry/except Exception→api_error(...), so the client gets a bare 500 instead of the formatted error the routes produce everywhere else.The CLI equivalent of this was fixed in #954 (
cf auth setup/cf auth rotatenow catch it and print the exception’s recovery text); the HTTP surface was not.Acceptance criteria
record_installationserialises its read-modify-write (reuse the_store_lockpattern); a two-thread test records both tools with neither lostapi_error(...)shape, not a bare 500; test asserts the status code and body shapeContext
claude-reviewbot on fix(core): make config, credential, installer and workspace-init writes crash-safe (#954) #1084 ("Nits / non-blocking" and the smaller same-shape issue)