Repository navigation
refactor(pstack): port upstream's playbook cuts, effort rule and Graphite removal - #6
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08bc5467ab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+1073
to
+1075
| "--state", | ||
| "all", | ||
| "--json", |
There was a problem hiding this comment.
Add an explicit --limit to this pr list call. gh pr list --help documents a default maximum of 30 items, so when a long-lived stack branch has more than 30 PRs across the requested all states, an older open child can be omitted behind newer closed or merged PRs. openChildren then silently treats that branch as the tip and persists an incomplete frontier rather than reporting the omitted PR.
Useful? React with 👍 / 👎.
Comment on lines
1130
to
1133
| if (rows.length > 1) { | ||
| throw new UserError( | ||
| `gt info output contains multiple PRs for branch ${branch}` | ||
| `branch ${branch} has multiple pull requests (${rows.map((item) => `#${item.pr}`).join(", ")}); close the stale ones first` | ||
| ); |
There was a problem hiding this comment.
Select the active/relevant PR rather than rejecting every branch with historical matches. Because the query deliberately uses --state all, reusing a branch name after an earlier PR was merged or closed produces multiple rows even when exactly one PR is currently open; the suggested remedy cannot close an already merged PR, so orch frontier set becomes permanently unusable for that otherwise valid chain.
Useful? React with 👍 / 👎.
…hain, not Graphite Ports the Graphite removal from upstream ce649c0 (cursor#613). orch frontier set walks the pull request base chain through the checkout's current branch with gh by default, or origin with --forge origin. The --source gt|gh option and the gt parsers go. Workers never retarget or merge PRs, and one stacker owns chain rebases, retargets, and merges. Adapted for T3: units.tsv keeps each unit's delegate_task taskId and child thread id. trunkBranch asks the remote's HEAD with git ls-remote --symref before the clone's origin/HEAD, because origin/HEAD here still says main for a repository whose default branch is lim (FORK.md Trunk row). The stacker's squash-merge recipe and the cloud workers placement stay. The three graphite PENDING entries in check-t3-port.mjs go with the drift they covered. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ports the Match the model to the job rule from upstream ce649c0 (cursor#613) into poteto-mode's Subagents section, and points the eleven routed skills and thirteen playbooks that spawn subagents at it with one sentence each. arena and interrogate count a composer-* model as the grok family, as upstream does. Adapted for T3: a spawn starts from its role line and may lower the effort option of the target for judgment or routine work, but never raises it above the role line. The hardest tasks line already covers the hardest work, so the role file stays the single source of model choice, and Claude's rate limits are not spent on a per-spawn raise. Upstream's fast-mode-when-urgent clause is dropped. Speed stays with the existing Speed rule, which is unchanged (FORK.md Speed and Model per role rows). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…fier for small follow-ups Ports the verification cadence from upstream ce649c0 (cursor#613). The plan skeleton ran the full swarm at every push that changed the patch. It now runs once at the code-ready head, and again only for new behavior, a redesign, or a large new diff. A rebase, a conflict resolution, or a minor fix gets one verifier at the new head. The autopilot playbooks take the same cadence in their own commit. Adapted for T3: the single verifier names its model, the counterpart of the code author's family from the role file's Counterparts list, so it is never the author (FORK.md Model per role row). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Ports the shipping rewrite from upstream ce649c0 (cursor#613). Nine steps become six, and each rule says why it holds. After a merge, the child PR is retargeted to trunk, its patch moves onto the merge result, and it is pushed with --force-with-lease. The patch-id rule then decides whether its verdict survives. opening-a-pr points stacked children at that step. Adapted for T3: the verifier is a delegate_task child on the counterpart of the code author's family from the role file, never the author, in its own worktree (FORK.md Subagents and Model per role rows). The landing watch is watch_pull_request on the run's bottom PR plus the fallback heartbeat from the role file's Waking and polling section, with the queued watcher's --status-only table read on each wake (Waking row). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… catch the operator up with /ketchup Ports the autopilot rewrite from upstream ce649c0 (cursor#613) and the /ketchup change from 8043881 (cursor#616). Autopilot-full and Autopilot-stack keep their rules, drop the procedure around them, and say why each holds. Autopilot-stack states only where it differs from Autopilot-full. Three rules are new: pin --force-with-lease to the SHA you last pushed or took over, run the patch's checks on the merge result when trunk changed a caller or a dependency, and hold for the root rather than take a third rebase past merge prep. A full swarm runs once at the code-ready head, as in the multi-phase plan. An audit tick that changed something ends with /ketchup, and the reply is a /ketchup. Adapted for T3: each owner is a delegate_task child on the cloud workers line, or local in its own worktree (Subagents row). The hourly wake is /loop 1h in a Claude root or a schedule_task interval, and each tick re-reads the playbook from origin/lim with the command check-plan.mjs requires (Waking and Trunk rows). Trails stay as the uncommitted decisions.tsv and children.tsv returned with reports, and children.tsv records each child's taskId and thread id so the root can probe it with task_status (Todo list row). The single follow-up verifier is the counterpart of the author's family. The upstream origin/main merge-base command is gone with the procedure, so the last PENDING entry in check-t3-port.mjs goes too, and the test now plants a stale entry itself so the rule stays covered with an empty list. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…d read the audit PR column from Origin Ports two fixes from upstream ce649c0 (cursor#613) unchanged. In watch-pr, a completed Code Review Gate check run with an ACTION_REQUIRED conclusion maps to the owner-approval gate on both read paths instead of a failed check, so the watcher neither fails nor waits on a human approval. In worktree-audit, the PR column reads origin pr list when the Origin CLI can read the repository and falls back to gh otherwise. Both shapes normalize to one lowercase state, and a draft PR also holds the worktree. The fork's --threads option and GitHub default-branch lookup in worktree-audit.sh stay as merged in sync PR B. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sets both FORK-SYNC-LOG.md rows to adapt, PR C, with the FORK.md rows each port applied. Adds an Effort row to the T3 port intent table: upstream raises and lowers effort per job, the fork lowers only and never above the role line, because a raise spends Claude rate limit per spawn and the role file stays the single source of model choice. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
leegmoore
force-pushed
the
sync/c-big-port
branch
from
October 17, 2026 15:25
08bc546 to
5c7537a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c7537abc4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| This is the half after `playbooks/babysit.md`. | ||
| 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, a `delegate_task` child on the counterpart of the code author's family (the Counterparts list in the role file, `~/lim-v2/AGENTS.md` or `~/lim-v2/CLAUDE.md`) and never the author, in its own worktree per poteto-mode's Subagents section, runs the change the way a user would with the matching control skill (such as `control-ui` or `control-cli`), 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. |
There was a problem hiding this comment.
For a multi-commit PR, this step no longer records the verdict's base SHA and only says to compute git patch-id “at” each head. The natural git show <head> | git patch-id hashes only the tip commit, so a chain rebase or fix that changes an earlier commit while leaving the tip patch unchanged can appear equivalent; step 4 may then reuse the old verdict and merge code no verifier saw. Record the verdict's base and head, then compare a stable patch ID of each complete base-to-head diff.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Reused branch names break frontier set
- branchPullRequest now prefers the single OPEN PR for a reused branch and only errors when multiple OPEN PRs exist or a pin is required for multiple historical PRs.
- ✅ Fixed: Trunk checkout silently empties the frontier
- resolveFrontier now throws when on the trunk branch, preventing frontier.set from writing an empty frontier.
You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5c7537a. Configure here.
| ); | ||
| } | ||
| return parseGtPullRequest({ branch, detail: rows[0] ?? "" }); | ||
| return row; |
There was a problem hiding this comment.
Reused branch names break frontier set
Medium Severity
branchPullRequest lists every PR for a head with --state all and errors if more than one row comes back. A reused branch with a merged PR plus a new open PR is a normal GitHub workflow. The previous gh walker listed only open PRs, so this used to resolve.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5c7537a. Configure here.
| // Init-plus-fetch checkouts and local fixtures lack origin/HEAD. | ||
| return "main"; | ||
| } | ||
| if (!Array.isArray(value)) { |
There was a problem hiding this comment.
Trunk checkout silently empties the frontier
Medium Severity
If HEAD is the trunk branch, resolveFrontier returns an empty list and frontier.set writes it. The previous gh walker errored when the current branch had no open PR, so it did not overwrite frontier.json. Running orch frontier set --repo from a default-branch checkout now wipes a valid stack.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5c7537a. Configure here.
…dren by thread id Review findings 1, 2 and 7 on PR #6. An owner's decisions.tsv and children.tsv now live under autopilot/<pr>/ in the pstack store, as upstream ce649c0 intended, so they survive a dead VM. A local owner writes there. A cloud owner cannot reach the store, so it posts each trail row and each new child's id in a marked PR comment, and the root copies them into the store at every drain and tick. The hourly tick reads the store instead of every owner's checkout. task_status reads only tasks the calling thread created, so the root probes its own owners with it, a recorded child with t3_thread_read on its thread id, and cloud work by its PR and branch, which a cloud owner records instead of a taskId it does not have. An owner cannot arm watch_pull_request (a child cannot watch a PR, a cloud VM has no T3 tools, and ending the turn ends its task), so it polls with the bare watch-pr and posts CODE-READY <sha> as a PR comment. The root arms watch_pull_request on each owner's PR, with the tick as fallback. babysit.md points owners at that paragraph. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ng through fallbacks Review findings 3, 11 and 12 on PR #6. poteto-mode's effort rule said verification stays on the swarm workers entries, which contradicted the counterpart verifier that shipping, autopilot-full and multi-phase-plan name. Now verification effort is never lowered: a full-swarm lane keeps its swarm workers entry, and a single verifier runs on the Counterparts entry for the author's family. swarm's effort sentence carries the same exception. interrogate and arena fall back to table defaults at effort=xhigh. Both now say the original role's effort ceiling survives every fallback, including the nearest supported level. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Review finding 4 on PR #6. Step 5 named the heartbeat only on the GitHub path. The heartbeat is what notices a merge on either forge, so it now covers Origin too, and only the watch_pull_request arm stays GitHub-only. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Review findings 5 and 8 on PR #6. Chain rebases may run on cloud workers the coordinator cannot reach, and store.ts reads head SHAs from local refs, so Stack safety now fetches and resets each chain branch to origin/<branch> in a local clone checked out on the tip before running orch frontier set. The ledger note says "after a chain rebase" instead of "after restack", since Graphite's restack is gone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… open PR Review findings 9 and 10 on PR #6. Both are upstream bugs in ce649c0's frontier resolver, fixed here on top of upstream. listPullRequests passed --state all with no limit, and gh returns 30 rows by default, so an open child beyond the first page of closed predecessors silently dropped out of the frontier. Children are now queried with --state open, every list passes --limit 500, and a page that fills the limit fails with a message instead of passing as complete. branchPullRequest rejected any branch with more than one PR, so an open PR plus a closed predecessor failed with "close the stale ones first", which closing cannot fix. A unique open PR now anchors the branch. Merged and closed PRs anchor it only when none is open, and several of those need the --prs pin to name the one that anchors the chain. Tests cover an open child beyond gh's default page, a truncated list, an open PR over a closed predecessor, and the --prs disambiguation, through a gh-shaped fake that honours --state and --limit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Review finding 6 on PR #6. Nothing guarded the lower-only effort rule. check-t3-port.mjs now bans "Raise it one step", "up to max" and "job is urgent" under pstack/skills and pstack/agents, and requires "Never raise it above the role line" in poteto-mode's SKILL.md. Regression tests plant each phrase and the missing anchor. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… for ce649c0 Review findings 1 and 10 on PR #6. The ce649c0 row now says owner trails live under autopilot/<pr>/ in the pstack store with cloud owners' rows relayed through PR comments, that owners poll and report by PR comment while the root holds the watch, and names the two frontier resolver fixes the fork carries on top of upstream. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A cloud VM has no pstack install, so it cannot run watch-pr. A local owner keeps the installed watch-pr. A cloud owner polls gh pr view for state, checks and review decision, and reads comments for new threads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…unk checkout in resolveFrontier to avoid empty frontier overwrite
…ch/store: adopt pinned-aware PR selection, prefer unique OPEN PR for reused branches, and keep trunk-checkout error to prevent empty frontier writes
…y review thread Both PR #6 round 2 reviewers found that gh pr view --comments leaves out inline review comments, so a cloud owner could report merge-ready with Bugbot threads open. A cloud owner now pages reviewThreads through gh api graphql. T3's PR watch ignores comments from its own account, which owners share, so a local owner also sends its report to the root with t3_thread_send, and the root polls cloud owners' PRs every 10 minutes while any is out. The root acts on a report only from the owners' account and at the PR's current head. Trails move to autopilot/<run-slug>/<repo>/<branch>/ so runs and repos never share one, and the root keeps its owners in autopilot/<run-slug>/children.tsv so a restarted root can find them. Cloud trail rows are copied by comment id. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…thread paging Round 3 review notes on PR #6: trail comments get the same author and PR checks as reports, and the reviewThreads query names its cursor and pageInfo. PR #7's review note: the ce649c0 sync-log row records how owners poll and report. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
leegmoore
added a commit
that referenced
this pull request
Oct 17, 2026
* test(orch): pin that frontier set refuses a trunk checkout resolveFrontier already throws a UserError when HEAD is the trunk, so `orch frontier set` can no longer overwrite a valid frontier with an empty one. Nothing tested it. The new test saves a frontier, checks out main, asserts the UserError, and asserts frontier.json is byte-for-byte unchanged. It fails when the guard is removed. Fixes #6 (comment) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(check-t3-port): scan extensionless text files and clean up test trees The extension allowlist skipped extensionless executables such as scripts/watch-pr/watch-pr, so drift planted there passed. The walker now also scans an extensionless file when its first 8 KB hold no NUL byte. New tests plant origin/main in watch-pr and show an extensionless binary is skipped. Each test fails against the matching broken walker. copyTree(t) now registers t.after cleanup right after creating its temp root, and every caller passes t, so runs no longer leave t3-port-* copies of the tree in the temp dir. Fixes #4 (comment) Fixes #4 (comment) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(pstack): compare the whole PR patch before reusing a verdict Shipping step 3 compared `git patch-id` at the verdict SHA with the head. Hashing a commit only covers the tip, so a rebase that changed an earlier commit of a multi-commit PR could keep an old verdict for code no verifier saw. A verdict now records the head it verified and that head's merge base with the PR's base branch, taken at verification time. Reuse compares `git diff <saved-merge-base> <verdict-head> | git patch-id --stable` with the same diff from the current merge base to the current head. The saved merge base is never recomputed, because a restack moves it. The build-equivalence exception for test, doc and lint changes stays. Shipping step 3 owns the rule. autopilot-full, autopilot-stack and multi-phase-plan point to it. Fixes #6 (comment) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(pstack): place /ketchup choices above the list and restore /swarm's N step /ketchup said to show a choice after the action list, but also that the html_render page, where a drawn choice goes, shows above the reply. It now says to show each choice above the list, either under the screenshots that already show its options or on the published page, with the action item ending "see the page above". Upstream's 99743e9 cut /swarm's step 3, so a bare "swarm this" left N undefined while Phase B still spawns all N workers. The step is back, and FORK-SYNC-LOG.md records it as a fork fix on top of 99743e9. Fixes #7 (comment) Fixes #7 (comment) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(pstack): compare verdict patches with whitespace kept git patch-id --stable strips whitespace, so a rebase that only dedents Python or changes YAML nesting kept the same ID and carried an old verdict to untested behavior. --verbatim keeps whitespace. Checked in a scratch repo: --stable gave the same ID for a dedent that changed behavior, --verbatim gave different IDs. Fixes #9 (comment) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Third of four sync PRs. Upstream
ce649c0reworked 35 files the fork had also changed: a per-job effort rule, Graphite removed from orchestrate, one full swarm per PR, and shipping and autopilot cut down to their principles.8043881has autopilot loops catch the operator up with/ketchup.What changed
autopilot/<run-slug>/in the pstack store. They read every review thread, and they report to the root byt3_thread_sendor a 10-minute poll, because T3's PR watch ignores comments from its own account.Scope
ce649c0is ported except the fast-when-urgent clause.Blast Radius
Every program playbook and the orch scripts. Running programs read the new playbooks at their next hourly tick, so reinstall right after the merge.
Verification
node scripts/check-t3-port.mjsis clean with 0 pending entries, its tests pass 28 of 28, andbun testpasses 72 of 72.limraised a Claude coder tomax. This PR's skills sent the work to thehardest tasksline instead.🤖 Generated with Claude Code