Make /progress effort-agnostic and add --full - #970
Conversation
/progress carried the redesign inside it: a hardcoded redesign/main, a table of seven redesign ticket numbers, and a description that asked "where is the redesign". It is a generic progress skill. It now resolves its own working branch from git and its integration branch from the open pull requests, finds the effort's spec under .claude/specs/ the way /handoff does, and derives what each ticket means to a person from that ticket at answer time. The ticket table is deleted rather than generalised: a table of numbers rots the moment a ticket is retitled, split or closed. The default scope moves to THIS SESSION, because mid-run he is asking what just happened and a whole-effort summary buries it. --full answers the session first and then the whole spec underneath, labelled per scope, and earns the extra room the writing contract normally refuses. Also reseeds two calibration entries that had drifted before this change: handoff/SKILL.md, last moved by a direct commit that ran no PR checks, and orchestrate/SKILL.md from #890. Both verdicts were reread and still describe their files, so they carry forward with new digests. Harnesses: node tools/test-tools.mjs and node .claude/hooks/test-hooks.mjs both pass. node tools/check-calibration.mjs reports 31 files stamped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
There was a problem hiding this comment.
Important
The new live-state procedure cannot yet identify one integration branch or enumerate all work in the current session, so /progress can return incomplete or incorrect answers.
Reviewed changes Reviewed the one-commit rewrite of /progress and its calibration updates.
- Scope model: The default report now covers the current session, while
--fullappends whole-effort state and an optional area narrows both scopes. - Live-state discovery: Redesign-specific branches and ticket meanings are replaced with Git, GitHub, run-state, effort-spec, and ticket lookups.
- Calibration: The progress verdict and related drifted digests are refreshed after the skill edit.
ℹ️ The cited test cannot exercise --full
The PR body says the skill commands hook would catch an invalid /progress --full argument, but the hook only scans lines beginning with node tools/*.mjs; this invocation never enters that scan. Its passing result therefore provides no coverage for the new argument behavior.
Technical details
# Replace the inapplicable test evidence
## Affected sites
- `.claude/hooks/test-hooks.mjs:1131` - the regex accepts only `node tools/<tool>.mjs ...` command lines.
- PR body, `Test evidence` - attributes `/progress --full` coverage to that unrelated scan.
## Required outcome
- Exercise the new argument contract with a check that can fail when it regresses, or accurately state that this behavior was validated manually and remains outside automated coverage.GPT Sol | 𝕏
There was a problem hiding this comment.
Important
The revised evidence model can still misstate what this session changed and what reached the integration branch.
Reviewed changes Reviewed the evidence and regression updates added since the prior Pullfrog review.
- Integration resolution: Anchored branch discovery to the checkout's exact open PR head, followed stacked bases, and fetched the resolved branch.
- Session evidence: Replaced the undeclared
mergedfield with a matchingreadinessLedger, live PR states, and a current-checkout status read. - Contract coverage: Added progress-skill assertions to the hook harness and refreshed the progress calibration.
GPT Sol | 𝕏
…ss-generic-full # Conflicts: # .claude/calibration.json
|
@pullfrog review |
There was a problem hiding this comment.
Important
The current head only merges the updated base, so the two open correctness threads from the prior Pullfrog review still apply; the external CLI evidence is also incomplete.
Reviewed changes Reviewed the complete four-file PR at 673918fc, including the new base merge and the unchanged /progress rewrite.
- Scope model:
/progressdefaults to session work, while--fullappends effort-wide state and an optional area narrows both scopes. - Evidence discovery: The skill resolves an integration branch, reads session-owned pull requests from run state, queries live pull request state, and inspects the current checkout.
- Contract guards and calibration: The hook harness adds text-level progress assertions, and the calibration digest and verdict are refreshed.
GPT Sol | 𝕏
There was a problem hiding this comment.
Important
The new ancestry proof still cannot reliably identify landed session work across sibling repositories or stacked squash merges.
Reviewed changes Reviewed the ancestry and missing-baseline fixes added since the prior Pullfrog review at 673918fc.
- Integration ancestry: Added live
mergeCommit.oidreads andgit merge-base --is-ancestorchecks before calling merged pull requests landed. - Missing baseline: Replaced the checkout fallback with an explicit no-baseline result and one effort-scope report while documenting the linked-worktree blind spot.
- Contract guards: Added textual assertions for ancestry and baseline behavior and refreshed the progress calibration digest.
GPT Sol | 𝕏
There was a problem hiding this comment.
Important
The new per-repository resolver depends on an external response field whose exact command shape is not yet evidenced.
Reviewed changes Reviewed the per-repository integration and squash-boundary fixes added since the prior Pullfrog review.
- Repository-specific resolution: Anchored each ledger pull request to its own live base, resolved the integration branch inside that mapped repository, and scoped fetch failures per repository.
- Squash-boundary handling: Limited ancestry proof to pull requests that directly target integration and made other merged bases explicitly unverifiable.
- Regression contract: Added hook assertions for both fixes and refreshed the progress calibration verdict.
GPT Sol | 𝕏
|
@pullfrog review |
There was a problem hiding this comment.
Important
The three-field gh pr view response that now drives per-repository branch resolution is still not evidenced in the live PR body.
Reviewed changes Reviewed the complete current head and rechecked the external-interface evidence after the latest thread was resolved.
- Scope model:
/progressnow defaults to current-session behavior, while--fullappends effort-wide state and an optional area narrows both scopes. - State evidence: The skill resolves integration branches per repository, enumerates session-owned pull requests from
readinessLedger, and distinguishes direct ancestry from unprovable stacked squash boundaries. - Regression contract: The hook harness adds source-level checks for the new progress contract, while calibration metadata is refreshed for the rewritten skill.
GPT Sol | 𝕏
|
@pullfrog review |
There was a problem hiding this comment.
Important
The integration walk still confuses closed stacked heads with integration branches, and the new external-interface evidence remains incomplete.
Reviewed changes Reviewed the complete six-commit rewrite of /progress, including its current PR-body evidence.
- Scope model:
/progressnow defaults to current-session behavior, while--fullappends effort-wide state and an optional area narrows both scopes. - State discovery: The skill resolves branches per repository, reads session-owned pull requests from
readinessLedger, and distinguishes direct ancestry from stacked squash boundaries. - Regression contract: The hook harness adds source-level checks for the progress procedure, while calibration metadata is refreshed for the rewritten skill.
GPT Sol | 𝕏
The base-chain walk searched only open pull request heads, then accepted every absent head as the integration branch. A stacked parent can be CLOSED while it is still the child's recorded base: PR 575's base is `feature/539-b5-apply-design`, that branch is PR 560's head, and 560 is CLOSED with `mergeCommit` null. The walk therefore named a feature branch as the integration branch for that whole chain. Each candidate now resolves in every state through `gh pr list --head <candidate> --state all`. An OPEN or MERGED parent is followed to its base; a CLOSED unmerged parent ends the walk as an unresolved chain that names the parent instead of reporting its head as integration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Important
The all-state walk has conflicting branch-reuse instructions and lacks valid evidence for its new CLI response; both must be resolved before merge.
Reviewed changes Reviewed the closed-parent fix added since the prior Pullfrog review at 81586aa3.
- All-state parent lookup: Extended base-chain traversal from open pull request heads to
OPEN,MERGED, andCLOSEDrecords. - Closed-chain handling: Stopped a closed, unmerged stacked parent from being reported as the integration branch.
- Regression contract: Added a hook assertion for closed-parent handling and refreshed the progress calibration verdict.
GPT Sol | 𝕏
|
@pullfrog review |
There was a problem hiding this comment.
Important
The merge from redesign/main does not address the two open correctness threads: reused heads still have conflicting outcomes, and the all-state gh pr list response remains unevidenced.
Reviewed changes Reviewed the complete current head at 196e503c, including the base merge after the prior Pullfrog review.
- Scope model:
/progressdefaults to current-session behavior, while--fullappends effort-wide state and an optional area narrows both scopes. - State discovery: The skill resolves integration branches per repository, reads session-owned pull requests from
readinessLedger, and distinguishes direct ancestry from stacked squash boundaries. - Closed-parent handling: The branch walk queries all pull request states and refuses to identify a closed, unmerged stacked parent as integration.
- Regression contract: The hook harness adds source-level checks for the progress procedure, while calibration metadata is refreshed for the rewritten skill.
GPT Sol | 𝕏
Step 3 gave two incompatible decisions for the same multi-row result: take the highest number, and treat a duplicate matching head as ambiguity. A branch name can be reused, so `/progress` could either continue the walk or refuse it depending on which sentence it read. A reused head is normal, not ambiguous, and now has one rule. Exactly one OPEN row decides whatever the numbers are, because a branch can have only one live pull request. With no OPEN row the highest number decides, because numbers increase. Two OPEN rows sharing a head is the real ambiguity and still refuses. A cycle still refuses. An empty array is what ends the walk: the candidate is no pull request's head, so it is the integration branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Important
The reused-head rule can select an unrelated fork pull request with the same branch name and resolve the wrong integration branch.
Reviewed changes Reviewed the deterministic reused-head fix and corrected gh pr list contract evidence added since the prior Pullfrog review at 196e503c.
- Head selection: Defined a sole-open, otherwise highest-numbered rule while retaining explicit ambiguity for multiple open rows and cycles.
- Regression contract: Added source assertions for the empty-array terminator and deterministic reused-head wording.
- External evidence: Replaced prose-formatted output with the exact JSON response, complete selected types, empty-array terminator, and observed state-set evidence in the PR body.
- Calibration: Refreshed the progress skill digest while retaining medium effort.
GPT Sol | 𝕏
`gh pr list --head` filters on the branch NAME only, and its own help states that the `<owner>:<branch>` form is not supported. This repository is public, so a fork's pull request carrying the same branch name comes back in that listing. It could then be the sole OPEN row, and the walk would follow a stranger's `baseRefName`. The listing now requests `isCrossRepository` and drops every row where it is true before any decision runs. An array left empty by that filter terminates the walk exactly as a genuinely empty one does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Important
The new fork filter leaves one anchor path exposed, so integration discovery can still begin from unrelated fork metadata.
Reviewed changes Reviewed the fork-collision fix added since the prior Pullfrog review at 98c61a64.
- Recursive head filtering: Added
isCrossRepositoryto the all-state lookup, dropped fork rows before selection, and treated a filtered-empty list as the integration terminator. - Regression contract: Added source assertions for the recursive filter wording and refreshed the progress calibration digest.
GPT Sol | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes Reviewed the repository-bound anchor fix and executable resolver added since the prior Pullfrog review at b56d1b5e.
- Repository-bound anchor: Added
isCrossRepositoryto the initial open-pull-request listing and prevented a same-named fork row from supplying the checkout's starting base. - Executable branch walk: Added
resolveIntegrationBranchas a testable implementation of the documented all-state traversal, including fork filtering, reused-head selection, ambiguity, and cycle handling. - Regression coverage: Registered eight resolver cases, strengthened the progress hook assertion for the initial anchor, and refreshed the progress calibration metadata.
GPT Sol | 𝕏
|




Closes thomasluizon/orbit-tickets#558
Summary
/progressnow identifies one integration branch, reports session work only from schema-backed ownership, and proves arrival by commit ancestry.readinessLedgeronly whensessionIdmatchescurrentRunIdentifier().MERGEDas an intermediate fact and verifies the merge commit is an ancestor of the integration branch.mergedkey.Branch and session evidence
Chosen integration rule: begin with the pull request whose
headRefNameequals this checkout branch. Follow eachbaseRefNamewhile it is another openheadRefName. The first base that is not an open head is the integration branch. This cannot tie on unrelated bases because traversal begins from one exact head. A duplicate matching head or cycle is reported as ambiguous.Exact real invocation:
Complete response: stdout was
chore/progress-generic-full, stderr was empty, and the exit code was 0.Exact real invocation:
Complete selected response key shape:
Array<{baseRefName:string, headRefName:string, number:number}>.[{"baseRefName":"redesign/main","headRefName":"feature/ticket-71-perfil-s10","number":983},{"baseRefName":"redesign/main","headRefName":"feature/ticket-67-onboarding-s1","number":981},{"baseRefName":"redesign/main","headRefName":"chore/progress-generic-full","number":970},{"baseRefName":"main","headRefName":"dependabot/github_actions/github-actions-ac39c799b5","number":881},{"baseRefName":"main","headRefName":"dependabot/npm_and_yarn/fast-uri-3.1.7","number":801},{"baseRefName":"main","headRefName":"dependabot/npm_and_yarn/xmldom/xmldom-0.8.15","number":799},{"baseRefName":"main","headRefName":"dependabot/npm_and_yarn/orval-8.27.0","number":798}]Session ownership reads
sessionId:string,remaining:Array<string>, andreadinessLedger. Each ledger row reads{repositoryKey:string, prNumber:number, receiptPath:string}. The receipt path supplies provenance, not merge status.The replacement for the undeclared
mergedfield reads livestateand nullablemergeCommit.oidfor each ledger pull request. It then tests the merge commit againstorigin/<integration-branch>. A stacked child cannot pass falsely because its merge commit remains only on its parent branch until that parent reaches the integration branch.With no matching run record, there is no session-start baseline. The skill now says so and reports effort scope instead. The orchestrating checkout also cannot see uncommitted pre-pull-request changes in linked worktrees, so the skill states that limitation.
Observed command contracts
Exact real invocation:
Complete response, with exit code 0:
Exact real invocation:
Complete response: stdout and stderr were both zero bytes, and the exit code was 0. The empty response therefore represented a clean checkout in this observed run.
Exact real invocation:
Complete selected response and types:
{"mergeCommit":{"oid":"<40-character commit oid>"},"state":"MERGED"}, shaped as{mergeCommit:{oid:string}|null,state:string}.Exact real invocation:
Complete selected response:
{"mergeCommit":null,"state":"OPEN"}. The previously observed completePullRequestStateset isOPEN,CLOSED, andMERGED.Exact real invocations:
Both commands returned empty stdout and stderr. The known integrated merge commit exited 0. The branch-only commit exited 1.
Assumptions
.claude/hooks/test-hooks.mjs; I rejected a standalone harness because this file already scans every skill contract.Test evidence
node .claude/hooks/test-hooks.mjspassed withORBIT HOOKS OKwhile both round-two defects were present.$hookOutput = & node .claude/hooks/test-hooks.mjs; $hookExit = $LASTEXITCODE; $hookOutput | Select-String -Pattern 'progress:|ORBIT HOOKS|FAIL'; exit $hookExitfailed the new ancestry and missing-baseline assertions and ended withORBIT HOOKS FAILED (2).node .claude/hooks/test-hooks.mjspassed withORBIT HOOKS OK.node tools/test-tools.mjspassed withORBIT TOOLS GATE OK.node tools/reseed-calibration.mjsstamped 31 files and renewed only the progress verdict.node tools/check-calibration.mjspassed with 31 calibrated files.git diff --checkpassed.Round 3 delivery: the worker was killed before it could push
The worker that wrote
81586aa3was killed by the harness after it committed and before it pushed orreported. It did not fail. Its tree was clean and its commit was complete.
The orchestrator verified that commit and pushed it. None of the worker's own numbers are quoted
for this commit. Everything under the next heading is a run the orchestrator performed itself, from
the clean tree at
81586aa3, inC:\Users\thoma\orca\workspaces\orbit-ui-mobile\ticket-541-progress-generic.Note that the
Test evidencesection above belongs to the EARLIER rounds. It reports 31 calibratedfiles;
81586aa3reseeds again and the current figure is 32.Test evidence for
81586aa3, produced by the orchestratorGreen.
node .claude/hooks/test-hooks.mjsexits 0 and endsORBIT HOOKS OK, with:Red first, proved after the fact rather than claimed. A red-first run that happened only inside a
session that no longer exists is not evidence, so the failure was reproduced on the commit itself.
Reverting ONLY the source file the new assertions read, and nothing else:
exits 1 and fails exactly the two assertions this commit adds, and no others:
git checkout 81586aa3 -- .claude/skills/progress/SKILL.mdthen restores the tree, andgit status --porcelainis empty.The rest, all from the restored clean tree:
node tools/test-tools.mjsexits 0, zeroFAILlines.node tools/check-calibration.mjsexits 0: "32 calibrated file(s) stamped 2026-09-16 against codextiers default, mechanical, oldest verdict 2 day(s) old."
No root
type-checkorlintrun is quoted for this commit. This pull request changes four files,all of them harness documentation and harness tests, and no application source. CI runs both gates on
this head and they are the evidence for them.
External interface:
gh pr view --json state,mergeCommit,baseRefNameSKILL.md:68prescribesgh pr view <number> --json state,mergeCommit,baseRefName. The previousrevision of this section proved only that
baseRefNameis an ACCEPTED field, from the no-value--jsonlisting, and that listing was elided and carried no types. Both gaps are closed here withthe real three-field invocation and its complete selected key and type shape, measured 2026-09-17
against
gh version 2.97.0 (2026-07-31).MERGED, the state whose
mergeCommit.oidthe skill feeds togit merge-base --is-ancestor:Its complete selected key set with each value's JSON type, from
--jq 'to_entries|map({key:.key, type:(.value|type)})':[{"key":"baseRefName","type":"string"},{"key":"mergeCommit","type":"object"},{"key":"state","type":"string"}]mergeCommitis an object, and its own complete key set, from--json mergeCommit --jq '.mergeCommit|to_entries|map({key:.key, type:(.value|type)})', is onestring field:
[{"key":"oid","type":"string"}]OPEN and CLOSED, the two states the skill reports without an ancestry check. Both carry
mergeCommit: null, which is why the skill requires a non-nullmergeCommit.oidbefore it runsmerge-base:So
stateis a string overOPEN,CLOSEDandMERGED;baseRefNameis a string in every state;mergeCommitis eithernullor an object with a single stringoid. That is the whole shape theskill consumes.
A closed stacked parent is not the integration branch
The base-chain walk searched only OPEN pull request heads and then accepted every absent head as the
integration branch. A stacked parent can be CLOSED while it is still the child's recorded base, and
this repository contains a live example, read on 2026-09-17:
PR 575's base is
feature/539-b5-apply-design, that branch is PR 560's head, and 560 is CLOSED withmergeCommitnull. The old walk stopped there and reported a feature branch as the integrationbranch for that whole chain.
SKILL.mdstep 3 now resolves every candidate in every state withgh pr list --head <candidate> --state all --limit 10 --json number,state,baseRefName, takes thehighest
numberbecause a branch name can be reused, follows anOPENorMERGEDparent to itsbase, and ends the walk on a
CLOSEDunmerged parent by naming that pull request instead ofreporting its head as integration. The first candidate that is no pull request's head, in any state,
is the integration branch.
--headand--state allare not assumed: the invocation above is a real run of the installedbinary and its typed output is quoted.
Test evidence for
c0c86de9Red first, in
.claude/hooks/test-hooks.mjs. The new caseprogress: a closed stacked parent is never reported as the integration branchwas added before theSKILL.mdchange andnode .claude/hooks/test-hooks.mjsexited 1 with exactly one failure:Green after the change:
node .claude/hooks/test-hooks.mjsexited 0 withORBIT HOOKS OKand zeroFAILlines.node tools/test-tools.mjsexited 0 withORBIT TOOLS GATE OK. Its output contains two linesmatching
FAIL, and both are PASS lines whose test NAME contains the word,verify-delivery.mjs: a red required check is CI_FAILING, never DELIVEREDandverify-delivery.mjs: CI_FAILING names the checks, so the report never says merely that something is red. Zero real failures. The complete runwas captured to a file before the result was quoted.
.claude/skills/progress/SKILL.mdis a calibrated file, so its digest changed andnode tools/check-calibration.mjsfailed. Its verdict was reconsidered rather than rubber-stamped,and stays at medium effort with one more judgement named in the reason: the walk now has to classify
a parent's state rather than look one up. After
node tools/reseed-calibration.mjs,check-calibrationexited 0:32 calibrated file(s) stamped 2026-09-17 against codex tiers default, mechanical, oldest verdict 3 day(s) old, with 1 verdict renewed and 31 carried forward.This round was written by the orchestrator under the step 8 review-fix authority, not by a worker.
Round 4: the all-state listing's real contract, and one rule for a reused head
Head
98c61a64.The evidence for
gh pr list --head <candidate> --state all, correctedThe previous round's section was wrong about this and the review was right. It quoted
560 CLOSED base=main, which is--jq-formatted output, under a--jsoninvocation that emitsJSON. That is a rendering, not a response. Here is the exact command this skill runs, unaltered,
measured 2026-09-17 against
gh version 2.97.0 (2026-07-31):The top level is an array, from
--jq 'type':Each row's complete key set with types, from
--jq '[.[]|(to_entries|map(.key+":"+(.value|type))|join(", "))]':["baseRefName:string, number:number, state:string"]So
baseRefNameis a string,numberis a number, andstateis a string.The terminator has its own shape and it is now load-bearing. A head no pull request owns returns
an empty array, which is what ends the walk:
Every
statevalue this listing can produce, counted across all 835 pull requests in the repositorywith
--state all --limit 1000 --json state --jq '[.[].state]|group_by(.)|map({value:.[0],count:length})':[{"count":46,"value":"CLOSED"},{"count":780,"value":"MERGED"},{"count":9,"value":"OPEN"}]Three values, all three present, and the skill branches on exactly those three.
One deterministic rule for a reused head
Step 3 said both "take the highest
number" and "a duplicate matching head or a cycle isambiguity". Those are two answers to one response, so the same listing could either continue the
walk or refuse it. A reused branch name makes several rows NORMAL, so the ambiguity line was the
wrong half:
OPEN: that row decides, whatever the numbers are, because a branch can haveonly one live pull request.
OPEN: the highestnumberdecides, because numbers increase.OPEN: that is the real ambiguity, it should not happen, and it refuses byname.
Test evidence for
98c61a64Red first. Two new
.claude/hooks/test-hooks.mjscases were written against the oldSKILL.md,with the file stashed so only the skill reverted:
noContradictoryDuplicateRuleis the one that asserts the old sentence is GONE, which is why it isthe only key already true.
Green after:
node .claude/hooks/test-hooks.mjsexits 0 withORBIT HOOKS OK, andnode tools/test-tools.mjsexits 0 withORBIT TOOLS GATE OKand zero real failures.node tools/reseed-calibration.mjsrenewed 1 verdict and carried 31 forward;check-calibrationthen exits 0 at32 calibrated file(s) stamped 2026-09-17.One local repair worth recording, because it is not a code change and it cost a round.
tools/test-tools.mjsfirst died onERR_MODULE_NOT_FOUNDforsharp, then forfast-check.Neither package was missing: this worktree's
tools/node_modulesis a JUNCTION to the maincheckout's
node_modules, and that directory held 956 empty package directories left behind byan
npm installthe machine killed for low memory. Node resolved the empty husk first and stopped.Removing the husks with
rmdir, which cannot touch a populated directory, let resolution fallthrough to the worktree's own healthy
node_modulesand the harness ran clean. No repository filewas involved.
External interface evidence
Measured with:
gh pr list --jsonwith no value makes the installed binary print every field it accepts.isCrossRepositoryis on that list, alongsideheadRepositoryandheadRepositoryOwner.Real invocation:
Unaltered response:
[{"baseRefName":"chore/ticket-560-sweep-order","headRefName":"fix/ticket-570-harness-gate","isCrossRepository":false,"number":1010}, {"baseRefName":"redesign/main","headRefName":"feature/ticket-67-onboarding","isCrossRepository":false,"number":1007}, {"baseRefName":"redesign/main","headRefName":"feature/ticket-73-static","isCrossRepository":false,"number":1005}]Complete selected key and type shape, taken from that same response:
Round 12 executable contract
tools/lib/integration-branch.mjsnow executes the base-chain rule. The walk is capped at 100candidates, far beyond a practical pull request stack, so an invalid lookup cannot return unique
branches forever. Explicit cycle detection stops shorter loops.
Red before the fix, after the fork-only case existed:
Green after the fix:
Final evidence:
node tools/test-tools.mjsexited 0 withORBIT TOOLS GATE OKand eight integration-branch assertions.node .claude/hooks/test-hooks.mjsexited 0 withORBIT HOOKS OK.node tools/check-calibration.mjsexited 0 with 32 calibrated files.git diff --checkexited 0.