Background
After the attach-mode pivot landed on feat/attach-mode, a review pass surfaced a handful of small cleanup items that aren't blockers but are worth handling together as one focused maintenance pass.
These are deliberately scoped to code — doc cleanups landed inline with the pivot.
Items
1. Extract shared set_dotted_path utility
src-tauri/src/cli_layer.rs:62-87 and src-tauri/src/env_layer.rs:64-87 carry character-for-character duplicate implementations of set_dotted_path + set_segments (~25 lines × 2). Both callers use it identically.
Move to one place — either a new src-tauri/src/dotted_path.rs module, or as a pub(crate) helper in one of the existing files and import from the other.
2. Delete unreachable defensive branch in cli_layer::append_to_array_path
src-tauri/src/cli_layer.rs:94-100 has an else arm whose own comment admits "shouldn't happen for a fresh cli layer, but be defensive." raw is freshly built per call (Value::Object(Map::new())), and the only writes at that path go through append_to_array_path itself, which always lands an array. The branch is unreachable.
Per CLAUDE.md: "Don't add error handling for scenarios that can't happen." Delete the else branch; the if let Some(arr) = entry.as_array_mut() can become an expect() or just unwrap, since the or_insert_with above guarantees an array.
3. Strip pivot/legacy framing from code comments
src-tauri/src/settings.rs carries comments referencing "the pivot" and "legacy behavior" — historical-transition language that will rot:
- Line 294:
/// over the legacy "knobs.cc's own CWD" fallback.
- Line 296:
/// Pre-pivot fallback: read project/project_local relative to whatever
- Line 299:
/// pivot will always supply Attached or Picked.
- Line 372:
// when available; otherwise they fall back to the legacy behaviors
- Line 439:
/// - neither — legacy "knobs.cc's own CWD" behavior, kept so existing
- Line 912: test name
current_dir_fallback_matches_legacy_behavior
- Line 913:
// The pre-pivot behavior: project root resolves to whatever
Per CLAUDE.md: "Don't reference the current task, fix, or callers… those belong in the PR description and rot as the codebase evolves." Rewrite to describe semantic intent without the historical framing. Test name → something like current_dir_fallback_uses_env_cwd.
4. Trim the rename_all war-story comment
src-tauri/src/settings.rs:325-358 carries a long block explaining "Without this attribute, attached_pid: 75618 from JS silently deserialized to None and the snapshot fell back to ProjectSource::CurrentDir." That's a record of a bug we hit, not a useful WHY for future readers.
Trim to one short line: something like "Tauri 2 defaults to camelCase command args; this project's wire format is snake_case throughout."
5. Add two missing test cases
cli_layer.rs: a stringArrayMulti-at-EOF case (e.g. argv = ["claude", "--add-dir"], no values follow). Symmetric with the existing string-at-EOF test.
envVars.test.ts: a "var only in attached, not in shell or settings" case. Symmetric with the existing 3-source test.
Out of scope
Items considered and rejected during the review:
- Renaming
PlatformStatus::Ok → Supported (minor mental collision with Result::Ok; not worth the churn).
- Extracting
useSessionGrounding hook from App.tsx (premature abstraction).
- Documenting magic number 40 in
SessionPill's truncatePath (visual tuning constant; would rot).
- Adding error handling around
expect() on catalog parse (project convention is fail-fast on required-at-compile-time data).
Estimate
Small focused PR — ~30-45 minutes of edits, low regression risk (mostly deletions and rewordings + 2 small tests).
Background
After the attach-mode pivot landed on
feat/attach-mode, a review pass surfaced a handful of small cleanup items that aren't blockers but are worth handling together as one focused maintenance pass.These are deliberately scoped to code — doc cleanups landed inline with the pivot.
Items
1. Extract shared
set_dotted_pathutilitysrc-tauri/src/cli_layer.rs:62-87andsrc-tauri/src/env_layer.rs:64-87carry character-for-character duplicate implementations ofset_dotted_path+set_segments(~25 lines × 2). Both callers use it identically.Move to one place — either a new
src-tauri/src/dotted_path.rsmodule, or as apub(crate)helper in one of the existing files and import from the other.2. Delete unreachable defensive branch in
cli_layer::append_to_array_pathsrc-tauri/src/cli_layer.rs:94-100has anelsearm whose own comment admits "shouldn't happen for a fresh cli layer, but be defensive."rawis freshly built per call (Value::Object(Map::new())), and the only writes at that path go throughappend_to_array_pathitself, which always lands an array. The branch is unreachable.Per CLAUDE.md: "Don't add error handling for scenarios that can't happen." Delete the else branch; the
if let Some(arr) = entry.as_array_mut()can become anexpect()or just unwrap, since theor_insert_withabove guarantees an array.3. Strip pivot/legacy framing from code comments
src-tauri/src/settings.rscarries comments referencing "the pivot" and "legacy behavior" — historical-transition language that will rot:/// over the legacy "knobs.cc's own CWD" fallback./// Pre-pivot fallback: read project/project_local relative to whatever/// pivot will always supply Attached or Picked.// when available; otherwise they fall back to the legacy behaviors/// - neither — legacy "knobs.cc's own CWD" behavior, kept so existingcurrent_dir_fallback_matches_legacy_behavior// The pre-pivot behavior: project root resolves to whateverPer CLAUDE.md: "Don't reference the current task, fix, or callers… those belong in the PR description and rot as the codebase evolves." Rewrite to describe semantic intent without the historical framing. Test name → something like
current_dir_fallback_uses_env_cwd.4. Trim the
rename_allwar-story commentsrc-tauri/src/settings.rs:325-358carries a long block explaining "Without this attribute,attached_pid: 75618from JS silently deserialized toNoneand the snapshot fell back toProjectSource::CurrentDir." That's a record of a bug we hit, not a useful WHY for future readers.Trim to one short line: something like "Tauri 2 defaults to camelCase command args; this project's wire format is snake_case throughout."
5. Add two missing test cases
cli_layer.rs: astringArrayMulti-at-EOF case (e.g.argv = ["claude", "--add-dir"], no values follow). Symmetric with the existingstring-at-EOF test.envVars.test.ts: a "var only inattached, not in shell or settings" case. Symmetric with the existing 3-source test.Out of scope
Items considered and rejected during the review:
PlatformStatus::Ok→Supported(minor mental collision withResult::Ok; not worth the churn).useSessionGroundinghook fromApp.tsx(premature abstraction).SessionPill'struncatePath(visual tuning constant; would rot).expect()on catalog parse (project convention is fail-fast on required-at-compile-time data).Estimate
Small focused PR — ~30-45 minutes of edits, low regression risk (mostly deletions and rewordings + 2 small tests).