Skip to content

refactor(planning): make apply ops the one write path for interview page text - #10610

Merged
melodic-automation-lanes[bot] merged 11 commits into
mainfrom
work-resolve-issue/issue-9797-40615014969-1
Oct 29, 2026
Merged

melodic-automation-lanes[bot] merged 11 commits into
mainfrom
work-resolve-issue/issue-9797-40615014969-1

Conversation

@melodic-automation-lanes

@melodic-automation-lanes melodic-automation-lanes Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Closes #9797

Summary

round.sh now takes interview page text only from files: every subcommand that took user text on its command line is removed in favor of its apply op (an ops.json file), or reads that text from a file (add --file takes a question JSON file; supersede --proposes-file and resolve --note-file take plain UTF-8 text files). This follows plugins/planning/skills/interview/context/surface.md:20: "User and dictated text never goes on a command line: it goes into an ops.json op written with the Write tool and run through apply."

Opened by the work-resolve-issue lane from work-resolve-issue/issue-9797-40615014969-1.

Fix

Each round.sh subcommand that took user text, and what happened to it (plugins/planning/surface/round.py):

Subcommand Text it took on the command line Now
group --title, --summary Removed; use the group op through apply.
reply --text, --rec, --why, --affects, --kind Removed; use the reply op through apply.
revise question fields as flags, --affects, --group Removed; use the revise op through apply.
note-reply --text Removed; use the note-reply op through apply.
record-terminal --decision, --alt, --text Removed; use the record-terminal op through apply.
archive --why Removed; use the archive op through apply.
reconfirm --why Removed; use the reconfirm op through apply.
add --short, --title, --facts, --rec, --basis, --waits-on, --commit, --alt Kept; --file is now required and is the only source of the question's text. It keeps the non-text flags --id, --group, --stage, --round, --depends, --follow-up-of, --supersedes, --repoint, --waiting, and refuses a file that is not one question object.
supersede --proposes TEXT Kept; the proposal is read from --proposes-file P. Abbreviation is off, so an old --proposes TEXT is refused instead of read as a file name.
resolve ... reconfirm --note TEXT Kept; the note is read from --note-file N, with abbreviation off as for supersede.

The seven removed names (APPLY_ONLY) stay registered and exit 1 with a pointer to round.sh --dir DATA_DIR apply --file OPS, writing nothing. Refusals reached through an op name the op's fields (affects, alternatives, keepAlternatives, seq, force, resolution, why), not the removed flags.

Files changed:

  • plugins/planning/surface/round.py
  • plugins/planning/surface/exporters.py
  • plugins/planning/surface/surface-checks.sh
  • plugins/planning/surface/README.md: op rows give op fields; supersede and resolve rows give --proposes-file and --note-file
  • plugins/planning/skills/plan/SKILL.md: the supersede and resolve commands use --proposes-file and --note-file
  • plugins/planning/skills/interview/context/surface.md: reply --rec, revise --rec, revise --commit and reply --resolution read as op fields
  • plugins/planning/surface/test_round.py: moves the removed commands' cases to apply; adds tests for the seven refusals, add without --file, and add with a non-object file
  • plugins/planning/surface/test_server.py
  • plugins/planning/surface/test_exporters.py
  • plugins/planning/surface/watch.test.sh
  • .changes/planning/refactor-9797-round-text-through-files-0fb95536.md

Verification

Run locally on 99b5bfc (origin/main merged in):

  • python3 -m unittest test_round test_server test_exporters in plugins/planning/surface/: 797 tests, OK (1 skipped).
  • bash plugins/planning/surface/watch.test.sh: PASS=32 FAIL=0.
  • bash plugins/planning/tests/interview-defenses.test.sh: PASS=172 FAIL=0; standards-binding.test.sh: PASS=9 FAIL=0; interview-surface-decision-mirror.test.sh: ok.
  • git grep over plugins/planning/skills/ and plugins/planning/surface/README.md for round.sh with a removed command and a flag, or supersede/resolve with --proposes/--note taking text: no match.

Run earlier on 744e8d9: bash plugins/planning/surface/surface-checks.sh PASS=283 FAIL=0 SKIP=1; bash scripts/check-changelog-fragments.sh --check all fragments valid; with the APPLY_ONLY refusal disabled, test_each_apply_only_command_refuses_and_points_at_apply fails for all seven names.

Related

#9797

🤖 Generated with Claude Code

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 99b5bfc0bf7a169ffd8a0f95372f2855f3244922: 0 finding(s), 0 as inline comments.

No security issues found.

critical-findings: 0

I read the full diff, the PR context, and the non-test source changes. The tests were skimmed only.

What the change is. It is a local-CLI refactor of the planning plugin's round.py. Page text that used to come in on command-line flags now comes only from files: apply ops, add --file, supersede --proposes-file and resolve --note-file. The seven old subcommands (APPLY_ONLY) now refuse with a pointer to apply. Nothing in the diff changes who is trusted, what a token may do, a kill switch, a merge hold, or any workflow. No changed file matches a trust control. The trusted-path globs matched none of the 11 changed files, so no extra trust-control scrutiny applied.

Checked and clear:

  • File reads: cmd_supersede and cmd_resolve read the proposal and note through the existing read_input helper. The path comes from the invoking user's own argument on a local tool, and the content is .strip()ped. It still passes through exporters.supersede_row and resolve_row, which keep their esc_field escaping and their empty-proposal and --note-file-on-restore refusals. This adds no new untrusted-input path.
  • Argument parsing: allow_abbrev=False on supersede and resolve closes a confusion where an old --proposes TEXT would be read as a file name. parse_known_args plus the extra check keeps unknown arguments rejected, so there is no silent flag swallowing. The removed commands exit 1 before any write, so no old call path writes unvalidated text.
  • add --file: it now rejects a file that is not one JSON object. The override flags are limited to non-text fields (id, group, stage, followUpOf, supersedes, round, depends, waiting). The validation that add_question does after this point is unchanged.
  • Instruction-surface edits: the edits to surface.md, plan/SKILL.md and the README rewrite flag references as op-field references. They reinforce the existing rule that user text never goes on a command line, and they remove no guardrail. The protected-class register was not consulted, because no rule is deleted, narrowed or softened.
  • Manifests: the diff has no plugin.json or marketplace.json change, so there are no manifest <field> lines.
  • surface-checks.sh: the new rapply and grev helpers build JSON from fixed literals and from sequence numbers read from status output. They run only in the local test script, so no untrusted input reaches them.

Spend: none; no billing, budget or model change.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 99b5bfc0bf7a169ffd8a0f95372f2855f3244922: 1 finding(s), 0 as inline comments.

Code review: #10610 (round.sh page text only from files)

I found one advisory documentation problem. No blocking correctness issue turned up in the code I read, and the closing issue's criteria are met. The only checks I ran myself were greps over the checked-out head. I did not run the test suites; the 797-test and PASS=… results in the PR description are author-claimed and unverified.

Stale references to removed CLI flags. The PR's own test and the issue's second criterion ("no instruction to pass user text on a round.sh command line") rely on the docs saying the old flags are gone. Two unchanged lines still describe them as live:

Neither line is in the diff, so I could not anchor an inline comment.

Closing issue #9797.

  • Per-subcommand list: met. The PR body lists each one.
  • No round.sh command line carries user text in plugins/planning/skills/interview/: met apart from the stale mention above, which names a flag but does not tell anyone to pass text.
  • Surface tests under plugins/planning/surface/ and scripts/check-changelog-fragments.sh passing: author-claimed, not run by me.
  • Test coverage: the new cases cover each refusal (test_each_apply_only_command_refuses_and_points_at_apply), add without --file, and add with a non-object file. The supersede and resolve helper converts --proposes and --note to the file flags.

Code observations, no finding.

  • parse_known_args plus the APPLY_ONLY check exits 1 with the pointer before any write.
  • Leftover extras are rejected through p.error.
  • allow_abbrev=False on supersede and resolve keeps an old --proposes TEXT from being read as --proposes-file.

Manifest items. manifest: not hit. The diff touches no plugin.json or marketplace.json. The changelog fragment is bump: minor for a removed CLI surface, which is acceptable for this repository's fragment convention.

Spend. The change adds no billed resources and no extra lane runs.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head c67d0a07866a1dcf50f727a405f5fcc64d79e4ba. A person must clear this reason, so the pr-merge lane added the needs-human label: fix the reason, then remove the label, and the next scheduled run judges it again.

its body carries the lane's own 'Hold: a human merges this' line: a person merges it by hand, or removes that line

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 28, 2026
kyle-sexton and others added 3 commits October 28, 2026 10:34
Three tests merged from main (#10578, #10622) drove `revise` and
`record-terminal` through command-line text, which this branch removes.
They now write one apply op each and keep their assertions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on 6399320f22c618d4705fbc5a9722feb338e8d1b7; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread plugins/planning/surface/round.py Resolved
Run every APPLY_ONLY name with old-style text flags and assert the refusal
points at `apply --file` and leaves questions.json byte-identical. Cover `add`
without --file and `add --file` with a non-object JSON.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-merge lane escalated this pull request, and its head has moved (head-moved): its escalation names head c67d0a07866a1dcf50f727a405f5fcc64d79e4ba, and the head is now 6399320f22c618d4705fbc5a9722feb338e8d1b7. pr-merge removed needs-human so it judges 6399320f22c618d4705fbc5a9722feb338e8d1b7 afresh, and escalates again if its refusal still holds. To keep a person on it, add needs-human back: a label a person adds is never removed.

@melodic-automation-lanes melodic-automation-lanes Bot removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 28, 2026
@melodic-automation-lanes

melodic-automation-lanes Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

Unattended merge: native auto-merge leaves this pull request unarmed at head 99b5bfc0bf7a169ffd8a0f95372f2855f3244922. The lanes clear this reason on their own; the next scheduled run judges it again.

pr-merge's gate holds it on its risk score (evidence-unbound:claude-review: not bound: no-run; evidence-unbound:claude-security-review: not bound: no-run)

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 28, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 1 of max 4 on 6399320.

@melodic-automation-lanes

melodic-automation-lanes Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped address-feedback on this pull request: the lane left a pr-verify note open (uncertain, deferred or answered incorrect, or still open after the round cap and one fresh fixer round); pr-verify judges the next head afresh. The lane sets no label: the run ends in a failing pr-unblock/address-feedback status on the head the lane gated.

When a retry on this head can change the outcome, the lane runs one on its own, up to its per-head cap, and edits this comment if it stops again; a run that does not stop clears the status. When no retry can (a review thread the lane does not take or settle, or a failing check after the retries are spent), a person resolves the threads or gets the checks green, then dispatches the workflow below: that run finds the cause gone and clears the status. On a pull request the lanes App opened, the dispatchers start that run once per stop; on any other, a person does. A new commit to the branch starts clean.

To run it again on this head now, dispatch the pr-unblock workflow:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=10610 -f activity=address-feedback

@kyle-sexton kyle-sexton removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 29, 2026
Comment thread plugins/planning/surface/round.py Resolved
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 2 of max 4 on 0dbf055.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on 0dbf055c6e6ebf3b340cdff8b756c9ad06d6cf49; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread plugins/planning/surface/test_round.py Resolved
Lane-Activity: pr-unblock/address-feedback
Lane-Thread: PRRT_kwDOTCGFQM6xYacL
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 3 of max 4 on 60fb0f6.

Comment thread plugins/planning/surface/README.md Resolved
Lane-Activity: pr-unblock/address-feedback
Lane-Thread: PRRT_kwDOTCGFQM6xZ2Gp
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

pr-unblock / address-feedback finished round 4 of max 4 on 204a0e2.

kyle-sexton and others added 2 commits October 28, 2026 23:33
… flags

reply, revise, archive and add's commits check are reached only through
apply ops or a question file now, so their refusals name `seq`,
`force: true`, `"commits": []`, `resolution` and `why` instead of
--seq, --force, --commit none, --resolution and --why.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The unattended merge path armed native auto-merge at head 99b5bfc0bf7a169ffd8a0f95372f2855f3244922.

@melodic-automation-lanes
melodic-automation-lanes Bot added this pull request to the merge queue Oct 29, 2026
Merged via the queue into main with commit 63f10a9 Oct 29, 2026
77 of 78 checks passed
@melodic-automation-lanes
melodic-automation-lanes Bot deleted the work-resolve-issue/issue-9797-40615014969-1 branch October 29, 2026 04:33
@github-actions

Copy link
Copy Markdown
Contributor

Post-merge mutation testing

Killed 0, survived 0, timeout 0, no coverage 0, invalid 0, not run 7.
Covered-code score n/a; mutation score n/a.

File Killed Survived Timeout No coverage Invalid Not run
plugins/planning/surface/exporters.py 0 0 0 0 0 1
plugins/planning/surface/round.py 0 0 0 0 0 5
plugins/planning/surface/surface-checks.sh 0 0 0 0 0 1

The run's summary has the full report.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(planning): make apply ops the one write path for interview page text

1 participant