Skip to content

fix(auto-pr): detect catstack work from a session in another repo - #472

Merged
EdbertChan merged 1 commit into
mainfrom
stack/EdbertChan/fix/auto-pr-foreign-session-20260911/detect-catstack-work-session-another-repo--77563afd
Sep 12, 2026
Merged

fix(auto-pr): detect catstack work from a session in another repo#472
EdbertChan merged 1 commit into
mainfrom
stack/EdbertChan/fix/auto-pr-foreign-session-20260911/detect-catstack-work-session-another-repo--77563afd

Conversation

@EdbertChan

@EdbertChan EdbertChan commented Sep 12, 2026

Copy link
Copy Markdown
Owner

Summary

The reminder to open a catstack PR only appears when the session is working
inside catstack.

It decides which repo it is looking at from the session's working directory. A
session sitting in another repo, editing catstack through a full path or a
worktree, gets nothing.

That happened today. A hook and an install.sh change were written from a
session working in another repo.

No reminder fired, and the PRs went up with no schema.

Now it also reads the file paths the session touched. If any live in catstack,
the reminder fires.

Review Claim

repo_root() falls back to the catstack paths this session's tool calls named
when the working directory belongs to another repository.

Review Lane

behavior

Review Unit

engine-runtime

Safety Invariant

The working-directory check runs first and is unchanged, so every session that
fired before still fires identically. The fallback only ever widens detection,
and _accept_root() — the same function the working-directory path uses — still
gates every candidate, so no directory outside catstack can be returned. Reads
are the transcript file and read-only git; nothing writes.

Slice Rationale

This is one function's input, separable from everything else in the hook. The
diff-debounce, the once-per-diff marker, and the instruction text are untouched,
so a reviewer only has to agree that touched paths are a valid second signal for
which repo the session edited.

Non-goals

  • Does not change when the reminder fires relative to editing (the debounce is untouched).
  • Does not change the instruction text or the once-per-diff marker.
  • Does not change RELEVANT_PREFIXES, so the same paths count as relevant.
  • Does not address the second cause found today: CLAUDE.core.md triggers the
    PR skill on being asked, so agreeing to an agent's own offer ("yes") matches
    no trigger. That is a prose change, filed separately.

Architecture

Before

graph TD
    A["Stop fires"] --> B["repo_root: read session cwd"]
    B --> C{"is cwd inside catstack?"}
    C -->|"yes"| D["check diff, remind once idle"]
    C -->|"no"| E["silent, even if catstack was edited"]
Loading

After

graph TD
    A["Stop fires"] --> B["repo_root: read session cwd"]
    B --> C{"is cwd inside catstack?"}
    C -->|"yes"| D["check diff, remind once idle"]
    C -->|"no"| F["read paths this session's tools named"]
    F --> G{"any path inside catstack?"}
    G -->|"yes"| D
    G -->|"no"| E["silent"]
Loading

Test Plan

Test Plan
  • cd engine/hooks/auto-pr && python3 -m unittest discover -s tests
  • python3 engine/skills/make-pr/scripts/preflight.py --base origin/main
  • python3 scripts/check_no_new_comments.py
  • python3 scripts/check_ecosystem_boundaries.py

Replayed against the real transcript of the session that missed it, cwd left as
that session's own (empathy_ai):

###### BEFORE (origin/main auto-pr) ######
repo_root -> None
decide    -> None

###### AFTER (this branch) ######
repo_root -> /Users/edbertchan/Documents/GitHub/catstack/.worktrees/autopr-foreign
decide    -> catstack changes detected on branch `fix/auto-pr-foreign-session-20260911`
             (engine/hooks/auto-pr/detect.py, engine/hooks/auto-pr/tests/test_hooks.py, ...

Suite and gates on this branch:

Ran 30 tests in 33.701s
OK

unit    engine-runtime: 2 file(s)
gate    python3 scripts/check_hook_test_coverage.py engine/hooks/auto-pr
        check_hook_test_coverage: OK (1 hook(s) checked)
ok      preflight passed
check_no_new_comments.py         PASS
check_ecosystem_boundaries.py    PASS

Fixture vs live: the before/after replay above is the real transcript, not a
fixture. The hook is installed on this machine as a symlink into the primary
checkout, so this branch's version has not itself run as the installed hook:

$ readlink ~/.claude/hooks/auto-pr
/Users/edbertchan/Documents/GitHub/catstack/engine/hooks/auto-pr

Revert Plan

Revert Plan
  • Safe to revert? Yes
  • Revert command: git revert <sha>
  • Post-revert steps: None. Reverting restores working-directory-only detection.
  • Data migration? No

repo_root() decided which repository it was looking at from the session's cwd
alone, so a session working in another repo that edits catstack through an
absolute path or a worktree got no reminder at all.

That is what happened today: engine/hooks/unverified-tag-ledger and install.sh
were written and committed from a session whose cwd was a different repo, the
Stop hook stayed silent every turn, and both PRs went up with no draft-pr
schema until the user noticed.

cwd stays the first check and is unchanged. When it does not resolve to
catstack, repo_root() now reads the file paths this session's tool calls named
and runs each through the same _accept_root() gate, so the fallback can only
ever widen detection and can never return a directory outside catstack.

Replayed against the real transcript of that session, cwd left as its own:

  BEFORE  repo_root -> None      decide -> None
  AFTER   repo_root -> .../catstack/.worktrees/autopr-foreign
          decide    -> "catstack changes detected on branch ..."

30 tests pass, including a positive case (foreign cwd + a touched catstack
path fires), a negative case (touched paths outside catstack stay silent), an
unchanged-behaviour case for a cwd inside catstack, and an unreadable
transcript reporting on stderr rather than failing quietly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018u8S5ct3kFhosinSbybc7W
Change-Id: I77563afdcde6b7dde30dc046ff69ab637648d84f
@cursor

cursor Bot commented Sep 12, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_94cd25ee-2aa3-4cdd-828f-376595bb7e92)

@mergify

mergify Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Queued — the merge queue status continues in this comment ↓.

@EdbertChan

Copy link
Copy Markdown
Owner Author

@Mergifyio queue

@mergify

mergify Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-09-12 06:11 UTC · Rule: admin-bypass · triggered by a @mergifyio queue command
  • 🟠 Checks running · on draft merge queue: checking #472 on main (e9a9750) #476
  • 🚫 Left the queue2026-09-12 06:14 UTC · at 745d936fce48708039c76d0271c9a8b44cf4acd7

This pull request spent 2 minutes 34 seconds in the queue, with no time running CI.

Waiting for
  • check-success = test
All conditions
  • check-success = test
  • check-success = lint

Reason

Pull request #472 has been merged manually at de78102

Hint

You were too fast!

Tick the box to put this pull request back in the merge queue (same as @mergifyio queue).

  • Requeue this pull request

@mergify mergify Bot added the queued label Sep 12, 2026
@EdbertChan
EdbertChan merged commit de78102 into main Sep 12, 2026
4 checks passed
@mergify

mergify Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

queue

☑️ The pull request has been merged manually

Details

The pull request has been merged manually at de78102

@mergify mergify Bot removed the queued label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant