From 3c6591a088dca1a7929fafbdbc68060e1bc01e70 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 15:07:54 -0700 Subject: [PATCH 1/5] Keep the Fork Iteration PR Open and Let a Fork Host a Handoff Chain upstream-contribution-workflow now says what happens to the fork-internal iteration PR: it targets the fork's mirror of the upstream base branch, stays open for the life of the contribution, and is never merged, since merging it stops the mirror following upstream and removes the place upstream drift is absorbed. Upstream movement is merged into the dirty branch and fixed there, the clean branch is a plain branch cut fresh from the mirror tip, and the iteration PR is closed unmerged only once upstream decides. handoff.py no longer sends a repository outside the fleet to the fleet label set. A fleet repository missing the handoff label still refuses. Outside the registry, reads warn and answer as an empty chain does, and new refuses until given --create-label, which creates and confirms that one label before filing the first link. Issues turned off stop it before any write. session-handoff and the carried AGENTS.md "Session Scope" sentence follow. Refs #2264 Co-Authored-By: Claude Opus 5.5 --- .agents/skills/session-handoff/SKILL.md | 17 +- .../upstream-contribution-workflow/SKILL.md | 63 ++++-- .../.source-digests/session-handoff | 2 +- .../upstream-contribution-workflow | 2 +- .../skills/session-handoff/SKILL.md | 17 +- .../upstream-contribution-workflow/SKILL.md | 63 ++++-- .github/skills/session-handoff/SKILL.md | 17 +- .../upstream-contribution-workflow/SKILL.md | 63 ++++-- AGENTS.md | 2 +- scripts/handoff.py | 146 ++++++++++++-- tests/test_handoff.py | 182 +++++++++++++++++- 11 files changed, 505 insertions(+), 69 deletions(-) diff --git a/.agents/skills/session-handoff/SKILL.md b/.agents/skills/session-handoff/SKILL.md index ee98d9cbf..10c2dadd9 100644 --- a/.agents/skills/session-handoff/SKILL.md +++ b/.agents/skills/session-handoff/SKILL.md @@ -308,13 +308,22 @@ It is accepted by `new` and `link`, the two subcommands that write, and by no ot Exit `0` is success, `1` a refusal the caller can act on, a usage error included, and `2` the command not having run to an answer, so a refusal and a failure to reach one never share a code. A -repository missing the `handoff` label is a refusal rather than a degraded empty answer, and it -names the command that applies the fleet label set except where the label read filled its window, -which is the one case where the label's absence is unproven rather than established. +fleet repository missing the `handoff` label, one the hub's `registry/repos.json` lists, is a +refusal rather than a degraded empty answer, and it names the command that applies the fleet label +set except where the label read filled its window, which is the one case where the label's absence +is unproven rather than established. + +A repository outside the fleet, such as a fork kept for an upstream contribution per +`upstream-contribution-workflow`, can host a chain to keep a session's state without taking on any +fleet configuration. Missing the label there, a read prints a warning and answers as an empty chain +does, and `new` refuses until it is given `--create-label`, which 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 repository. 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 -being a write of its own. `link` edits a body, comments, and closes. Each of them is bound by +being a write of its own, and `new --create-label` adds one write ahead of those, the label itself. +`link` edits a body, comments, and closes. Each of them is bound by `GOVERNANCE.md` "Repository Boundaries and Write Safety" exactly as any other write is. Point them at the repository `AGENTS.md` "Session Scope" sends the link to, the one holding the work the next session resumes, and at no other. `link` also reaches an issue this chain never created, since the diff --git a/.agents/skills/upstream-contribution-workflow/SKILL.md b/.agents/skills/upstream-contribution-workflow/SKILL.md index 654586148..0b2c8b761 100644 --- a/.agents/skills/upstream-contribution-workflow/SKILL.md +++ b/.agents/skills/upstream-contribution-workflow/SKILL.md @@ -3,12 +3,14 @@ name: upstream-contribution-workflow description: >- Governs how the maintainer contributes to a third-party repository he does not control (for example esphome/esphome), distinct from the fleet's own internal branching model: a dirty work - branch on his own fork for the actual work and review iteration, squashed once clean to a second - branch that carries only the intended minimal history, that clean branch opened as the PR - against the upstream repo, and reviewer feedback applied to the dirty branch first, then - re-squashed into the clean one. Use this whenever about to open a pull request against a - repository outside the ptr727 fleet, whenever forking a third-party project to contribute a fix - or feature, whenever an upstream reviewer requests changes on a PR opened this way, and whenever + branch on his own fork for the actual work and review iteration, its fork-internal PR kept open + and never merged, squashed once clean to a second branch that carries only the intended minimal + history, that clean branch opened as the PR against the upstream repo, and reviewer feedback + applied to the dirty branch first, then re-squashed into the clean one. Use this whenever about + to open a pull request against a repository outside the ptr727 fleet, whenever forking a + third-party project to contribute a fix or feature, whenever an upstream reviewer requests + changes on a PR opened this way, whenever a fork-internal iteration PR looks ready to merge or + close, whenever the upstream base branch moves under a contribution in flight, and whenever deciding which issue or PR template to use for a third-party repository. Triggers regardless of the target repo's own type or workflow model, since this skill is about the shape of a contribution to someone else's repo, not the target repo's own internal conventions, which this @@ -29,15 +31,24 @@ fleet's internal model so the two are never conflated. ## The two-branch shape -1. **Fork the upstream repo**, if not already forked. +1. **Fork the upstream repo**, if not already forked. The fork's copy of the upstream base branch + (`main`, `dev`, whichever upstream PRs target) is the **mirror branch**. It moves only by + syncing from upstream, and nothing of the maintainer's is ever merged into it. 2. **Do the actual work on a dirty work branch**, on the maintainer's own fork. This branch is allowed to be messy: false starts, fixup commits, back-and-forth in response to review, whatever - the real work looks like while it's happening. Open a PR from this branch into a branch on the - maintainer's **own fork** (not upstream), so all the iteration happens there, visible and - reviewable, without touching the upstream repo at all. + the real work looks like while it's happening. Open a PR from this branch into the mirror + branch on the maintainer's **own fork** (not upstream), so all the iteration happens there, + visible and reviewable by bots and the maintainer, without touching the upstream repo at all. + **That fork-internal PR is the iteration record, never a merge candidate.** It stays open for + the life of the contribution, however green it gets, and is never merged, per "The iteration PR + is never merged" below. 3. **Once the dirty branch is clean and the change is ready, squash it to a second branch** that carries only the intended, minimal commit history, one commit (or a small, deliberate set) that - states what the change is, not how it was arrived at. + states what the change is, not how it was arrived at. Cut that branch fresh from the mirror + branch's current tip and squash the dirty branch's changes onto it. It is a plain branch, not a + PR on the fork, since the review it needs already happened on the iteration PR. One dirty branch + may feed several clean branches when upstream wants the change split, each carrying only its + own slice. 4. **Open the PR against the upstream repo from that second, clean branch.** This is the only branch upstream ever sees. An upstream draft may be opened only after that clean presentation branch exists and is published. When more preparation is needed, continue on the dirty branch, @@ -63,6 +74,29 @@ fleet's internal model so the two are never conflated. Never reverse this: never iterate directly on the branch that's open against upstream, and never skip the squash step because the dirty branch "looks clean enough." +## The iteration PR is never merged + +Merging the iteration PR into the mirror branch looks like the natural finish once it is green, and +it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it can +no longer be fast-forwarded from upstream, and every later sync conflicts with the maintainer's own +change. And the iteration PR is the place where upstream drift is absorbed and its fallout fixed +before the upstream PR has to absorb the same drift, so merging it removes that place. Closing it +loses the review record the same way. + +When the upstream base moves, sync the mirror branch from upstream, merge the mirror branch into +the dirty branch, and fix whatever breaks in the iteration PR. That is a merge rather than a rebase, +since the dirty branch is append-only and is never force-pushed. Then re-squash onto the new mirror +tip whenever the clean branch needs to follow, per step 5. + +For example, the fork's mirror branch and upstream both sit at commit `A`, with the iteration PR +from `work/x` into the mirror branch green. Upstream advances to `B`. The fork syncs its mirror +branch to `B`, merges `B` into `work/x`, and fixes the breakage in the iteration PR. Had `work/x` +been merged into the mirror branch at `A`, the mirror branch could not have followed `B` cleanly. + +The contribution ends when upstream merges or declines the upstream PR. Only then close the +iteration PR, unmerged, and delete the dirty and clean branches. A merged change then reaches the +mirror branch through the next sync from upstream, the same way anyone else's change does. + ## Use the upstream repo's own conventions, not the fleet's Always use the upstream repo's own issue and PR templates, its own contribution guidelines, and @@ -82,3 +116,10 @@ write-safety rules (never write to a repository outside explicit authorization, GitHub id) also still apply in full. A fork the maintainer owns is within scope to push to, and the upstream repository itself is written to only through the PR the maintainer explicitly asked for. + +The fork is not a fleet repository. It carries none of the fleet's instruction files, labels, or +repository settings, and a resync or the fleet label set is never applied to it. A session working +the contribution can still keep its state in the `session-handoff` chain on the fork. That needs +issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new +--create-label` creates on a repository outside the fleet, and nothing else of the fleet's +configuration. diff --git a/.claude-plugin/fleet-skills/.source-digests/session-handoff b/.claude-plugin/fleet-skills/.source-digests/session-handoff index cb8a9fd89..bec7185d6 100644 --- a/.claude-plugin/fleet-skills/.source-digests/session-handoff +++ b/.claude-plugin/fleet-skills/.source-digests/session-handoff @@ -1 +1 @@ -8dfe7946b9693ac7 +e8c168644e946836 diff --git a/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow b/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow index baf4d8084..9a4ce5fe5 100644 --- a/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow +++ b/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow @@ -1 +1 @@ -f98c8047451b8448 +1529d1d88434ef19 diff --git a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md index ee98d9cbf..10c2dadd9 100644 --- a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md @@ -308,13 +308,22 @@ It is accepted by `new` and `link`, the two subcommands that write, and by no ot Exit `0` is success, `1` a refusal the caller can act on, a usage error included, and `2` the command not having run to an answer, so a refusal and a failure to reach one never share a code. A -repository missing the `handoff` label is a refusal rather than a degraded empty answer, and it -names the command that applies the fleet label set except where the label read filled its window, -which is the one case where the label's absence is unproven rather than established. +fleet repository missing the `handoff` label, one the hub's `registry/repos.json` lists, is a +refusal rather than a degraded empty answer, and it names the command that applies the fleet label +set except where the label read filled its window, which is the one case where the label's absence +is unproven rather than established. + +A repository outside the fleet, such as a fork kept for an upstream contribution per +`upstream-contribution-workflow`, can host a chain to keep a session's state without taking on any +fleet configuration. Missing the label there, a read prints a warning and answers as an empty chain +does, and `new` refuses until it is given `--create-label`, which 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 repository. 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 -being a write of its own. `link` edits a body, comments, and closes. Each of them is bound by +being a write of its own, and `new --create-label` adds one write ahead of those, the label itself. +`link` edits a body, comments, and closes. Each of them is bound by `GOVERNANCE.md` "Repository Boundaries and Write Safety" exactly as any other write is. Point them at the repository `AGENTS.md` "Session Scope" sends the link to, the one holding the work the next session resumes, and at no other. `link` also reaches an issue this chain never created, since the diff --git a/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md b/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md index 654586148..0b2c8b761 100644 --- a/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md @@ -3,12 +3,14 @@ name: upstream-contribution-workflow description: >- Governs how the maintainer contributes to a third-party repository he does not control (for example esphome/esphome), distinct from the fleet's own internal branching model: a dirty work - branch on his own fork for the actual work and review iteration, squashed once clean to a second - branch that carries only the intended minimal history, that clean branch opened as the PR - against the upstream repo, and reviewer feedback applied to the dirty branch first, then - re-squashed into the clean one. Use this whenever about to open a pull request against a - repository outside the ptr727 fleet, whenever forking a third-party project to contribute a fix - or feature, whenever an upstream reviewer requests changes on a PR opened this way, and whenever + branch on his own fork for the actual work and review iteration, its fork-internal PR kept open + and never merged, squashed once clean to a second branch that carries only the intended minimal + history, that clean branch opened as the PR against the upstream repo, and reviewer feedback + applied to the dirty branch first, then re-squashed into the clean one. Use this whenever about + to open a pull request against a repository outside the ptr727 fleet, whenever forking a + third-party project to contribute a fix or feature, whenever an upstream reviewer requests + changes on a PR opened this way, whenever a fork-internal iteration PR looks ready to merge or + close, whenever the upstream base branch moves under a contribution in flight, and whenever deciding which issue or PR template to use for a third-party repository. Triggers regardless of the target repo's own type or workflow model, since this skill is about the shape of a contribution to someone else's repo, not the target repo's own internal conventions, which this @@ -29,15 +31,24 @@ fleet's internal model so the two are never conflated. ## The two-branch shape -1. **Fork the upstream repo**, if not already forked. +1. **Fork the upstream repo**, if not already forked. The fork's copy of the upstream base branch + (`main`, `dev`, whichever upstream PRs target) is the **mirror branch**. It moves only by + syncing from upstream, and nothing of the maintainer's is ever merged into it. 2. **Do the actual work on a dirty work branch**, on the maintainer's own fork. This branch is allowed to be messy: false starts, fixup commits, back-and-forth in response to review, whatever - the real work looks like while it's happening. Open a PR from this branch into a branch on the - maintainer's **own fork** (not upstream), so all the iteration happens there, visible and - reviewable, without touching the upstream repo at all. + the real work looks like while it's happening. Open a PR from this branch into the mirror + branch on the maintainer's **own fork** (not upstream), so all the iteration happens there, + visible and reviewable by bots and the maintainer, without touching the upstream repo at all. + **That fork-internal PR is the iteration record, never a merge candidate.** It stays open for + the life of the contribution, however green it gets, and is never merged, per "The iteration PR + is never merged" below. 3. **Once the dirty branch is clean and the change is ready, squash it to a second branch** that carries only the intended, minimal commit history, one commit (or a small, deliberate set) that - states what the change is, not how it was arrived at. + states what the change is, not how it was arrived at. Cut that branch fresh from the mirror + branch's current tip and squash the dirty branch's changes onto it. It is a plain branch, not a + PR on the fork, since the review it needs already happened on the iteration PR. One dirty branch + may feed several clean branches when upstream wants the change split, each carrying only its + own slice. 4. **Open the PR against the upstream repo from that second, clean branch.** This is the only branch upstream ever sees. An upstream draft may be opened only after that clean presentation branch exists and is published. When more preparation is needed, continue on the dirty branch, @@ -63,6 +74,29 @@ fleet's internal model so the two are never conflated. Never reverse this: never iterate directly on the branch that's open against upstream, and never skip the squash step because the dirty branch "looks clean enough." +## The iteration PR is never merged + +Merging the iteration PR into the mirror branch looks like the natural finish once it is green, and +it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it can +no longer be fast-forwarded from upstream, and every later sync conflicts with the maintainer's own +change. And the iteration PR is the place where upstream drift is absorbed and its fallout fixed +before the upstream PR has to absorb the same drift, so merging it removes that place. Closing it +loses the review record the same way. + +When the upstream base moves, sync the mirror branch from upstream, merge the mirror branch into +the dirty branch, and fix whatever breaks in the iteration PR. That is a merge rather than a rebase, +since the dirty branch is append-only and is never force-pushed. Then re-squash onto the new mirror +tip whenever the clean branch needs to follow, per step 5. + +For example, the fork's mirror branch and upstream both sit at commit `A`, with the iteration PR +from `work/x` into the mirror branch green. Upstream advances to `B`. The fork syncs its mirror +branch to `B`, merges `B` into `work/x`, and fixes the breakage in the iteration PR. Had `work/x` +been merged into the mirror branch at `A`, the mirror branch could not have followed `B` cleanly. + +The contribution ends when upstream merges or declines the upstream PR. Only then close the +iteration PR, unmerged, and delete the dirty and clean branches. A merged change then reaches the +mirror branch through the next sync from upstream, the same way anyone else's change does. + ## Use the upstream repo's own conventions, not the fleet's Always use the upstream repo's own issue and PR templates, its own contribution guidelines, and @@ -82,3 +116,10 @@ write-safety rules (never write to a repository outside explicit authorization, GitHub id) also still apply in full. A fork the maintainer owns is within scope to push to, and the upstream repository itself is written to only through the PR the maintainer explicitly asked for. + +The fork is not a fleet repository. It carries none of the fleet's instruction files, labels, or +repository settings, and a resync or the fleet label set is never applied to it. A session working +the contribution can still keep its state in the `session-handoff` chain on the fork. That needs +issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new +--create-label` creates on a repository outside the fleet, and nothing else of the fleet's +configuration. diff --git a/.github/skills/session-handoff/SKILL.md b/.github/skills/session-handoff/SKILL.md index ee98d9cbf..10c2dadd9 100644 --- a/.github/skills/session-handoff/SKILL.md +++ b/.github/skills/session-handoff/SKILL.md @@ -308,13 +308,22 @@ It is accepted by `new` and `link`, the two subcommands that write, and by no ot Exit `0` is success, `1` a refusal the caller can act on, a usage error included, and `2` the command not having run to an answer, so a refusal and a failure to reach one never share a code. A -repository missing the `handoff` label is a refusal rather than a degraded empty answer, and it -names the command that applies the fleet label set except where the label read filled its window, -which is the one case where the label's absence is unproven rather than established. +fleet repository missing the `handoff` label, one the hub's `registry/repos.json` lists, is a +refusal rather than a degraded empty answer, and it names the command that applies the fleet label +set except where the label read filled its window, which is the one case where the label's absence +is unproven rather than established. + +A repository outside the fleet, such as a fork kept for an upstream contribution per +`upstream-contribution-workflow`, can host a chain to keep a session's state without taking on any +fleet configuration. Missing the label there, a read prints a warning and answers as an empty chain +does, and `new` refuses until it is given `--create-label`, which 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 repository. 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 -being a write of its own. `link` edits a body, comments, and closes. Each of them is bound by +being a write of its own, and `new --create-label` adds one write ahead of those, the label itself. +`link` edits a body, comments, and closes. Each of them is bound by `GOVERNANCE.md` "Repository Boundaries and Write Safety" exactly as any other write is. Point them at the repository `AGENTS.md` "Session Scope" sends the link to, the one holding the work the next session resumes, and at no other. `link` also reaches an issue this chain never created, since the diff --git a/.github/skills/upstream-contribution-workflow/SKILL.md b/.github/skills/upstream-contribution-workflow/SKILL.md index 654586148..0b2c8b761 100644 --- a/.github/skills/upstream-contribution-workflow/SKILL.md +++ b/.github/skills/upstream-contribution-workflow/SKILL.md @@ -3,12 +3,14 @@ name: upstream-contribution-workflow description: >- Governs how the maintainer contributes to a third-party repository he does not control (for example esphome/esphome), distinct from the fleet's own internal branching model: a dirty work - branch on his own fork for the actual work and review iteration, squashed once clean to a second - branch that carries only the intended minimal history, that clean branch opened as the PR - against the upstream repo, and reviewer feedback applied to the dirty branch first, then - re-squashed into the clean one. Use this whenever about to open a pull request against a - repository outside the ptr727 fleet, whenever forking a third-party project to contribute a fix - or feature, whenever an upstream reviewer requests changes on a PR opened this way, and whenever + branch on his own fork for the actual work and review iteration, its fork-internal PR kept open + and never merged, squashed once clean to a second branch that carries only the intended minimal + history, that clean branch opened as the PR against the upstream repo, and reviewer feedback + applied to the dirty branch first, then re-squashed into the clean one. Use this whenever about + to open a pull request against a repository outside the ptr727 fleet, whenever forking a + third-party project to contribute a fix or feature, whenever an upstream reviewer requests + changes on a PR opened this way, whenever a fork-internal iteration PR looks ready to merge or + close, whenever the upstream base branch moves under a contribution in flight, and whenever deciding which issue or PR template to use for a third-party repository. Triggers regardless of the target repo's own type or workflow model, since this skill is about the shape of a contribution to someone else's repo, not the target repo's own internal conventions, which this @@ -29,15 +31,24 @@ fleet's internal model so the two are never conflated. ## The two-branch shape -1. **Fork the upstream repo**, if not already forked. +1. **Fork the upstream repo**, if not already forked. The fork's copy of the upstream base branch + (`main`, `dev`, whichever upstream PRs target) is the **mirror branch**. It moves only by + syncing from upstream, and nothing of the maintainer's is ever merged into it. 2. **Do the actual work on a dirty work branch**, on the maintainer's own fork. This branch is allowed to be messy: false starts, fixup commits, back-and-forth in response to review, whatever - the real work looks like while it's happening. Open a PR from this branch into a branch on the - maintainer's **own fork** (not upstream), so all the iteration happens there, visible and - reviewable, without touching the upstream repo at all. + the real work looks like while it's happening. Open a PR from this branch into the mirror + branch on the maintainer's **own fork** (not upstream), so all the iteration happens there, + visible and reviewable by bots and the maintainer, without touching the upstream repo at all. + **That fork-internal PR is the iteration record, never a merge candidate.** It stays open for + the life of the contribution, however green it gets, and is never merged, per "The iteration PR + is never merged" below. 3. **Once the dirty branch is clean and the change is ready, squash it to a second branch** that carries only the intended, minimal commit history, one commit (or a small, deliberate set) that - states what the change is, not how it was arrived at. + states what the change is, not how it was arrived at. Cut that branch fresh from the mirror + branch's current tip and squash the dirty branch's changes onto it. It is a plain branch, not a + PR on the fork, since the review it needs already happened on the iteration PR. One dirty branch + may feed several clean branches when upstream wants the change split, each carrying only its + own slice. 4. **Open the PR against the upstream repo from that second, clean branch.** This is the only branch upstream ever sees. An upstream draft may be opened only after that clean presentation branch exists and is published. When more preparation is needed, continue on the dirty branch, @@ -63,6 +74,29 @@ fleet's internal model so the two are never conflated. Never reverse this: never iterate directly on the branch that's open against upstream, and never skip the squash step because the dirty branch "looks clean enough." +## The iteration PR is never merged + +Merging the iteration PR into the mirror branch looks like the natural finish once it is green, and +it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it can +no longer be fast-forwarded from upstream, and every later sync conflicts with the maintainer's own +change. And the iteration PR is the place where upstream drift is absorbed and its fallout fixed +before the upstream PR has to absorb the same drift, so merging it removes that place. Closing it +loses the review record the same way. + +When the upstream base moves, sync the mirror branch from upstream, merge the mirror branch into +the dirty branch, and fix whatever breaks in the iteration PR. That is a merge rather than a rebase, +since the dirty branch is append-only and is never force-pushed. Then re-squash onto the new mirror +tip whenever the clean branch needs to follow, per step 5. + +For example, the fork's mirror branch and upstream both sit at commit `A`, with the iteration PR +from `work/x` into the mirror branch green. Upstream advances to `B`. The fork syncs its mirror +branch to `B`, merges `B` into `work/x`, and fixes the breakage in the iteration PR. Had `work/x` +been merged into the mirror branch at `A`, the mirror branch could not have followed `B` cleanly. + +The contribution ends when upstream merges or declines the upstream PR. Only then close the +iteration PR, unmerged, and delete the dirty and clean branches. A merged change then reaches the +mirror branch through the next sync from upstream, the same way anyone else's change does. + ## Use the upstream repo's own conventions, not the fleet's Always use the upstream repo's own issue and PR templates, its own contribution guidelines, and @@ -82,3 +116,10 @@ write-safety rules (never write to a repository outside explicit authorization, GitHub id) also still apply in full. A fork the maintainer owns is within scope to push to, and the upstream repository itself is written to only through the PR the maintainer explicitly asked for. + +The fork is not a fleet repository. It carries none of the fleet's instruction files, labels, or +repository settings, and a resync or the fleet label set is never applied to it. A session working +the contribution can still keep its state in the `session-handoff` chain on the fork. That needs +issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new +--create-label` creates on a repository outside the fleet, and nothing else of the fleet's +configuration. diff --git a/AGENTS.md b/AGENTS.md index 5ffbb17fe..c4c5f82c0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,7 @@ An agent session is billed on the context it carries, not the work it does. Ever - **One deliverable, one session.** A session covers one branch and one deliverable, and ends when that work merges. A multi-step task is one deliverable and stays in one session. Two unrelated tasks are two sessions even when they run back to back. - **End a session at any of these, without being asked:** the branch changes, the pull request merges, or the next task is unrelated to the last. A review round is none of them. A loop still producing findings is the deliverable in progress, and a round count is not a reason to leave one open. - **A session orchestrating dispatched work is an exception, and a narrow one.** Its deliverable is the run rather than any branch, so it spans many branches and many merges by construction, and ending it at the first dispatched merge would end the run. The triggers above land on each dispatched task instead, one branch and one deliverable each, which is this rule applied rather than waived. What keeps the exception narrow is that such a session holds no branch of its own and authors none of the work it dispatches, so the file context every other session accumulates is context it never takes on, and it re-derives each round's state from live sources rather than holding it, per "Re-derive state, do not carry it" below. A session that starts editing the files a dispatched task would have edited is an ordinary one again and ends on the triggers above. -- **Hand off in an issue, never in a scratch file and never in context.** Close a session by filing the next link in the handoff chain of the repository holding the work the next session resumes, which for a session that stayed in one repository is this one, and a session that spanned several names the others in the handoff's state section. A track is a lane of work named by a short slug, `default` where a session names none, and a track in use holds exactly one open issue carrying the `handoff` label. The track and the predecessor are recorded in the issue body rather than in its title, so a retitled or hand-edited issue still chains and a reader can tell which lane an open issue belongs to. The new link names its predecessor that way, a forward-link comment then goes onto that predecessor, and the predecessor is closed last, in that order, so a failure part way leaves a discoverable new issue rather than a closed chain with no successor. Each of those three is an outward-facing write, so the write-safety rules in `GOVERNANCE.md` "Repository Boundaries and Write Safety" bind all three exactly as they bind any other write, the identifier rule most of all, since the link a comment and a close target is read live in the same run rather than remembered. The chain needs the `handoff` label to be findable at all, so a repository not carrying it cannot host one until the fleet label set is applied there, which is a change to that repository's configuration rather than anything a handoff writes. A session that cannot file a link reports that it could not hand off and leaves the previous link open, whether it is stopped by a repository carrying no such label, by a write it may not make, or by anything else. That is the one alternative this rule allows to a track still in use, and it is a report rather than a file, because a report says the round's record is missing while a file claims to be it. A track whose work is complete is closed out instead, its last link carrying the outcome as a comment and closed with no successor, which leaves the track no longer in use rather than breaking its chain. A scratch file fails three ways the chain closes. It is not found where the next session looks. More than one candidate is found and nothing says which is current. And it holds no history, so a later round re-runs a path an earlier round already tried and already wrote down, which is the one thing a handoff exists to prevent. The handoff carries the next steps in priority order, the external blockers and internal dependencies among them, the state a resume re-reads rather than trusts, listed so the resume knows what to re-read, the account of parked decisions that `GOVERNANCE.md` "Communicating with the User" requires, what the last round did, what not to repeat, and what was learned. That section states the account whole, and it requires the session to present those decisions as well as record them. **The size rule is stated per section.** An entry earns its place by being specific enough to change a later session's behavior, a section ranks what it keeps and drops whatever does not meet that bar, and what belongs somewhere durable goes there and appears here as one line and a pointer, a defect as an issue, a rule as rule text, a lesson as governance prose. The parked-decision account keeps the count and the ranked questions one round can carry, and names every issue past those by number alone, which is what keeps a queue larger than one round inside this rule. A summary held in context is re-billed until the session ends, a scratch file is read only on the machine holding it, and a closed link stays readable from any machine to every session after it. +- **Hand off in an issue, never in a scratch file and never in context.** Close a session by filing the next link in the handoff chain of the repository holding the work the next session resumes, which for a session that stayed in one repository is this one, and a session that spanned several names the others in the handoff's state section. A track is a lane of work named by a short slug, `default` where a session names none, and a track in use holds exactly one open issue carrying the `handoff` label. The track and the predecessor are recorded in the issue body rather than in its title, so a retitled or hand-edited issue still chains and a reader can tell which lane an open issue belongs to. The new link names its predecessor that way, a forward-link comment then goes onto that predecessor, and the predecessor is closed last, in that order, so a failure part way leaves a discoverable new issue rather than a closed chain with no successor. Each of those three is an outward-facing write, so the write-safety rules in `GOVERNANCE.md` "Repository Boundaries and Write Safety" bind all three exactly as they bind any other write, the identifier rule most of all, since the link a comment and a close target is read live in the same run rather than remembered. The chain needs the `handoff` label to be findable at all, so a repository not carrying it cannot host one until the label exists there, which is a change to that repository's configuration rather than anything a handoff writes. A session that cannot file a link reports that it could not hand off and leaves the previous link open, whether it is stopped by a repository carrying no such label, by a write it may not make, or by anything else. That is the one alternative this rule allows to a track still in use, and it is a report rather than a file, because a report says the round's record is missing while a file claims to be it. A track whose work is complete is closed out instead, its last link carrying the outcome as a comment and closed with no successor, which leaves the track no longer in use rather than breaking its chain. A scratch file fails three ways the chain closes. It is not found where the next session looks. More than one candidate is found and nothing says which is current. And it holds no history, so a later round re-runs a path an earlier round already tried and already wrote down, which is the one thing a handoff exists to prevent. The handoff carries the next steps in priority order, the external blockers and internal dependencies among them, the state a resume re-reads rather than trusts, listed so the resume knows what to re-read, the account of parked decisions that `GOVERNANCE.md` "Communicating with the User" requires, what the last round did, what not to repeat, and what was learned. That section states the account whole, and it requires the session to present those decisions as well as record them. **The size rule is stated per section.** An entry earns its place by being specific enough to change a later session's behavior, a section ranks what it keeps and drops whatever does not meet that bar, and what belongs somewhere durable goes there and appears here as one line and a pointer, a defect as an issue, a rule as rule text, a lesson as governance prose. The parked-decision account keeps the count and the ranked questions one round can carry, and names every issue past those by number alone, which is what keeps a queue larger than one round inside this rule. A summary held in context is re-billed until the session ends, a scratch file is read only on the machine holding it, and a closed link stays readable from any machine to every session after it. - **Re-derive state, do not carry it.** "This session already has the context" is the signal to split, not to continue. Context that has gone stale is worse than absent, because a file read hundreds of requests ago no longer describes the file. - **Compaction is a fallback, not the strategy.** It restarts context from a floor and climbs again, where a fresh session starts from zero. diff --git a/scripts/handoff.py b/scripts/handoff.py index 748128105..0f86a7713 100755 --- a/scripts/handoff.py +++ b/scripts/handoff.py @@ -39,8 +39,8 @@ Exit codes 0 the command did what it says. - 1 a refusal the caller can act on: the label is missing from the repository, no handoff is - open on the track, a track is ambiguous, a handoff carries no metadata block, a body is + 1 a refusal the caller can act on: the label is missing from a fleet repository, or from one + outside the fleet where `new` was not given `--create-label`, no handoff is open on the track, a track is ambiguous, a handoff carries no metadata block, a body is over the hard cap, or the command line itself was wrong. A usage error is a refusal, so `Parser` below moves it here off argparse's own 2. 2 the command did not run to an answer: `gh` failed, a write did not confirm, or an exception @@ -52,12 +52,19 @@ python3 scripts/handoff.py resume --repo OWNER/NAME [--track T] [--history N] python3 scripts/handoff.py chain --repo OWNER/NAME [--track T] [--limit N] [--grep PATTERN] python3 scripts/handoff.py new --repo OWNER/NAME [--track T] --title S --body-file PATH + [--create-label] python3 scripts/handoff.py link --repo OWNER/NAME --new N --previous N python3 scripts/handoff.py tracks --repo OWNER/NAME Every subcommand takes `--repo`, with no default, for the reason `pr_review.py` requires one: an issue number resolves in every repository, and a chain read out of the wrong one is well formed. Add `--dry-run` to any writing subcommand to print the calls it would make and write nothing. + +A repository missing the label is drift on a fleet repository, one `registry/repos.json` lists, and +every subcommand refuses there. A repository outside the fleet, such as a fork kept for an upstream +contribution, can still use a chain to hold state, and the fleet label set does not belong on it. +There the reads warn and answer as an empty chain does, and `new --create-label` creates the one +`handoff` label and nothing else. """ from __future__ import annotations @@ -84,6 +91,10 @@ # A `handoff` label declared and not applied fails the same silent way, so this refuses instead. APPLY = "repo-config/configure.sh apply OWNER/NAME release|operational" +REGISTRY = Path(__file__).resolve().parent.parent / "registry" / "repos.json" + +READS = frozenset({"current", "resume", "chain", "tracks"}) + DEFAULT_TRACK = "default" # A track is a short kebab-case slug naming a lane of work. @@ -215,11 +226,11 @@ def rows_of(data: object, what: str, field: str) -> list[dict]: return data -def require_label(repo: str) -> None: - """Refuse where the target repository does not carry the label the chain is indexed by. +def label_present(repo: str) -> bool: + """Whether the target repository carries the label the chain is indexed by. - Degrading instead would report an empty chain on a repository that has one, which is the - failure mode `decision` already demonstrated fleet-wide. + A read that filled its window refuses rather than answering, since there the label's absence is + unproven rather than established. """ rows = rows_of( gh_json(["label", "list", "--repo", repo, "--limit", str(WINDOW), "--json", "name"]), @@ -233,11 +244,105 @@ def require_label(repo: str) -> None: "could sit past it. Reporting the label as absent here would send you to re-apply a " "set that may already be applied." ) - if LABEL not in names: + return LABEL in names + + +def in_fleet(repo: str) -> bool: + """Whether the hub's registry lists the repository, compared case-insensitively as GitHub does. + + An unreadable registry is a failure to answer rather than an answer, since reading it as empty + would move every fleet repository onto the warning path. + """ + try: + data = json.loads(REGISTRY.read_text(encoding="utf-8")) + owner = data["owner"] + names = {row["name"].lower() for row in data["repos"]} + except (OSError, ValueError, KeyError, TypeError) as exc: + raise Execution(f"could not read the fleet registry at {REGISTRY}: {exc}") from exc + repo_owner, _, name = repo.partition("/") + return repo_owner.lower() == owner.lower() and name.lower() in names + + +def issues_enabled(repo: str) -> bool: + data = gh_json(["repo", "view", repo, "--json", "hasIssuesEnabled"]) + if not isinstance(data, dict) or not isinstance(data.get("hasIssuesEnabled"), bool): + raise Execution(f"repo view for {repo} returned no hasIssuesEnabled: {data!r}") + return data["hasIssuesEnabled"] + + +def create_label(repo: str, dry_run: bool) -> None: + """Create the one `handoff` label and confirm it by reading the label set back.""" + out = gh( + [ + "label", + "create", + LABEL, + "--repo", + repo, + "--description", + "A session handoff link", + "--color", + "5319e7", + ], + dry_run=dry_run, + ).strip() + if dry_run: + return + print(f" label created: {out or LABEL}") + if not label_present(repo): + raise Execution( + f"{repo} still carries no `{LABEL}` label after the create, so the create is not " + "confirmed. A write that appears to have failed is verified, never assumed harmless." + ) + + +def without_label(a: argparse.Namespace) -> int | None: + """What a repository missing the label gets, or None where the subcommand goes on to run. + + On a fleet repository it is drift, and every subcommand refuses, since degrading there would + report an empty chain on a repository that has one, the failure `decision` demonstrated. + Outside the fleet no issue can carry a label that does not exist, so an empty chain is the true + answer there rather than a degraded one, and only `new --create-label` writes anything. + """ + repo = a.repo + create = getattr(a, "create_label", False) + if in_fleet(repo): + flag = " `--create-label` is for a repository outside the fleet." if create else "" raise Refusal( - f"{repo} carries no `{LABEL}` label, so no handoff can be found or filed there. " - f"Apply the fleet label set from a hub checkout: {APPLY}" + f"{repo} carries no `{LABEL}` label, so no handoff can be found or filed there." + f"{flag} Apply the fleet label set from a hub checkout: {APPLY}" ) + fix = f"gh label create {LABEL} --repo {repo}" + if not issues_enabled(repo): + fix = f"gh repo edit {repo} --enable-issues, then {fix}" + if create: + raise Refusal( + f"{repo} has issues turned off, so no handoff can be filed there. Run " + f"gh repo edit {repo} --enable-issues first, then rerun with --create-label." + ) + if a.cmd in READS: + print( + f"warning: {repo} is outside the fleet and carries no `{LABEL}` label, so it holds no " + f"handoff chain. `new --create-label` creates that one label, or run: {fix}", + file=sys.stderr, + ) + if a.cmd == "tracks": + print(f"(no open `{LABEL}` issues in {repo})") + return 0 + raise Refusal( + f"{repo} has no handoff on track {a.track!r}. Zero is the state before the first " + "handoff on a track, so `new` is what follows, not a retry of this." + ) + if a.cmd == "new" and create: + print(f"0. create the `{LABEL}` label on {repo}, which is outside the fleet") + create_label(repo, a.dry_run) + a.fresh_label = True + return None + raise Refusal( + f"{repo} is outside the fleet and carries no `{LABEL}` label, so no handoff can be filed " + f"there. Rerun `new` with --create-label to create that one label, or run: {fix}. " + "The fleet label set does not belong on a repository outside the fleet." + ) def parse_marker(body: str, number: int) -> dict[str, str] | None: @@ -764,11 +869,18 @@ def cmd_new(a: argparse.Namespace) -> int: filing on one track at once both resolve the same predecessor and both create. That leaves the two open handoffs every later command refuses over, which is detectable rather than silent, and naming a track per lane is what keeps two sessions off one chain in the first place. + + After `--create-label` the chain is empty by construction, since a label this run just created + sits on no issue yet, so no chain read is made, which also keeps a dry run from querying a label + it never created. """ body = body_from(Path(a.body_file)) - rows = open_handoffs(a.repo) - require_marked(rows) - previous = on_track(rows, a.track) or newest_closed(a.repo, a.track) + if getattr(a, "fresh_label", False): + rows, previous = [], None + else: + rows = open_handoffs(a.repo) + require_marked(rows) + previous = on_track(rows, a.track) or newest_closed(a.repo, a.track) previous_number = previous["number"] if previous else None round_ = int(previous["marker"]["round"]) + 1 if previous else 1 # A head can already have a successor, whatever key resolved it. @@ -1040,6 +1152,11 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument( "--body-file", required=True, metavar="PATH", help="the handoff body, per the skill" ) + p.add_argument( + "--create-label", + action="store_true", + help="on a repository outside the fleet with no handoff label, create that one label first", + ) p = sub.add_parser("link", help="finish a chain that half-applied") add_repo(p) @@ -1080,7 +1197,10 @@ 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_label(a.repo) + if not label_present(a.repo): + answered = without_label(a) + if answered is not None: + return answered return HANDLERS[a.cmd](a) except Refusal as exc: print(f"refused: {exc}", file=sys.stderr) diff --git a/tests/test_handoff.py b/tests/test_handoff.py index 0b782b87d..00e541d7a 100755 --- a/tests/test_handoff.py +++ b/tests/test_handoff.py @@ -7,8 +7,10 @@ each write case asserts the argv it produced rather than only its effect. Two refusals earn cases of their own because the chain exists to fix them. An ambiguous track is -never resolved by picking, and a repository missing the label refuses rather than reporting an -empty chain, which is the silent failure the `decision` label already demonstrated fleet-wide. +never resolved by picking, and a fleet repository missing the label refuses rather than reporting +an empty chain, which is the silent failure the `decision` label already demonstrated fleet-wide. +A repository outside the fleet warns instead, and its cases prove the warning path never writes +anything but the one label `new --create-label` asks for. Run as `python3 tests/test_handoff.py`, or under `python3 -m unittest discover -s tests`. @@ -70,15 +72,21 @@ class FakeGh: It honors `--json`, `--state`, `--label`, and `--limit`, because a stub looser than the tool it stands in for is a stub that green-lights a crash. - It also refuses any call carrying no `--repo`, which real `gh` would answer by resolving the - repository from the working directory's remote. That is the failure this whole script is built + It also refuses any call carrying no `--repo`, apart from `repo view`, which names its + repository positionally and is refused instead where that name is no OWNER/NAME. Real `gh` + would answer a call missing its repository by resolving it from the working directory's + remote. That is the failure this whole script is built against, and without this the argument could be dropped from any of nine call sites with every case still green. """ - def __init__(self, issues: dict[int, dict] | None = None, *, label: bool = True) -> None: + def __init__( + self, issues: dict[int, dict] | None = None, *, label: bool = True, issues_on: bool = True + ) -> None: self.issues = issues or {} self.label = label + self.issues_on = issues_on + self.refuse_label = False self.calls: list[list[str]] = [] self.next_number = 1001 # A close that reports success and leaves the issue open. @@ -87,6 +95,10 @@ def __init__(self, issues: dict[int, dict] | None = None, *, label: bool = True) def __call__(self, argv: list[str]) -> str: self.calls.append(list(argv)) + if argv[:2] == ["repo", "view"]: + if "/" not in argv[2]: + raise AssertionError(f"{' '.join(argv)} names no OWNER/NAME repository") + return json.dumps({"hasIssuesEnabled": self.issues_on}) if "--repo" not in argv: raise AssertionError( f"{' '.join(argv)} carries no --repo, so real gh would resolve the repository " @@ -95,6 +107,10 @@ def __call__(self, argv: list[str]) -> str: head = (argv[0], argv[1]) if head == ("label", "list"): return json.dumps([{"name": handoff.LABEL}] if self.label else [{"name": "bug"}]) + if head == ("label", "create"): + if not self.refuse_label: + self.label = argv[2] == handoff.LABEL + return "" if head == ("issue", "list"): return json.dumps([projected(row, argv) for row in self._list(argv)]) if head == ("issue", "view"): @@ -258,8 +274,18 @@ def test_a_retitled_issue_still_chains(self) -> None: self.assertEqual(read_marker(row["body"], 20)["track"], "default") +def fleet(case: unittest.TestCase, member: bool = True) -> None: + """Pin whether `o/r` reads as a fleet repository, for the rest of the case.""" + patcher = unittest.mock.patch.object(handoff, "in_fleet", lambda repo: member) + patcher.start() + case.addCleanup(patcher.stop) + + class LabelCase(unittest.TestCase): - """A repository missing the label refuses rather than reporting an empty chain.""" + """A fleet repository missing the label refuses rather than reporting an empty chain.""" + + def setUp(self) -> None: + fleet(self) def test_a_missing_label_refuses_and_names_the_fix(self) -> None: code, _, err = run(FakeGh(label=False), "current", "--repo", "o/r") @@ -267,6 +293,25 @@ def test_a_missing_label_refuses_and_names_the_fix(self) -> None: self.assertIn("configure.sh apply", err) self.assertIn(handoff.LABEL, err) + def test_create_label_is_refused_on_a_fleet_repository(self) -> None: + """There the fleet label set is the fix, and one label alone would hide the drift.""" + fake = FakeGh(label=False) + code, _, err = run( + fake, + "new", + "--repo", + "o/r", + "--title", + "T", + "--body-file", + body_file(self, "w"), + "--create-label", + ) + self.assertEqual(code, 1) + self.assertIn("outside the fleet", err) + self.assertIn("configure.sh apply", err) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) + def test_a_full_label_window_refuses_rather_than_reporting_the_label_absent(self) -> None: """Otherwise the caller is sent to re-apply a set that may already be applied.""" @@ -303,6 +348,127 @@ def test_the_label_is_checked_before_every_subcommand(self) -> None: self.assertEqual(fake.calls[0][:2], ["label", "list"]) +class OutsideFleetLabelCase(unittest.TestCase): + """A repository outside the fleet with no label warns, and writes only the label it is asked to.""" + + def setUp(self) -> None: + fleet(self, member=False) + + def new(self, fake: FakeGh, *extra: str) -> tuple[int, str, str]: + return run( + fake, + "new", + "--repo", + "o/r", + "--title", + "T", + "--body-file", + body_file(self, "w"), + *extra, + ) + + def test_a_read_warns_and_answers_as_an_empty_chain(self) -> None: + for cmd in ("current", "resume", "chain"): + with self.subTest(cmd=cmd): + fake = FakeGh(label=False) + code, _, err = run(fake, cmd, "--repo", "o/r") + self.assertEqual(code, 1) + self.assertIn("warning:", err) + self.assertIn(f"gh label create {handoff.LABEL} --repo o/r", err) + self.assertIn("no handoff on track", err) + self.assertNotIn("configure.sh", err) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) + + def test_tracks_warns_and_lists_nothing(self) -> None: + code, out, err = run(FakeGh(label=False), "tracks", "--repo", "o/r") + self.assertEqual(code, 0) + self.assertIn("(no open", out) + self.assertIn("warning:", err) + + def test_new_without_the_flag_refuses_and_names_it(self) -> None: + fake = FakeGh(label=False) + code, _, err = self.new(fake) + self.assertEqual(code, 1) + self.assertIn("--create-label", err) + self.assertNotIn("configure.sh", err) + self.assertFalse(any(c[:2] == ["label", "create"] for c in fake.calls)) + self.assertFalse(any(c[:2] == ["issue", "create"] for c in fake.calls)) + + def test_new_with_the_flag_creates_the_label_then_the_first_link(self) -> None: + """The label is confirmed before the issue that needs it, and no chain read is made.""" + fake = FakeGh(label=False) + code, out, _ = self.new(fake, "--create-label") + self.assertEqual(code, 0, out) + self.assertEqual( + [c[:2] for c in fake.calls], + [ + ["label", "list"], + ["repo", "view"], + ["label", "create"], + ["label", "list"], + ["issue", "create"], + ], + ) + self.assertEqual(fake.calls[2][2], handoff.LABEL) + filed = fake.issues[1001] + self.assertEqual([label["name"] for label in filed["labels"]], [handoff.LABEL]) + marker = read_marker(filed["body"], 1001) + self.assertEqual((marker["round"], marker["previous"]), ("1", "none")) + + def test_a_dry_run_with_the_flag_writes_nothing(self) -> None: + fake = FakeGh(label=False) + code, out, _ = self.new(fake, "--create-label", "--dry-run") + 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.assertFalse(fake.label) + + def test_an_unconfirmed_label_create_fails_before_filing(self) -> None: + fake = FakeGh(label=False) + fake.refuse_label = True + code, _, err = self.new(fake, "--create-label") + self.assertEqual(code, 2) + self.assertIn("not confirmed", err) + self.assertFalse(any(c[:2] == ["issue", "create"] for c in fake.calls)) + + def test_issues_turned_off_are_named_and_stop_the_label_create(self) -> None: + code, _, err = run(FakeGh(label=False, issues_on=False), "current", "--repo", "o/r") + self.assertEqual(code, 1) + self.assertIn("gh repo edit o/r --enable-issues", err) + fake = FakeGh(label=False, issues_on=False) + code, _, err = self.new(fake, "--create-label") + self.assertEqual(code, 1) + self.assertIn("--enable-issues", err) + self.assertFalse(any(c[:2] == ["label", "create"] for c in fake.calls)) + + def test_link_refuses_without_the_label(self) -> None: + fake = FakeGh(label=False) + 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"]]) + + +class FleetRegistryCase(unittest.TestCase): + """Membership is read from the hub's registry, and an unreadable one is not an empty one.""" + + def test_the_hub_itself_is_in_the_fleet_and_a_stranger_is_not(self) -> None: + self.assertTrue(handoff.in_fleet("ptr727/ProjectTemplate")) + self.assertTrue(handoff.in_fleet("PTR727/projecttemplate")) + self.assertFalse(handoff.in_fleet("ptr727/not-a-registered-repo")) + self.assertFalse(handoff.in_fleet("someone-else/ProjectTemplate")) + + def test_an_unreadable_registry_fails_rather_than_reading_as_empty(self) -> None: + missing = Path(tempfile.mkdtemp()) / "repos.json" + self.addCleanup(shutil.rmtree, missing.parent) + with ( + unittest.mock.patch.object(handoff, "REGISTRY", missing), + self.assertRaises(handoff.Execution), + ): + handoff.in_fleet("ptr727/ProjectTemplate") + + class CurrentCase(unittest.TestCase): """Exactly one open handoff per track, and anything else is reported rather than resolved.""" @@ -1154,7 +1320,7 @@ def test_a_row_that_is_not_an_object_is_named_too(self) -> None: ), self.assertRaises(handoff.Execution) as caught, ): - handoff.require_label("o/r") + handoff.label_present("o/r") self.assertIn("carrying no name", str(caught.exception)) def test_a_label_list_that_is_not_an_array_is_an_execution_failure(self) -> None: @@ -1163,7 +1329,7 @@ def test_a_label_list_that_is_not_an_array_is_an_execution_failure(self) -> None unittest.mock.patch.object(handoff, "run_gh", lambda argv: '{"not": "an array"}'), self.assertRaises(handoff.Execution) as caught, ): - handoff.require_label("o/r") + handoff.label_present("o/r") self.assertIn("did not read as an array", str(caught.exception)) def test_an_unreadable_body_file_refuses_rather_than_filing_an_empty_handoff(self) -> None: From 365d7149dc706f6d483a4ffcc246e440e591c259 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 15:17:14 -0700 Subject: [PATCH 2/5] Bound the Handoff Label Fallback to an Unregistered Fork Under the Owner The local strict review found that "outside the fleet" meant only "not in the registry", so --create-label would put a label and an issue on a repository under another owner, such as a mistyped --repo, and on an unregistered repository of the owner's that is registry drift rather than a fork. The fallback now needs both the registry's owner and isFork, and everything else refuses before any write. The label created takes its color and description from repo-config/labels.json rather than a second definition, a malformed registry row now reports as an unreadable registry, scripts/README.md states the new rule, and two overstated claims in upstream-contribution-workflow are corrected: closing a PR loses no review record, and a diverged mirror merges upstream rather than always conflicting. Refs #2264 Co-Authored-By: Claude Opus 5.5 --- .agents/skills/session-handoff/SKILL.md | 14 ++- .../upstream-contribution-workflow/SKILL.md | 14 +-- .../.source-digests/session-handoff | 2 +- .../upstream-contribution-workflow | 2 +- .../skills/session-handoff/SKILL.md | 14 ++- .../upstream-contribution-workflow/SKILL.md | 14 +-- .github/skills/session-handoff/SKILL.md | 14 ++- .../upstream-contribution-workflow/SKILL.md | 14 +-- scripts/README.md | 2 +- scripts/handoff.py | 114 +++++++++++++----- tests/test_handoff.py | 78 +++++++++++- 11 files changed, 202 insertions(+), 80 deletions(-) diff --git a/.agents/skills/session-handoff/SKILL.md b/.agents/skills/session-handoff/SKILL.md index 10c2dadd9..5dd83ca7a 100644 --- a/.agents/skills/session-handoff/SKILL.md +++ b/.agents/skills/session-handoff/SKILL.md @@ -313,12 +313,14 @@ refusal rather than a degraded empty answer, and it names the command that appli set except where the label read filled its window, which is the one case where the label's absence is unproven rather than established. -A repository outside the fleet, such as a fork kept for an upstream contribution per -`upstream-contribution-workflow`, can host a chain to keep a session's state without taking on any -fleet configuration. Missing the label there, a read prints a warning and answers as an empty chain -does, and `new` refuses until it is given `--create-label`, which 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 repository. +A fork under the registry's owner that the registry does not list, such as one kept for an +upstream contribution per `upstream-contribution-workflow`, can host a chain to keep a session's +state without taking on any fleet configuration. Missing the label there, a read prints a warning +and answers as an empty chain does, and `new` refuses until it is given `--create-label`, which +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. 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/.agents/skills/upstream-contribution-workflow/SKILL.md b/.agents/skills/upstream-contribution-workflow/SKILL.md index 0b2c8b761..feb3cf1b9 100644 --- a/.agents/skills/upstream-contribution-workflow/SKILL.md +++ b/.agents/skills/upstream-contribution-workflow/SKILL.md @@ -77,11 +77,11 @@ skip the squash step because the dirty branch "looks clean enough." ## The iteration PR is never merged Merging the iteration PR into the mirror branch looks like the natural finish once it is green, and -it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it can -no longer be fast-forwarded from upstream, and every later sync conflicts with the maintainer's own -change. And the iteration PR is the place where upstream drift is absorbed and its fallout fixed -before the upstream PR has to absorb the same drift, so merging it removes that place. Closing it -loses the review record the same way. +it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it stops +being a mirror: it can no longer be fast-forwarded from upstream, every later sync has to merge +upstream into the maintainer's own change, and any sync touching the same lines conflicts. And the +iteration PR is the place where upstream drift is absorbed and its fallout fixed before the +upstream PR has to absorb the same drift, so merging or closing it removes that place. When the upstream base moves, sync the mirror branch from upstream, merge the mirror branch into the dirty branch, and fix whatever breaks in the iteration PR. That is a merge rather than a rebase, @@ -91,7 +91,7 @@ tip whenever the clean branch needs to follow, per step 5. For example, the fork's mirror branch and upstream both sit at commit `A`, with the iteration PR from `work/x` into the mirror branch green. Upstream advances to `B`. The fork syncs its mirror branch to `B`, merges `B` into `work/x`, and fixes the breakage in the iteration PR. Had `work/x` -been merged into the mirror branch at `A`, the mirror branch could not have followed `B` cleanly. +been merged into the mirror branch at `A`, the mirror branch could not have fast-forwarded to `B`. The contribution ends when upstream merges or declines the upstream PR. Only then close the iteration PR, unmerged, and delete the dirty and clean branches. A merged change then reaches the @@ -121,5 +121,5 @@ The fork is not a fleet repository. It carries none of the fleet's instruction f repository settings, and a resync or the fleet label set is never applied to it. A session working the contribution can still keep its state in the `session-handoff` chain on the fork. That needs issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new ---create-label` creates on a repository outside the fleet, and nothing else of the fleet's +--create-label` creates on a fork under the fleet's owner, and nothing else of the fleet's configuration. diff --git a/.claude-plugin/fleet-skills/.source-digests/session-handoff b/.claude-plugin/fleet-skills/.source-digests/session-handoff index bec7185d6..24500fc8c 100644 --- a/.claude-plugin/fleet-skills/.source-digests/session-handoff +++ b/.claude-plugin/fleet-skills/.source-digests/session-handoff @@ -1 +1 @@ -e8c168644e946836 +c1684259d7d09c94 diff --git a/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow b/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow index 9a4ce5fe5..8bb423243 100644 --- a/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow +++ b/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow @@ -1 +1 @@ -1529d1d88434ef19 +b5d0ef3835766d65 diff --git a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md index 10c2dadd9..5dd83ca7a 100644 --- a/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/session-handoff/SKILL.md @@ -313,12 +313,14 @@ refusal rather than a degraded empty answer, and it names the command that appli set except where the label read filled its window, which is the one case where the label's absence is unproven rather than established. -A repository outside the fleet, such as a fork kept for an upstream contribution per -`upstream-contribution-workflow`, can host a chain to keep a session's state without taking on any -fleet configuration. Missing the label there, a read prints a warning and answers as an empty chain -does, and `new` refuses until it is given `--create-label`, which 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 repository. +A fork under the registry's owner that the registry does not list, such as one kept for an +upstream contribution per `upstream-contribution-workflow`, can host a chain to keep a session's +state without taking on any fleet configuration. Missing the label there, a read prints a warning +and answers as an empty chain does, and `new` refuses until it is given `--create-label`, which +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. 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/skills/upstream-contribution-workflow/SKILL.md b/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md index 0b2c8b761..feb3cf1b9 100644 --- a/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md @@ -77,11 +77,11 @@ skip the squash step because the dirty branch "looks clean enough." ## The iteration PR is never merged Merging the iteration PR into the mirror branch looks like the natural finish once it is green, and -it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it can -no longer be fast-forwarded from upstream, and every later sync conflicts with the maintainer's own -change. And the iteration PR is the place where upstream drift is absorbed and its fallout fixed -before the upstream PR has to absorb the same drift, so merging it removes that place. Closing it -loses the review record the same way. +it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it stops +being a mirror: it can no longer be fast-forwarded from upstream, every later sync has to merge +upstream into the maintainer's own change, and any sync touching the same lines conflicts. And the +iteration PR is the place where upstream drift is absorbed and its fallout fixed before the +upstream PR has to absorb the same drift, so merging or closing it removes that place. When the upstream base moves, sync the mirror branch from upstream, merge the mirror branch into the dirty branch, and fix whatever breaks in the iteration PR. That is a merge rather than a rebase, @@ -91,7 +91,7 @@ tip whenever the clean branch needs to follow, per step 5. For example, the fork's mirror branch and upstream both sit at commit `A`, with the iteration PR from `work/x` into the mirror branch green. Upstream advances to `B`. The fork syncs its mirror branch to `B`, merges `B` into `work/x`, and fixes the breakage in the iteration PR. Had `work/x` -been merged into the mirror branch at `A`, the mirror branch could not have followed `B` cleanly. +been merged into the mirror branch at `A`, the mirror branch could not have fast-forwarded to `B`. The contribution ends when upstream merges or declines the upstream PR. Only then close the iteration PR, unmerged, and delete the dirty and clean branches. A merged change then reaches the @@ -121,5 +121,5 @@ The fork is not a fleet repository. It carries none of the fleet's instruction f repository settings, and a resync or the fleet label set is never applied to it. A session working the contribution can still keep its state in the `session-handoff` chain on the fork. That needs issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new ---create-label` creates on a repository outside the fleet, and nothing else of the fleet's +--create-label` creates on a fork under the fleet's owner, and nothing else of the fleet's configuration. diff --git a/.github/skills/session-handoff/SKILL.md b/.github/skills/session-handoff/SKILL.md index 10c2dadd9..5dd83ca7a 100644 --- a/.github/skills/session-handoff/SKILL.md +++ b/.github/skills/session-handoff/SKILL.md @@ -313,12 +313,14 @@ refusal rather than a degraded empty answer, and it names the command that appli set except where the label read filled its window, which is the one case where the label's absence is unproven rather than established. -A repository outside the fleet, such as a fork kept for an upstream contribution per -`upstream-contribution-workflow`, can host a chain to keep a session's state without taking on any -fleet configuration. Missing the label there, a read prints a warning and answers as an empty chain -does, and `new` refuses until it is given `--create-label`, which 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 repository. +A fork under the registry's owner that the registry does not list, such as one kept for an +upstream contribution per `upstream-contribution-workflow`, can host a chain to keep a session's +state without taking on any fleet configuration. Missing the label there, a read prints a warning +and answers as an empty chain does, and `new` refuses until it is given `--create-label`, which +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. 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/upstream-contribution-workflow/SKILL.md b/.github/skills/upstream-contribution-workflow/SKILL.md index 0b2c8b761..feb3cf1b9 100644 --- a/.github/skills/upstream-contribution-workflow/SKILL.md +++ b/.github/skills/upstream-contribution-workflow/SKILL.md @@ -77,11 +77,11 @@ skip the squash step because the dirty branch "looks clean enough." ## The iteration PR is never merged Merging the iteration PR into the mirror branch looks like the natural finish once it is green, and -it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it can -no longer be fast-forwarded from upstream, and every later sync conflicts with the maintainer's own -change. And the iteration PR is the place where upstream drift is absorbed and its fallout fixed -before the upstream PR has to absorb the same drift, so merging it removes that place. Closing it -loses the review record the same way. +it breaks the fork two ways. The mirror branch then carries a commit upstream does not, so it stops +being a mirror: it can no longer be fast-forwarded from upstream, every later sync has to merge +upstream into the maintainer's own change, and any sync touching the same lines conflicts. And the +iteration PR is the place where upstream drift is absorbed and its fallout fixed before the +upstream PR has to absorb the same drift, so merging or closing it removes that place. When the upstream base moves, sync the mirror branch from upstream, merge the mirror branch into the dirty branch, and fix whatever breaks in the iteration PR. That is a merge rather than a rebase, @@ -91,7 +91,7 @@ tip whenever the clean branch needs to follow, per step 5. For example, the fork's mirror branch and upstream both sit at commit `A`, with the iteration PR from `work/x` into the mirror branch green. Upstream advances to `B`. The fork syncs its mirror branch to `B`, merges `B` into `work/x`, and fixes the breakage in the iteration PR. Had `work/x` -been merged into the mirror branch at `A`, the mirror branch could not have followed `B` cleanly. +been merged into the mirror branch at `A`, the mirror branch could not have fast-forwarded to `B`. The contribution ends when upstream merges or declines the upstream PR. Only then close the iteration PR, unmerged, and delete the dirty and clean branches. A merged change then reaches the @@ -121,5 +121,5 @@ The fork is not a fleet repository. It carries none of the fleet's instruction f repository settings, and a resync or the fleet label set is never applied to it. A session working the contribution can still keep its state in the `session-handoff` chain on the fork. That needs issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new ---create-label` creates on a repository outside the fleet, and nothing else of the fleet's +--create-label` creates on a fork under the fleet's owner, and nothing else of the fleet's configuration. diff --git a/scripts/README.md b/scripts/README.md index 3c825cd08..6016bc73b 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 missing `handoff` label is a refusal naming `repo-config/configure.sh apply`, never 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. 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 under another owner, and an unregistered repository of the owner's that is not a fork, refuse 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. `--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 0f86a7713..ad9aea497 100755 --- a/scripts/handoff.py +++ b/scripts/handoff.py @@ -39,9 +39,11 @@ Exit codes 0 the command did what it says. - 1 a refusal the caller can act on: the label is missing from a fleet repository, or from one - outside the fleet where `new` was not given `--create-label`, no handoff is open on the track, a track is ambiguous, a handoff carries no metadata block, a body is - over the hard cap, or the command line itself was wrong. A usage error is a refusal, so + 1 a refusal the caller can act on: the label is missing from a repository that is not an + unregistered fork under the fleet's owner, or from one that is where `new` was not given + `--create-label` or its issues are turned off, no handoff is open on the track, a track is + ambiguous, a handoff carries no metadata block, a body is over the hard cap, or the command + line itself was wrong. A usage error is a refusal, so `Parser` below moves it here off argparse's own 2. 2 the command did not run to an answer: `gh` failed, a write did not confirm, or an exception nobody modeled reached the top. A refusal and a failure to reach one never share a code, @@ -61,10 +63,12 @@ Add `--dry-run` to any writing subcommand to print the calls it would make and write nothing. A repository missing the label is drift on a fleet repository, one `registry/repos.json` lists, and -every subcommand refuses there. A repository outside the fleet, such as a fork kept for an upstream -contribution, can still use a chain to hold state, and the fleet label set does not belong on it. -There the reads warn and answer as an empty chain does, and `new --create-label` creates the one -`handoff` label and nothing else. +every subcommand refuses there. A fork under the registry's owner that the registry does not list, +such as one kept for an upstream contribution, can still use a chain to hold state, and the fleet +label set does not belong on it. There the reads warn and answer as an empty chain does, and +`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. """ from __future__ import annotations @@ -92,6 +96,7 @@ APPLY = "repo-config/configure.sh apply OWNER/NAME release|operational" REGISTRY = Path(__file__).resolve().parent.parent / "registry" / "repos.json" +LABELS = Path(__file__).resolve().parent.parent / "repo-config" / "labels.json" READS = frozenset({"current", "resume", "chain", "tracks"}) @@ -247,8 +252,8 @@ def label_present(repo: str) -> bool: return LABEL in names -def in_fleet(repo: str) -> bool: - """Whether the hub's registry lists the repository, compared case-insensitively as GitHub does. +def registry() -> tuple[str, set[str]]: + """The registry's owner and its repository names, lowercased, as GitHub compares them. An unreadable registry is a failure to answer rather than an answer, since reading it as empty would move every fleet repository onto the warning path. @@ -257,21 +262,49 @@ def in_fleet(repo: str) -> bool: data = json.loads(REGISTRY.read_text(encoding="utf-8")) owner = data["owner"] names = {row["name"].lower() for row in data["repos"]} - except (OSError, ValueError, KeyError, TypeError) as exc: + if not isinstance(owner, str): + raise TypeError(f"owner is {owner!r}, not a string") + except (OSError, ValueError, KeyError, TypeError, AttributeError) as exc: raise Execution(f"could not read the fleet registry at {REGISTRY}: {exc}") from exc + return owner.lower(), names + + +def in_fleet(repo: str) -> bool: + """Whether the hub's registry lists the repository.""" + owner, names = registry() repo_owner, _, name = repo.partition("/") - return repo_owner.lower() == owner.lower() and name.lower() in names + return repo_owner.lower() == owner and name.lower() in 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"]) + 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} -def issues_enabled(repo: str) -> bool: - data = gh_json(["repo", "view", repo, "--json", "hasIssuesEnabled"]) - if not isinstance(data, dict) or not isinstance(data.get("hasIssuesEnabled"), bool): - raise Execution(f"repo view for {repo} returned no hasIssuesEnabled: {data!r}") - return data["hasIssuesEnabled"] +def label_definition() -> tuple[str, str]: + """The `handoff` label's color and description, read from the fleet label set it belongs to. + + One definition keeps a label created here identical to the one the fleet set applies, should the + repository it lands on join the fleet later. + """ + try: + rows = json.loads(LABELS.read_text(encoding="utf-8")) + row = next(row for row in rows if row["name"] == LABEL) + color, description = row["color"], row["description"] + if not isinstance(color, str) or not isinstance(description, str): + raise TypeError(f"the `{LABEL}` entry is {row!r}") + except (OSError, ValueError, KeyError, TypeError, StopIteration) as exc: + raise Execution(f"could not read the `{LABEL}` label from {LABELS}: {exc!r}") from exc + return color, description def create_label(repo: str, dry_run: bool) -> None: """Create the one `handoff` label and confirm it by reading the label set back.""" + color, description = label_definition() out = gh( [ "label", @@ -280,9 +313,9 @@ def create_label(repo: str, dry_run: bool) -> None: "--repo", repo, "--description", - "A session handoff link", + description, "--color", - "5319e7", + color, ], dry_run=dry_run, ).strip() @@ -301,19 +334,36 @@ def without_label(a: argparse.Namespace) -> int | None: On a fleet repository it is drift, and every subcommand refuses, since degrading there would report an empty chain on a repository that has one, the failure `decision` demonstrated. - Outside the fleet no issue can carry a label that does not exist, so an empty chain is the true - answer there rather than a degraded one, and only `new --create-label` writes anything. + On an unregistered fork under the registry's owner no issue can carry a label that does not + exist, so an empty chain is the true answer there rather than a degraded one, and only + `new --create-label` writes anything. + + Everything else refuses before any write. A repository under another owner is outside what this + may write to at all, and 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. """ repo = a.repo create = getattr(a, "create_label", False) - if in_fleet(repo): - flag = " `--create-label` is for a repository outside the fleet." if create else "" + owner, names = registry() + repo_owner, _, name = repo.partition("/") + missing = f"{repo} carries no `{LABEL}` label, so no handoff can be found or filed there." + if repo_owner.lower() != owner: + raise Refusal( + f"{missing} It is not under {owner}, so this script neither reads a chain there nor " + "creates the label." + ) + 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) + if not flags["isFork"]: raise Refusal( - f"{repo} carries no `{LABEL}` label, so no handoff can be found or filed there." - f"{flag} Apply the fleet label set from a hub checkout: {APPLY}" + f"{missing} It is neither in registry/repos.json nor a fork, so it is registry drift " + f"rather than a fork keeping state. Register it, then apply the fleet label set: {APPLY}" ) fix = f"gh label create {LABEL} --repo {repo}" - if not issues_enabled(repo): + if not flags["hasIssuesEnabled"]: fix = f"gh repo edit {repo} --enable-issues, then {fix}" if create: raise Refusal( @@ -322,8 +372,8 @@ def without_label(a: argparse.Namespace) -> int | None: ) if a.cmd in READS: print( - f"warning: {repo} is outside the fleet and carries no `{LABEL}` label, so it holds no " - f"handoff chain. `new --create-label` creates that one label, or run: {fix}", + f"warning: {repo} is a fork outside the fleet and carries no `{LABEL}` label, so it " + f"holds no handoff chain. `new --create-label` creates that one label, or run: {fix}", file=sys.stderr, ) if a.cmd == "tracks": @@ -334,14 +384,14 @@ def without_label(a: argparse.Namespace) -> int | None: "handoff on a track, so `new` is what follows, not a retry of this." ) if a.cmd == "new" and create: - print(f"0. create the `{LABEL}` label on {repo}, which is outside the fleet") + print(f"0. create the `{LABEL}` label on {repo}, a fork outside the fleet") create_label(repo, a.dry_run) a.fresh_label = True return None raise Refusal( - f"{repo} is outside the fleet and carries no `{LABEL}` label, so no handoff can be filed " - f"there. Rerun `new` with --create-label to create that one label, or run: {fix}. " - "The fleet label set does not belong on a repository outside the fleet." + f"{repo} is a fork outside the fleet and carries no `{LABEL}` label, so no handoff can be " + f"filed there. Rerun `new` with --create-label to create that one label, or run: {fix}. " + "The fleet label set does not belong on a fork outside the fleet." ) @@ -1155,7 +1205,7 @@ def build_parser() -> argparse.ArgumentParser: p.add_argument( "--create-label", action="store_true", - help="on a repository outside the fleet with no handoff label, create that one label first", + help="on an unregistered fork with no handoff label, create that one label first", ) p = sub.add_parser("link", help="finish a chain that half-applied") diff --git a/tests/test_handoff.py b/tests/test_handoff.py index 00e541d7a..92226b136 100755 --- a/tests/test_handoff.py +++ b/tests/test_handoff.py @@ -81,11 +81,17 @@ class FakeGh: """ def __init__( - self, issues: dict[int, dict] | None = None, *, label: bool = True, issues_on: bool = True + self, + issues: dict[int, dict] | None = None, + *, + label: bool = True, + issues_on: bool = True, + fork: bool = True, ) -> None: self.issues = issues or {} self.label = label self.issues_on = issues_on + self.fork = fork self.refuse_label = False self.calls: list[list[str]] = [] self.next_number = 1001 @@ -98,7 +104,8 @@ def __call__(self, argv: list[str]) -> str: if argv[:2] == ["repo", "view"]: if "/" not in argv[2]: raise AssertionError(f"{' '.join(argv)} names no OWNER/NAME repository") - return json.dumps({"hasIssuesEnabled": self.issues_on}) + flags = {"hasIssuesEnabled": self.issues_on, "isFork": self.fork} + return json.dumps(projected(flags, argv)) if "--repo" not in argv: raise AssertionError( f"{' '.join(argv)} carries no --repo, so real gh would resolve the repository " @@ -275,8 +282,9 @@ def test_a_retitled_issue_still_chains(self) -> None: def fleet(case: unittest.TestCase, member: bool = True) -> None: - """Pin whether `o/r` reads as a fleet repository, for the rest of the case.""" - patcher = unittest.mock.patch.object(handoff, "in_fleet", lambda repo: member) + """Pin a registry owned by `o` that lists `o/r` or not, for the rest of the case.""" + names = {"r"} if member else set() + patcher = unittest.mock.patch.object(handoff, "registry", lambda: ("o", names)) patcher.start() case.addCleanup(patcher.stop) @@ -308,7 +316,7 @@ def test_create_label_is_refused_on_a_fleet_repository(self) -> None: "--create-label", ) self.assertEqual(code, 1) - self.assertIn("outside the fleet", err) + self.assertIn("unregistered fork", err) self.assertIn("configure.sh apply", err) self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) @@ -349,7 +357,7 @@ def test_the_label_is_checked_before_every_subcommand(self) -> None: class OutsideFleetLabelCase(unittest.TestCase): - """A repository outside the fleet with no label warns, and writes only the label it is asked to.""" + """An unregistered fork with no label warns, and writes only the label it is asked to.""" def setUp(self) -> None: fleet(self, member=False) @@ -442,6 +450,51 @@ def test_issues_turned_off_are_named_and_stop_the_label_create(self) -> None: self.assertIn("--enable-issues", err) self.assertFalse(any(c[:2] == ["label", "create"] for c in fake.calls)) + def test_a_repository_under_another_owner_refuses_before_any_other_call(self) -> None: + """A mistyped owner would otherwise put a label and an issue on a stranger's repository.""" + for argv in (("current",), ("tracks",)): + with self.subTest(argv=argv): + fake = FakeGh(label=False) + code, _, err = run(fake, *argv, "--repo", "stranger/r") + self.assertEqual(code, 1) + self.assertIn("not under o", err) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) + fake = FakeGh(label=False) + code, _, _ = run( + fake, + "new", + "--repo", + "stranger/r", + "--title", + "T", + "--body-file", + body_file(self, "w"), + "--create-label", + ) + self.assertEqual(code, 1) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) + + 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.""" + fake = FakeGh(label=False, fork=False) + code, _, err = run(fake, "tracks", "--repo", "o/r") + self.assertEqual(code, 1) + self.assertIn("registry drift", err) + self.assertIn("configure.sh apply", err) + 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"]]) + + def test_the_label_created_is_the_fleet_set_s_own_definition(self) -> None: + rows = json.loads(handoff.LABELS.read_text(encoding="utf-8")) + want = next(row for row in rows if row["name"] == handoff.LABEL) + fake = FakeGh(label=False) + self.assertEqual(self.new(fake, "--create-label")[0], 0) + argv = next(c for c in fake.calls if c[:2] == ["label", "create"]) + self.assertEqual(argv[argv.index("--color") + 1], want["color"]) + self.assertEqual(argv[argv.index("--description") + 1], want["description"]) + def test_link_refuses_without_the_label(self) -> None: fake = FakeGh(label=False) code, _, err = run(fake, "link", "--repo", "o/r", "--new", "2", "--previous", "1") @@ -468,6 +521,19 @@ def test_an_unreadable_registry_fails_rather_than_reading_as_empty(self) -> None ): handoff.in_fleet("ptr727/ProjectTemplate") + def test_a_malformed_registry_fails_rather_than_reading_as_empty(self) -> None: + bad = Path(tempfile.mkdtemp()) / "repos.json" + self.addCleanup(shutil.rmtree, bad.parent) + for data in ({"owner": "o", "repos": [{"name": 7}]}, {"owner": 7, "repos": []}): + with self.subTest(data=data): + bad.write_text(json.dumps(data), encoding="utf-8") + with ( + unittest.mock.patch.object(handoff, "REGISTRY", bad), + self.assertRaises(handoff.Execution) as caught, + ): + handoff.in_fleet("o/r") + self.assertIn("could not read the fleet registry", str(caught.exception)) + class CurrentCase(unittest.TestCase): """Exactly one open handoff per track, and anything else is reported rather than resolved.""" From c2f947d94a87843febb57d09afc339ea3d117376 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 15:23:25 -0700 Subject: [PATCH 3/5] Scope the Handoff Fallback's Prose to What the Code Checks The second strict review pass found the owner boundary described as total when it runs only on the missing-label path, so the docstring, README, and test now say a repository under another owner never gets a label created. AGENTS.md "Session Scope" narrows its configuration clause to a fleet repository, since on a fork the handoff command creates the label, and upstream-contribution-workflow no longer says the fork carries none of the fleet's labels. in_fleet had no production caller, so it is gone, and the case-insensitive comparison is now tested on the live path. The exit-code paragraph names the two cases that answer rather than refuse. Refs #2264 Co-Authored-By: Claude Opus 5.5 --- .../upstream-contribution-workflow/SKILL.md | 4 +-- .../upstream-contribution-workflow | 2 +- .../upstream-contribution-workflow/SKILL.md | 4 +-- .../upstream-contribution-workflow/SKILL.md | 4 +-- AGENTS.md | 2 +- scripts/README.md | 2 +- scripts/handoff.py | 20 +++++--------- tests/test_handoff.py | 26 +++++++++++++------ 8 files changed, 33 insertions(+), 31 deletions(-) diff --git a/.agents/skills/upstream-contribution-workflow/SKILL.md b/.agents/skills/upstream-contribution-workflow/SKILL.md index feb3cf1b9..6ba840d67 100644 --- a/.agents/skills/upstream-contribution-workflow/SKILL.md +++ b/.agents/skills/upstream-contribution-workflow/SKILL.md @@ -117,8 +117,8 @@ GitHub id) also still apply in full. A fork the maintainer owns is within scope the upstream repository itself is written to only through the PR the maintainer explicitly asked for. -The fork is not a fleet repository. It carries none of the fleet's instruction files, labels, or -repository settings, and a resync or the fleet label set is never applied to it. A session working +The fork is not a fleet repository. It carries none of the fleet's instruction files or repository +settings, and a resync or the fleet label set is never applied to it. A session working the contribution can still keep its state in the `session-handoff` chain on the fork. That needs issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new --create-label` creates on a fork under the fleet's owner, and nothing else of the fleet's diff --git a/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow b/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow index 8bb423243..3ab96a457 100644 --- a/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow +++ b/.claude-plugin/fleet-skills/.source-digests/upstream-contribution-workflow @@ -1 +1 @@ -b5d0ef3835766d65 +1436e605eb7e114f diff --git a/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md b/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md index feb3cf1b9..6ba840d67 100644 --- a/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md +++ b/.claude-plugin/fleet-skills/skills/upstream-contribution-workflow/SKILL.md @@ -117,8 +117,8 @@ GitHub id) also still apply in full. A fork the maintainer owns is within scope the upstream repository itself is written to only through the PR the maintainer explicitly asked for. -The fork is not a fleet repository. It carries none of the fleet's instruction files, labels, or -repository settings, and a resync or the fleet label set is never applied to it. A session working +The fork is not a fleet repository. It carries none of the fleet's instruction files or repository +settings, and a resync or the fleet label set is never applied to it. A session working the contribution can still keep its state in the `session-handoff` chain on the fork. That needs issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new --create-label` creates on a fork under the fleet's owner, and nothing else of the fleet's diff --git a/.github/skills/upstream-contribution-workflow/SKILL.md b/.github/skills/upstream-contribution-workflow/SKILL.md index feb3cf1b9..6ba840d67 100644 --- a/.github/skills/upstream-contribution-workflow/SKILL.md +++ b/.github/skills/upstream-contribution-workflow/SKILL.md @@ -117,8 +117,8 @@ GitHub id) also still apply in full. A fork the maintainer owns is within scope the upstream repository itself is written to only through the PR the maintainer explicitly asked for. -The fork is not a fleet repository. It carries none of the fleet's instruction files, labels, or -repository settings, and a resync or the fleet label set is never applied to it. A session working +The fork is not a fleet repository. It carries none of the fleet's instruction files or repository +settings, and a resync or the fleet label set is never applied to it. A session working the contribution can still keep its state in the `session-handoff` chain on the fork. That needs issues turned on and the one `handoff` label, which the hub's `scripts/handoff.py new --create-label` creates on a fork under the fleet's owner, and nothing else of the fleet's diff --git a/AGENTS.md b/AGENTS.md index c4c5f82c0..22d505ab5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,7 +37,7 @@ An agent session is billed on the context it carries, not the work it does. Ever - **One deliverable, one session.** A session covers one branch and one deliverable, and ends when that work merges. A multi-step task is one deliverable and stays in one session. Two unrelated tasks are two sessions even when they run back to back. - **End a session at any of these, without being asked:** the branch changes, the pull request merges, or the next task is unrelated to the last. A review round is none of them. A loop still producing findings is the deliverable in progress, and a round count is not a reason to leave one open. - **A session orchestrating dispatched work is an exception, and a narrow one.** Its deliverable is the run rather than any branch, so it spans many branches and many merges by construction, and ending it at the first dispatched merge would end the run. The triggers above land on each dispatched task instead, one branch and one deliverable each, which is this rule applied rather than waived. What keeps the exception narrow is that such a session holds no branch of its own and authors none of the work it dispatches, so the file context every other session accumulates is context it never takes on, and it re-derives each round's state from live sources rather than holding it, per "Re-derive state, do not carry it" below. A session that starts editing the files a dispatched task would have edited is an ordinary one again and ends on the triggers above. -- **Hand off in an issue, never in a scratch file and never in context.** Close a session by filing the next link in the handoff chain of the repository holding the work the next session resumes, which for a session that stayed in one repository is this one, and a session that spanned several names the others in the handoff's state section. A track is a lane of work named by a short slug, `default` where a session names none, and a track in use holds exactly one open issue carrying the `handoff` label. The track and the predecessor are recorded in the issue body rather than in its title, so a retitled or hand-edited issue still chains and a reader can tell which lane an open issue belongs to. The new link names its predecessor that way, a forward-link comment then goes onto that predecessor, and the predecessor is closed last, in that order, so a failure part way leaves a discoverable new issue rather than a closed chain with no successor. Each of those three is an outward-facing write, so the write-safety rules in `GOVERNANCE.md` "Repository Boundaries and Write Safety" bind all three exactly as they bind any other write, the identifier rule most of all, since the link a comment and a close target is read live in the same run rather than remembered. The chain needs the `handoff` label to be findable at all, so a repository not carrying it cannot host one until the label exists there, which is a change to that repository's configuration rather than anything a handoff writes. A session that cannot file a link reports that it could not hand off and leaves the previous link open, whether it is stopped by a repository carrying no such label, by a write it may not make, or by anything else. That is the one alternative this rule allows to a track still in use, and it is a report rather than a file, because a report says the round's record is missing while a file claims to be it. A track whose work is complete is closed out instead, its last link carrying the outcome as a comment and closed with no successor, which leaves the track no longer in use rather than breaking its chain. A scratch file fails three ways the chain closes. It is not found where the next session looks. More than one candidate is found and nothing says which is current. And it holds no history, so a later round re-runs a path an earlier round already tried and already wrote down, which is the one thing a handoff exists to prevent. The handoff carries the next steps in priority order, the external blockers and internal dependencies among them, the state a resume re-reads rather than trusts, listed so the resume knows what to re-read, the account of parked decisions that `GOVERNANCE.md` "Communicating with the User" requires, what the last round did, what not to repeat, and what was learned. That section states the account whole, and it requires the session to present those decisions as well as record them. **The size rule is stated per section.** An entry earns its place by being specific enough to change a later session's behavior, a section ranks what it keeps and drops whatever does not meet that bar, and what belongs somewhere durable goes there and appears here as one line and a pointer, a defect as an issue, a rule as rule text, a lesson as governance prose. The parked-decision account keeps the count and the ranked questions one round can carry, and names every issue past those by number alone, which is what keeps a queue larger than one round inside this rule. A summary held in context is re-billed until the session ends, a scratch file is read only on the machine holding it, and a closed link stays readable from any machine to every session after it. +- **Hand off in an issue, never in a scratch file and never in context.** Close a session by filing the next link in the handoff chain of the repository holding the work the next session resumes, which for a session that stayed in one repository is this one, and a session that spanned several names the others in the handoff's state section. A track is a lane of work named by a short slug, `default` where a session names none, and a track in use holds exactly one open issue carrying the `handoff` label. The track and the predecessor are recorded in the issue body rather than in its title, so a retitled or hand-edited issue still chains and a reader can tell which lane an open issue belongs to. The new link names its predecessor that way, a forward-link comment then goes onto that predecessor, and the predecessor is closed last, in that order, so a failure part way leaves a discoverable new issue rather than a closed chain with no successor. Each of those three is an outward-facing write, so the write-safety rules in `GOVERNANCE.md` "Repository Boundaries and Write Safety" bind all three exactly as they bind any other write, the identifier rule most of all, since the link a comment and a close target is read live in the same run rather than remembered. The chain needs the `handoff` label to be findable at all, so a repository not carrying it cannot host one until the label exists there, which on a fleet repository is a change to that repository's configuration rather than anything a handoff writes. A session that cannot file a link reports that it could not hand off and leaves the previous link open, whether it is stopped by a repository carrying no such label, by a write it may not make, or by anything else. That is the one alternative this rule allows to a track still in use, and it is a report rather than a file, because a report says the round's record is missing while a file claims to be it. A track whose work is complete is closed out instead, its last link carrying the outcome as a comment and closed with no successor, which leaves the track no longer in use rather than breaking its chain. A scratch file fails three ways the chain closes. It is not found where the next session looks. More than one candidate is found and nothing says which is current. And it holds no history, so a later round re-runs a path an earlier round already tried and already wrote down, which is the one thing a handoff exists to prevent. The handoff carries the next steps in priority order, the external blockers and internal dependencies among them, the state a resume re-reads rather than trusts, listed so the resume knows what to re-read, the account of parked decisions that `GOVERNANCE.md` "Communicating with the User" requires, what the last round did, what not to repeat, and what was learned. That section states the account whole, and it requires the session to present those decisions as well as record them. **The size rule is stated per section.** An entry earns its place by being specific enough to change a later session's behavior, a section ranks what it keeps and drops whatever does not meet that bar, and what belongs somewhere durable goes there and appears here as one line and a pointer, a defect as an issue, a rule as rule text, a lesson as governance prose. The parked-decision account keeps the count and the ranked questions one round can carry, and names every issue past those by number alone, which is what keeps a queue larger than one round inside this rule. A summary held in context is re-billed until the session ends, a scratch file is read only on the machine holding it, and a closed link stays readable from any machine to every session after it. - **Re-derive state, do not carry it.** "This session already has the context" is the signal to split, not to continue. Context that has gone stale is worse than absent, because a file read hundreds of requests ago no longer describes the file. - **Compaction is a fallback, not the strategy.** It restarts context from a floor and climbs again, where a fresh session starts from zero. diff --git a/scripts/README.md b/scripts/README.md index 6016bc73b..17fd3c052 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 under another owner, and an unregistered repository of the owner's that is not a fork, refuse 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. 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 ad9aea497..9ca22b7fd 100755 --- a/scripts/handoff.py +++ b/scripts/handoff.py @@ -39,11 +39,10 @@ Exit codes 0 the command did what it says. - 1 a refusal the caller can act on: the label is missing from a repository that is not an - unregistered fork under the fleet's owner, or from one that is where `new` was not given - `--create-label` or its issues are turned off, no handoff is open on the track, a track is - ambiguous, a handoff carries no metadata block, a body is over the hard cap, or the command - line itself was wrong. A usage error is a refusal, so + 1 a refusal the caller can act on: the label is missing, other than where `tracks` answers + an unregistered fork under the fleet's owner or `new --create-label` creates the label there, + no handoff is open on the track, a track is ambiguous, a handoff carries no metadata block, a + body is over the hard cap, or the command line itself was wrong. A usage error is a refusal, so `Parser` below moves it here off argparse's own 2. 2 the command did not run to an answer: `gh` failed, a write did not confirm, or an exception nobody modeled reached the top. A refusal and a failure to reach one never share a code, @@ -269,13 +268,6 @@ def registry() -> tuple[str, set[str]]: return owner.lower(), names -def in_fleet(repo: str) -> bool: - """Whether the hub's registry lists the repository.""" - owner, names = registry() - repo_owner, _, name = repo.partition("/") - return repo_owner.lower() == owner and name.lower() in 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"]) @@ -338,8 +330,8 @@ def without_label(a: argparse.Namespace) -> int | None: exist, so an empty chain is the true answer there rather than a degraded one, and only `new --create-label` writes anything. - Everything else refuses before any write. A repository under another owner is outside what this - may write to at all, and these calls run as subprocesses a write guard on the caller never sees. + 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. """ diff --git a/tests/test_handoff.py b/tests/test_handoff.py index 92226b136..cf3e465ab 100755 --- a/tests/test_handoff.py +++ b/tests/test_handoff.py @@ -451,7 +451,7 @@ def test_issues_turned_off_are_named_and_stop_the_label_create(self) -> None: self.assertFalse(any(c[:2] == ["label", "create"] for c in fake.calls)) def test_a_repository_under_another_owner_refuses_before_any_other_call(self) -> None: - """A mistyped owner would otherwise put a label and an issue on a stranger's repository.""" + """A mistyped owner would otherwise put a label on a stranger's repository, then an issue.""" for argv in (("current",), ("tracks",)): with self.subTest(argv=argv): fake = FakeGh(label=False) @@ -506,11 +506,21 @@ def test_link_refuses_without_the_label(self) -> None: class FleetRegistryCase(unittest.TestCase): """Membership is read from the hub's registry, and an unreadable one is not an empty one.""" - def test_the_hub_itself_is_in_the_fleet_and_a_stranger_is_not(self) -> None: - self.assertTrue(handoff.in_fleet("ptr727/ProjectTemplate")) - self.assertTrue(handoff.in_fleet("PTR727/projecttemplate")) - self.assertFalse(handoff.in_fleet("ptr727/not-a-registered-repo")) - self.assertFalse(handoff.in_fleet("someone-else/ProjectTemplate")) + def test_the_registry_reads_lowercased_with_the_hub_in_it(self) -> None: + owner, names = handoff.registry() + self.assertEqual(owner, "ptr727") + self.assertIn("projecttemplate", names) + + def test_membership_is_compared_case_insensitively_on_the_live_path(self) -> None: + """A mixed-case registered repository is drift on the fleet path, never a fork's warning.""" + fleet(self) + for repo in ("O/R", "o/R"): + with self.subTest(repo=repo): + fake = FakeGh(label=False) + code, _, err = run(fake, "tracks", "--repo", repo) + self.assertEqual(code, 1) + self.assertIn("configure.sh apply", err) + self.assertEqual([c[:2] for c in fake.calls], [["label", "list"]]) def test_an_unreadable_registry_fails_rather_than_reading_as_empty(self) -> None: missing = Path(tempfile.mkdtemp()) / "repos.json" @@ -519,7 +529,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.in_fleet("ptr727/ProjectTemplate") + handoff.registry() def test_a_malformed_registry_fails_rather_than_reading_as_empty(self) -> None: bad = Path(tempfile.mkdtemp()) / "repos.json" @@ -531,7 +541,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.in_fleet("o/r") + handoff.registry() self.assertIn("could not read the fleet registry", str(caught.exception)) From e25bae8ee7fccf5493e382bf6884bf4557686430 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 15:30:50 -0700 Subject: [PATCH 4/5] Read the Handoff Body Before Creating the Label Copilot's review of #2265 noted that new --create-label created the label before reading the body file, so a body new then refused, unreadable or over the cap, left a label on the fork behind a refusal. The body is now read first, and a test proves no label create runs for a missing body file. Refs #2264 Co-Authored-By: Claude Opus 5.5 --- scripts/handoff.py | 4 +++- tests/test_handoff.py | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/scripts/handoff.py b/scripts/handoff.py index 9ca22b7fd..133959fa3 100755 --- a/scripts/handoff.py +++ b/scripts/handoff.py @@ -330,7 +330,8 @@ def without_label(a: argparse.Namespace) -> int | None: exist, so an empty chain is the true answer there rather than a degraded one, and only `new --create-label` writes anything. - Everything else refuses before any write. A repository under another owner never gets a label + The body is read before the label is created, 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. @@ -376,6 +377,7 @@ def without_label(a: argparse.Namespace) -> int | None: "handoff on a track, so `new` is what follows, not a retry of this." ) if a.cmd == "new" and create: + body_from(Path(a.body_file)) print(f"0. create the `{LABEL}` label on {repo}, a fork outside the fleet") create_label(repo, a.dry_run) a.fresh_label = True diff --git a/tests/test_handoff.py b/tests/test_handoff.py index cf3e465ab..8f1cb9179 100755 --- a/tests/test_handoff.py +++ b/tests/test_handoff.py @@ -432,6 +432,26 @@ def test_a_dry_run_with_the_flag_writes_nothing(self) -> None: self.assertEqual([c[:2] for c in fake.calls], [["label", "list"], ["repo", "view"]]) self.assertFalse(fake.label) + def test_a_body_new_would_refuse_creates_no_label(self) -> None: + """Otherwise the refusal comes after a write, which leaves the fork half-changed.""" + fake = FakeGh(label=False) + missing = str(Path(tempfile.mkdtemp()) / "absent.md") + self.addCleanup(shutil.rmtree, Path(missing).parent) + code, _, err = run( + fake, + "new", + "--repo", + "o/r", + "--title", + "T", + "--body-file", + missing, + "--create-label", + ) + self.assertEqual(code, 1) + self.assertIn("could not read the body file", err) + self.assertFalse(any(c[:2] == ["label", "create"] for c in fake.calls)) + def test_an_unconfirmed_label_create_fails_before_filing(self) -> None: fake = FakeGh(label=False) fake.refuse_label = True From e5770c6958ade66a47a71ee84bfe956a837d8098 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Thu, 1 Oct 2026 15:33:42 -0700 Subject: [PATCH 5/5] Read the Handoff Body Once on the Label-Creating Path Reading the body in without_label and again in cmd_new printed the size warning twice and let a file changed between the reads be filed after the label already existed. The first read is now the one new files, and a test proves the body is read exactly once. Refs #2264 Co-Authored-By: Claude Opus 5.5 --- scripts/handoff.py | 16 +++++++++------- tests/test_handoff.py | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/scripts/handoff.py b/scripts/handoff.py index 133959fa3..efa19b311 100755 --- a/scripts/handoff.py +++ b/scripts/handoff.py @@ -330,11 +330,11 @@ def without_label(a: argparse.Namespace) -> int | None: exist, so an empty chain is the true answer there rather than a degraded one, and only `new --create-label` writes anything. - The body is read before the label is created, 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. + 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. """ repo = a.repo create = getattr(a, "create_label", False) @@ -377,7 +377,7 @@ def without_label(a: argparse.Namespace) -> int | None: "handoff on a track, so `new` is what follows, not a retry of this." ) if a.cmd == "new" and create: - body_from(Path(a.body_file)) + a.body = body_from(Path(a.body_file)) print(f"0. create the `{LABEL}` label on {repo}, a fork outside the fleet") create_label(repo, a.dry_run) a.fresh_label = True @@ -918,7 +918,9 @@ def cmd_new(a: argparse.Namespace) -> int: sits on no issue yet, so no chain read is made, which also keeps a dry run from querying a label it never created. """ - body = body_from(Path(a.body_file)) + body = getattr(a, "body", None) + if body is None: + body = body_from(Path(a.body_file)) if getattr(a, "fresh_label", False): rows, previous = [], None else: diff --git a/tests/test_handoff.py b/tests/test_handoff.py index 8f1cb9179..facbde260 100755 --- a/tests/test_handoff.py +++ b/tests/test_handoff.py @@ -452,6 +452,32 @@ def test_a_body_new_would_refuse_creates_no_label(self) -> None: self.assertIn("could not read the body file", err) self.assertFalse(any(c[:2] == ["label", "create"] for c in fake.calls)) + def test_the_body_is_read_once_and_that_read_is_filed(self) -> None: + """A second read could see a changed file after the label exists, and warns twice.""" + fake = FakeGh(label=False) + path = body_file(self, "x" * (handoff.WARN_BYTES + 1)) + reads: list[Path] = [] + real = handoff.body_from + + def counted(p: Path) -> str: + reads.append(p) + return real(p) + + with unittest.mock.patch.object(handoff, "body_from", counted): + code, out, err = run( + fake, + "new", + "--repo", + "o/r", + "--title", + "T", + "--body-file", + path, + "--create-label", + ) + self.assertEqual(code, 0, out + err) + self.assertEqual(len(reads), 1) + def test_an_unconfirmed_label_create_fails_before_filing(self) -> None: fake = FakeGh(label=False) fake.refuse_label = True