From fd3bf4ca29b48c14a239e36ae6c5918129f9ee06 Mon Sep 17 00:00:00 2001 From: Will Metz Date: Sat, 24 Oct 2026 13:38:16 +0000 Subject: [PATCH 1/2] fix(pstack): align delegated task lifecycle with T3 --- CHANGES.md | 8 +++ plugins/pstack/skills/poteto-mode/SKILL.md | 8 ++- .../poteto-mode/playbooks/autopilot-full.md | 2 +- .../skills/poteto-mode/playbooks/feature.md | 2 +- .../poteto-mode/playbooks/opening-a-pr.md | 2 +- .../poteto-mode/playbooks/orchestrate.md | 4 +- .../skills/poteto-mode/playbooks/shipping.md | 2 +- plugins/pstack/skills/swarm/SKILL.md | 4 +- tests/sync.test.mjs | 2 +- tests/t3-delegation.test.mjs | 56 +++++++++++++++++++ tools/forks.json | 8 +-- tools/substitutions.json | 34 +++++------ 12 files changed, 99 insertions(+), 33 deletions(-) create mode 100644 tests/t3-delegation.test.mjs diff --git a/CHANGES.md b/CHANGES.md index 43f41068..f958ab13 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,14 @@ This file is the release changelog, with one `## - ` entry per release, newest first. The Cursor-to-T3 rewrite rules live in [`tools/substitutions.json`](tools/substitutions.json), and the [sync boundary](CONTRIBUTING.md#the-sync-boundary) in `CONTRIBUTING.md` defines how a change to upstream's skill content is declared. +## Unreleased - keep delegated work in T3 child tasks + +Independent review rounds now use fresh `delegate_task` calls with the original brief, prior findings, responses, and unresolved objections. Each round keeps its own retry-stable `clientRequestId` and returned `taskId`. The costly-state exception still permits `t3_thread_send` for work that needs the same child's conversation, but it cannot continue an independent review round. + +Autopilot-full owners are delegated children, and orchestrate manages their lifecycle through `delegate_task`, `task_status`, and `task_cancel`. Ordinary top-level conversations require an explicit user request. Writing children use T3's `isolation: "worktree"` from the parent's local commit, with project setup and without uncommitted edits. The substitutions and existing declared forks carry these rules. Regression tests read the distributed skills through the existing CI test job. All three upstream pins remain at `4ec7ffa`. + +This contributor patch leaves the release number unchanged. Include it in the next maintainer release so installed copies receive the fix. + ## 2.0.0-rc.18 - Lauren's `4ec7ffa`: statechart stops on inputs it used to chart wrong without saying so t3-pstack moves to Lauren Tan's pstack at cursor/plugins `4ec7ffa` for all three components. Only pstack changed upstream, only in the `statechart` skill, and its version stays at 0.15.30 ([pstack #776](https://github.com/cursor/plugins/pull/776) and [pstack #779](https://github.com/cursor/plugins/pull/779)). Before this, a run could draw a wrong chart or exit with the wrong code and not say so. The label map now stops on a variable or field that the `--fixed` or `--against` model lacks, where it used to give it the current model's start value. New `--config`, `--fixed-config`, and `--against-config` flags name each model's config. The agreed spec is never checked with the code model's config, and a missing config stops the run and lists the configs beside the spec. A rule the agreed config does not check reads "not checked" in the stories, where they used to say the agreed spec keeps it. When TLC finds a broken rule and then throws a Java exception, the run keeps the paths TLC printed, marks the rule broken, and warns that the charts can miss states. On a file system that cannot rename into the output folder (`ENOSYS`, `EXDEV`, or `EISDIR`, as on FUSE mounts), it copies the files into place. TLC checks each rule with one worker per CPU and checks a broken rule again with one worker, which gives the shortest paths. Each `PROPERTY` gets its own TLC run, `--expect-broken` accepts it, and a rule listed as both `INVARIANT` and `PROPERTY` is checked once. With `--against`, each story says whether a config beside the agreed spec expects the bug. With `--fixed` or `--against`, the run lists each condition of an action that no `\* file:line` comment ties to code. `SKILL.md` asks for those citations, says what a valid replay is, and asks the hand-off to say whether the spec already lists each bug. The test of each TLC run's temp folder accepts `Naturals.tla` anywhere under it, where TLC 1.8.0 puts it. #779 changes no behavior. It deletes script comments that repeat the code, and renames helpers so the name says what the comment did. `splitComments` becomes `splitCommentsKeepingOffsets`, `checkTlcOutput` becomes `tlcOutputWarning`, `quote` becomes `escapeMermaidQuotes`, and `luminance` becomes `wcagRelativeLuminance`, and one `PRIMES_OR_UNCHANGED` regex replaces two copies. `bun tools/sync.mjs` updated `SKILL.md`, `lib.mjs`, `statechart.mjs`, `tlc-to-chart.mjs`, and the test file cleanly, and each is byte-identical to upstream. No substitution, fork, or hook changed, and the 15 declared forks are untouched. At `7698375`, with TLC 1.7.4 on Linux and Node 20, 66 of the 67 skill tests pass and the PID-namespace test skips. With the TLC 1.8.0 prerelease, the `PROPERTY` test fails because 1.8.0 words a broken temporal property as "Temporal property X was violated.", which the scripts do not read yet, as #776 says. At `4ec7ffa` without TLC, 29 pass and 38 skip, and `from-spec` on the bundled `OutputFolder` model with TLC 1.7.4 exits 0 with all five rules holding. The README's skill count stays at 65. The copies #776 and #779 change under `harness/pstack/claude-code/` sit outside every synced component. At `4ec7ffa` each component's dry run reports no undeclared fork, no denylist hit, and no substitution that matches nothing. Neither hooks file changes. diff --git a/plugins/pstack/skills/poteto-mode/SKILL.md b/plugins/pstack/skills/poteto-mode/SKILL.md index d79c74f2..468b5b23 100644 --- a/plugins/pstack/skills/poteto-mode/SKILL.md +++ b/plugins/pstack/skills/poteto-mode/SKILL.md @@ -99,11 +99,13 @@ Read the leaf skill in full for any principle you apply. Each entry names when i You own every subagent's work. Review the diff and write your own summary, don't pass through what it said. A second opinion is the same prompt against a different model. Agreement is high-signal. -**Fresh subagents by default.** Give new work to a fresh subagent with consolidated scope, meaning the original brief, every later directive, and the prior agent's report and branch. This holds for a fix round, a follow-up, a retry, and the next queue item. Send a follow-up to an existing child (`t3_thread_send` to its `childThreadId`) only when the new work strictly needs state that lives in that agent and is costly to move: its local checkout, its uncommitted changes, or a process it still runs, such as a dev server, a simulator, or a babysit watcher. A stop or hold order to a running agent is not reuse. A role such as a PR owner outlives its agent. Once that agent returns, a fresh agent takes the role's next round. Interrupt-chained resumes silently drop directives, so fire a fresh subagent with consolidated scope rather than trusting a "done" summary. **Stop the abandoned agent first, and confirm it stopped,** when its writes could overlap its replacement's, in the same worktree or the same files. Autopilot-full's audit still replaces a stuck agent at once, without waiting for its stop, per that playbook. A child whose turn completed can still have live work, and it can keep editing after a fresh agent takes its role. `task_cancel` it, which also stops the tasks it delegated, and read `task_status` until the task is terminal with no `hasPendingChildRuns`. A claim about the working tree that `git status` contradicts means it is still running. +**Fresh subagents by default.** Give new work to a fresh subagent with consolidated scope, meaning the original brief, every later directive, and the prior agent's report and branch. This holds for a fix round, a follow-up, a retry, and the next queue item. Send a follow-up to an existing child (`t3_thread_send` to its `childThreadId`) only when the new work strictly needs state that lives in that agent and is costly to move: its local checkout, its uncommitted changes, or a process it still runs, such as a dev server, a simulator, or a babysit watcher. This same-conversation exception never applies to independent review rounds. A stop or hold order to a running agent is not reuse. A role such as a PR owner outlives its agent. Once that agent returns, a fresh agent takes the role's next round. Interrupt-chained resumes silently drop directives, so fire a fresh subagent with consolidated scope rather than trusting a "done" summary. **Stop the abandoned agent first, and confirm it stopped,** when its writes could overlap its replacement's, in the same worktree or the same files. Autopilot-full's audit still replaces a stuck agent at once, without waiting for its stop, per that playbook. A child whose turn completed can still have live work, and it can keep editing after a fresh agent takes its role. `task_cancel` it, which also stops the tasks it delegated, and read `task_status` until the task is terminal with no `hasPendingChildRuns`. A claim about the working tree that `git status` contradicts means it is still running. -**Dispatch on T3.** Every subagent is a `delegate_task` call from the chat that runs the playbook, same model family included, so every child shows up in T3. Give each call its own `clientRequestId`, stable across retries of that call, and keep each returned `taskId`. A child's completion wakes this thread, so end the turn instead of polling, and call `task_status` only when a result is needed mid-turn. `task_cancel` stops a child, and `task_status` confirms it stopped. A child gets only its brief, never your context, so pass absolute paths. `<plugin root>` is the absolute path of the directory that holds this plugin's `skills/` and `agents/`. A persona (`poteto-agent`, `comment-sicko`, `thermo-nuclear-review-subagent`, `thermo-nuclear-code-quality-review-subagent`) is a brief that opens with "Read `<plugin root>/agents/<name>.md` in full and act as it says", and a `poteto-agent` brief also names this file's absolute path. +**Dispatch on T3.** Every subagent is a `delegate_task` call from the chat that runs the playbook, same model family included, so every child shows up in T3. Give each call its own `clientRequestId`, stable across retries of that call, and keep each returned `taskId`. A child's completion wakes this thread, so end the turn instead of polling, and call `task_status` only when a result is needed mid-turn. `task_cancel` stops a child, and `task_status` confirms it stopped. A child gets only its brief, never your context, so include explicit task context and absolute paths. `childThreadId` is the child's backing conversation. `t3_thread_launch` and `create_threads` create ordinary top-level conversations only for an explicit user request for separate threads; a request for parallel workers or subagents alone does not authorize them. `<plugin root>` is the absolute path of the directory that holds this plugin's `skills/` and `agents/`. A persona (`poteto-agent`, `comment-sicko`, `thermo-nuclear-review-subagent`, `thermo-nuclear-code-quality-review-subagent`) is a brief that opens with "Read `<plugin root>/agents/<name>.md` in full and act as it says", and a `poteto-agent` brief also names this file's absolute path. -A child starts in your checkout. One that writes gets its own worktree, created with `git worktree add` from `HEAD` so it builds on your branch's commits. The brief names that absolute path as the only place it writes, and tells it to run every command there (`workdir` on Codex) and to edit by absolute path. Record your own `git status --porcelain` before you dispatch, check after the result that it is unchanged, then remove the worktree and its branch. A step a playbook says to delegate goes to `delegate_task` on that role's sheet entry, not into your own tool calls, unless you state a reason such as a step too small to brief. A research delegate no role names runs on the `how explorer` entry. A model's family is the first segment of its ID's last `/` part, such as `claude`, `gpt`, or `grok`, whatever provider instance serves it, and your own family is the one in `orchestrator_capabilities`' `inheritedModel`. A rejected target runs on another target of the same family from `orchestrator_capabilities`, and you say so. A required review with no reviewer from another family stays `BLOCKED: independent review`. +**Independent review rounds.** Call a fresh `delegate_task` for every new round, with the original brief, prior findings, responses, and unresolved objections. Give each round a distinct `clientRequestId`, stable across retries of that round, and retain its returned `taskId`. Never start a new review round through `t3_thread_send` to the previous child's `childThreadId`. + +A child that needs a separate checkout uses `delegate_task` with `isolation: "worktree"`. Every child that writes gets this isolation. T3 creates the worktree from the parent's local commit and runs project setup. Uncommitted edits are not copied, so commit what it builds on before dispatch. The brief tells it to write and run every command only in its assigned worktree (`workdir` on Codex). Record your own `git status --porcelain` before dispatch and check after the result that it is unchanged. A step a playbook says to delegate goes to `delegate_task` on that role's sheet entry, not into your own tool calls, unless you state a reason such as a step too small to brief. A research delegate no role names runs on the `how explorer` entry. A model's family is the first segment of its ID's last `/` part, such as `claude`, `gpt`, or `grok`, whatever provider instance serves it, and your own family is the one in `orchestrator_capabilities`' `inheritedModel`. A rejected target runs on another target of the same family from `orchestrator_capabilities`, and you say so. A required review with no reviewer from another family stays `BLOCKED: independent review`. **A review gate stays closed when no independent reviewer can run.** A reviewer child that fails or is interrupted gets a fresh `delegate_task` in its place. Keep the results of reviewers that finished. Never count your own review, passing tests, or CI as the independent verdict. While the gate is blocked, record `BLOCKED: independent review` in the todolist, keep merge-ready and shipping closed, and report which reviewers ran and which are missing. Work that doesn't depend on the review continues. A fan-out run only to explore may fall back to one labelled sequential pass. diff --git a/plugins/pstack/skills/poteto-mode/playbooks/autopilot-full.md b/plugins/pstack/skills/poteto-mode/playbooks/autopilot-full.md index 258724ae..a4fd277b 100644 --- a/plugins/pstack/skills/poteto-mode/playbooks/autopilot-full.md +++ b/plugins/pstack/skills/poteto-mode/playbooks/autopilot-full.md @@ -5,7 +5,7 @@ A verdict covers one patch and green CI covers one head. Step 4 runs a full swarm once, and again only for a major change. The patch-id rule in `playbooks/shipping.md` can carry a lane's result, and every new head needs its own green CI. 1. **The operator's gates come first.** The grant covers only the queue's items, so items the operator names as theirs and PRs outside the queue stop at merge-ready. A request to state the plan is not a go. -2. **Spawn one owner per PR, and keep its work where a replacement can find it.** Resolve the forge once for the program. GitHub CLI (`gh`) is the default. If `command -v origin` succeeds and Origin can resolve the repository, use `origin pr` for every PR operation. Otherwise stay on `gh` and record the fallback. Never require Graphite (`gt`). Each owner is a top-level T3 thread that `t3_thread_launch` starts with `workspaceStrategy: {type: "worktree", baseRef, branch, startFromOrigin: false}` and a brief that names this playbook and this thread's ID, so the owner reports back with `t3_thread_send`. It builds, opens its PR per `playbooks/opening-a-pr.md`, proves the change on the real artifact, babysits to green per `playbooks/babysit.md`, and merges once steps 4 and 5 clear it. +2. **Spawn one owner per PR, and keep its work where a replacement can find it.** Resolve the forge once for the program. GitHub CLI (`gh`) is the default. If `command -v origin` succeeds and Origin can resolve the repository, use `origin pr` for every PR operation. Otherwise stay on `gh` and record the fallback. Never require Graphite (`gt`). Each owner is a `delegate_task` child with `mode: "async"` and `isolation: "worktree"`, started from the parent's committed trunk checkout. Its brief names this playbook and the PR scope. Retain its `taskId`; completion wakes the parent, and `task_status` reads progress when needed. It builds, opens its PR per `playbooks/opening-a-pr.md`, proves the change on the real artifact, babysits to green per `playbooks/babysit.md`, and merges once steps 4 and 5 clear it. Open the PR early so CI and bots run alongside the work. The code is ready only after the slop-strip (the **deslop** skill, `/deslop`) and the `/no-comments` pass, so the full swarm runs on finished code. Then rebase onto current trunk and report the code-ready head before self-proof, so verification runs on the base the patch lands on. Report every later head, and merge-ready once self-proof, CI, and babysit are done. Fix rounds stay on that base unless trunk conflicts or breaks CI, because each rebase restarts CI. Run the repo's checks for the touched paths on the committed head before the code-ready push and before a later push that needs a verifier. diff --git a/plugins/pstack/skills/poteto-mode/playbooks/feature.md b/plugins/pstack/skills/poteto-mode/playbooks/feature.md index e5d3fd08..c677aa43 100644 --- a/plugins/pstack/skills/poteto-mode/playbooks/feature.md +++ b/plugins/pstack/skills/poteto-mode/playbooks/feature.md @@ -9,7 +9,7 @@ - **Independent workstreams.** Disjoint files, services, or layers parallelize. Shared writes serialize. - **Shared mutable state.** Default to splitting the target (the **separate-before-serializing-shared-state** principle skill). Serialize only for real invariants. - **Smallest safe decomposition.** If one worker is best, name why. -4. Delegate code-writing to a subagent using your configured feature model (default in poteto-mode's [Models](../SKILL.md#models)) with a specific scope (file paths, named data shape and its organizing structure per **principle-model-the-domain**, a state machine over scattered booleans, a table/registry over branching, a typed model over repeated shape assumptions, chosen before the delegate writes logic, and success criteria). Pick each subagent's reasoning level and fast or standard speed for the job, per the Subagents section. When the implementation admits multiple valid shapes (error handling, abstraction layer, test structure), delegate via the **arena** skill instead so the runners surface the alternatives and the cross-judge guards the pick. Mandatory: no skip-with-reason escape, and Laziness Protocol does not override it (the gain is review separation, not lines saved). A subagent forbidden to spawn satisfies this by owning the diff directly with the same review separation. No "standing by" reply that waits on a nested agent. **Every file-writing delegate gets its own worktree from `HEAD`, per poteto-mode's [Subagents](../SKILL.md#subagents) section.** Commit what it builds on first, and name that base commit in the brief. Do not write files or run a suite in a worktree a delegate still holds. Fencing a file in the brief's prose is not a lock (**principle-separate-before-serializing-shared-state**). Comments per **Comments**. Surgical edits, re-ground against the source for upstream-derived files. Port shared-primitive improvements to all consumers and verify each. Commit liberally. +4. Delegate code-writing to a subagent using your configured feature model (default in poteto-mode's [Models](../SKILL.md#models)) with a specific scope (file paths, named data shape and its organizing structure per **principle-model-the-domain**, a state machine over scattered booleans, a table/registry over branching, a typed model over repeated shape assumptions, chosen before the delegate writes logic, and success criteria). Pick each subagent's reasoning level and fast or standard speed for the job, per the Subagents section. When the implementation admits multiple valid shapes (error handling, abstraction layer, test structure), delegate via the **arena** skill instead so the runners surface the alternatives and the cross-judge guards the pick. Mandatory: no skip-with-reason escape, and Laziness Protocol does not override it (the gain is review separation, not lines saved). A subagent forbidden to spawn satisfies this by owning the diff directly with the same review separation. No "standing by" reply that waits on a nested agent. **Every file-writing delegate uses `delegate_task` with `isolation: "worktree"` from the parent's local commit, per poteto-mode's [Subagents](../SKILL.md#subagents) section.** Commit what it builds on first, and name that base commit in the brief. Do not write files or run a suite in a worktree a delegate still holds. Fencing a file in the brief's prose is not a lock (**principle-separate-before-serializing-shared-state**). Comments per **Comments**. Surgical edits, re-ground against the source for upstream-derived files. Port shared-primitive improvements to all consumers and verify each. Commit liberally. 5. Verify on the matching surface. "Inconclusive" or wrong-surface is not a pass. Flag it. 6. Rebase into small, ordered commits. Stack follow-ups. Use the **sequence-verifiable-units** principle skill, building, verifying, and committing each small unit before the next. diff --git a/plugins/pstack/skills/poteto-mode/playbooks/opening-a-pr.md b/plugins/pstack/skills/poteto-mode/playbooks/opening-a-pr.md index 6378dc3b..9f3b6658 100644 --- a/plugins/pstack/skills/poteto-mode/playbooks/opening-a-pr.md +++ b/plugins/pstack/skills/poteto-mode/playbooks/opening-a-pr.md @@ -2,7 +2,7 @@ Invoked at the end of every other playbook. -**Worktree.** Work from a git worktree off main. Subagents inherit it. Multiple `delegate_task` calls on the same branch each get their own worktree. Dirty branch with unrelated work: patch out, fresh worktree, apply. Snarled worktree: reset from main, redo minimally. Before you commit, merge, or deploy from a worktree, stop every live agent that holds it with `task_cancel`, which also stops the tasks it delegated, and confirm each stop with `task_status`. A delegate's children do not inherit its brief, so a read-only instruction never reaches them. Then run `git status` and read the tree you are about to ship. +**Worktree.** Work from a git worktree off main. Writing subagents use `delegate_task` with `isolation: "worktree"`. T3 creates each worktree from the parent's local commit and runs project setup; uncommitted edits are not copied. Dirty branch with unrelated work: patch out, fresh worktree, apply. Snarled worktree: reset from main, redo minimally. Before you commit, merge, or deploy from a worktree, stop every live agent that holds it with `task_cancel`, which also stops the tasks it delegated, and confirm each stop with `task_status`. A delegate's children do not inherit its brief, so a read-only instruction never reaches them. Then run `git status` and read the tree you are about to ship. **Commits.** Commit liberally. Rebase into small, ordered commits before opening PRs. Each commit is a future PR: landable, ordered to tell the story. Amend when the fix belongs in a just-made commit. New commit when separable. Commit bodies don't restate the subject. diff --git a/plugins/pstack/skills/poteto-mode/playbooks/orchestrate.md b/plugins/pstack/skills/poteto-mode/playbooks/orchestrate.md index a9816a16..f0a5ba31 100644 --- a/plugins/pstack/skills/poteto-mode/playbooks/orchestrate.md +++ b/plugins/pstack/skills/poteto-mode/playbooks/orchestrate.md @@ -12,9 +12,9 @@ Three rules carry the rest. #### Roles and placement -- **Coordinator (this chat).** Local. Frames, authors briefs, drains the inbox, owns the human report, makes judgment calls. It never authors or edits code. Conflicted merges, chain rebases, and code changes are always tasks. Mechanically landing a verified unit (fast-forward or clean cherry-pick of a worker's commit, then push) is bookkeeping the coordinator may do itself on repos where local git is cheap. Queueing finished work behind an idle stacker is how a deadline harvests nothing. The loop is agentic end to end. Agents are spawned, resumed, and drained only through `delegate_task`, `t3_thread_send` to a child's `childThreadId`, and `task_status`. Pick each subagent's reasoning level and fast or standard speed for the job, per the Subagents section. State reads and writes go through `scripts/orch/orch.ts` at drain points, one command in and one line out. The CLI never spawns, waits, or wakes anything. +- **Coordinator (this chat).** Local. Frames, authors briefs, drains the inbox, owns the human report, makes judgment calls. It never authors or edits code. Conflicted merges, chain rebases, and code changes are always tasks. Mechanically landing a verified unit (fast-forward or clean cherry-pick of a worker's commit, then push) is bookkeeping the coordinator may do itself on repos where local git is cheap. Queueing finished work behind an idle stacker is how a deadline harvests nothing. The loop is agentic end to end. Manage the child lifecycle through `delegate_task`, `task_status`, and `task_cancel`. A costly-state same-conversation continuation may use `t3_thread_send` to its `childThreadId` per poteto-mode's Subagents section, but independent review rounds always require a fresh `delegate_task` with the full review history and a distinct round `clientRequestId`, stable across retries. Retain each returned `taskId`. Completion wakes this thread; do not poll for it. Pick each subagent's reasoning level and fast or standard speed for the job, per the Subagents section. State reads and writes go through `scripts/orch/orch.ts` at drain points, one command in and one line out. The CLI never spawns, waits, or wakes anything. - **Sub-coordinator.** Always local, durable, one per track, and only when the program exceeds what one coordinator's drains can manage. A track the coordinator can drain itself needs no middle layer. Each nested layer re-pays a full orientation preamble, and a blocking sub-coordinator hides its children while the parent idles. Owns its track's units and boards, authors its workers' briefs, spawns its own workers and verifiers (a `delegate_task` child can delegate its own). Rolls up aggregates at wave boundaries. Never forwards raw child reports. Cap in-flight children at what one drain can process, roughly ten, as a rolling window. Never as blocking batches, which cost the slowest child of every batch. -- **Worker / verifier.** Always a `delegate_task` child in its own git worktree, named by absolute path in its brief as the only place it writes. Every child runs on this machine, so it can drive the real surface, read T3 threads, and use local auth. A child starts without your context, so its brief inlines what it needs or points at repo paths. Prefer fewer, broader workers. One writer per worktree or branch (principle-separate-before-serializing-shared-state). Run a unit's verifier on a different model family from its worker. +- **Worker / verifier.** Always a `delegate_task` child with `isolation: "worktree"`, using its assigned worktree as the only place it writes. Every child runs on this machine, so it can drive the real surface, read T3 threads, and use local auth. A child starts without your context, so its brief inlines what it needs or points at repo paths. Prefer fewer, broader workers. One writer per worktree or branch (principle-separate-before-serializing-shared-state). Run a unit's verifier on a different model family from its worker. Depth stays at coordinator, track, worker. Author the track decomposition per project (build, landing, and verification are common cuts, not a required shape). Hard-coded swarm trees were tried and parked as too rigid. diff --git a/plugins/pstack/skills/poteto-mode/playbooks/shipping.md b/plugins/pstack/skills/poteto-mode/playbooks/shipping.md index 001870eb..a80cf90d 100644 --- a/plugins/pstack/skills/poteto-mode/playbooks/shipping.md +++ b/plugins/pstack/skills/poteto-mode/playbooks/shipping.md @@ -2,7 +2,7 @@ **You own what lands. Verify each PR independently, land only the verified run from the root, then keep your hands off the stack.** -1. **Resolve the forge, then verify each PR independently before it lands.** GitHub CLI (`gh`) is the default. If `command -v origin` succeeds and Origin can resolve the repository, use `origin pr` for every PR operation. Otherwise stay on `gh` and record the fallback. Never require Graphite (`gt`). One verifier per PR, each a `delegate_task` child in its own worktree, runs the change the way a user would with the matching control skill in poteto-mode's Non-negotiables, parent against head, and posts `PASS`, `PASS+NOTES`, or `FAIL` on that PR. Only a verdict from an agent that did not write the code counts, because an author checks what it meant to build. Green CI and an approving bot review are not verdicts, because neither runs the change as a user would. +1. **Resolve the forge, then verify each PR independently before it lands.** GitHub CLI (`gh`) is the default. If `command -v origin` succeeds and Origin can resolve the repository, use `origin pr` for every PR operation. Otherwise stay on `gh` and record the fallback. Never require Graphite (`gt`). One verifier per PR, each a `delegate_task` child with `isolation: "worktree"` from the parent's committed PR head, runs the change the way a user would with the matching control skill in poteto-mode's Non-negotiables, parent against head, and posts `PASS`, `PASS+NOTES`, or `FAIL` on that PR. Only a verdict from an agent that did not write the code counts, because an author checks what it meant to build. Green CI and an approving bot review are not verdicts, because neither runs the change as a user would. 2. **Land only the contiguous verified run rooted at the bottom.** Walk up from the lowest unmerged PR and stop at the first one without `PASS` or `PASS+NOTES`. The PRs below it are the run, and the top one is the ceiling. A verified PR above an unverified one waits, because it cannot land without the code below it. One whose branch neither holds nor needs that code can move onto trunk and land on its own. It needs a fresh verdict first, because its old one ran with that code beneath it and trunk does not have it. 3. **Re-check that the verdicts still describe the code.** A verdict covers a patch, and a chain rebase moves every PR above it to a new SHA without failing any check. Compare `git patch-id` at the verdict SHA with the current head before trusting an older verdict. When the patches differ only in tests, docs, or lint config, a lane's result stands if what it ran builds the same at both SHAs apart from noise. Noise is a difference that also shows up between two builds of the verdict SHA, or an embedded commit SHA. Report each kind of noise with its files, so a reader can check the call. The change itself still gets fresh checks and a review, because the old verdict never saw it. A lane with no build output, such as a dev server, cannot show this, so rerun it, and re-verify anything else that drifted. 4. **Land bottom-up, one PR at a time.** Always merge the run's lowest unmerged PR, with `origin pr merge <pr>` or `gh pr merge <pr> --squash --match-head-commit <head>`, where `<head>` is the current head SHA whose verdict survives step 3's patch-id rule, so a head pushed after that check cannot land. On Origin the plain command also merges every PR below the one you name, and on GitHub a PR whose base is still its parent's branch merges into that branch, not trunk. Read the PR back for the result (`origin pr view <pr>` or `gh pr view <pr> --json state,mergedAt,baseRefName`), because a merge command can report success when a merge queue only queued the PR, and a requested auto-merge is not a merge. A landed PR reads merged with trunk as its base, not its parent's branch. Arm `--auto` (Origin merge-when-ready or GitHub auto-merge) only on a PR whose verdict covers its current head, because it lands whatever head is ready. After a merge lands, retarget the PR whose base was the merged branch to trunk (`origin pr edit <pr> --base <trunk>` or `gh pr edit <pr> --base <trunk>`), move its patch onto the merge result, and push with `--force-with-lease`. The verdict covers the patch, not its commits, so step 3's patch-id rule decides whether the verdict survives. Merge it once its checks pass and its verdict survives. diff --git a/plugins/pstack/skills/swarm/SKILL.md b/plugins/pstack/skills/swarm/SKILL.md index b6d2ca45..e33246e9 100644 --- a/plugins/pstack/skills/swarm/SKILL.md +++ b/plugins/pstack/skills/swarm/SKILL.md @@ -24,9 +24,9 @@ Open a todolist with one entry per phase before launching anything. ## Phase B: Fan out -Spawn all N workers in one response as `delegate_task` calls with `mode: "async"` and the step 4 `target`, left unset for `auto` or `inherit-parent`. Every worker runs on this machine, so a worker that writes gets its own git worktree, named by absolute path in its brief as the only place it writes. +Spawn all N workers in one response as `delegate_task` calls with `mode: "async"` and the step 4 `target`, left unset for `auto` or `inherit-parent`. Every worker runs on this machine. A worker that writes uses `isolation: "worktree"` and writes only in its assigned worktree. -When a worker must start from a non-default branch, create its worktree from that branch. +When a worker must start from a non-default branch, put the parent on that branch and commit the required changes before dispatch. T3 worktree isolation starts from the parent's local commit, without uncommitted edits. Every brief stands alone. Include the goal, scope, exact slice or race arm, how to verify, and what to report. Reports use `PASS`, `ISSUES`, or `BLOCKED` with evidence. A worker that can prove a defect reports `ISSUES` and lists every issue it can prove, not only the first. diff --git a/tests/sync.test.mjs b/tests/sync.test.mjs index 45803765..c9fab750 100644 --- a/tests/sync.test.mjs +++ b/tests/sync.test.mjs @@ -168,7 +168,7 @@ describe("applySubstitutions", () => { ["skills/poteto-mode/SKILL.md", "- Contested design → the **interrogate** skill (multi-model adversarial) before shipping.", ["before Opening a PR", "After three fix rounds, stop"]], ["skills/arena/SKILL.md", "Prefer a different model family from the parent's.", ["no model scores its own work"]], ["skills/poteto-mode/playbooks/babysit.md", "Run `drive` and `background` under `/loop` in dynamic mode.", ["--timeout 540", "watch_pull_request", "--settle"]], - ["skills/poteto-mode/playbooks/autopilot-full.md", "Each owner is a Cursor cloud agent that builds, opens its PR", ["t3_thread_launch", "t3_thread_send"]], + ["skills/poteto-mode/playbooks/autopilot-full.md", "Each owner is a Cursor cloud agent that builds, opens its PR", ["`delegate_task` child", 'isolation: "worktree"', "taskId", "task_status"]], ["skills/no-comments/SKILL.md", '1. Spawn `Task` with `subagent_type: "comment-sicko"`. Pass the scope.', ["<plugin root>/agents/comment-sicko.md", "no worktree"]], ])("%s keeps its T3 insert", (rel, upstream, needles) => { const { text } = applySubstitutions(upstream, RULES.substitutions, rel); diff --git a/tests/t3-delegation.test.mjs b/tests/t3-delegation.test.mjs new file mode 100644 index 00000000..a07e7687 --- /dev/null +++ b/tests/t3-delegation.test.mjs @@ -0,0 +1,56 @@ +import { expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; + +const skill = (path) => readFileSync(new URL(`../plugins/pstack/skills/${path}`, import.meta.url), "utf8"); +const poteto = skill("poteto-mode/SKILL.md"); +const playbook = (name) => skill(`poteto-mode/playbooks/${name}.md`); + +test("same-conversation continuation excludes independent review rounds", () => { + const fresh = poteto.split("**Fresh subagents by default.**")[1].split("**Dispatch on T3.**")[0]; + expect(fresh).toContain("`t3_thread_send`"); + expect(fresh).toContain("`childThreadId`"); + expect(fresh).toMatch(/only when.*state that lives in that agent/s); + expect(fresh).toMatch(/exception never applies to independent review rounds/); +}); + +test("each independent review round delegates with history and its own retry-stable ID", () => { + const review = poteto.match(/\*\*Independent review rounds\.\*\*[^\n]+/)?.[0] ?? ""; + expect(review).toMatch(/fresh `delegate_task`/); + for (const context of ["original brief", "prior findings", "responses", "unresolved objections"]) { + expect(review).toContain(context); + } + expect(review).toMatch(/distinct `clientRequestId`.*stable across retries/); + expect(review).toContain("`taskId`"); + expect(review).toMatch(/Never.*`t3_thread_send`.*`childThreadId`/); +}); + +test("worker ownership stays in child tasks and top-level conversations require an explicit request", () => { + const autopilot = playbook("autopilot-full"); + expect(autopilot).toMatch(/Each owner is a `delegate_task` child/); + expect(autopilot).toContain('isolation: "worktree"'); + expect(autopilot).not.toMatch(/t3_thread_launch|create_threads/); + expect(poteto).toMatch(/`t3_thread_launch` and `create_threads`.*only.*explicit.*user request/); + + const orchestrate = playbook("orchestrate"); + expect(orchestrate).toMatch(/lifecycle.*`delegate_task`.*`task_status`.*`task_cancel`/); + expect(orchestrate).toMatch(/same-conversation.*independent review rounds.*fresh `delegate_task`/); +}); + +test("writing children use T3 worktree isolation from committed parent state", () => { + for (const [name, text] of [ + ["poteto-mode", poteto], + ["swarm", skill("swarm/SKILL.md")], + ["orchestrate", playbook("orchestrate")], + ["autopilot-full", playbook("autopilot-full")], + ["feature", playbook("feature")], + ["opening-a-pr", playbook("opening-a-pr")], + ["shipping", playbook("shipping")], + ]) { + expect(text, name).toContain('isolation: "worktree"'); + expect(text, name).not.toContain("git worktree add"); + } + expect(poteto).toMatch(/parent's local commit.*project setup/); + expect(poteto).toMatch(/Uncommitted edits are not copied/); + expect(skill("swarm/SKILL.md")).toMatch(/parent.*branch.*commit.*before dispatch/); + expect(playbook("opening-a-pr")).not.toContain("Subagents inherit it"); +}); diff --git a/tools/forks.json b/tools/forks.json index c85d1d20..1eb108b3 100644 --- a/tools/forks.json +++ b/tools/forks.json @@ -2,25 +2,25 @@ "pstack": { "plugins/pstack/skills/poteto-mode/SKILL.md": { "kind": "policy", - "why": "Carries four rules from Michael Denyer's pstack-claude in T3 wording: load a managed platform's skill before deploying, stop an abandoned agent whose writes could overlap its replacement's with task_cancel and confirm with task_status that it stopped, which Autopilot-full's immediate replacement of a stuck agent overrides, keep a review gate closed while no independent reviewer can run, and keep the todolist in .audit/<task-slug>.todo.md when the session has no task tool.", + "why": "Carries four rules from Michael Denyer's pstack-claude in T3 wording: load a managed platform's skill before deploying, stop an abandoned agent whose writes could overlap its replacement's with task_cancel and confirm with task_status that it stopped, which Autopilot-full's immediate replacement of a stuck agent overrides, keep a review gate closed while no independent reviewer can run, and keep the todolist in .audit/<task-slug>.todo.md when the session has no task tool. The T3 dispatch insert also requires fresh independent review rounds, reserves top-level conversations for explicit user requests, and uses delegate_task worktree isolation.", "since": "2.0.0-rc.2", "upstream": "not-proposed" }, "plugins/pstack/skills/poteto-mode/playbooks/feature.md": { "kind": "policy", - "why": "Points step 4 at the Subagents rule that every file-writing delegate gets its own worktree from HEAD, and carries the rule from Michael Denyer's pstack-claude that the delegator commits what the delegate builds on, names that commit in the brief, and never writes or runs a suite in a worktree a delegate still holds.", + "why": "Points step 4 at the Subagents rule that every file-writing delegate uses delegate_task with isolation: \"worktree\" from the parent local commit, and carries the rule from Michael Denyer's pstack-claude that the delegator commits what the delegate builds on, names that commit in the brief, and never writes or runs a suite in a worktree a delegate still holds.", "since": "2.0.0-rc.2", "upstream": "not-proposed" }, "plugins/pstack/skills/poteto-mode/playbooks/opening-a-pr.md": { "kind": "policy", - "why": "Carries the rule from Michael Denyer's pstack-claude that every live agent holding a worktree is stopped, and the stop confirmed, before a commit, merge, or deploy from it, and drops the reset --hard between delegates on one branch so that each gets its own worktree.", + "why": "Carries the rule from Michael Denyer's pstack-claude that every live agent holding a worktree is stopped, and the stop confirmed, before a commit, merge, or deploy from it, and drops the reset --hard between delegates on one branch so that each gets its own worktree. Writing children use T3 worktree isolation from committed parent state rather than inheriting the parent worktree.", "since": "2.0.0-rc.2", "upstream": "not-proposed" }, "plugins/pstack/skills/poteto-mode/playbooks/shipping.md": { "kind": "policy", - "why": "Adds --match-head-commit <head> to the GitHub merge in step 4, the head guard from Michael Denyer's pstack-claude, with the current head whose verdict survives step 3's patch-id rule, so a head pushed after that check cannot land, and stops step 5's watcher loop at a merge gate that needs a person.", + "why": "Adds --match-head-commit <head> to the GitHub merge in step 4, the head guard from Michael Denyer's pstack-claude, with the current head whose verdict survives step 3's patch-id rule, so a head pushed after that check cannot land, and stops step 5's watcher loop at a merge gate that needs a person. Verification children use delegate_task worktree isolation from the parent committed PR head.", "since": "2.0.0-rc.2", "upstream": "not-proposed" }, diff --git a/tools/substitutions.json b/tools/substitutions.json index 6b2cfa3d..993a0ab7 100644 --- a/tools/substitutions.json +++ b/tools/substitutions.json @@ -259,15 +259,15 @@ "files": "^skills/poteto-mode/SKILL\\.md$" }, { - "pattern": "Resume, message, or queue a follow-up on an existing subagent only when", - "replacement": "Send a follow-up to an existing child (`t3_thread_send` to its `childThreadId`) only when", - "rationale": "Tool actions: Continue a subagent", + "pattern": "Resume, message, or queue a follow-up on an existing subagent only when the new work strictly needs state that lives in that agent and is costly to move: its local checkout, its uncommitted changes, or a process it still runs, such as a dev server, a simulator, or a babysit watcher.", + "replacement": "Send a follow-up to an existing child (`t3_thread_send` to its `childThreadId`) only when the new work strictly needs state that lives in that agent and is costly to move: its local checkout, its uncommitted changes, or a process it still runs, such as a dev server, a simulator, or a babysit watcher. This same-conversation exception never applies to independent review rounds.", + "rationale": "T3 child lifecycle: preserve costly-state conversation continuation, but every independent review round is a fresh delegated task (live T3 contract, 2026-10-24)", "files": "^skills/poteto-mode/SKILL\\.md$" }, { "pattern": "Interrupt-chained resumes silently drop directives, so fire a fresh subagent with consolidated scope rather than trusting a \"done\" summary.", - "replacement": "Interrupt-chained resumes silently drop directives, so fire a fresh subagent with consolidated scope rather than trusting a \"done\" summary.\n\n**Dispatch on T3.** Every subagent is a `delegate_task` call from the chat that runs the playbook, same model family included, so every child shows up in T3. Give each call its own `clientRequestId`, stable across retries of that call, and keep each returned `taskId`. A child's completion wakes this thread, so end the turn instead of polling, and call `task_status` only when a result is needed mid-turn. `task_cancel` stops a child, and `task_status` confirms it stopped. A child gets only its brief, never your context, so pass absolute paths. `<plugin root>` is the absolute path of the directory that holds this plugin's `skills/` and `agents/`. A persona (`poteto-agent`, `comment-sicko`, `thermo-nuclear-review-subagent`, `thermo-nuclear-code-quality-review-subagent`) is a brief that opens with \"Read `<plugin root>/agents/<name>.md` in full and act as it says\", and a `poteto-agent` brief also names this file's absolute path.\n\nA child starts in your checkout. One that writes gets its own worktree, created with `git worktree add` from `HEAD` so it builds on your branch's commits. The brief names that absolute path as the only place it writes, and tells it to run every command there (`workdir` on Codex) and to edit by absolute path. Record your own `git status --porcelain` before you dispatch, check after the result that it is unchanged, then remove the worktree and its branch. A step a playbook says to delegate goes to `delegate_task` on that role's sheet entry, not into your own tool calls, unless you state a reason such as a step too small to brief. A research delegate no role names runs on the `how explorer` entry. A model's family is the first segment of its ID's last `/` part, such as `claude`, `gpt`, or `grok`, whatever provider instance serves it, and your own family is the one in `orchestrator_capabilities`' `inheritedModel`. A rejected target runs on another target of the same family from `orchestrator_capabilities`, and you say so. A required review with no reviewer from another family stays `BLOCKED: independent review`.\n\nEnd every reply that ran a playbook with a dispatch ledger: one line per delegated step, naming the role and the `providerInstanceId/model` it ran on, or `in-thread: <reason>`.", - "rationale": "inserts the T3 dispatch policy at the end of the Subagents section (Tool actions; Subagent policy)", + "replacement": "Interrupt-chained resumes silently drop directives, so fire a fresh subagent with consolidated scope rather than trusting a \"done\" summary.\n\n**Dispatch on T3.** Every subagent is a `delegate_task` call from the chat that runs the playbook, same model family included, so every child shows up in T3. Give each call its own `clientRequestId`, stable across retries of that call, and keep each returned `taskId`. A child's completion wakes this thread, so end the turn instead of polling, and call `task_status` only when a result is needed mid-turn. `task_cancel` stops a child, and `task_status` confirms it stopped. A child gets only its brief, never your context, so include explicit task context and absolute paths. `childThreadId` is the child's backing conversation. `t3_thread_launch` and `create_threads` create ordinary top-level conversations only for an explicit user request for separate threads; a request for parallel workers or subagents alone does not authorize them. `<plugin root>` is the absolute path of the directory that holds this plugin's `skills/` and `agents/`. A persona (`poteto-agent`, `comment-sicko`, `thermo-nuclear-review-subagent`, `thermo-nuclear-code-quality-review-subagent`) is a brief that opens with \"Read `<plugin root>/agents/<name>.md` in full and act as it says\", and a `poteto-agent` brief also names this file's absolute path.\n\n**Independent review rounds.** Call a fresh `delegate_task` for every new round, with the original brief, prior findings, responses, and unresolved objections. Give each round a distinct `clientRequestId`, stable across retries of that round, and retain its returned `taskId`. Never start a new review round through `t3_thread_send` to the previous child's `childThreadId`.\n\nA child that needs a separate checkout uses `delegate_task` with `isolation: \"worktree\"`. Every child that writes gets this isolation. T3 creates the worktree from the parent's local commit and runs project setup. Uncommitted edits are not copied, so commit what it builds on before dispatch. The brief tells it to write and run every command only in its assigned worktree (`workdir` on Codex). Record your own `git status --porcelain` before dispatch and check after the result that it is unchanged. A step a playbook says to delegate goes to `delegate_task` on that role's sheet entry, not into your own tool calls, unless you state a reason such as a step too small to brief. A research delegate no role names runs on the `how explorer` entry. A model's family is the first segment of its ID's last `/` part, such as `claude`, `gpt`, or `grok`, whatever provider instance serves it, and your own family is the one in `orchestrator_capabilities`' `inheritedModel`. A rejected target runs on another target of the same family from `orchestrator_capabilities`, and you say so. A required review with no reviewer from another family stays `BLOCKED: independent review`.\n\nEnd every reply that ran a playbook with a dispatch ledger: one line per delegated step, naming the role and the `providerInstanceId/model` it ran on, or `in-thread: <reason>`.", + "rationale": "T3-owned dispatch, independent review rounds, top-level thread boundary, and app-managed worktree isolation (live T3 contract, 2026-10-24)", "files": "^skills/poteto-mode/SKILL\\.md$" }, { @@ -470,13 +470,13 @@ }, { "pattern": "Spawn all N workers in one message with `subagent_type: generalPurpose`, `environment: \"cloud\"`, `run_in_background: true`, and the step 4 model, left unset for `auto` or `inherit-parent`. Use `environment: \"local\"` only when the worker needs access to something on the user's computer.", - "replacement": "Spawn all N workers in one response as `delegate_task` calls with `mode: \"async\"` and the step 4 `target`, left unset for `auto` or `inherit-parent`. Every worker runs on this machine, so a worker that writes gets its own git worktree, named by absolute path in its brief as the only place it writes.", - "rationale": "Per-skill notes: swarm" + "replacement": "Spawn all N workers in one response as `delegate_task` calls with `mode: \"async\"` and the step 4 `target`, left unset for `auto` or `inherit-parent`. Every worker runs on this machine. A worker that writes uses `isolation: \"worktree\"` and writes only in its assigned worktree.", + "rationale": "Per-skill notes: swarm; T3 creates writing children's worktrees through isolation, not a shared task pointed at a manually created worktree" }, { "pattern": "When a worker must start from a non-default pushed branch, pass `cloud_base_branch`.", - "replacement": "When a worker must start from a non-default branch, create its worktree from that branch.", - "rationale": "no cloud base branch on T3" + "replacement": "When a worker must start from a non-default branch, put the parent on that branch and commit the required changes before dispatch. T3 worktree isolation starts from the parent's local commit, without uncommitted edits.", + "rationale": "T3 worktree isolation starts at the parent local commit, not a cloud_base_branch or a manually created worktree" }, { "pattern": "Transcripts live at `~/.cursor/projects/<slug>/agent-transcripts/<uuid>/<uuid>.jsonl`, where `<slug>` is the workspace path with the leading slash dropped and each \"/\" turned into \"-\" (so `/Users/you/proj` becomes `Users-you-proj`). Every line is one chat message.", @@ -564,14 +564,14 @@ { "files": "^skills/poteto-mode/playbooks/", "pattern": "Each owner is a Cursor cloud agent that builds, opens its PR", - "replacement": "Each owner is a top-level T3 thread that `t3_thread_launch` starts with `workspaceStrategy: {type: \"worktree\", baseRef, branch, startFromOrigin: false}` and a brief that names this playbook and this thread's ID, so the owner reports back with `t3_thread_send`. It builds, opens its PR", - "rationale": "Playbooks: Autopilot-full (each PR owner is a t3_thread_launch thread in its own worktree)" + "replacement": "Each owner is a `delegate_task` child with `mode: \"async\"` and `isolation: \"worktree\"`, started from the parent's committed trunk checkout. Its brief names this playbook and the PR scope. Retain its `taskId`; completion wakes the parent, and `task_status` reads progress when needed. It builds, opens its PR", + "rationale": "Autopilot-full owners are child tasks in T3-managed worktrees; worker fan-out never authorizes top-level conversations" }, { "files": "^skills/poteto-mode/playbooks/", "pattern": "Assume the VM dies.", "replacement": "Assume the owner's session dies.", - "rationale": "no cloud VMs on T3; an owner is a T3 thread" + "rationale": "no cloud VMs on T3; an owner is a delegated child task" }, { "files": "^skills/poteto-mode/playbooks/", @@ -701,8 +701,8 @@ }, { "pattern": "Agents are spawned, resumed, and drained only through the Task tool.", - "replacement": "Agents are spawned, resumed, and drained only through `delegate_task`, `t3_thread_send` to a child's `childThreadId`, and `task_status`.", - "rationale": "Tool actions", + "replacement": "Manage the child lifecycle through `delegate_task`, `task_status`, and `task_cancel`. A costly-state same-conversation continuation may use `t3_thread_send` to its `childThreadId` per poteto-mode's Subagents section, but independent review rounds always require a fresh `delegate_task` with the full review history and a distinct round `clientRequestId`, stable across retries. Retain each returned `taskId`. Completion wakes this thread; do not poll for it.", + "rationale": "T3 child lifecycle, costly-state continuation exception, and fresh independent review override (live T3 contract, 2026-10-24)", "files": "^skills/poteto-mode/playbooks/" }, { @@ -713,7 +713,7 @@ }, { "pattern": "- **Worker / verifier.** Always `environment: \"cloud\"` unless the task needs this machine: `control-ui` or `control-cli` runtime verification (from `cursor-team-kit`). Reading local transcripts under `agent-transcripts/`. Simulators and local IDE state. Auth that exists only here. Cloud agents cannot read the local store, so their briefs inline what they need or point at repo paths.", - "replacement": "- **Worker / verifier.** Always a `delegate_task` child in its own git worktree, named by absolute path in its brief as the only place it writes. Every child runs on this machine, so it can drive the real surface, read T3 threads, and use local auth. A child starts without your context, so its brief inlines what it needs or points at repo paths.", + "replacement": "- **Worker / verifier.** Always a `delegate_task` child with `isolation: \"worktree\"`, using its assigned worktree as the only place it writes. Every child runs on this machine, so it can drive the real surface, read T3 threads, and use local auth. A child starts without your context, so its brief inlines what it needs or points at repo paths.", "rationale": "Playbooks: Orchestrate; Subagent policy (worktree)", "files": "^skills/poteto-mode/playbooks/" }, @@ -762,7 +762,7 @@ { "files": "^skills/poteto-mode/playbooks/", "pattern": "One verifier per PR, each a Cursor cloud agent, runs the change the way a user would with the matching control skill (such as `control-ui` or `control-cli` from `cursor-team-kit`), parent against head,", - "replacement": "One verifier per PR, each a `delegate_task` child in its own worktree, runs the change the way a user would with the matching control skill in poteto-mode's Non-negotiables, parent against head,", + "replacement": "One verifier per PR, each a `delegate_task` child with `isolation: \"worktree\"` from the parent's committed PR head, runs the change the way a user would with the matching control skill in poteto-mode's Non-negotiables, parent against head,", "rationale": "Playbooks: Shipping; UI evidence" }, { @@ -1007,7 +1007,7 @@ }, { "regex": "\\bcloud[- ](?:agents?|VMs?|workers?|work|default)\\b|cloud_base_branch|environment: \"|Cursor dashboard", - "hint": "Cursor cloud agents; delegate_task children in worktrees or t3_thread_launch threads" + "hint": "Cursor cloud agents; delegate_task children with isolation: \"worktree\"" }, { "token": "create-skill", From a0bfd09d2e460a055e25bceeccd75474f9b0c232 Mon Sep 17 00:00:00 2001 From: Will Metz <metzwa.2@gmail.com> Date: Sat, 24 Oct 2026 13:47:17 +0000 Subject: [PATCH 2/2] fix: verify delegated targets inside assigned worktrees --- CHANGES.md | 2 + .../skills/poteto-mode/playbooks/shipping.md | 2 +- plugins/pstack/skills/swarm/SKILL.md | 2 +- tests/t3-delegation.test.mjs | 87 ++++++++++++++++++- tools/forks.json | 2 +- tools/substitutions.json | 8 +- 6 files changed, 94 insertions(+), 9 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index f958ab13..54134ceb 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -8,6 +8,8 @@ Independent review rounds now use fresh `delegate_task` calls with the original Autopilot-full owners are delegated children, and orchestrate manages their lifecycle through `delegate_task`, `task_status`, and `task_cancel`. Ordinary top-level conversations require an explicit user request. Writing children use T3's `isolation: "worktree"` from the parent's local commit, with project setup and without uncommitted edits. The substitutions and existing declared forks carry these rules. Regression tests read the distributed skills through the existing CI test job. All three upstream pins remain at `4ec7ffa`. +Verifiers resolve the committed branch or PR head to an exact SHA before dispatch. The parent keeps its checkout. After T3 creates the isolated worktree and runs setup at the parent's commit, the verifier checks out the target SHA detached inside its assigned worktree, confirms HEAD, and reruns setup as needed before behavioral verification. A Git fixture executes the distributed recipe with the PR branch held by an active owner and the parent on main. + This contributor patch leaves the release number unchanged. Include it in the next maintainer release so installed copies receive the fix. ## 2.0.0-rc.18 - Lauren's `4ec7ffa`: statechart stops on inputs it used to chart wrong without saying so diff --git a/plugins/pstack/skills/poteto-mode/playbooks/shipping.md b/plugins/pstack/skills/poteto-mode/playbooks/shipping.md index a80cf90d..0c2e58c1 100644 --- a/plugins/pstack/skills/poteto-mode/playbooks/shipping.md +++ b/plugins/pstack/skills/poteto-mode/playbooks/shipping.md @@ -2,7 +2,7 @@ **You own what lands. Verify each PR independently, land only the verified run from the root, then keep your hands off the stack.** -1. **Resolve the forge, then verify each PR independently before it lands.** GitHub CLI (`gh`) is the default. If `command -v origin` succeeds and Origin can resolve the repository, use `origin pr` for every PR operation. Otherwise stay on `gh` and record the fallback. Never require Graphite (`gt`). One verifier per PR, each a `delegate_task` child with `isolation: "worktree"` from the parent's committed PR head, runs the change the way a user would with the matching control skill in poteto-mode's Non-negotiables, parent against head, and posts `PASS`, `PASS+NOTES`, or `FAIL` on that PR. Only a verdict from an agent that did not write the code counts, because an author checks what it meant to build. Green CI and an approving bot review are not verdicts, because neither runs the change as a user would. +1. **Resolve the forge, then verify each PR independently before it lands.** GitHub CLI (`gh`) is the default. If `command -v origin` succeeds and Origin can resolve the repository, use `origin pr` for every PR operation. Otherwise stay on `gh` and record the fallback. Never require Graphite (`gt`). Resolve each PR head to an exact committed SHA before dispatch and include it in the verifier brief. One verifier per PR, each a `delegate_task` child with `isolation: "worktree"`, uses its assigned worktree and follows the **swarm** skill's target checkout and setup instructions. It runs the change the way a user would with the matching control skill in poteto-mode's Non-negotiables, parent against head, and posts `PASS`, `PASS+NOTES`, or `FAIL` on that PR. Only a verdict from an agent that did not write the code counts, because an author checks what it meant to build. Green CI and an approving bot review are not verdicts, because neither runs the change as a user would. 2. **Land only the contiguous verified run rooted at the bottom.** Walk up from the lowest unmerged PR and stop at the first one without `PASS` or `PASS+NOTES`. The PRs below it are the run, and the top one is the ceiling. A verified PR above an unverified one waits, because it cannot land without the code below it. One whose branch neither holds nor needs that code can move onto trunk and land on its own. It needs a fresh verdict first, because its old one ran with that code beneath it and trunk does not have it. 3. **Re-check that the verdicts still describe the code.** A verdict covers a patch, and a chain rebase moves every PR above it to a new SHA without failing any check. Compare `git patch-id` at the verdict SHA with the current head before trusting an older verdict. When the patches differ only in tests, docs, or lint config, a lane's result stands if what it ran builds the same at both SHAs apart from noise. Noise is a difference that also shows up between two builds of the verdict SHA, or an embedded commit SHA. Report each kind of noise with its files, so a reader can check the call. The change itself still gets fresh checks and a review, because the old verdict never saw it. A lane with no build output, such as a dev server, cannot show this, so rerun it, and re-verify anything else that drifted. 4. **Land bottom-up, one PR at a time.** Always merge the run's lowest unmerged PR, with `origin pr merge <pr>` or `gh pr merge <pr> --squash --match-head-commit <head>`, where `<head>` is the current head SHA whose verdict survives step 3's patch-id rule, so a head pushed after that check cannot land. On Origin the plain command also merges every PR below the one you name, and on GitHub a PR whose base is still its parent's branch merges into that branch, not trunk. Read the PR back for the result (`origin pr view <pr>` or `gh pr view <pr> --json state,mergedAt,baseRefName`), because a merge command can report success when a merge queue only queued the PR, and a requested auto-merge is not a merge. A landed PR reads merged with trunk as its base, not its parent's branch. Arm `--auto` (Origin merge-when-ready or GitHub auto-merge) only on a PR whose verdict covers its current head, because it lands whatever head is ready. After a merge lands, retarget the PR whose base was the merged branch to trunk (`origin pr edit <pr> --base <trunk>` or `gh pr edit <pr> --base <trunk>`), move its patch onto the merge result, and push with `--force-with-lease`. The verdict covers the patch, not its commits, so step 3's patch-id rule decides whether the verdict survives. Merge it once its checks pass and its verdict survives. diff --git a/plugins/pstack/skills/swarm/SKILL.md b/plugins/pstack/skills/swarm/SKILL.md index e33246e9..0a7eca88 100644 --- a/plugins/pstack/skills/swarm/SKILL.md +++ b/plugins/pstack/skills/swarm/SKILL.md @@ -26,7 +26,7 @@ Open a todolist with one entry per phase before launching anything. Spawn all N workers in one response as `delegate_task` calls with `mode: "async"` and the step 4 `target`, left unset for `auto` or `inherit-parent`. Every worker runs on this machine. A worker that writes uses `isolation: "worktree"` and writes only in its assigned worktree. -When a worker must start from a non-default branch, put the parent on that branch and commit the required changes before dispatch. T3 worktree isolation starts from the parent's local commit, without uncommitted edits. +When a worker needs a different branch or PR head, leave the parent checkout unchanged. Before dispatch, resolve the intended committed target to an exact SHA with `git rev-parse --verify <target>^{commit}` (fetch first if needed), and include that SHA in the brief. Use `isolation: "worktree"`; T3 initially creates the worktree from the parent's local commit and runs project setup at that commit, without copying uncommitted edits. Inside its assigned worktree, the worker runs `git checkout --detach <sha>`, confirms `git rev-parse HEAD` matches the requested SHA, and reruns the project's setup as needed for that target before behavioral verification. Keep the assigned worktree path and the live owner's branch unchanged; never force the checkout. Every brief stands alone. Include the goal, scope, exact slice or race arm, how to verify, and what to report. Reports use `PASS`, `ISSUES`, or `BLOCKED` with evidence. A worker that can prove a defect reports `ISSUES` and lists every issue it can prove, not only the first. diff --git a/tests/t3-delegation.test.mjs b/tests/t3-delegation.test.mjs index a07e7687..a26de4b2 100644 --- a/tests/t3-delegation.test.mjs +++ b/tests/t3-delegation.test.mjs @@ -1,5 +1,8 @@ import { expect, test } from "bun:test"; -import { readFileSync } from "node:fs"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; const skill = (path) => readFileSync(new URL(`../plugins/pstack/skills/${path}`, import.meta.url), "utf8"); const poteto = skill("poteto-mode/SKILL.md"); @@ -51,6 +54,86 @@ test("writing children use T3 worktree isolation from committed parent state", ( } expect(poteto).toMatch(/parent's local commit.*project setup/); expect(poteto).toMatch(/Uncommitted edits are not copied/); - expect(skill("swarm/SKILL.md")).toMatch(/parent.*branch.*commit.*before dispatch/); expect(playbook("opening-a-pr")).not.toContain("Subagents inherit it"); }); + +test("verifiers select a committed target inside their assigned worktree and refresh setup", () => { + const swarm = skill("swarm/SKILL.md"); + expect(swarm).toContain("leave the parent checkout unchanged"); + expect(swarm).toMatch(/Before dispatch.*exact SHA.*brief/); + expect(swarm).toMatch(/initially.*parent's local commit.*setup at that commit/); + expect(swarm).toMatch(/Inside.*assigned worktree.*`git checkout --detach <sha>`.*`git rev-parse HEAD`/); + expect(swarm).toMatch(/reruns the project's setup as needed for that target before.*verification/); + expect(swarm).not.toContain("put the parent on that branch"); + + const shipping = playbook("shipping"); + expect(shipping).toMatch(/Resolve each PR head to an exact committed SHA.*brief/); + expect(shipping).toMatch(/assigned worktree.*\*\*swarm\*\*.*target checkout and setup/); + expect(shipping).not.toContain("from the parent's committed PR head"); + expect(playbook("autopilot-full")).toContain("at the code-ready SHA per the **swarm** skill"); +}); + +test("swarm's documented recipe verifies an owned PR head while the parent stays on main", () => { + const swarm = skill("swarm/SKILL.md"); + // Execute the commands shipped to workers, so stale or missing guidance fails CI. + const resolve = swarm.match(/`(git rev-parse --verify <target>\^\{commit\})`/)?.[1]; + const checkout = swarm.match(/`(git checkout --detach <sha>)`/)?.[1]; + const head = swarm.match(/`(git rev-parse HEAD)`/)?.[1]; + expect(resolve).toBeDefined(); + expect(checkout).toBeDefined(); + expect(head).toBeDefined(); + + const root = mkdtempSync(join(tmpdir(), "t3-verifier-target-")); + const parent = join(root, "parent"); + const owner = join(root, "owner"); + const verifier = join(root, "verifier"); + const git = (cwd, ...args) => execFileSync("git", args, { + cwd, + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + env: { + ...process.env, + GIT_CONFIG_GLOBAL: "/dev/null", + GIT_CONFIG_NOSYSTEM: "1", + GIT_AUTHOR_NAME: "Fixture", + GIT_AUTHOR_EMAIL: "fixture@example.invalid", + GIT_COMMITTER_NAME: "Fixture", + GIT_COMMITTER_EMAIL: "fixture@example.invalid", + }, + }).trim(); + const setup = (cwd) => execFileSync(process.execPath, ["setup.mjs"], { cwd }); + try { + git(root, "init", "-q", "-b", "main", parent); + writeFileSync(join(parent, ".gitignore"), "installed.txt\n"); + writeFileSync(join(parent, "requirements.txt"), "trunk dependencies\n"); + writeFileSync(join(parent, "setup.mjs"), 'import { copyFileSync } from "node:fs"; copyFileSync("requirements.txt", "installed.txt");\n'); + git(parent, "add", "."); + git(parent, "commit", "-qm", "trunk setup fixture"); + const parentHead = git(parent, "rev-parse", "HEAD"); + git(parent, "worktree", "add", "-qb", "pr-owner", owner); + writeFileSync(join(owner, "requirements.txt"), "PR dependencies\n"); + git(owner, "commit", "-qam", "PR target fixture"); + const target = git(parent, ...resolve.replace("<target>", "pr-owner").split(" ").slice(1)); + writeFileSync(join(owner, "live-owner.txt"), "owner still active\n"); + const ownerStatus = git(owner, "status", "--porcelain"); + expect(() => git(parent, "checkout", "pr-owner")).toThrow(/already (used by worktree|checked out)/); + + // Model T3's initial worktree and setup at the parent's commit; no live task is spawned. + git(parent, "worktree", "add", "-q", "--detach", verifier, parentHead); + setup(verifier); + expect(readFileSync(join(verifier, "installed.txt"), "utf8")).toBe("trunk dependencies\n"); + git(verifier, ...checkout.replace("<sha>", target).split(" ").slice(1)); + expect(git(verifier, ...head.split(" ").slice(1))).toBe(target); + setup(verifier); + expect(readFileSync(join(verifier, "installed.txt"), "utf8")).toBe("PR dependencies\n"); + expect(git(verifier, "branch", "--show-current")).toBe(""); + expect(git(parent, "branch", "--show-current")).toBe("main"); + expect(git(parent, "rev-parse", "HEAD")).toBe(parentHead); + expect(git(parent, "status", "--porcelain")).toBe(""); + expect(git(owner, "branch", "--show-current")).toBe("pr-owner"); + expect(git(owner, "rev-parse", "HEAD")).toBe(target); + expect(git(owner, "status", "--porcelain")).toBe(ownerStatus); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/tools/forks.json b/tools/forks.json index 1eb108b3..5bd0048e 100644 --- a/tools/forks.json +++ b/tools/forks.json @@ -20,7 +20,7 @@ }, "plugins/pstack/skills/poteto-mode/playbooks/shipping.md": { "kind": "policy", - "why": "Adds --match-head-commit <head> to the GitHub merge in step 4, the head guard from Michael Denyer's pstack-claude, with the current head whose verdict survives step 3's patch-id rule, so a head pushed after that check cannot land, and stops step 5's watcher loop at a merge gate that needs a person. Verification children use delegate_task worktree isolation from the parent committed PR head.", + "why": "Adds --match-head-commit <head> to the GitHub merge in step 4, the head guard from Michael Denyer's pstack-claude, with the current head whose verdict survives step 3's patch-id rule, so a head pushed after that check cannot land, and stops step 5's watcher loop at a merge gate that needs a person. Verification children use delegate_task worktree isolation, then select the exact committed PR SHA and refresh setup in their assigned worktree per swarm, leaving the parent checkout unchanged.", "since": "2.0.0-rc.2", "upstream": "not-proposed" }, diff --git a/tools/substitutions.json b/tools/substitutions.json index 993a0ab7..bc9faead 100644 --- a/tools/substitutions.json +++ b/tools/substitutions.json @@ -475,8 +475,8 @@ }, { "pattern": "When a worker must start from a non-default pushed branch, pass `cloud_base_branch`.", - "replacement": "When a worker must start from a non-default branch, put the parent on that branch and commit the required changes before dispatch. T3 worktree isolation starts from the parent's local commit, without uncommitted edits.", - "rationale": "T3 worktree isolation starts at the parent local commit, not a cloud_base_branch or a manually created worktree" + "replacement": "When a worker needs a different branch or PR head, leave the parent checkout unchanged. Before dispatch, resolve the intended committed target to an exact SHA with `git rev-parse --verify <target>^{commit}` (fetch first if needed), and include that SHA in the brief. Use `isolation: \"worktree\"`; T3 initially creates the worktree from the parent's local commit and runs project setup at that commit, without copying uncommitted edits. Inside its assigned worktree, the worker runs `git checkout --detach <sha>`, confirms `git rev-parse HEAD` matches the requested SHA, and reruns the project's setup as needed for that target before behavioral verification. Keep the assigned worktree path and the live owner's branch unchanged; never force the checkout.", + "rationale": "T3 initially isolates at the parent local commit; select an explicit target and refresh setup inside the assigned worktree without moving the parent onto a branch held by a live owner" }, { "pattern": "Transcripts live at `~/.cursor/projects/<slug>/agent-transcripts/<uuid>/<uuid>.jsonl`, where `<slug>` is the workspace path with the leading slash dropped and each \"/\" turned into \"-\" (so `/Users/you/proj` becomes `Users-you-proj`). Every line is one chat message.", @@ -762,8 +762,8 @@ { "files": "^skills/poteto-mode/playbooks/", "pattern": "One verifier per PR, each a Cursor cloud agent, runs the change the way a user would with the matching control skill (such as `control-ui` or `control-cli` from `cursor-team-kit`), parent against head,", - "replacement": "One verifier per PR, each a `delegate_task` child with `isolation: \"worktree\"` from the parent's committed PR head, runs the change the way a user would with the matching control skill in poteto-mode's Non-negotiables, parent against head,", - "rationale": "Playbooks: Shipping; UI evidence" + "replacement": "Resolve each PR head to an exact committed SHA before dispatch and include it in the verifier brief. One verifier per PR, each a `delegate_task` child with `isolation: \"worktree\"`, uses its assigned worktree and follows the **swarm** skill's target checkout and setup instructions. It runs the change the way a user would with the matching control skill in poteto-mode's Non-negotiables, parent against head,", + "rationale": "Playbooks: Shipping; verify an explicit PR head in the assigned child worktree through swarm, leaving the parent and PR owner checkouts unchanged" }, { "pattern": "`/loop` per component until the diff is zero.",