[HemSoft]

Summary

A refused settings save could still replace an existing account's key or leave a new account's key orphaned. The account dialog now reads what the key write replaces, then restores it or removes the new entry when settings are refused. It preserves a key another writer changed meanwhile, keeps OpenCode's long-secret format and lock, and reports an undo failure in the dialog.

The PR also exercises the real dialog with in-memory credential failures and renders the two new error messages in the existing Linux CI job.

Acceptance Criteria

• Blank-name and busy-lock refusals preserve an existing account's old key, while the dialog is open and after cancel.
• A refused new account leaves no key under its generated id.
• Store-read or key-write failure saves no account; successful saves replace the key, and a blank key keeps it.
• Refused OpenCode OAuth-to-cookie changes restore the prior chunked sign-in, or leave a signed-out account without an abandoned cookie.
• A key replaced by another writer is preserved.
• Failed restoration or deletion is reported beside the settings error, without exposing the key.

Definition of Done

• Real GPUI dialog regressions cover refusals, successful saves, store failures, concurrent replacement and OpenCode rollback.
• All seven required checks pass at 51072ef in CI run 42357712838, including the metrics and mutation gates.
• Four current-head screenshots are embedded below through verified GitHub attachments.
• SFL Round 1 PASS, zero findings covers production head 547453f. Subsequent changes are CI, the test-only capture fixture, CI binding tests and a unit test inside the credential module's #[cfg(test)] section. The credential source before that section is byte-identical to the reviewed head; shipped code is unchanged, preserving PASS under the Charter's CI/test-only rule.

Validation

• RED with the new dialog tests and original 58a0945 production code: four failures reproduced overwritten/orphaned keys and missing rollback-error messages; six controls passed.
• GREEN at production head 547453f: eleven focused UI cases pass. Local workspace tests pass with 444 app tests and one ignored capture; fmt, workspace clippy and the production-binary check pass.
• Linux, Windows and macOS workspace tests, clippy and production-binary checks pass in the current-head CI run. The Windows native credential test remains enabled.
• CI workflow bindings: seven pass; the new case fails against the previous workflow. CI input-selection tests: 21 run, one skipped.

Metrics follow-up

The first CI run rejected lost coverage in MemoryCredentialStore::write: the corrected dialog now returns on a failed pre-read, so its former failure test no longer reaches that write branch. Added refused_memory_store_operations_leave_saved_secrets_unchanged to test refused reads, replacement/new writes and deletion directly, then verify the saved value remains and the new id is absent.

The exact in-memory test, store clippy and fmt pass locally. The new hosted metrics artifact has failures: [], with both write branches exercised. No metric scope, threshold or production implementation changed.

An earlier broad local credential-test filter also invoked the native Windows integration test; it failed with error 1312 in the SSH session. Four in-memory cases passed there. The focused follow-up uses memory only, and the native test passes in hosted Windows CI.

Rendered dialog states

The existing Ubuntu rust job rendered these at 51072ef838fbacc5db9410bfe340e46b734b18cc, run 42357712838, artifact account-dialog-captures / 13568198480. The command is CODEXBAR_CAPTURE_DIR=<absolute-path> cargo test -p codexbar-app --bin codexbar --locked -- --ignored --exact captures::captures_refused_key_320 --nocapture.

The fixture uses an in-memory store, temporary settings and no providers. It selects Light and Dark explicitly at 1440x960 logical pixels. The parent verified artifact head/digest, all four PNG hashes and their readable dialog messages. The current frames byte-match the previously inspected render.

Existing account: previous key could not be restored, Light.

Existing account: previous key could not be restored, Dark.

New account: abandoned key could not be removed, Light.

New account: abandoned key could not be removed, Dark.

No recording: these are static dialog messages; the headless fixture produces still frames.

The standing Chief review rule preserves SFL PASS for test, CI, coverage and metrics-only follow-ups. The status helper currently has no exact-head AI marker; the recorded review at production head 547453f and the unchanged-production proof above satisfy that rule.

Risks and deferred work

A save now requires a credential read before writing. If the read fails, no key is written. If rollback fails, the dialog explicitly describes the remaining saved-key state. Cleanup of orphaned keys from older app versions is outside this issue.

Closes #320

[HemSoft]

@sfl review

[hemsoft-sfl-reviewer]

✅ Verdict: PASS • Round 1

Confidence: 8/10

Review done by claude-opus-5-5 (same family: same-family: codex gated: codex Weekly 168% SUS 60 🔴 pause, defer)

The account dialog now records the secret a key write replaces and, if the settings save is refused, restores it, or removes a key that was written for a new account. It also adds UI tests and headless captures for refused saves, failed undo and concurrent replacement. I found no evidenced Critical, High, Medium or Low defect: the rollback uses the same id, the same short or long write path and the same OpenCode exclusive lock as the original write, and it skips the undo if another writer changed the secret since.

🔨 Full-Spectrum Review Summary

Effort: high
Review time: 2m 49s
Token count: Unavailable

Pass	🔴 Critical	🟠 High	🟡 Medium	⚪ Low
🔒 Security	0	0	0	0
✅ Accuracy	0	0	0	0
🧰 Quality & Maintainability	0	0	0	0
Total	0	0	0	0

Key Findings

No material defects found.

Coverage

• Files reviewed:
  • crates/codexbar-app/src/settings_view.rs
  • crates/codexbar-app/src/ui_tests.rs
  • crates/codexbar-app/src/captures.rs
• Context read:
  • crates/codexbar-store/src/credentials.rs
  • crates/codexbar-store/src/settings.rs
  • crates/codexbar-store/src/lock.rs
  • crates/codexbar-app/src/settings_hub.rs
  • crates/codexbar-app/src/catalog.rs
  • crates/codexbar-app/src/opencode_sign_in.rs
  • crates/codexbar-providers/src/opencode_console.rs
  • rust-toolchain.toml
  • Cargo.toml
  • AGENTS.md
• Verified:
  • Compare base_commit.sha is 58a0945970ab95f5a9e3c9e770473813f7c32105, the last commit is 547453ff6cf2dee4856ec0fc92281a5daf1c62fb, the branch is 1 commit ahead and 0 behind, and the PR's head and base match
  • ReplacedSecret::write reads the previous secret before writing and returns an error without writing if that read fails, so a key write failure still saves no account
  • ReplacedSecret::restore re-reads under the same long or short mode and the same exclusive lock, and changes nothing unless the stored secret still equals the one written
  • The long path restores with write_long. The cookie write deleted the old parts, so write_long writes the earlier sign-in again as fresh parts, because the cookie is not chunked
  • The short path restoring the head of a chunked secret is safe: plain store.write never deletes parts
  • When the billing owner changes, the record gets a new id, and record_id is taken after that change, so the undo targets the id that was written. GitHub Billing has secret: None, so it never takes this path
  • SettingsHub::update returns Err only before write_replacing or when that fails, so a refused save leaves the settings file unchanged and the rollback matches what is on disk
  • exclusive<T> is generic over its return type, so returning Result<Self, _> works
  • hold_settings_lock matches FileLock::acquire: no sharing on Windows, try_lock elsewhere. The Busy message the tests expect matches SettingsError Display
  • is_managed is true for any OpenCode Go account on OAuth, so both new OpenCode tests take the long path. The helpers and fixtures the new tests call exist at the head SHA
  • Every new error-message string the tests assert is built by undo's format! with STORE_SHORT_NAME
• Not verified:
  • CI: the rust jobs on all three platforms, security-scan and rust-metrics were still queued or running at review time. build and rust-mutations passed
  • Whether the stable toolchain CI resolves supports AtomicUsize::try_update, used in the FailsAfter test store
  • rustfmt and clippy cleanliness of the new code, judged by reading only
  • Behavior of the real Windows, Keychain and Secret Service stores during rollback; only their source and the in-memory store were read
  • Interleaving with an OpenCode renewal between the key write and the rollback was reasoned from source only
• Tests run: None. Reviewer execution is disabled.

---

HemSoft full-spectrum review • 3 evidence-based passes • Reviewed head 547453ff6cf2dee4856ec0fc92281a5daf1c62fb • SFL v0