Skip to content

sessionScope's uncovered-cwd comment claims a dispatcher pre-check that does not exist #709

Description

@zzet

internal/mcp/server.go's sessionScope ends with a fall-through for a cwd that neither lives inside nor contains a tracked repo:

// cwd neither lives inside nor contains a tracked repo. The daemon
// dispatcher rejects unreachable cwds before dispatch, so this is
// defensive: the sentinel matches no node, so the session sees
// nothing rather than the whole global graph.
ss.scopeWorkspaceID = unresolvedWorkspacePrefix + cwd

The premise in that comment is not true. No dispatcher rejects an unreachable cwd — cmd/gortex/daemon_mcp.go:403 logs "mcp session cwd is not covered by any tracked repo" once and continues, and :218 says as much ("repair an uncovered cwd. Every graph-backed call remains fail-closed."). So this branch is not defensive-only; it is the live path for any uncovered cwd.

The behaviour is still correct — the sentinel matches no node, so the session sees nothing rather than the global graph, which is the fail-closed direction. Two things to settle:

  1. Fix the comment so the next reader doesn't rely on a guard that isn't there. This is the load-bearing half: the claim invites someone to skip a check on the assumption it already happened upstream.
  2. Decide whether an uncovered cwd should be rejected at the entrypoint rather than silently resolving to a scope that returns nothing. Today a caller passing a typo'd X-Gortex-Cwd gets empty results with no explanation, which is hard to distinguish from a genuinely empty answer. An explicit error naming the uncovered cwd would be more useful, and would make the comment true.

Context: this surfaced while reviewing #649, which started attaching WithSessionCWD on the POST /v1/tools/{name} path (previously it went unset there, leaving sessionScope unbound). That change is right — binding narrows, and the fall-through above is what an unreachable value now lands on instead of an unbounded session — but it makes this branch reachable from one more entrypoint, so the stale comment is worth correcting now.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions