Skip to content

Code graph and spill-store follow-ups from the #433 review #435

Open

Description

@whykusanagi

Four review findings (CodeRabbit, Major) on the round-3 hardening in #433. They were moved to the 2.0.1 sprint so 2.0.0 could ship. None of them is a security issue. Each needs a test that fails before the fix.

  1. Code graph .gitignore is read by path during a pass.
    • Build and Update walk the pinned workspace root, but LoadGitignore still reads .gitignore through the workspace path.
    • Fix: read it through the pass's pinned root (idx.passRoot).
    • This also covers the Minor at codegraph/index.go:1144.
  2. Spill pruning identifies session directories by name only.
    • Any directory under tool-results whose name looks like a session id (for example backup) is counted toward the quota, and can be removed by pruning.
    • Fix: write a marker file into each spill directory, and only count or prune directories that carry it.
  3. Case variants of the reserved pruned store.
    • On a case-insensitive filesystem, session id PRUNED resolves to the compaction recall store.
    • Fix: apply the reserved-name rule case-insensitively in session validation, in remapping, and in directory recognition.
  4. Remapped session ids can collide.
    • Session pruned is remapped to pruned-session, but a real session named pruned-session is also accepted, so two sessions can share spill files.
    • Fix: use a collision-free remap, or reject the reserved name before CapToolResult.

Also from the same review (Minor):

  • When /set-model fails, ChangeModel still clears the resumed session's hold. The hold should be kept on failure (tui/app.go:1825).

Activity

  1. added this to the v2.0.1 milestone on Oct 9, 2026
  2. whykusanagi commented on Oct 14, 2026

    @whykusanagi
    OwnerAuthor

    Status after #446 merged: the pinned .gitignore read, spill-directory ownership marker, case-insensitive pruned reservation, and collision-prone remap have regression tests and are on main. Still open: the failed /set-model/ChangeModel resumed-session hold behavior in tui/app.go; leaving this issue open until that test and fix land. Pre-marker spill directories intentionally remain unpruned for safety.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions