Skip to content

fix: associate backlog sessions with a worktree by their launch directory (#647) - #677

Closed
bess-agent wants to merge 2 commits into
mainfrom
fix/issue-647-session-cwd-match
Closed

fix: associate backlog sessions with a worktree by their launch directory (#647)#677
bess-agent wants to merge 2 commits into
mainfrom
fix/issue-647-session-cwd-match

Conversation

@bess-agent

Copy link
Copy Markdown
Collaborator

Summary

  • The backlog digest (scripts/backlog-digest.sh) now associates a claude session with a worktree's issue by the session's launch directory (cwd) as well as by its issue-<n> name, so the backlog rhythm stops re-proposing resume_implementation on work a live session is already doing.
  • A session whose name IS issue-<m> always counts as 's — a dispatch launched from inside the wrong worktree cannot mask that worktree's genuinely stalled work.
  • Three CONFIRMED code-review findings addressed: the misattribution guard above, a deterministic pick when several sessions share a worktree, and a stale comment claiming foreground sessions never appear in claude agents --json.

Root cause

session_for($n) in scripts/backlog-digest.sh matched a session to an issue only by exact name issue-<n>. A session started by hand inside a worktree carries a generated name (e.g. bess-manager-84), so it was invisible and its unlocked worktree read as abandoned — backlog-rhythm.sh proposed resume_implementation every tick against live work. (Verified against the current code: the issue's premise that the digest matched "by cwd" was inverted — it matched by name, and the cwd gap is the real defect.)

Fix

session_for now matches two ways: the exact issue-<n> name (covers the documented claude --bg -n "issue-<n>" dispatch), and a cwd inside a worktree (exact path or subdirectory prefix) that joins the session to the same issue the worktree path/branch already resolves to. The name match is authoritative — a session named issue-<m> is excluded from the cwd join for other issues — and a deterministic sort | .[0] replaces the raw listing-order pick when several sessions share a worktree.

Test plan

  • ./scripts/quality-check.sh passes locally (unsandboxed), re-run on the committed tree.
  • .venv/bin/pytest backend/tests/test_backlog_digest.py backend/tests/test_backlog_rhythm.py — 129 passed.
  • .venv/bin/pytest -m slow — 554 passed, 8 skipped.
  • Step 8 script-level verification on the real board (unsandboxed): digest runs clean; name-match preserved for real dispatched sessions; an injected worktree-resident session associates; the non-issue feat-agent-fleet-sandboxing worktree resolves to no issue (no false positive). Real claude agents --json confirms interactive sessions now appear with their launch cwd (a live bess-manager-84 sits in feat-agent-fleet-sandboxing).

Evidence the test discriminates

  • Reverted: removed the cwd join from session_for (back to the exact-name-only match).
  • Result: test_a_session_inside_a_worktree_is_associated_with_its_issue FAILED — item["session"] was None, expected "bess-manager-84"; 129 tests total.
  • Restored: tree clean; test passes.
  • Misattribution guard (test_a_dispatch_named_session_launched_inside_another_worktree_is_not_misattributed) verified via jq probe: the unguarded cwd join returns issue-700 for a dispatch launched inside another worktree (the reviewer-confirmed false positive); the guarded version returns null.

Outcome-level coverage

None, because this is a bash/jq tooling change with no optimizer outcome pin. The behaviour is pinned by the digest unit tests (129 passed, including the new RED and misattribution-guard tests) plus the real-board digest run in the Test plan.

Docs check

docs/agents/bess-knowledge.md and docs/SOFTWARE_DESIGN.md were grep'd for sessions, backlog, cwd, and worktree matching; neither mentions anything this change touches — N/A.

Refs #647

The digest matched a claude session to an issue only by its exact
`issue-<n>` name, so a session started by hand inside a worktree
(generated name like `bess-manager-84`) was invisible and its unlocked
worktree read as abandoned -- the backlog rhythm proposed
resume_implementation every tick against live work.

The session cwd is the directory it was launched from, so match it
against each worktree path (exact, or a subdirectory prefix) and join the
session to the same issue the worktree path-or-branch join already
resolves. The exact `issue-<m>` name stays authoritative so a dispatch
launched from inside the wrong worktree cannot mask that worktree's
genuinely stalled work, and a deterministic pick replaces the raw
listing-order pick when several sessions share a worktree. Refs #647.
@bess-agent

Copy link
Copy Markdown
Collaborator Author

@claude-bot review

@bess-manager-claude-bot bess-manager-claude-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: PR #677 — associate backlog sessions by launch directory

Root-cause match: The PR's own "Root cause" section corrects the linked issue's premise — #647 claims the digest matched sessions "by cwd" when in fact main's code (verified directly: select(.name? == "issue-\($n)") in scripts/backlog-digest.sh) already matched by exact name only. The actual gap this PR closes is the second candidate fix from #647: a hand-started session inside a worktree, carrying a generated name like bess-manager-84, was never joined to its issue at all. This is exactly the kind of "verify before proposing a fix" CLAUDE.md asks for, and the PR documents the discrepancy rather than silently reinterpreting the issue.

New session_for logic (scripts/backlog-digest.sh diff, jq function around line 281):

def session_for($n):
    ([ $sessions[] | . as $s
      | select(
          ($s.name? == "issue-\($n)")
          or (
            (($s.name? // "") | test("^issue-[0-9]+$") | not)
            and any($worktrees[] | select(matches_issue(.; $n)); session_is_in_worktree($s; .))
          )
        )
      | ($s.name // $s.cwd)
    ]) as $matches
    | if ($matches | length) == 0 then null else ($matches | sort | .[0]) end;

Traced by hand and confirmed by test: exact issue-<n> name is authoritative and short-circuits the cwd join for any other issue number (blocks the misattribution case where an issue-<m> dispatch is launched inside <n>'s worktree), a non-dispatch-named session joins via cwd exact-or-prefix match against a worktree already resolved to the issue, and sort | .[0] replaces the old raw-listing-order pick for determinism when several sessions share a worktree.

Test discrimination — verified independently, not just trusted from the PR body: I checked out the PR branch in a worktree, ran the full test_backlog_digest.py suite (55 passed), then manually reverted session_for to the pre-PR exact-name-only version and reran: test_a_session_inside_a_worktree_is_associated_with_its_issue and test_a_session_launched_from_a_worktree_subdirectory_is_associated both failed with None != 'bess-manager-84', exactly matching the PR's "Evidence the test discriminates" claim. These tests would catch a regression of the cwd join. The other three new tests (dispatch-name-associated, misattribution-guard, main-checkout-matches-nothing) cover the boundary conditions the fix explicitly reasons about.

Minimality: The diff touches exactly scripts/backlog-digest.sh (comments + the one jq function + a new helper session_is_in_worktree), its test file, and CHANGELOG.md under ## [Unreleased] per the release workflow rule. No unrelated refactoring. shellcheck -x on the PR's version of the script is clean, and bash -n passes.

Checklist:

  • No Optional[x]/hasattr/getattr-with-default/new classes — N/A, this is bash/jq + pytest, no such constructs present.
  • No hardcoded entity IDs, no exception-string matching, no credentials — none present.
  • Tests check behavior (session/worktree association outcomes) via shimmed gh/git/claude CLIs, not implementation details — consistent with the existing suite's style.

Minor, non-blocking observation: ($s.name // $s.cwd) in the match-value projection falls back to .cwd when .name is absent, but every session shape in the new tests (and the real claude agents --json sample quoted in the PR body) always carries a name. If name is in fact always present on real output, this fallback branch is unreachable — not worth blocking on, just flagging in case it was meant as a placeholder for a currently-untested shape.

No blockers. Root cause is verified against actual source (not guessed), the fix stays within session_for's existing contract, the new tests demonstrably catch the regression they target, and the change is scoped tightly to the stated problem.

@johanzander
johanzander marked this pull request as ready for review August 22, 2026 13:13
@johanzander

Copy link
Copy Markdown
Owner

wont do.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants