From 6bf848ab10f6b08dfe186cdfe712c2221be1bb5b Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 15:57:57 -0700 Subject: [PATCH 1/2] Bound handoff.py's Writes Whatever the Label State Copilot's review of promotion #2254 found that the owner and fork checks ran only where the handoff label was missing, so new or link against another owner's repository went on to write whenever that repository carried a handoff label, and an unregistered non-fork under the owner evaded the registry-drift refusal the same way. require_write_scope now runs before the label is read, for new and link only. A repository under another owner refuses with no call at all, and an unregistered repository of the owner's refuses unless it is a fork, the same in-process owner boundary pr_review.py keeps. The reads stay open, and a registered repository costs no extra request, since the fork answer is read once a run and shared with the missing-label path. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/session-handoff/SKILL.md | 3 +- .../.source-digests/session-handoff | 2 +- .../skills/session-handoff/SKILL.md | 3 +- .github/skills/session-handoff/SKILL.md | 3 +- scripts/README.md | 2 +- scripts/handoff.py | 57 +++++++++++--- tests/test_handoff.py | 76 +++++++++++++++++-- 7 files changed, 124 insertions(+), 22 deletions(-) diff --git a/.agents/skills/session-handoff/SKILL.md b/.agents/skills/session-handoff/SKILL.md index 5dd83ca7..1ddeb792 100644 --- a/.agents/skills/session-handoff/SKILL.md +++ b/.agents/skills/session-handoff/SKILL.md @@ -320,7 +320,8 @@ and answers as an empty chain does, and `new` refuses until it is given `--creat creates the one `handoff` label, confirms it, and then files the first link. Issues turned off stop it before any write, naming the command that turns them on. Never apply the fleet label set to such a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, -refuses before any write. +refuses before any write. `new` and `link` refuse both whatever the label state, since a label on +such a repository opens no write there, while the reads stay open everywhere. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than diff --git a/.claude-plugin/fleet-skills/.source-digests/session-handoff b/.claude-plugin/fleet-skills/.source-digests/session-handoff index 24500fc8..c4cce7b6 100644 --- a/.claude-plugin/fleet-skills/.source-digests/session-handoff +++ b/.claude-plugin/fleet-skills/.source-digests/session-handoff @@ -1 +1 @@ -c1684259d7d09c94 +551f09ebd1b0a691 diff --git a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md index 5dd83ca7..1ddeb792 100644 --- a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md @@ -320,7 +320,8 @@ and answers as an empty chain does, and `new` refuses until it is given `--creat creates the one `handoff` label, confirms it, and then files the first link. Issues turned off stop it before any write, naming the command that turns them on. Never apply the fleet label set to such a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, -refuses before any write. +refuses before any write. `new` and `link` refuse both whatever the label state, since a label on +such a repository opens no write there, while the reads stay open everywhere. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than diff --git a/.github/skills/session-handoff/SKILL.md b/.github/skills/session-handoff/SKILL.md index 5dd83ca7..1ddeb792 100644 --- a/.github/skills/session-handoff/SKILL.md +++ b/.github/skills/session-handoff/SKILL.md @@ -320,7 +320,8 @@ and answers as an empty chain does, and `new` refuses until it is given `--creat creates the one `handoff` label, confirms it, and then files the first link. Issues turned off stop it before any write, naming the command that turns them on. Never apply the fleet label set to such a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, -refuses before any write. +refuses before any write. `new` and `link` refuse both whatever the label state, since a label on +such a repository opens no write there, while the reads stay open everywhere. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than diff --git a/scripts/README.md b/scripts/README.md index 17fd3c05..4e405cca 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -261,7 +261,7 @@ The invariant is one open handoff per track per repository, read from the metada `new` creates the new issue, comments the forward link on the previous one, and then closes it, in that order, printing each step's own result. Creating first means a failure at any later step leaves a discoverable new issue rather than a closed chain with no successor, and `link` finishes what a failure interrupted without a second issue being filed for it. `link` takes no `--track`, so the two issues' own metadata blocks are the only thing that can say they belong to one lane, and it refuses an issue named as its own predecessor and a pair whose blocks name different tracks rather than closing one lane's handoff into another's. Nothing suppresses a write's output or converts a failure into success, per [GOVERNANCE.md][governance] "Repository Boundaries and Write Safety". Where the caller names an issue, which is `link` alone, that issue is read live before the write and only what the read returned is written back, and every other identifier a write consumes comes from a read in the same run rather than being constructed. A close is confirmed by reading the state back, because a write that appears to have failed may have succeeded on the server. -`--repo` is required and carries no default, for the reason `pr_review.py` states about a pull request number: an issue number resolves in every repository, and a chain read out of the wrong one is well formed. `--track` defaults to `default`, and omitting it on a named lane does more than read the wrong chain, since `new` then files onto `default` and comments on and closes whatever that lane had open. A fleet repository missing the `handoff` label refuses, naming `repo-config/configure.sh apply`, never with a degraded empty answer, because `decision` was declared on the hub and applied nowhere else and every downstream session then enumerated an empty queue and reported it healthy. The one exception is a fork under the registry's owner that the registry does not list, such as one kept for an upstream contribution, where no issue can carry a label that does not exist: a read there warns and answers as an empty chain does, and `new --create-label` creates the one `handoff` label from `repo-config/labels.json`, confirms it, and files the first link, refusing first where the fork has issues turned off. A repository missing the label under another owner, or an unregistered one of the owner's that is not a fork, refuses before any write, the first because these calls run as subprocesses a write guard on the caller never sees and the second because it is registry drift a lone label would hide. Exit `0` is success, `1` a refusal the caller can act on, and `2` the command not having run to an answer, so the two never share a code: a usage error is moved off argparse's own `2` onto `1` because it is a refusal, and an exception nobody modeled is caught onto `2` rather than reaching CPython's own `1`. A body over 12 KB warns and one over 60 KB refuses, under GitHub's own 65536-character limit, and both are backstops against a body GitHub would reject: what bounds a handoff is the Skill's per-section rules. +`--repo` is required and carries no default, for the reason `pr_review.py` states about a pull request number: an issue number resolves in every repository, and a chain read out of the wrong one is well formed. `--track` defaults to `default`, and omitting it on a named lane does more than read the wrong chain, since `new` then files onto `default` and comments on and closes whatever that lane had open. A fleet repository missing the `handoff` label refuses, naming `repo-config/configure.sh apply`, never with a degraded empty answer, because `decision` was declared on the hub and applied nowhere else and every downstream session then enumerated an empty queue and reported it healthy. The one exception is a fork under the registry's owner that the registry does not list, such as one kept for an upstream contribution, where no issue can carry a label that does not exist: a read there warns and answers as an empty chain does, and `new --create-label` creates the one `handoff` label from `repo-config/labels.json`, confirms it, and files the first link, refusing first where the fork has issues turned off. A repository missing the label under another owner, or an unregistered one of the owner's that is not a fork, refuses before any write, the first because these calls run as subprocesses a write guard on the caller never sees and the second because it is registry drift a lone label would hide. The two writing subcommands, `new` and `link`, refuse both whatever the label state, checked before the label is read, so a label a stranger's repository happens to carry opens no write there, the same in-process owner boundary `pr_review.py` keeps. The reads stay open everywhere. Exit `0` is success, `1` a refusal the caller can act on, and `2` the command not having run to an answer, so the two never share a code: a usage error is moved off argparse's own `2` onto `1` because it is a refusal, and an exception nobody modeled is caught onto `2` rather than reaching CPython's own `1`. A body over 12 KB warns and one over 60 KB refuses, under GitHub's own 65536-character limit, and both are backstops against a body GitHub would reject: what bounds a handoff is the Skill's per-section rules. `--dry-run` prints the calls a writing subcommand would make and sends none of them, which is the first thing anyone wants before trusting this to close an issue. It sits on the one path every call goes through rather than on each write separately. diff --git a/scripts/handoff.py b/scripts/handoff.py index efa19b31..04a6f14c 100755 --- a/scripts/handoff.py +++ b/scripts/handoff.py @@ -68,6 +68,11 @@ `new --create-label` creates the one `handoff` label and nothing else. Every other repository missing the label refuses, a repository under another owner and an unregistered repository of the owner's that is not a fork among them. + +The two writing subcommands, `new` and `link`, are bounded whatever the label state, as +`pr_review.py` bounds its own writes. They refuse a repository under another owner before any +call, and an unregistered repository of the owner's unless it is a fork, since these writes run as +subprocesses a write guard on the caller never sees. The reads stay open everywhere. """ from __future__ import annotations @@ -268,13 +273,47 @@ def registry() -> tuple[str, set[str]]: return owner.lower(), names -def repo_flags(repo: str) -> dict[str, bool]: - """Whether the repository has issues turned on and whether it is a fork, read live.""" - data = gh_json(["repo", "view", repo, "--json", "hasIssuesEnabled,isFork"]) +def repo_flags(a: argparse.Namespace) -> dict[str, bool]: + """Whether the repository has issues turned on and whether it is a fork, read live once a run. + + The write scope and the missing-label path both ask, so the answer rides on the parsed + arguments rather than costing a second request. + """ + cached = getattr(a, "repo_flags", None) + if cached is not None: + return cached + data = gh_json(["repo", "view", a.repo, "--json", "hasIssuesEnabled,isFork"]) fields = ("hasIssuesEnabled", "isFork") if not isinstance(data, dict) or not all(isinstance(data.get(f), bool) for f in fields): - raise Execution(f"repo view for {repo} returned no {' and '.join(fields)}: {data!r}") - return {field: data[field] for field in fields} + raise Execution(f"repo view for {a.repo} returned no {' and '.join(fields)}: {data!r}") + a.repo_flags = {field: data[field] for field in fields} + return a.repo_flags + + +def require_write_scope(a: argparse.Namespace) -> None: + """Refuse a write outside the registry's owner, or to an unregistered repository not a fork. + + It runs before the label is read, so a label a stranger's repository happens to carry opens no + write there. A registered repository costs no request, and the reads are never bounded here. + """ + if a.cmd in READS: + return + owner, names = registry() + repo_owner, _, name = a.repo.partition("/") + if repo_owner.lower() != owner: + raise Refusal( + f"{a.repo} is not under {owner}, so this script writes nothing there. A different " + "owner goes through the runbook's own `gh` path, where the write guard reads the " + "maintainer's grant." + ) + if name.lower() in names: + return + if not repo_flags(a)["isFork"]: + raise Refusal( + f"{a.repo} is neither in registry/repos.json nor a fork, so it is registry drift " + "rather than a fork keeping state, and this script writes nothing there. Register it, " + f"then apply the fleet label set: {APPLY}" + ) def label_definition() -> tuple[str, str]: @@ -332,9 +371,8 @@ def without_label(a: argparse.Namespace) -> int | None: The body is read once, before the label is created, and that read is the one `new` files, so a body `new` would refuse leaves no label behind. Everything else refuses before any write. A - repository under another owner never gets a label created here, since these calls run as - subprocesses a write guard on the caller never sees. An unregistered repository of the owner's - that is not a fork is registry drift rather than a fork, and a lone label there would hide it. + repository under another owner, or an unregistered one of the owner's that is not a fork, + refuses here for a read and in `require_write_scope` for a write, which runs first. """ repo = a.repo create = getattr(a, "create_label", False) @@ -349,7 +387,7 @@ def without_label(a: argparse.Namespace) -> int | None: if name.lower() in names: flag = " `--create-label` is for an unregistered fork." if create else "" raise Refusal(f"{missing}{flag} Apply the fleet label set from a hub checkout: {APPLY}") - flags = repo_flags(repo) + flags = repo_flags(a) if not flags["isFork"]: raise Refusal( f"{missing} It is neither in registry/repos.json nor a fork, so it is registry drift " @@ -1243,6 +1281,7 @@ def main(argv: list[str] | None = None) -> int: if getattr(a, flag, None) is not None and getattr(a, flag) < 1: ap.error(f"--{flag} takes an issue number, so it cannot be below 1") try: + require_write_scope(a) if not label_present(a.repo): answered = without_label(a) if answered is not None: diff --git a/tests/test_handoff.py b/tests/test_handoff.py index facbde26..b292f1ac 100755 --- a/tests/test_handoff.py +++ b/tests/test_handoff.py @@ -32,6 +32,19 @@ sys.path.insert(0, str(SCRIPTS)) import handoff +REAL_REGISTRY = handoff.registry + + +def setUpModule() -> None: + """Read `o/r` as a fleet repository unless a case says otherwise. + + The write scope reads the registry before every write, and the real one lists no `o`, so + without this every write case would test the scope refusal instead of what it names. + """ + patcher = unittest.mock.patch.object(handoff, "registry", lambda: ("o", {"r"})) + patcher.start() + unittest.addModuleCleanup(patcher.stop) + def marked(body: str, track: str, round_: int, previous: int | None) -> str: return handoff.with_marker(body, track, round_, previous) @@ -410,8 +423,8 @@ def test_new_with_the_flag_creates_the_label_then_the_first_link(self) -> None: self.assertEqual( [c[:2] for c in fake.calls], [ - ["label", "list"], ["repo", "view"], + ["label", "list"], ["label", "create"], ["label", "list"], ["issue", "create"], @@ -429,7 +442,7 @@ def test_a_dry_run_with_the_flag_writes_nothing(self) -> None: self.assertEqual(code, 0, out) self.assertIn(f"would run: gh label create {handoff.LABEL}", out) self.assertIn("would run: gh issue create", out) - self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"], ["label", "list"]]) self.assertFalse(fake.label) def test_a_body_new_would_refuse_creates_no_label(self) -> None: @@ -518,7 +531,7 @@ def test_a_repository_under_another_owner_refuses_before_any_other_call(self) -> "--create-label", ) self.assertEqual(code, 1) - self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) + self.assertEqual([c[:2] for c in fake.calls], []) def test_an_unregistered_repository_that_is_not_a_fork_is_drift(self) -> None: """It belongs in the registry, so it refuses as drift rather than taking a lone label.""" @@ -530,7 +543,7 @@ def test_an_unregistered_repository_that_is_not_a_fork_is_drift(self) -> None: fake = FakeGh(label=False, fork=False) code, _, _ = self.new(fake, "--create-label") self.assertEqual(code, 1) - self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"]]) def test_the_label_created_is_the_fleet_set_s_own_definition(self) -> None: rows = json.loads(handoff.LABELS.read_text(encoding="utf-8")) @@ -546,14 +559,61 @@ def test_link_refuses_without_the_label(self) -> None: code, _, err = run(fake, "link", "--repo", "o/r", "--new", "2", "--previous", "1") self.assertEqual(code, 1) self.assertIn("--create-label", err) - self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"], ["label", "list"]]) + + +class WriteScopeCase(unittest.TestCase): + """The writes are bounded whatever the label state, since a label opens no write by itself.""" + + def setUp(self) -> None: + fleet(self, member=False) + + def new(self, fake: FakeGh, repo: str) -> tuple[int, str, str]: + return run(fake, "new", "--repo", repo, "--title", "T", "--body-file", body_file(self, "w")) + + def test_a_labeled_repository_under_another_owner_takes_no_call_and_no_write(self) -> None: + fake = FakeGh() + code, _, err = self.new(fake, "stranger/r") + self.assertEqual(code, 1) + self.assertIn("writes nothing there", err) + self.assertEqual(fake.calls, []) + fake = FakeGh() + code, _, _ = run(fake, "link", "--repo", "stranger/r", "--new", "2", "--previous", "1") + self.assertEqual(code, 1) + self.assertEqual(fake.calls, []) + + def test_a_labeled_unregistered_repository_that_is_not_a_fork_takes_no_write(self) -> None: + fake = FakeGh(fork=False) + code, _, err = self.new(fake, "o/r") + self.assertEqual(code, 1) + self.assertIn("registry drift", err) + self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"]]) + + def test_a_labeled_unregistered_fork_files_and_asks_once_whether_it_is_one(self) -> None: + fake = FakeGh() + code, out, err = self.new(fake, "o/r") + self.assertEqual(code, 0, out + err) + self.assertEqual(sum(c[:2] == ["repo", "view"] for c in fake.calls), 1) + self.assertTrue(any(c[:2] == ["issue", "create"] for c in fake.calls)) + + def test_a_read_of_a_labeled_repository_under_another_owner_stays_open(self) -> None: + fake = FakeGh() + code, out, _ = run(fake, "tracks", "--repo", "stranger/r") + self.assertEqual(code, 0) + self.assertIn("(no open", out) + + def test_a_registered_repository_costs_no_extra_request(self) -> None: + fleet(self) + fake = FakeGh() + self.assertEqual(self.new(fake, "o/r")[0], 0) + self.assertFalse(any(c[:2] == ["repo", "view"] for c in fake.calls)) class FleetRegistryCase(unittest.TestCase): """Membership is read from the hub's registry, and an unreadable one is not an empty one.""" def test_the_registry_reads_lowercased_with_the_hub_in_it(self) -> None: - owner, names = handoff.registry() + owner, names = REAL_REGISTRY() self.assertEqual(owner, "ptr727") self.assertIn("projecttemplate", names) @@ -575,7 +635,7 @@ def test_an_unreadable_registry_fails_rather_than_reading_as_empty(self) -> None unittest.mock.patch.object(handoff, "REGISTRY", missing), self.assertRaises(handoff.Execution), ): - handoff.registry() + REAL_REGISTRY() def test_a_malformed_registry_fails_rather_than_reading_as_empty(self) -> None: bad = Path(tempfile.mkdtemp()) / "repos.json" @@ -587,7 +647,7 @@ def test_a_malformed_registry_fails_rather_than_reading_as_empty(self) -> None: unittest.mock.patch.object(handoff, "REGISTRY", bad), self.assertRaises(handoff.Execution) as caught, ): - handoff.registry() + REAL_REGISTRY() self.assertIn("could not read the fleet registry", str(caught.exception)) From 5ff97d28b58a5ead37f4bf44fdb79c697439572f Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 16:02:26 -0700 Subject: [PATCH 2/2] Say the Write Scope Bounds No Read Rather Than Opening Every Read The local strict review found "the reads stay open everywhere" false, since a read of an unlabeled repository under another owner still refuses on the missing-label path, and the pr_review.py comparison overstated parity, since that script reads its owner from origin and this one from the registry. Both now say what the code does, and link against a labeled unregistered non-fork gains its own case. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/session-handoff/SKILL.md | 2 +- .../fleet-skills/.source-digests/session-handoff | 2 +- .../fleet-skills/skills/session-handoff/SKILL.md | 2 +- .github/skills/session-handoff/SKILL.md | 2 +- scripts/README.md | 2 +- scripts/handoff.py | 9 +++++---- tests/test_handoff.py | 4 ++++ 7 files changed, 14 insertions(+), 9 deletions(-) diff --git a/.agents/skills/session-handoff/SKILL.md b/.agents/skills/session-handoff/SKILL.md index 1ddeb792..b84ad5f5 100644 --- a/.agents/skills/session-handoff/SKILL.md +++ b/.agents/skills/session-handoff/SKILL.md @@ -321,7 +321,7 @@ creates the one `handoff` label, confirms it, and then files the first link. Iss it before any write, naming the command that turns them on. Never apply the fleet label set to such a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, refuses before any write. `new` and `link` refuse both whatever the label state, since a label on -such a repository opens no write there, while the reads stay open everywhere. +such a repository opens no write there, and that refusal bounds no read. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than diff --git a/.claude-plugin/fleet-skills/.source-digests/session-handoff b/.claude-plugin/fleet-skills/.source-digests/session-handoff index c4cce7b6..6d54cf54 100644 --- a/.claude-plugin/fleet-skills/.source-digests/session-handoff +++ b/.claude-plugin/fleet-skills/.source-digests/session-handoff @@ -1 +1 @@ -551f09ebd1b0a691 +71ea890bf0050608 diff --git a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md index 1ddeb792..b84ad5f5 100644 --- a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md @@ -321,7 +321,7 @@ creates the one `handoff` label, confirms it, and then files the first link. Iss it before any write, naming the command that turns them on. Never apply the fleet label set to such a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, refuses before any write. `new` and `link` refuse both whatever the label state, since a label on -such a repository opens no write there, while the reads stay open everywhere. +such a repository opens no write there, and that refusal bounds no read. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than diff --git a/.github/skills/session-handoff/SKILL.md b/.github/skills/session-handoff/SKILL.md index 1ddeb792..b84ad5f5 100644 --- a/.github/skills/session-handoff/SKILL.md +++ b/.github/skills/session-handoff/SKILL.md @@ -321,7 +321,7 @@ creates the one `handoff` label, confirms it, and then files the first link. Iss it before any write, naming the command that turns them on. Never apply the fleet label set to such a fork. A repository under another owner, or an unregistered one of the owner's that is not a fork, refuses before any write. `new` and `link` refuse both whatever the label state, since a label on -such a repository opens no write there, while the reads stay open everywhere. +such a repository opens no write there, and that refusal bounds no read. Creating an issue, commenting on one, closing one, and editing a body are each outward-facing writes. `new` creates, comments, and closes, the label riding inside the one create call rather than diff --git a/scripts/README.md b/scripts/README.md index 4e405cca..4c9e2a1e 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -261,7 +261,7 @@ The invariant is one open handoff per track per repository, read from the metada `new` creates the new issue, comments the forward link on the previous one, and then closes it, in that order, printing each step's own result. Creating first means a failure at any later step leaves a discoverable new issue rather than a closed chain with no successor, and `link` finishes what a failure interrupted without a second issue being filed for it. `link` takes no `--track`, so the two issues' own metadata blocks are the only thing that can say they belong to one lane, and it refuses an issue named as its own predecessor and a pair whose blocks name different tracks rather than closing one lane's handoff into another's. Nothing suppresses a write's output or converts a failure into success, per [GOVERNANCE.md][governance] "Repository Boundaries and Write Safety". Where the caller names an issue, which is `link` alone, that issue is read live before the write and only what the read returned is written back, and every other identifier a write consumes comes from a read in the same run rather than being constructed. A close is confirmed by reading the state back, because a write that appears to have failed may have succeeded on the server. -`--repo` is required and carries no default, for the reason `pr_review.py` states about a pull request number: an issue number resolves in every repository, and a chain read out of the wrong one is well formed. `--track` defaults to `default`, and omitting it on a named lane does more than read the wrong chain, since `new` then files onto `default` and comments on and closes whatever that lane had open. A fleet repository missing the `handoff` label refuses, naming `repo-config/configure.sh apply`, never with a degraded empty answer, because `decision` was declared on the hub and applied nowhere else and every downstream session then enumerated an empty queue and reported it healthy. The one exception is a fork under the registry's owner that the registry does not list, such as one kept for an upstream contribution, where no issue can carry a label that does not exist: a read there warns and answers as an empty chain does, and `new --create-label` creates the one `handoff` label from `repo-config/labels.json`, confirms it, and files the first link, refusing first where the fork has issues turned off. A repository missing the label under another owner, or an unregistered one of the owner's that is not a fork, refuses before any write, the first because these calls run as subprocesses a write guard on the caller never sees and the second because it is registry drift a lone label would hide. The two writing subcommands, `new` and `link`, refuse both whatever the label state, checked before the label is read, so a label a stranger's repository happens to carry opens no write there, the same in-process owner boundary `pr_review.py` keeps. The reads stay open everywhere. Exit `0` is success, `1` a refusal the caller can act on, and `2` the command not having run to an answer, so the two never share a code: a usage error is moved off argparse's own `2` onto `1` because it is a refusal, and an exception nobody modeled is caught onto `2` rather than reaching CPython's own `1`. A body over 12 KB warns and one over 60 KB refuses, under GitHub's own 65536-character limit, and both are backstops against a body GitHub would reject: what bounds a handoff is the Skill's per-section rules. +`--repo` is required and carries no default, for the reason `pr_review.py` states about a pull request number: an issue number resolves in every repository, and a chain read out of the wrong one is well formed. `--track` defaults to `default`, and omitting it on a named lane does more than read the wrong chain, since `new` then files onto `default` and comments on and closes whatever that lane had open. A fleet repository missing the `handoff` label refuses, naming `repo-config/configure.sh apply`, never with a degraded empty answer, because `decision` was declared on the hub and applied nowhere else and every downstream session then enumerated an empty queue and reported it healthy. The one exception is a fork under the registry's owner that the registry does not list, such as one kept for an upstream contribution, where no issue can carry a label that does not exist: a read there warns and answers as an empty chain does, and `new --create-label` creates the one `handoff` label from `repo-config/labels.json`, confirms it, and files the first link, refusing first where the fork has issues turned off. A repository missing the label under another owner, or an unregistered one of the owner's that is not a fork, refuses before any write, the first because these calls run as subprocesses a write guard on the caller never sees and the second because it is registry drift a lone label would hide. The two writing subcommands, `new` and `link`, refuse both whatever the label state, checked before the label is read, so a label a stranger's repository happens to carry opens no write there. That is an in-process refusal of the kind `pr_review.py` makes, with the owner read from the registry rather than from `origin`, and it bounds no read. Exit `0` is success, `1` a refusal the caller can act on, and `2` the command not having run to an answer, so the two never share a code: a usage error is moved off argparse's own `2` onto `1` because it is a refusal, and an exception nobody modeled is caught onto `2` rather than reaching CPython's own `1`. A body over 12 KB warns and one over 60 KB refuses, under GitHub's own 65536-character limit, and both are backstops against a body GitHub would reject: what bounds a handoff is the Skill's per-section rules. `--dry-run` prints the calls a writing subcommand would make and sends none of them, which is the first thing anyone wants before trusting this to close an issue. It sits on the one path every call goes through rather than on each write separately. diff --git a/scripts/handoff.py b/scripts/handoff.py index 04a6f14c..71eed74c 100755 --- a/scripts/handoff.py +++ b/scripts/handoff.py @@ -69,10 +69,11 @@ missing the label refuses, a repository under another owner and an unregistered repository of the owner's that is not a fork among them. -The two writing subcommands, `new` and `link`, are bounded whatever the label state, as -`pr_review.py` bounds its own writes. They refuse a repository under another owner before any -call, and an unregistered repository of the owner's unless it is a fork, since these writes run as -subprocesses a write guard on the caller never sees. The reads stay open everywhere. +The two writing subcommands, `new` and `link`, are bounded whatever the label state, an +in-process refusal of the kind `pr_review.py` makes, with the owner read from the registry. They +refuse a repository under another owner before any call, and an unregistered repository of the +owner's unless it is a fork, since these writes run as subprocesses a write guard on the caller +never sees. That write scope bounds no read. """ from __future__ import annotations diff --git a/tests/test_handoff.py b/tests/test_handoff.py index b292f1ac..c73842c5 100755 --- a/tests/test_handoff.py +++ b/tests/test_handoff.py @@ -588,6 +588,10 @@ def test_a_labeled_unregistered_repository_that_is_not_a_fork_takes_no_write(sel self.assertEqual(code, 1) self.assertIn("registry drift", err) self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"]]) + fake = FakeGh(fork=False) + code, _, _ = run(fake, "link", "--repo", "o/r", "--new", "2", "--previous", "1") + self.assertEqual(code, 1) + self.assertEqual([c[:2] for c in fake.calls], [["repo", "view"]]) def test_a_labeled_unregistered_fork_files_and_asks_once_whether_it_is_one(self) -> None: fake = FakeGh()