Repository navigation
Plan and completion review diff against the spec's own base - #570
Merged
Merged
Conversation
Plan review and completion review no longer default --base to a literal main. Plan review falls back to the repository's default branch, like the other review commands. Completion review uses the base work recorded for this spec, then the chain parent's branch, then the default branch, on every backend including host. Work records the spec id and base branch name beside the merge-base SHA so the review computes the merge-base at review time. Fixes #538, fixes #569. Task: fn-304-plan-and-completion-review-diff-against.1 Claude-Session: https://claude.ai/code/session_01LbHwMnkD6ovBw6n8cnD8hQ
…chain lookup when recorded The host completion-review fence compared the work record against the spec id as invoked, so a short alias discarded a valid recorded base. It now compares against the canonical id, and queries the chain only when no valid record exists, as the CLI path does. Task: fn-304-plan-and-completion-review-diff-against.1 Claude-Session: https://claude.ai/code/session_01LbHwMnkD6ovBw6n8cnD8hQ
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
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
Plan and completion review stop defaulting --base to a literal main. Plan review falls back to the repository's default branch; completion review uses the base work recorded for this spec, then the chain parent's branch, then the default branch, on every backend including host. Fixes #538 and #569.
What changes for a user or operator
Repositories whose default branch is master can run plan and completion review without --base. Specs that land on an integration branch (a release branch, a chained parent, a flow --goal child) get a completion review of their own commits only.
Scope
19 files changed; +410/-71 lines; 2 generated, 2 mechanical.
1. Hardcoded main base
Both commands defaulted --base to main, failing on master repos and judging the wrong diff on integration branches.
.flow/specs/fn-304-plan-and-completion-review-diff-against.md: The spec: resolution order, boundaries, and why goal derivation was dropped. [R1, R2]2. CLI base resolution
Check the completion resolver's order and that a foreign or dead record falls through.
plugins/flow-next/scripts/flowctl.py: Parsers default --base to None; plan uses _default_review_base, completion uses the new _completion_review_base. [R1, R2, R3, R4]plugins/flow-next/tests/test_claude_review_commands.py: Real-CLI cases: master default, recorded integration base excludes another child, foreign/dead record, chain parent, explicit base, no base. [R1, R2, R3, R4]3. Work records the base branch
Check the fence writes the canonical spec id and branch name beside the unchanged SHA.
plugins/flow-next/skills/flow-next-work/phases.md: Branch fence writes .flow/tmp/spec_base_branch. [R6]plugins/flow-next/tests/test_chain_consumer_fixtures.py: Chain and --base fence runs assert the record. [R6]4. Host workflow follows the same order
Check the host fence matches a short spec alias against the canonical id and only queries the chain without a valid record.
plugins/flow-next/skills/flow-next-spec-completion-review/workflow-host.md: Replaces the main/master fallback with the shared resolution order. [R5]plugins/flow-next/tests/test_host_review_backend.py: Runs the fence: record, short alias, foreign record, chain parent, explicit and unresolvable BASE_COMMIT. [R5]5. Release and generated files
Version bump to 9.0.2, changelog, Codex mirror, tracker manifest and Flow bookkeeping.
CHANGELOG.md: 9.0.2 entry crediting the reporter of both issues. [requirement undeclared]Rest of diff: 2 mechanical files; 2 generated files; 7 not described files
Coverage: R1 → groups 1, 2; R2 → groups 1, 2; R3 → group 2; R4 → group 2; R5 → group 4; R6 → group 3
Blast radius
Read the resolver in flowctl.py first, then the host fence that mirrors its order, then work's one-line record. Implementation review is unchanged. The work record lives in .flow/tmp, so a completion review run by hand in a cold session after work --base falls back to the chain parent or the default branch.
Verification
Tradeoffs
A review.base config key and a base_branch field on the spec were left out: the run record, the chain parent and the default branch cover the reported setups. Goal-branch derivation in flowctl was dropped because nothing links a child to its goal in data; goal children run work with --base=goal/<branch>, which the record carries.
Fixes #538
Fixes #569
https://claude.ai/code/session_01LbHwMnkD6ovBw6n8cnD8hQ