fix(T-1032): Claude session resolver fails when listing all projects

[ArjenSchwarz]

Fixes Transit ticket T-1032: Claude session resolver fails when listing all projects

[claude]

Code Review

Overview

This PR fixes T-1032 by refactoring resolveClaude / ResolvePath into a shared findClaudeSessionPath helper that searches all project subdirectories under ~/.claude/projects/ when projectPath is empty, rather than failing because no specific project directory is set.

---

What's good

• Clean extraction – The new findClaudeSessionPath method eliminates the code duplication that previously existed between resolveClaude and ResolvePath for the same path-building logic.
• Proactive security hardening – Adding the ContainsAny("/\\") / Contains("..") guard at the function entry is a good addition even though web.IsPathWithinDir already provides defense-in-depth further down.
• Correct IsPathWithinDir boundary – In the all-project search loop the check is against projectsRoot (not the per-entry baseDir). Since we're iterating over known subdirectories of projectsRoot and sessionID has already been validated, this is the right choice and correctly mirrors the intended containment.
• Tests verify real behavior – Both new tests use the actual filesystem and make real assertions, consistent with the project testing guidelines.

---

Issues / suggestions

1. os.ReadDir error swallows the underlying cause (minor)

entries, err := os.ReadDir(projectsRoot)
if err != nil {
    return "", fmt.Errorf("session not found: %s", sessionID)
}

If ~/.claude/projects/ doesn't exist or has a permissions problem the caller just sees "session not found" with no hint of why. Wrapping the error would help debugging:

if err != nil {
    return "", fmt.Errorf("session not found: %s (could not read projects directory: %w)", sessionID, err)
}

Or at minimum keep the underlying error in a %w form so callers using errors.Is / errors.As can inspect it.

2. No test for the input-validation guard (minor)

The ContainsAny/Contains("..") block is new security code, but neither of the new tests exercises it. Adding a small table-driven case would close that gap:

for _, bad := range []string{"../other", "sub/session", "foo\\bar", "a..b"} {
    _, err := resolver.Resolve(SourceClaude, bad)
    assert.Error(t, err, "expected error for session ID: %s", bad)
}

Note: the "a..b" case shows that the current check is slightly over-broad – it rejects IDs that merely contain .. as a substring (e.g. "my..session"). Whether that's acceptable depends on whether Claude ever produces IDs like that, but it's the safe trade-off.

3. No test for missing projects root (minor)

TestResolveClaudeSessionEmptyProjectPathNotFound creates the projects root but leaves it empty of matching files. It doesn't cover the path where ~/.claude/projects/ itself doesn't exist. Worth a one-liner test since the error path is different (os.ReadDir fails vs. directory-scan finds nothing).

4. First-match semantics are undocumented (informational)

If the same session ID somehow appears in two project directories, the function returns the first one found by os.ReadDir (alphabetical order). This is fine in practice since Claude session IDs are UUIDs, but a brief comment on the search-all branch would make the intent explicit.

---

Summary

The fix is correct, the refactor is clean, and the tests cover the primary happy-path and not-found cases. The main action items are:

1. Wrap the os.ReadDir error rather than discarding it.
2. Add a test for the path-traversal rejection path.
3. Optionally, a test for a missing ~/.claude/projects/ root.

Happy to approve once those are addressed (or if you're comfortable accepting the minor gaps as-is).

[claude]

Code Review: fix(T-1032) — Claude session resolver fails when listing all projects

Overview

This PR fixes a bug where resolveClaude always required r.projectPath to be set. When the resolver is used without a project context (e.g., listing all sessions), the old code constructed a path from an empty claudeProjectPath, silently returning the wrong directory. The fix extracts findClaudeSessionPath with two code paths: specific-project lookup (original behavior) and an all-projects search when projectPath is empty.

---

What Works Well

• Good refactor: Extracting findClaudeSessionPath eliminates the duplication that previously existed between resolveClaude and ResolvePath. Both now share a single implementation.
• Defense in depth on path traversal: The up-front strings.ContainsAny(sessionID, "/\\") || strings.Contains(sessionID, "..") check is a good first gate before doing any filesystem work. The original code relied solely on web.IsPathWithinDir after path construction, which is weaker.
• Correct boundary check in the search loop: Using web.IsPathWithinDir(sessionFile, projectsRoot) (not baseDir) in the all-project scan is correct — it guards against any symlink tricks inside a project subdirectory without being overly restrictive.
• Test coverage is solid: Both the happy path and not-found path are covered, and TestResolveClaudeSessionEmptyProjectPath also exercises ResolvePath, which is good given the shared helper.

---

Issues and Suggestions

Minor: .. check is overly broad

strings.Contains(sessionID, "..")

This rejects session IDs that contain .. anywhere — e.g. session..abc would be rejected even though it cannot escape a directory. Since Claude session IDs are UUIDs in practice this won't matter, but the intent is to block ../ traversal sequences. A tighter check would be strings.Contains(sessionID, "../") (plus ..\ for Windows). The current check is safe but slightly noisier if session ID formats ever evolve.

Minor: redundant IsPathWithinDir in the specific-path branch

if r.projectPath != "" {
    ...
    if !web.IsPathWithinDir(sessionFile, baseDir) {   // <-- redundant after the up-front ID check
        return "", fmt.Errorf("session not found: %s", sessionID)
    }

After the up-front / and .. check, sessionFile is always baseDir + "/" + sessionID + ".jsonl" — it cannot escape baseDir. The IsPathWithinDir call here is dead code. Not harmful, but worth removing for clarity.

Observation: first-match wins when session ID collides across projects

If the same session ID exists in two different project directories (unlikely but possible), the search returns whichever subdirectory os.ReadDir yields first (filesystem order). This is reasonable, but worth a comment since it's a silent disambiguation — or alternatively the function could return an error if more than one match is found to avoid ambiguity.

Style note (CLAUDE.md): multi-line comment block

The function comment on findClaudeSessionPath spans three lines. CLAUDE.md asks for "one short line max" on comments. The current comment is useful, but could be condensed:

// findClaudeSessionPath locates sessionID under ~/.claude/projects/, searching all subdirs when projectPath is empty.

Similarly, the test functions have multi-paragraph docstrings explaining expected behavior — per CLAUDE.md these should be trimmed or removed.

---

Summary

The fix is correct and the approach is sound. The main actionable suggestions are:

1. Remove the now-redundant IsPathWithinDir check in the specific-path branch.
2. Optionally tighten the .. traversal check to ../ sequences only.
3. Trim comment blocks to match project style.

None of these are blockers — the bug fix is solid and the tests verify the new behavior properly.