Skip to content

Post-pivot maintenance: code cleanup punch-list from attach-mode review #14

@samkeen

Description

@samkeen

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::OkSupported (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).

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions