Phase 4: Worktree and phase lifecycle declare #502

Merged

mph merged 22 commits from IW-463-phase-04 into IW-463 2026-09-03 04:56:15 +00:00

Conversation 0 Commits 22 Files changed 60 +3314 -711

mph commented 2026-09-02 15:25:30 +00:00

Owner

Copy link

Phase 04 implementation for IW-463.

Files

Browse changed files: https://code.iterative.works/iterative-works/iw-cli/-/blob/IW-463-phase-04/

Phase 04 implementation for IW-463. ### Files Browse changed files: https://code.iterative.works/iterative-works/iw-cli/-/blob/IW-463-phase-04/

mph added 20 commits 2026-09-02 15:25:30 +00:00

chore(IW-463): update review-state for phase 04 a841b73a16

IW-463 Phase 4 Step 1: add Args.mapN, port PhasePr to it ... 7cdc0179f8

Adds Args.mapN overloads (arity 2-8), desugaring to the existing
zip/map chain with no new Args case and no parser change. Ports
PhasePr's eight-way zip to it, replacing the eleven-line left-nested
tuple pattern with a direct case-class constructor call. Declared
content and PhasePrHarnessTest are unchanged.

Authorised by implementation-log.md:1083 (Phase 3 hand-off).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4 Step 2: shared Args[CiGateConfig], CiGateConfig's duration defaults removed ... fa23516205

Adds CiGateArgsDeclaration.args: one declared Args[CiGateConfig] for
--timeout/--poll-window/--poll-interval, with PhaseMerge.parseDuration
as each leaf's validator, producing a CiGateConfig rather than three
strings a caller re-parses. phase-pr composes it in place of its own
three duration leaves; declared content is unchanged and
PhasePrHarnessTest is green with no changed expectation.

CiGateConfig's pollWindowMs/timeoutMs/pollIntervalMs parameter
defaults are removed (D4.3): a default here is a second answer to a
question the shared declaration already answers. Test fixtures that
constructed CiGateConfig() or a partial CiGateConfig(...) now state
all three durations explicitly.

CommandHelp.usage is made public as usageLine, so a command's own
post-parse hint (Step 3/4) can print the same "Usage: iw …" line
renderRejection does, from one place (D4.4).

CiGateArgs (core/commands/CiGate.scala) is not yet deleted: its two
remaining callers, PhaseMerge and CiGateCommand, are migrated in
Steps 5 and 7. Deleting it now would break compilation before those
commits land.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4 Step 3: declare worktrees, status, open, analyze ... a7524a5d5c

Declares the four smallest commands: worktrees (--all/--json
switches), status (optional <key> operand + --json, keeping its
post-parse "branch is not an issue id" hint but printing it through
CommandHelp.usageLine per D4.4), open (optional <key> operand +
--prompt as Args.resolved per D4.1), and analyze (required
<issue-id> operand, pass-through exit codes per D4.5, WorktreeLifecycle
group per the context's grouping rationale).

Deletes the args.contains/foldLeft/extractPrompt argv scrapes each
replaced. A missing --prompt value now says "--prompt requires a
value." (was "a text argument") and analyze's usage line moves
stdout -> stderr, per the Testing strategy table.

Adds FakeStateReader.readCallList so a --help harness case can prove
no state was read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4 Step 4: declare rm, phase-start, start ... 29262acd57

Declares the operand commands. rm: required <key> operand plus
--force switch; parseArgs and its stdout usage literal go, replaced
by CommandRejection.reject on stderr. phase-start: required
<phase-number> operand validated with PhaseNumber.parse (D3.6) plus
--issue-id resolved from the branch; the positional filter and inline
usage go.

start re-tests D3.3's alternation and holds: no Args.Alt. Both
<issue-id> and --effort are declared optional
(Args.optionalOperand / Args.resolved), so an empty argv parses
cleanly and checkArgCombination — kept verbatim, printing
CommandHelp.usageLine instead of the Start.usage literal — is the
only place that tells a caller what start needs. The operand's and
--effort's `does` each name the other as the alternative, and the
declaration carries two examples, one per way in.

Rejection text changes at the sites the Testing strategy table names:
rm's and phase-start's missing-argument/usage lines move to the
placeholder-named MissingOperand form and from stdout to stderr;
start's --prompt/--effort MissingValue messages read "requires a
value." StartHarnessTest.scala's "neither issue id nor --effort"
case (checkArgCombination's own text) stays green unchanged, which is
the proof D4.4 was implemented rather than lost.

Behavioural note for implementation-log.md: rm's argument rejection
now happens before readConfig, so an unconfigured project invoked
with no argument sees "Missing required argument: <key>" instead of
"Cannot read configuration".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4 Step 5: declare phase-advance, phase-commit, phase-merge ... 969fe50782

Declares the three phase commands. phase-advance: both flags
Args.resolved from the branch, per the context's worked recipe.
phase-commit: --title required, --items/--issue-id/--phase-number
Args.resolved; the inline usage string goes.

phase-merge composes the Step 2 shared Args[CiGateConfig] plus
--max-retries (validated with PhaseMergeArgs.parseMaxRetries),
--issue-id and --phase-number; CiGateArgs.parseConfig is no longer
called from this file. Declares exit 3 (it reaches
CommandResult.resume) and exit 1 naming --timeout in both meaning and
caller action, matching PhasePr's exit 1 text for a spent budget.

Adds FakeGit.getCurrentBranchCallList so a --help harness case can
prove no git call was made.

LESSONS: running a new --help/unknown-flag harness test against an
unmigrated gated command (phase-merge, before this commit) can hang
the JVM forever rather than fail an assertion, because FakeClock's
sleep is a no-op and its now never advances without an explicit call
— the poll loop spins with no exit condition. Two Mill daemon
processes for this worktree were left in that spin state and had to
be killed by PID; the daemon's own state directory
(out/mill-daemon) was left corrupt afterward ("Worker wire broken")
and had to be removed to let a fresh daemon start.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4 Step 6: declare gate and gate check ... 35d17401bf

Declares gate as a parent (Args.unit, no flags of its own) carrying
one subcommand, check, via CommandDeclaration.subcommand — the same
shape Config.scala uses. check's <point> stays a required positional
operand (D3.6: the validator accepts any String, its `does` names
known points as examples, not a closed list) because iw-run:601-610
reads it from argv position 2 to choose which hook files to load; a
--point flag would compile and pass every test here while silently
loading no gates.

GateCommand.run becomes CommandArgParser.dispatch + parse; the
hand-rolled usage literal and the `case "check" :: point :: _` match
go. group = PhaseLifecycle for both, per the context's reasoning:
GateCommand resolves --issue-id/--phase-number from the branch the
same way every phase command does, so its answer is about the phase
the branch is on.

Manual check (Risks 4, nothing in core/test/ can catch a shape
change here): `./iw gate check phase-pr` reports the same workflow
gate phase-pr itself evaluates, since both call WorkflowGateCheck
with the same context.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4 Step 7: declare ci-gate; delete CiGateArgs and ReviewStatePath.resolve's List[String] entry point ... fc3ffcc9c6

Declares ci-gate: the shared Args[CiGateConfig] from Step 2, plus
--pr-url/--input/--issue-id, all Args.resolved (D4.1) since each has
an honest "when absent" sentence. resolve/resolveKey/pullRequest now
take the parsed options instead of scraping argv a second time.
ci-gate calls ReviewStatePath.resolveFrom(options.input, "--input",
key, env) with its own parsed --input value; the List[String]
resolve entry point ReviewStateCliParser used to extract for it is
deleted, closing the doc comment's own note that ci-gate was its
last caller. Declares exit 3 (reaches CommandResult.resume) with
exit 1 naming --timeout in both meaning and caller action.

With ci-gate migrated (phase-merge already was, Step 5), CiGateArgs
(core/commands/CiGate.scala) is now unreferenced and is deleted,
along with the comment calling it a second copy of the duration
defaults.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4 Step 8: deletions, the registry walk, the sweep, the log ... cace8ac65a

Deletes PhaseArgs.namedArg/hasFlag (no callers remain outside their
own now-deleted tests; ReviewStateCliParser.hasFlag is a distinct
helper and does not move). Adds all twelve declared commands to
CommandDeclarationTest's subjects — phase-merge and ci-gate marked
canResume/waitsOnABudget = true — reaching 20 declarations from 15
top-level subjects with no existing assertion rewritten. Adds a
CommandHelpTest case pinning that start's does strings name the
alternative flag/operand, alongside gate's parent/subcommand
rendering cases.

Adds one BATS wiring case (test/phase-start.bats): an unknown flag
on iw phase-start names the flag, creates no branch, exits 1.

Runs and records: the in-repo unknown-flag sweep (no in-repo caller
of the twelve passes an unknown flag, a --leading value, or an extra
bare token; commands/, iw-run and iw-bootstrap are unchanged from
this phase's starting commit, confirmed by diff, not just absence of
a stat line); the Open questions 3 leaf/field audit (thirteen
declarations, one documented exception — gate check's <point>, read
by iw-run from argv, not from the parsed value, unchanged from the
pre-Phase-4 code's own unused match binding); and the legibility
gate on start's rendered help (verdict: legible, Args.Alt not added,
with the caveat that no subagent tool was available in this run so
the read was not genuinely isolated).

Full implementation-log.md entry added: D4.1-D4.7 outcomes, the two
inventory corrections (10 remaining not 11; 25+9=34 declarations not
8/33), the group assignment table for all 25 top-level commands with
Open questions 1 flagged for Michal, the correction to
implementation-log.md:158-159's claim about which CiGate comments
name a phase, and what Phase 5 inherits. LESSONS.md lesson 5's
undercount is fixed to 25/34 with gate check named as the ninth
subcommand.

./iw ./test unit: core 186/186 (+ this phase's additions), dashboard
241/241. ./iw ./test e2e: 150/150 ok, including the new phase-start
case.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4: pin start's unknown-flag rejection in help-intercept case 7a ... 18efa82f1c

test/help-intercept.bats:203 was a ninth rejection-text site, outside the
eight the phase context's Testing strategy table enumerated. Under the
declared parse `iw start --help-me` is refused by name as an unknown flag
instead of falling through to the issue-ID parser.

The case's intent is unchanged: the token still reaches the command rather
than the launcher's help intercept, and the new text proves that more
directly because only the command's own parser can emit it.

Records the ninth site in implementation-log.md and two LESSONS entries.

IW-463 Phase 4: start takes one <key>, replacing the <issue-id>/--effort alternation ... 4d5e6a35e9

The cold-legibility re-run of D4.4's gate found the rendered usage line
contradicted the prose it was meant to disambiguate. Michal chose to remove
the asymmetry at its source, matching the shape open/rm already use, rather
than add Args.Alt. checkArgCombination is deleted along with it: a required
operand refuses an empty argv on its own, and there is only one positional
slot left to fill, so "both given" cannot arise either.

iw start --effort <slug> becomes iw start <slug>; iw start <issue-id> is
unchanged. Effort grounding (isEffortOnHead, the two ungroundedEffort
recovery hints) is untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4: one shared --issue-id leaf for the five phase commands ... 2da26f0375

phase-advance, phase-commit, phase-start, phase-merge and phase-pr each
declared a byte-identical Args[Option[String]] for --issue-id, flagged as
deferred duplication in the phase's own code review. Extracted to
PhaseIssueIdArgsDeclaration.args, composed the way CiGateArgsDeclaration.args
already is. GateCommand's and CiGateCommand's own issueIdArgs are untouched
- their from/does prose differs for genuinely different semantics.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4: add IssueTracking, a seventh CommandGroup ... de6dd076a4

Resolves Open questions 1 from the phase's group-assignment record: issue
(and the create subcommand Phase 5 declares) fit CommandGroup's six existing
groups no better than a forced placement under ProjectSettings. No
declaration binds to IssueTracking yet - issue is Phase 5's.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

chore(IW-463): record the three post-review Phase 4 changes ... d5d8c0688b

Records, in implementation-log.md: D4.4's supersession (the gate passed on
its stated criterion, but the cold-legibility reader found the usage line
contradicting the prose, and Michal chose to remove the asymmetry at its
source rather than add Args.Alt); checkArgCombination's deletion and why;
the exact user-visible start change and every doc/test site updated for it;
and changes 2/3 with the corrected 25-command group map, so Phase 5
inherits a map with no blanks.

Annotates every phase-04-tasks.md entry whose recorded outcome these three
changes supersede, and adds a new section for the three changes themselves.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4: fix help-intercept case 7b for start's new key resolution ... 6bf11a15ad

"helper" used to fail IssueId parsing outright. With start's <key> now
resolved through WorktreeKey.parseArg (issue-first, effort fallback),
"helper" is a syntactically valid effort slug, so it now fails at the
grounding check instead. Update the assertion to the actual reachable
error, keeping the case's real point intact: a bare value token containing
"help" still reaches the command, not the help intercept.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4: LESSONS - a literal 'iw start' grep under-covers BATS's own quoting ... e7b861c92a

Recorded at the moment found while sweeping the repo for start's
--effort/<issue-id> callers: test/help-intercept.bats:207 escaped a literal
"iw start" search because the binary name is quoted separately from the
subcommand ("$PROJECT_ROOT/iw" start), which is this project's own BATS
idiom throughout. The stale assertion only surfaced when ./iw ./test ran
the BATS suite, not from ./iw ./test unit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WehHdHp9MRKJkrhGFiKNxa

IW-463 Phase 4: act on code review iteration 1 ... 9935642066

Six review skills ran in parallel: 1 critical, 4 warnings, 7 suggestions,
no correctness defect. Full report in review-phase-04-20260902-090538.md.

Fixed here:
- PhaseArgs.scala's PURPOSE line still named namedArg/hasFlag, deleted in
  this phase (critical).
- ReviewStatePath.resolveFrom's docstring narrated the deleted List[String]
  overload, which the project's comment rules forbid.
- GateCommandHarnessTest's check --help case pinned "<point>", which the
  parent's help also renders, so it could not fail on the dispatch
  regression it covered. It now pins "Usage: iw gate check".

Adds the review packet and the aggregated review report.

IW-463 Phase 4: act on code review iteration 2 ... 5b88f65017

- PhaseIssueIdArgsDeclaration's PURPOSE line carried a bare phase
  reference, which the comment rules forbid. Removed. The D-number
  citations elsewhere stay: they name a design decision, which has
  precedent in Config.scala, rather than a point in time.
- CommandGroup.IssueTracking has no declaration bound to it until
  Phase 5 declares `issue`, so it now says so in the code rather
  than only in the issue folder.

IW-463 Phase 4 review fix: name a mistyped issue ID in the not-framed hint ... 409de7c10c

'iw start iw463' silently fell through IssueId parsing (needs a dash) into
EffortSlug parsing (dash-less charset), then failed effort grounding with
only a /sl-frame hint -- steering a mistyped issue ID toward framing an
effort nobody wanted. EffortSlug.looksLikeMangledIssueId is a new pure
predicate (lowercase letters immediately followed by digits, no dash);
Start.ungroundedEffort's not-framed branch appends a second sentence
naming the TEAM-123 form only when it matches. The primary message and
the framed-but-uncommitted branch are untouched. The uppercase form
(IW463) was already safe, since EffortSlug's charset rejects uppercase
before grounding is ever checked.

IW-463 Phase 4 review fix: close --issue-id and start-prompt coverage gaps ... 98b6cab585

All five phase-lifecycle commands compose PhaseIssueIdArgsDeclaration.args,
but phase-merge and phase-pr only ever resolved --issue-id from the branch
name in their harness tests -- an unguarded silent-swap risk in Args.mapN
(two adjacent Option[String] fields). PhaseIssueIdArgsDeclarationTest (new,
mirrors CiGateArgsDeclarationTest) covers the leaf's own contract; one
explicit --issue-id case was added to each of PhaseMergeHarnessTest and
PhasePrHarnessTest, asserting the value reaches behaviour (the review-state
path, the default PR body, the JSON output). Both were proven RED by
swapping the two args' declared order without touching the case-class
field order, then reverted.

test/start-prompt.bats's "fails without a key" case asserted
Missing-required-argument OR Usage, and the Usage branch is true for
almost any start error -- it discriminated nothing. Dropped the fallback
to match StartHarnessTest's unit-level assertion for the same case.

mph added 1 commit 2026-09-02 15:34:34 +00:00

chore(IW-463): update review-state for phase 04 16ee93123e

mph added 1 commit 2026-09-02 15:50:35 +00:00

chore(IW-463): update review-state for phase 04 a719db24d1

mph merged commit ff67bdb6a7 into IW-463 2026-09-03 04:56:15 +00:00

mph deleted branch IW-463-phase-04 2026-09-03 04:56:15 +00:00

mph referenced this pull request from a commit 2026-09-03 04:56:17 +00:00

Merge pull request 'Phase 4: Worktree and phase lifecycle declare' (#502) from IW-463-phase-04 into IW-463

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

agent-ready

  

bug

Something isn't working

  

contract

Run the tool-contract suite on this PR

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

needs-human

  

needs-input

Intake asked the human a question on the issue; waiting for the answer

  

question

Further information is requested

  

wontfix

This will not be worked on

No labels

agent-ready

bug

contract

documentation

duplicate

enhancement

good first issue

help wanted

invalid

needs-human

needs-input

question

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

iterative-works/iw-cli!502

No description provided.