Merged
Conversation
…d, not the script's own location (#590) A bare `--check`/`--count`/`--check-links` used to walk up from `__file__` for a `.git`, so the plugin's own bundled copy always resolved to the plugin's own repository regardless of where or against what it was invoked -- a clean `ok` about the wrong tree's fragments, from any caller. It now walks from `Path.cwd()` instead and refuses, the same way the fold already does, when no `.git` is found above it; the `ok`/`skipped` receipts also name the resolved directory so the answer's subject is never implicit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wr8bkvu4ac3hpnHcG99fec
The reviewer spawn for the #590 commit caught two user-visible strings this plugin's own docs-currency rule warns about leaving stale: the fold's own refusal message (`scripts/assemble_changelog.py`, printed on every fold run missing `--dir`/`--changelog`) still said the derivation "walks up from itself" and names "the repository it is stored in", and `commands/ changelog.md` still described the pre-fix behavior as current. Both now describe the caller's-cwd derivation #590 actually shipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wr8bkvu4ac3hpnHcG99fec
Deriving `REPO` from `Path.cwd()` reads the process's own current directory at import time, unconditionally -- fold included, which never falls back to `REPO` at all. `os.getcwd()` raises `FileNotFoundError` when that directory has been removed out from under the process (a real race in this loop's own worktree lifecycle, not a hypothetical one), which crashed the import with an unhandled traceback: exit 1, empty stdout, indistinguishable from a shell that never launched. `_safe_cwd()` catches it and folds the case into the same "no default to derive" refusal `_resolve` already prints for "no .git above cwd", so every read-only mode reports the same way for both causes of "could not resolve" instead of one of them reporting nothing at all. Caught by the reviewer spawned against the #590 commit; reproduced and pinned red-then-green in tests/test_assemble_changelog_vanished_cwd_590.py. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wr8bkvu4ac3hpnHcG99fec
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/assemble_changelog.py's read-only modes (--check,--count,--check-links), invokedwith neither
--dirnor--changelog, used to derive their default fragment/changelog paths bywalking up from the script's own
__file__install location for a.git. For the plugin's ownbundled copy (a plugin cache unrelated to whatever repository the caller is working against), that
walk always succeeded and always resolved to the plugin's own repository -- so a bare
--checkrun from any caller's directory silently reported
okabout the plugin's own changelog fragments,never the caller's.
This now derives from the caller's current working directory instead, and refuses -- the same
way the mutating "fold" mode already refuses -- when no
.gitis found above it, rather thanfalling back to a repository nobody asked about. The
ok/skippedreceipts also now name thefully resolved directory so the answer's subject is never implicit.
Closes #590
What changed
scripts/assemble_changelog.py:REPOnow derives fromPath.cwd()rather than__file__'slocation. A
_safe_cwd()wrapper catchesOSErroraround that module-level read (a vanishedcwd -- e.g. a worktree removed mid-run -- used to crash the import with an unhandled traceback;
now it reports the same stated
skippedreceipt as "no.gitabove cwd").check()'sok/skippedreceipts now name the resolved directory. All adjacent prose that described the oldfile-location-based derivation was rewritten to match: the module docstring,
_resolve's and_fold_target's docstrings, the fold's own printed refusal message,CLAUDE.md's trap bullet,and
commands/changelog.md.tests/test_check_resolves_callers_repo_590.py(new): the composition control -- a script copiedinto one "vendor" tree, invoked with cwd inside a second, unrelated repository, must never answer
about the vendor's fragments, paired with the positive control that it does resolve and answer
about the caller's own repository. Plus a refusal control (no
.gitanywhere above cwd).tests/test_assemble_changelog_vanished_cwd_590.py(new): reproduces a vanished-cwd crash viaos.chdir()+os.rmdir()+os.execv(), asserting a statedskippedreceipt rather than anunhandled traceback. Skips loudly (naming why) on a platform where this construction is not
reachable, rather than asserting nothing.
tests/test_changelog_root_default.py,tests/test_assemble_empty_dir_346_349.py: updatedwhere their docstrings or assertions described (or, in one case, explicitly required) the old
file-location-based derivation -- one test's whole point used to be "must come from where the
script lives, not the caller's cwd", which this fix deliberately reverses.
changelog.d/590.fixed.md: the release-facing fragment, naming assemble_changelog.py --check resolves its repo from the script's install path, so it reportsokabout the plugin's own tree from any caller #590 and noting that scaffoldedrepositories' vendored
.oss/assemble_changelog.pycopies are unaffected in practice (theircwd was already, and remains, inside the repository they serve).
Review
Two spawns reviewed the initial commit (
db9f657): a diff reviewer (Explore) andoss:auditor.Both returned real findings, both fixed in follow-up commits (
ec94bd8,ba9ab82) on this branch.Reviewer (2 findings, both fixed): the fold's own printed refusal message still said the
derivation "walks up from itself";
commands/changelog.mdstill described the pre-fix behaviouras current. Both rewritten.
Auditor (1 finding, fixed): deriving
REPOfromPath.cwd()unconditionally at module importtime meant a vanished caller cwd (a real race in this loop's own worktree lifecycle) crashed the
import with an unhandled traceback rather than a stated receipt -- a violation of the module's own
documented three-exit-code contract. Fixed with
_safe_cwd(); below-bar: the auditor's ownreport ranked this finding
unrankedbecause it does not cleanly fit any of the ten named rows inthe ranking table this repo's manager skill owns (closest is
misreports, but that row is aboutwrong information rendered, and here the failure mode is no information rendered at all) -- worth
a maintainer glance at whether the table needs an eleventh row for "an absence rendered as a crash
rather than as a stated refusal."
Test plan
tests/test_check_resolves_callers_repo_590.py-- new, red against the pre-fix script,green against the fix.
tests/test_assemble_changelog_vanished_cwd_590.py-- new, red against the fix's firstcommit (
db9f657, before the_safe_cwd()guard), green after (ba9ab82).python3 -m pytest tests/ -q): 3768 passed, 7 skipped (all pre-existing,unrelated), 0 failed.
python3 scripts/assemble_changelog.py --checkfrom an empty scratch directory outside anyrepository: reproduces the issue's own reported symptom being fixed --
skipped, naming the cwdit looked above, rather than
okabout this repository's fragments.python3 scripts/assemble_changelog.py --check --dir changelog.d --changelog CHANGELOG.md(the explicit-flags path scaffolded repos actually use): still
ok, 2 fragments.