Phase 3: Declarations that cannot be forgotten #500

Merged

mph merged 13 commits from IW-463-phase-03 into IW-463 2026-09-01 19:34:00 +00:00

Conversation 0 Commits 13 Files changed 28 +3490 -655

mph commented 2026-09-01 16:28:58 +00:00

Owner

Copy link

Phase 03 implementation for IW-463.

Files

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

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

mph added 8 commits 2026-09-01 16:28:59 +00:00

chore(IW-463): update review-state for phase 03 8f04f5cfb2

Phase 3 Step 1: rewrite CommandArgParser onto an applicative Args[A] ... 0c06acdfef

Replaces the map-keyed ParsedArgs/Builder parser with a hand-rolled
applicative Args[A] (Pure/OneFlag/OneOperand/Repeated/Mapped/Zipped),
per the decision recorded at the end of Phase 2
(implementation-log.md, "Decision recorded for Phase 3: adopt an
applicative parser"). CommandDeclaration is now generic and carries
subcommands/dispatchToken ahead of Step 4. Adds operand and repeated
leaves, subcommand dispatch classification and their renderer/rejection
cases now so Steps 2-4 land as tests rather than a second rewrite.

PhasePr is ported to build Args[PhasePrOptions]; its declared content
is unchanged. Phase 2's compileErrors test is deleted (D3.7: under
Args[A] there is no accessor left to pin) and replaced by a
CommandDeclarationTest content rule asserting every leaf the parser
accepts is named in the rendered help. The Builder's duplicate-name
guard is likewise replaced by a content rule rather than reintroducing
require (LESSONS 9).

User-visible-neutral by construction; no command besides phase-pr
declares yet. ./mill core.test green; bats test/phase-pr.bats 3/3.

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

Phase 3 Step 5: config declares (parent --json, get <field> subcommand) ... da38329d13

config now declares its whole surface: --json as the parent's own
switch, get as a subcommand carrying a required <field> operand.
showUsage, handleGetMissingField and handleUnknownArgs are deleted;
Config.run dispatches through CommandArgParser.dispatch/parse and
renders rejections through CommandHelp. The "Available fields" content
moves into the get operand's own prose and two worked examples.

Design-consistency finding recorded in the log: config's usage/error
output is unified onto the one-channel (stderr) rendering convention
every other declared command uses, rather than keeping its own
stdout/stderr split. ConfigHarnessTest's channel assertions updated
accordingly; string content assertions are unchanged. Unknown-flag
wording changes from "Unknown option" to the shared "Unknown flag".

13/13 ConfigHarnessTest green; bats test/config.bats 1/1; config and
config get pass every CommandDeclarationTest content rule.

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

Phase 3 Steps 6-7: review-state declares; registry walk reaches all seven ... a62e1b71c0

review-state now declares its whole surface: the parent (three
subcommands, no flags), validate (optional <file-path> operand plus
--stdin), write (19 flags) and update (35 flags, five arrayMode
families). All four hand-rolled help bodies are deleted; run
dispatches through CommandArgParser.dispatch/parse.

Preserves parseWriteArgs/parseUpdateArgs as the boundary (Risks 4):
both untouched, all 48 ReviewStateCliParserTest cases pass unedited.
write/update's declared Args interprets to just the parsed
--output/--input value; every other leaf is composed into Args[Unit]
via a new combine helper, still gatekeeping (rejects an unknown or
invalid flag before the legacy parse runs) and still documented.

ReviewStatePath gains resolveFrom, taking the already-parsed
Option[String] instead of re-extracting --output/--input; the
existing resolve(args, ...) (CiGateCommand's entry point) delegates
to it and is otherwise untouched.

D3.5 (--needs-attention false) needed no special case: as a Switch
with no declared operands, a bare 'false' token is structurally
UnexpectedArgument. D3.6 closes --version banana (Int-typed leaf
validator) and confirms LESSONS 9's disposition for --checkpoint (a
thin (Int, String)-typed wrapper around the untouched parseCheckpoint,
so 1:context_sha=abc still passes — the SHA half stays untightened).

CommandDeclarationTest's subject list grows to phase-pr/config/
review-state, expanding subcommands to seven declarations; every
content rule (including duplicate-leaf-name and rendered-leaf) passes
on all seven with array-mode's templated does strings unweakened.

./mill core.test green; bats test/review-state.bats 2/2.

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

Phase 3 Step 8: annotate, sweep, wrap up ... 38e8e82d54

Annotates commands/review-state.scala's // USAGE: line as superseded
by ReviewState.declaration and slated for deletion in Phase 6 (D3.4);
docs/commands/review-state.md gains a top-of-file note that it will be
generated from the declaration starting Phase 8 and must not be
hand-edited meanwhile. tasks.md's phase-to-story table no longer
claims Phase 3 closes #202; the Phase 6 row now carries it, with the
reason (Phase 1's launcher --help intercept answers before dispatch
until Phase 6 routes it to the registry).

Adds one BATS wiring case to test/review-state.bats: an unknown flag
on `review-state update` is refused by name, writes nothing, exits 1
-- proving the declaration-derived parse is what `iw review-state`
actually reaches at the CLI, not just the in-process harness (one
case, not one per rejection kind, per LESSONS 16).

Records the in-repo unknown-flag sweep (clean: no in-repo caller
passes a flag this phase's declarations reject) and the one-edit-check
diff in implementation-log.md. LESSONS.md gets three updates: LESSONS
7 (--needs-attention) disposed fixed-at-source with how; LESSONS 9
(--checkpoint) disposed not-closed-here, confirming the settled leaf
rule in code; a new entry records the `--stdin`/`--from-stdin` naming
inconsistency Story 3's declarations made visible, left unrenamed per
Risks 5, proposed home a new tracker issue.

./iw ./test unit: green (core.test + dashboard.test, exit 0, no
failures). ./iw ./test e2e: green (205/205 bats cases ok, 0 not ok,
exit 0), both run in the foreground.

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

fix(IW-463): close two silent-success defects found in phase 3 review ... 0b4a44ed78

Critical 1 — a flag the gate accepted was silently discarded.
CommandArgParser.splitInline recognises --flag=value, but extractFlag
matched the exact token only, so `review-state write --issue-id=IW-500`
exited 0 and persisted the branch-inferred id instead. extractFlag and
extractRepeatedFlag now recognise the same inline syntax the gate does,
so the accept-set and the read-set agree. parseWriteArgs/parseUpdateArgs
stay untouched as the domain-construction boundary.

Critical 2 — the rule enforcing this phase's headline claim could not fail.
CommandDeclarationTest asserted rendered.contains(leaf.name), which the
Usage: line satisfies before flagsSection runs. Deleting every flag's
documentation left 186/186 green. The assertion is now carried by
leaf.does, which only flagsSection and subcommandLine emit; the same
mutation now fails 6 tests.

Also: validate --issue-id through WorktreeKey.parseArg so a malformed
value gives a clean error rather than an os.PathError stack trace;
restore the per-field explanations config get lost in the migration;
extract the duplicated reject helper into CommandRejection; assert the
subcommand-help hint on text unique to it.

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

test(IW-463): assert nothing is written when --issue-id is refused ... 76ed5c4469

The three invalid --issue-id cases pinned the exit code and the error
text but not the absence of a write, unlike every sibling "refused
cleanly" case in the file. A regression that reported the error but
still left a partial file on disk would have passed.

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

Declarations that cannot be forgotten ... 30cdf82355

- Args[A] applicative drives both parsing and help rendering
- review-state and config declare in full — 4 hand-rolled help bodies deleted
- subcommands and positional operands added to the declaration type
- two silent-success defects found and fixed in review

mph added 1 commit 2026-09-01 16:29:52 +00:00

chore(IW-463): update review-state for phase 03 a909f48257

mph added 1 commit 2026-09-01 16:40:22 +00:00

chore(IW-463): update review-state for phase 03 b422841b30

mph added 1 commit 2026-09-01 16:50:09 +00:00

chore(IW-463): update review-state for phase 03 cbf312d253

mph added 2 commits 2026-09-01 17:03:22 +00:00

chore(IW-463): update review-state to awaiting_review c4f30f9642

docs(IW-463): record two CI gate defects found during phase 3 ... 8326a715bd

A stale ci_deadline carried from phase 2 failed a fresh PR instantly,
and re-entering the gate pushes a commit that restarts the CI it waits
on. Both are phase-pr/phase-commit behaviour in core/, unrelated to this
phase's subject, so neither is fixed here.

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

mph merged commit 7f1a97fe46 into IW-463 2026-09-01 19:34:00 +00:00

mph deleted branch IW-463-phase-03 2026-09-01 19:34:00 +00:00

mph referenced this pull request from a commit 2026-09-01 19:34:00 +00:00

Phase 3: Declarations that cannot be forgotten (#500)

claude referenced this pull request 2026-10-15 21:57:41 +00:00

review-state: --git-sha is documented on 'write' and silently ignored there #495

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!500

No description provided.