Skip to content

fix(session-handoff): write Open PRs only on a git-attached (developer-mode) checkout - #5385

Merged
john-the-dev merged 2 commits into
mainfrom
fix/session-handoff-open-prs-dev-only
Oct 16, 2026
Merged

john-the-dev merged 2 commits into
mainfrom
fix/session-handoff-open-prs-dev-only

Conversation

@john-the-dev

@john-the-dev john-the-dev commented Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

src/session-handoff.sh queried Sutando's own GitHub repo on every install and wrote the results into <workspace>/session-state.md, in two places:

  • ## Open PRs ran gh pr list --repo sonichi/sutando.
  • ## Repo Stats ran gh api repos/sonichi/sutando (star and fork counts). Susan caught this one in review.

Agents read that file at session start, so a regular user's agent could raise Sutando repo PRs nobody asked about. Tracked as P1 row rmummbsx2s25lvab ("unrequested Sutando repo checks"). History: #1725 and #1737 removed the original hardcoded-author pieces.

Change

One flag is set once, right after REPO resolves:

DEV_CHECKOUT=0; [ -e "$REPO/.git" ] && DEV_CHECKOUT=1

It gates both sections. No new env var or helper: this is the signal the repo already uses.

  • The packaged engine ships as an rsync copy without .git.
  • Sutando Developer Mode git-attaches engine/sutando, and contributor clones have .git too.
  • -e, not -d, so worktrees and submodules still count.

Worst-case disruption: a developer-mode or contributor install sees no change. A packaged install loses two sections it never asked for and makes two fewer gh calls per compaction.

Evidence

tests/session-handoff-repo-queries-dev-only.test.py (renamed from …-open-prs-dev-only) extracts the live flag and both sections. It runs them with a stub gh that logs each call, against a temp repo with and without .git. The packaged arm asserts 0 gh calls and neither section. The developer arm asserts both sections and 2 calls.

At HEAD 806e91b1:

test_developer_mode_checkout_keeps_both_sections ... ok
test_packaged_install_queries_nothing ... ok
OK

With only Repo Stats ungated (the shape of a3f943ef):

test_packaged_install_queries_nothing ... FAIL
AssertionError: '## Repo Stats' unexpectedly found in '## Repo Stats\n7 stars, 3 forks\n'

At parent 17118e34, the original Open PRs leak, with the first version of this test:

test_packaged_install_has_no_open_prs_section ... FAIL
AssertionError: '## Open PRs' unexpectedly found in '## Open PRs\n#1 stub-pr\n\n'

Other checks:

  • All 5 tests/session-handoff-*.test.py pass.
  • bash -n is clean.
  • shellcheck is unchanged versus a3f943ef (3× SC1091, 2× SC2012).
  • git diff --cached | bash scripts/review-checks.sh → review-checks: PASS.

🤖 Generated with Claude Code

Every install queried sonichi/sutando's open PRs into session-state.md, so a
regular owner's agent could raise Sutando repo PRs nobody asked about. The
packaged engine ships without .git; developer mode git-attaches the checkout.
Gate the section on that existing signal ($REPO/.git, as _repo_ok and
sutando-config.sh already use).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@john-the-dev john-the-dev added the ag2product AG2 product related (packages, PyPI, task-relay) label Oct 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 91%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Rui's Codex-core agent posted a PASS at a3f943ef: the change is scoped, the regression reproduces at the parent, and all five session-handoff suites pass at the head. It's ready for a code-owner review.

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Request changes at a3f943e: the Open PRs gate is correct and well tested, but the same unrequested repo check survives 70 lines below.

Blocker (incomplete fix / sibling not swept):

  • src/session-handoff.sh:304-314 ("## Repo Stats") still runs gh api repos/sonichi/sutando on every install, packaged or not, and writes Sutando's star/fork counts into session-state.md. That is the same class as the P1 row this PR cites ("unrequested Sutando repo checks"), in the same file, and the same [ -e "$REPO/.git" ] predicate applies unchanged.
  • Ask: put Repo Stats behind the same gate (one shared check, e.g. a _dev_checkout var set once, not a second copy of the test), and extend tests/session-handoff-open-prs-dev-only.test.py so the packaged arm also asserts the stub gh is never called for either section. If Repo Stats is meant to stay for every install, say why in the PR body.

Verified at a3f943e (scratch clone, env -i, bundled python3):

  • New test: Ran 2 tests OK. Mutation if false && [ -e ... ] -> FAILED ('## Open PRs' not found); mutation if true -> FAILED ('## Open PRs' unexpectedly found). It discriminates in both directions; it extracts the live section and executes it, so REVIEW.md #14 is not triggered.
  • All 5 tests/session-handoff-*.test.py OK (9/28/2/8/3); bash -n clean.
  • -e not -d matches scripts/sutando-config.sh:661-670 and _repo_ok at src/session-handoff.sh:28.
  • Added lines: no ~/.sutando, $HOME/.sutando or SUTANDO_WORKSPACE literals; scripts/review-checks.sh PASS.
  • CI: all checks pass; license/cla success (1 status context).
  • Not verified: a live post-compaction round trip on a packaged install (named, not blocking).

No reviews on the PR yet, so after the fix it still needs approvals.

— Sutando (susans-MacBook-Pro)

…out flag

Repo Stats still ran gh api repos/sonichi/sutando on every install. One
DEV_CHECKOUT flag, set once after REPO resolves, now gates both sections;
the test asserts the stub gh is never called on a packaged install.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

@liususan091219 Fixed at 806e91b1. Thanks, and sorry it sat 12h. Repo Stats now sits behind the same gate. One DEV_CHECKOUT flag is set once after REPO resolves, and both sections read it. The test now runs both sections against a call-logging stub gh: the packaged arm asserts 0 calls and neither section, and the developer arm asserts both sections and 2 calls. Ungating only Repo Stats fails the packaged arm with '## Repo Stats' unexpectedly found. There are no other gh or sonichi/ calls left in the file (grep -nE '(^|[^a-z])gh |sonichi' src/session-handoff.sh → the two gated lines). The PR body has the before/after.

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review at 806e91b. My one blocker from a3f943e is fixed. Ready to approve.

What changed and how I checked it (scratch clone at 806e91b, bundled python3):

  • DEV_CHECKOUT is set once (src/session-handoff.sh:52) and both ## Open PRs (:237) and ## Repo Stats (:307) sit behind it. No third copy of the .git test.
  • Sweep: the only gh / sonichi/sutando calls left in the script are those two, both gated (:239, :310).
  • tests/session-handoff-repo-queries-dev-only.test.py: Ran 2 tests OK. The packaged arm asserts 0 stub-gh calls.
  • Mutations, each run on its own: ungate Open PRs -> FAILED; ungate Repo Stats -> FAILED; flag never set -> FAILED. The test catches each section separately and the flag itself.
  • All 5 tests/session-handoff-*.test.py OK; bash -n clean; scripts/review-checks.sh --diff PASS (no host-path literals in added lines).

Not yet verified:

  • CI at this head: 18 success, 1 skipped, 13 still pending when I checked.
  • A live post-compaction round trip on a packaged install. This is named here but does not block approval.

This review is not an approval. My earlier REQUEST_CHANGES stays on record until Susan decides.

— Sutando (susans-MacBook-Pro)

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

Rui's Codex-core agent PASSed 806e91b1. It independently enumerated the script and found exactly two gh/sonichi queries (lines 239 and 310), both behind DEV_CHECKOUT (line 52). Its isolated mutants confirm the test fails when either section is ungated. CI was still running at that point with no failures. Susan's bot re-reviewed and confirmed the Repo Stats blocker is fixed. Ready for a code-owner review once CI is green.

@john-the-dev

Copy link
Copy Markdown
Collaborator Author

@liususan091219 Status at 806e91b1. Nothing new to push: the one finding was fixed at this head, and your re-review (2026-10-12 14:18Z) confirmed it. What changed since then is that CI has finished.

Per finding

# Finding (CR at a3f943ef) Resolution
1 ## Repo Stats still ran gh api repos/sonichi/sutando on every install Fixed in 806e91b1. DEV_CHECKOUT is set once (src/session-handoff.sh:52) and gates both ## Open PRs (:237) and ## Repo Stats (:307). These two are the only gh/sonichi calls in the file (:239, :310).
1b Extend the test so the packaged arm asserts the stub gh is never called Done. tests/session-handoff-repo-queries-dev-only.test.py: the packaged arm asserts 0 calls and neither section; the developer arm asserts both sections and 2 calls.

Re-verified in a fresh --filter=blob:none clone at 806e91b1

== HEAD 806e91b1
Ran 2 tests in 0.151s
OK
== mutant: ungate Open PRs
FAIL: test_packaged_install_queries_nothing
AssertionError: '## Open PRs' unexpectedly found in '## Open PRs\n#1 stub-pr\n\n'
== mutant: ungate Repo Stats
FAIL: test_packaged_install_queries_nothing
AssertionError: '## Repo Stats' unexpectedly found in '## Repo Stats\n7 stars, 3 forks\n'
  • All 5 tests/session-handoff-*.test.py pass, and bash -n is clean.
  • gh pr diff 5385 | scripts/review-checks.sh --diff → review-checks: PASS (145-line diff).
  • CI at 806e91b1: all 15 required checks are SUCCESS, including license/cla. They were 13 pending when you re-reviewed.

Still open

  • Your CHANGES_REQUESTED from a3f943ef still stands. The COMMENTED re-review didn't replace it, so reviewDecision is CHANGES_REQUESTED and the PR is BLOCKED. If you're satisfied, please submit a formal review at this head, or dismiss the old one.
  • Not verified: a live post-compaction round trip on a packaged install. Same as before.
  • Merging also needs Rui's Sutando Release (Codex-core) verification at this exact head. It PASSed 806e91b1 on 10-12, and the head hasn't moved since.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approve at 806e91b.

There was no earlier review from qingyun-wu on this PR, so nothing of mine was outstanding. The only CHANGES_REQUESTED on record is liususan091219's at a3f943e (5485944018). Her re-review at this head (5492050455) is a COMMENTED review, not an approval, so reviewDecision stays CHANGES_REQUESTED until she approves or dismisses it. This approval does not clear it.

Verified in a fresh clone at 806e91b:

  • Her blocker is fixed. DEV_CHECKOUT is set once at src/session-handoff.sh:52, and both ## Open PRs (:237) and ## Repo Stats (:307) read it. The only gh/sonichi calls left in the file are :239 and :310, and both are gated.
  • bash -n src/session-handoff.sh is clean. All five tests/session-handoff-*.test.py pass, including the new repo-queries-dev-only test.
  • I ran three mutants, each on its own. Ungating Open PRs, ungating Repo Stats, and forcing DEV_CHECKOUT=1 all FAIL test_packaged_install_queries_nothing. The test catches each section and the flag.
  • scripts/review-checks.sh --diff on the PR diff passes (hardcoded-paths, root-artifacts, prose-cap). Both new comment blocks are 2 lines.
  • CI at this head: the newest run of each check is 32 success and 1 skipped. license/cla is success.
  • review-preflight.py 5385 ran from the clone. No REVIEW.md lesson applies beyond the ones above.

Not verified: a live post-compaction round trip on a packaged install. That is not blocking.

@john-the-dev
john-the-dev dismissed liususan091219’s stale review October 16, 2026 18:44

Resolved by 806e91b: Open PRs and Repo Stats now use the same DEV_CHECKOUT gate; packaged installs make zero gh calls. The reviewer confirmed her sole finding fixed in review 5492050455 (a COMMENT, not an approval), and Qingyun approved this exact head in 5527773377. Reverified all five session-handoff suites: 50 tests PASS; bash -n PASS; all 15 required checks including CLA PASS. Dismissing the resolved change request; this does not record an approval from Susan or waive any required merge check.

@john-the-dev
john-the-dev merged commit 8c23230 into main Oct 16, 2026
34 checks passed
@john-the-dev
john-the-dev deleted the fix/session-handoff-open-prs-dev-only branch October 16, 2026 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ag2product AG2 product related (packages, PyPI, task-relay)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants