Skip to content

fix(tui): resolve the state root before opening session-side state - #14

Open
SparkofSpike wants to merge 4 commits into
mirror/upstream-mainfrom
fix/state-root-junction
Open

SparkofSpike wants to merge 4 commits into
mirror/upstream-mainfrom
fix/state-root-junction

Conversation

@SparkofSpike

@SparkofSpike SparkofSpike commented Oct 26, 2026 •

Copy link
Copy Markdown
Owner

fix(tui): resolve the state root before opening session-side state files

Summary

A user who relocates application state to another volume — the documented way
to keep ~/.codewhale off the system drive — cannot save a session at all.
Every save fails with a red "cannot save session" notice, and the log says:

WARN persistence: persistence write failed what=session:1db78329-... \
  error_kind=InvalidInput error=Fleet artifact path must stay within the workspace

The save itself never starts. SessionManager::save_session_inner hydrates the
approval receipts before it serializes anything, and
ApprovalReceiptStore::log_file opens <sessions>/<id>/approval_receipts.jsonl
through the strict WorkspaceFile::open. On Windows that opener walks the root
component by component and refuses the junction that is the state root, so
every save dies on a state directory the user deliberately relocated.

Changes

  • approval_log.rs: the receipt log opens through
    WorkspaceFile::open_resolved_root, the opener added in Workflow resume: replay completed steps from a prior run codewhale-hq/Codewhale#6900 for exactly
    this case. The root is canonicalized once; every component below it is still
    refused when it is a link. This is the change that unsticks session saving.
  • session_export.rs: the artifact reads and both directory checks in a
    session archive take the same opener, so sessions export works on the same
    layout instead of failing where saving already failed.
  • tui/clipboard.rs, tui/file_mention.rs, tui/pet_watch/*: the
    clipboard image sink, the screencapture attachment sink and the pet habitat
    store all sit under the state root too. Same one-call change each; the
    link-refusing walk below the root is unchanged. The two sinks that fall back
    to a workspace root when no home directory resolves pick the strict opener
    for that fallback, so a caller-supplied root keeps its old rule.

Nothing widens the boundary: open_resolved_root resolves the root and then
runs the same per-component refusal, so a link below the state root is still
an error. WorkspaceFile::open remains the opener for caller-supplied
workspace roots.

Reproduction

  1. Make %USERPROFILE%\.codewhale a junction to another volume
    (mklink /J %USERPROFILE%\.codewhale D:\CodeWhaleData).
  2. Start a terminal session and let it autosave, or run /save.
  3. Before: the save is refused and logs/tui-*.log records
    persistence write failed ... error=Fleet artifact path must stay within the workspace
    once per attempt, with no sessions/<id>.json written.
  4. After: the session writes to the junction target and the notice is gone.

The same failure reproduces on 0.10.2 without any repository change, so this is
not a regression from an unreleased branch.

Testing

  • cargo check -p codewhale-tui — exit 0, no warnings.
  • cargo fmt --all -- --check — clean.
  • cargo build --release --locked -p codewhale-cli -p codewhale-tui — builds
    the shipped binaries from this branch.
  • End-to-end on the reported layout. Built binary, with
    %USERPROFILE%\.codewhale left as a junction to D:\CodeWhaleData:
    codewhale exec --auto "Run the shell command: echo CWJUNCTION_OK" finished
    with exit 0 and wrote sessions/cf02b940-8436-4289-b16b-756d7a516228.json
    (58,986 bytes) through the junction. Before the change the same state root
    produced persistence write failed what=session:... error=Fleet artifact path must stay within the workspace and no document.
  • The Windows link-refusal test
    (linked_state_root_resolves_once_and_keeps_the_inner_rule in
    fleet/files.rs) already pins the property this change relies on: the
    resolved-root opener writes through a root junction and still refuses a link
    below it. It was not run locally on this machine — the codewhale-tui test
    build runs out of memory here — and PR CI's Windows lane only runs
    cargo check, so treat that test as inherited rather than re-verified.

Checklist

  • No behavior change for an unlinked state root (Unix open_resolved_root
    is the same call as open).
  • Trust boundary unchanged: links below the root stay refused, and the two
    sinks that can fall back to a workspace root keep the strict opener
    there.
  • No new tests or comments beyond the call-site rationale already carried
    by open_resolved_root.

署名

🤖 由 SpikeBot 000(CodeWhale-LOCAL) 生成

@SparkofSpike

Copy link
Copy Markdown
Owner Author

Review

Verdict: the fix is correct and complete for the reported failure. Save on a relocated state root now gets past approval-receipt hydration.

Traced path: SessionManager::save_session_inner (crates/tui/src/session_manager.rs:2720) → hydrate_approval_receipts (:2766, fn at :1641) → approval_receipt_store() (:1633) → ApprovalReceiptStore::log_file (crates/tui/src/approval_log.rs:461) → the converted open at :466. That is the only confined open on the save path: the session document itself goes through plain write_atomic (session_manager.rs:2770) and checkpoints the same (:2812), which is why the junction never blocked them. Session load uses the same log_file, so it is fixed by the same change, as are replay/compact/load_if_present.

Trust boundary. Correct per file:

  • approval_log.rs:466 — root is SessionManager::sessions_dir (default_sessions_dir → ensure_state_dir, session_manager.rs:4610), the user's own state root.
  • session_export.rs:218/281/399 — root is the canonicalized sessions_dir param, state root.
  • pet_watch/owner.rs:51/316/364, persistence.rs:29-30 — ~/.codewhale/pet-shared or CODEWHALE_PET_HOME, user-owned.
    Strict open is retained where the root is caller-supplied (runtime_api/workspace.rs:636, fs_confined.rs:21, fleet/artifacts.rs:27,51, fleet/store.rs), so the invariant holds there. open_resolved_root runs the same per-component refusal below the resolved root, so an inner link is still an error — pinned by the test cited below.

Findings

  1. nit — crates/tui/src/tui/clipboard.rs:961-970 and crates/tui/src/tui/file_mention.rs:812-818 pass dir.parent() as the root, and both sinks fall back to a workspace root when the home directory is unavailable (clipboard_images_dir_for_home(workspace, None); screenshot_stabilization_dir, file_mention.rs:764-769). In that fallback the root is caller-supplied, and on Windows it now gets canonicalized instead of refused. Extremely narrow (no home dir), but the two call sites should either branch on which root they hold or keep open for the fallback.
  2. nit — test coverage. linked_state_root_resolves_once_and_keeps_the_inner_rule (crates/tui/src/fleet/files.rs:1167) already pins both halves (junction root accepted after resolve; inner junction still refused, and the strict opener still refuses the root), so the property does not need a new test. Note this PR's CI will not run it: test-windows-pr only does cargo check --workspace --all-targets plus the installer helper test on PRs (ci.yml:1224-1256); the Windows suite runs in final validation. There is also no test tying hydrate_approval_receipts to a junctioned sessions_dir, i.e. the report's end-to-end shape.
  3. nit — session_export.rs:168 already canonicalizes sessions_dir, so the resolved opener canonicalizes it a second time. Harmless.

Unlinked case: unchanged. On Unix open_resolved_root is literally open (files.rs:89-96); on Windows it canonicalizes then runs the identical walk (files.rs:685-695), which for an unlinked absolute root is the same directory. The NotFound arm in session_artifacts_dir still maps a missing store to Ok(None).

Nothing missed on save/load: no other confined open sits under the state root there, and artifacts.rs:218 / runtime_api/sessions.rs:1433,1519 already used the resolved variant. Not verified: whether any reject_linked_path caller outside these paths targets a state root (mcp.rs:104 etc. were not traced).

Attribution

🤖 Generated by SpikeBot 003(ClaudeCode-JP)

@SparkofSpike

Copy link
Copy Markdown
Owner Author

Follow-up review

Nit 1 is closed. Commit 4d7cd8629 does what it claims at both sites.

  • crates/tui/src/tui/clipboard.rs:962-978 and crates/tui/src/tui/file_mention.rs:815-827 now derive the opener from the root they actually hold (dir.parent() / artifact_dir.parent()) and compare it against user_home().join(CODEWHALE_APP_DIR). The resolved opener is reachable only for <home>/.codewhale; everything else goes back to WorkspaceFile::open.
  • The comparison matches both producers. clipboard_images_dir (crates/tui/src/config/paths.rs:264-266) calls clipboard_images_dir_for_home(workspace, user_home()) → home.join(".codewhale").join("clipboard-images"), so parent() is exactly home.join(".codewhale"); screenshot_stabilization_dir (file_mention.rs:764-769) is home.join(".codewhale").join("attachments"), same parent. Both read codewhale_paths::user_home(), so the guard cannot disagree with them about whether a home resolved — in the no-home fallback the guard is false by construction, which is exactly the case where the root is caller-supplied.
  • Fallback boundary restored: with root = <workspace>, WorkspaceFile::open runs, and its Windows walk (crates/tui/src/fleet/files.rs:655-677 → open_component, :584-604) refuses a reparse point on every component including the root. On Unix open and open_resolved_root are the same fn (:89-96), so nothing changes there.

No case found where the two disagree and the wrong opener wins. The only construct that can hit the guard with a non-user_home home is the test-facing clipboard_images_dir_for_home(workspace, Some(home)) (clipboard.rs:1291), and it fails safe — strict opener, no link, write still succeeds.

Minor, non-blocking:

  • nit — P3, consistency: the guard uses codewhale_paths::CODEWHALE_APP_DIR, but both producers still hardcode ".codewhale" (config/paths.rs:271, file_mention.rs:766). They agree today (crates/paths/src/lib.rs:13); if the literal ever changed the guard would silently stop matching. Fails closed (junction save breaks again, no boundary widened), so it is a readability/drift nit, not a defect — using the constant in the producers closes it.
  • nit — P3, coverage: the state_root == true branch of the guard has no test (existing tests go through temp homes, i.e. the strict branch). Optional given files.rs:1167 already pins the two openers.

No redundancy worth removing, and no remaining site in this PR passes a root it does not own: approval_log.rs:466 and session_export.rs:218/281/399 hold default_sessions_dir → ensure_state_dir (and unsealed_sessions_dir() is None outside tests, session_manager.rs:4646-4648); pet_watch/owner.rs:51/316/364 and persistence.rs:29-30 hold ~/.codewhale/pet-shared or CODEWHALE_PET_HOME.

Unrelated to the code: the PR closes an issue check still fails on this PR (body has neither Refs #… nor No-Issue: …), so it will not merge as-is.

Attribution

🤖 Generated by SpikeBot 003(ClaudeCode-JP)

A user who relocates application state to another volume cannot save a
session at all. Every save fails with the red "cannot save session"
notice and the log records

  persistence write failed what=session:<id> error_kind=InvalidInput
  error=Fleet artifact path must stay within the workspace

`SessionManager::save_session_inner` hydrates approval receipts before it
serializes anything, and `ApprovalReceiptStore::log_file` opened
`<sessions>/<id>/approval_receipts.jsonl` through the strict
`WorkspaceFile::open`. On Windows that opener walks the root component by
component and refuses the junction that is the state root, so the save died
before producing a document.

Storing session receipt data went from plain file I/O (0.10.2) to the
confined opener, which is the right direction, but the state root is a root
the user owns and may relocate. `open_resolved_root` exists for exactly
that (codewhale-hq#6947): it resolves the root once and then runs the same link-refusing
walk below it. These call sites now use it:

- approval receipt log (the one that blocked saving)
- session archive artifact reads and the two directory checks
- clipboard image sink, screencapture attachment sink
- pet habitat store, descriptor and lifetime lock

No boundary is widened: a link below the state root is still refused, and
`WorkspaceFile::open` stays the opener for caller-supplied workspace roots.

Verified with cargo check -p codewhale-tui (exit 0) and cargo fmt. The
0.10.2 binary with the same junction layout reproduces the failure, so this
is not a regression from an unreleased branch.
The review of the previous commit found that two of the converted call sites
do not always hold the state root. `clipboard_images_dir_for_home` and
`screenshot_stabilization_dir` fall back to a directory under the workspace
when no home directory resolves, so `dir.parent()` is then a caller-supplied
workspace root — and resolving it would have widened the rule that a link at
the workspace root is refused.

Both sites now pick the opener from the root they actually hold: the resolved
opener only for `<home>/.codewhale`, the strict opener otherwise. The normal
path (a home directory exists) keeps the junction fix; the narrow fallback
keeps the boundary it had.
@SparkofSpike
SparkofSpike force-pushed the fix/state-root-junction branch from 4d7cd86 to 3c31785 Compare October 27, 2026 02:17
CodeWhale Bot and others added 2 commits October 26, 2026 22:02
8155802 added three Unreleased entries to CHANGELOG.md without running
scripts/sync-changelog.sh, so the Version drift check fails on every PR
based on main (seen on codewhale-hq#7097 and codewhale-hq#7094). Regenerated from origin/main's
root CHANGELOG.md; scripts/sync-changelog.sh --check passes on the result.
Built with a temporary index so the shared checkout's uncommitted work is
untouched.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants