[melodic-automation-lanes]

Closes #14688

Summary

Opened by the work-resolve-issue lane. The run's own summary:

Simplified the functions that start in the last fifth of plugins/planning/shared/web/app.js (lines 6506-8132 of 8132) without changing behavior. Repeated code is now shared: a closeOther helper replaces five copies of the other-box close, a held() note replaces three copies of the lock/read-only ternary in renderPicks, a beenHere() check replaces two inline copies in render(), and render() reuses its existing gone list. Dead logic is gone: the always-true v8() checks in this range (v8 is () => true) were inlined or their no-op .hidden = false resets deleted after confirming nothing else hides those elements, along with a Math.max on a non-negative length and a tools.hidden write that the next line overwrote. laterItems() is now read once in renderHost.

Not verified

• Browser/e2e tests for the planning web page were not run here (no test runner is available in this lane); left to the PR's CI
• ci-status green is a PR CI check, left to the PR's CI
• Tidy pass 2 could not prove any comment deletion (change-shape.sh is not runnable in this lane); proposal: app.js:7656 // the welcome seen, or something answered restates beenHere's body and could be deleted
• Tidy passes 4-5 skipped on the changelog fragment: its body is the exact text the issue mandates, so no rewrite applies
• typos check skipped by check-affected (typos not installed)

Fix

• plugins/planning/shared/web/app.js: in functions starting in range 5 of 5, added closeOther and held/beenHere helpers, reused render()'s gone, inlined or removed always-true v8() checks, dropped a redundant Math.max, a duplicate tools.hidden write and a second laterItems() call, merged the…
• .changes/planning/work-resolve-issue-issue-14688-43008361129-1-4f4b3623.md: new patch fragment, ＃＃＃ Changed / Internal simplification, no behavior change.

Verification

The runner ran each check from the base copy against 4ae729e:

• scripts/check-changelog-fragments.sh --check: passed
• scripts/check-changed-skills.sh --merge-base: passed

The run reports:

• check-changelog-fragments: all 8 fragments valid (run before and after tidy)
• check-changed-skills: no changed skills, nothing to gate (run before and after tidy)
• check-affected: 12 passed, 7 skipped, 0 failed over 2 changed files (run before and after tidy)
• fresh-context review subagent over the whole diff: no behavior change found in any hunk (checked TDZ for closeOther, ternary equivalence for held, gone scope/shadowing, and that no other code sets hidden on the elements whose !v8() resets were removed)
• grep of plugins/planning for the changed identifiers and strings: no test asserts on the edited lines; snapshot-map.md already documents v8() as always true
• scope measure: 2 files, 68 changed lines in app.js, 1 area; every edited hunk is at or after line 6541, inside functions that start in range 5 (6506+)
• The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/43008361129

Left out: the items under Not verified.

Related

#14688

Merge-Risk: max=MEDIUM demote=false head=4ae729eb6faeffff54dcae5578196f85d122f5f8 rubric=8.4.2 rater=claude-opus-5-5 checker=claude-opus-5-5

[github-actions]

claude-security-review reviewed 4ae729eb6faeffff54dcae5578196f85d122f5f8: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #15343, head 4ae729e

No security issues found.

I read every hunk of both changed files:

• plugins/planning/shared/web/app.js is a client-side UI refactor:
  • Repeated code now goes through shared helpers: closeOther, held, beenHere, and the existing gone list in render().
  • Checks on v8() were inlined or removed. It always returns true, so the removed !v8() terms were always false.
  • A redundant Math.max and a duplicate tools.hidden write are gone.
• .changes/planning/...md is a new changelog fragment that bumps the patch version.

Security-relevant points I checked:

• Owner-only controls stay owner-only. Removing v8() drops only an always-true term. hGoalEdit, goalAdd and goalEdit are still hidden when the user is not the owner (!owner / !me.isOwner). The Ask panel's apModeBtns.explore and the settings switch switches.hideClaude were only ever hidden by !v8(), which was always false. Owner checks still cover the containers they sit in (apModes.hidden = !Core.me.isOwner, and the settings panel behind hSettings.hidden = !owner). The page can't be trusted to enforce permissions anyway; the server is where those checks belong.
• No new HTML-injection risk. Every changed DOM write (fmtNote, k.qref, the aria-label attributes, PP.note) still uses textContent or setAttribute. None uses innerHTML.
• The removed-people filter behaves the same. !(S.removed || []).includes(p) becomes !gone.includes(p), using the gone list render() already has, so removed people are still left out of the asked-of names, the ask-for list and the joined count.
• Nothing in the diff touches workflows, tokens, secrets, trust controls, merge holds, plugin manifests or standing instructions.

The PR description and the closing-issue text contain no embedded instructions aimed at this lane. The prior-findings file shows this is the lane's first review of this PR (blocking only: no).

critical-findings: 0

[github-actions]

claude-review reviewed 4ae729eb6faeffff54dcae5578196f85d122f5f8: read 2 of 2 changed file(s); 0 finding(s), 0 as inline comments.

Code review: no findings. I read every hunk of both changed files: plugins/planning/shared/web/app.js and the new changelog fragment.

What I checked against the code at head (I read the source; I ran no tests):

• closeOther (app.js:6541) is a const at the same 4-space scope as the handlers that call it: otherCancel.onclick, toggleMulti, fmtSave.onclick and fmtClear.onclick. Each of those runs only on a user event, after module setup, so there is no temporal-dead-zone risk. Every call site does the same two writes the inline code did.
• held(what) in renderPicks gives the same result as the old ternaries. It returns a non-empty string when a lock or read-only state applies and "" otherwise, so held(...) || (...) falls through exactly where the old : ... branch did. The "choice" and "order" wording is kept for each format.
• gone reuse in render(). const gone = S.removed || [] (app.js:7394) is at the top level of render(), which starts at line 7278. That is before the three new uses at 7709, 7718 and 7869. No nested block in between declares another gone; the other gone declarations sit in other functions (renderHost, personActs, and others).
• v8() removals. v8 is () => true (app.js:56), so each !v8() term is always false. I grepped for the elements whose .hidden = !v8()… resets were deleted: hLater, hGoalLabel, hGoalRow, apModeBtns.explore and switches.hideClaude. Nothing else in the file hides them, and hsec, btn and swRow do not create them hidden. The askMode "explore" fallback was dead for the same reason.
• Smaller cleanups, all equivalent:
  • beenHere(), the qref restructure and the scale keydown o local behave as before; ?? binds inside the ?: else branch, as it did.
  • Dropping Math.max(total, 0) is safe because total is a filter length.
  • The dropped tools.hidden = true was overwritten by tools.hidden = !q (app.js:7912).
  • In renderHost, later is now computed once with the same .sort.

Closing issue #14688, acceptance criteria:

• Behavior unchanged: met, per the checks above. No test asserts on these lines; the PR body says this was confirmed by a grep, which I did not repeat.
• Only listed files and fragments changed: met.
• Size: met. The PR is 2 files and about 70 changed lines, well under 400 lines and 15 files.
• Fragment: met. It is bump: patch with ### Changed and the exact line the issue requires.
• Only functions starting in range 5 changed: met. Every hunk is at line 6538 or later; the range starts at 6506.
• ci-status green: a CI check; this review did not confirm it.

The browser/e2e results and the check output in the PR body are the author's claims; I did not verify them.

No settled threads or prior findings apply: this is the first review of this PR by this lane.

[melodic-automation-lanes]

Unattended merge: native auto-merge leaves this pull request unarmed at head 4ae729eb6faeffff54dcae5578196f85d122f5f8. No person needs to act: a lane clears this reason, and the next scheduled run judges it again.

a pr-unblock lane stopped its head (pr-unblock/fix-ci is failure), which clears when the lane runs
  again

[melodic-automation-lanes]

The pr-unblock lane stopped fix-ci on this pull request: the lane could not fix every failing check, after one retry a model rung up, so a session builder takes it; its run log names each one and why. The lane labels it needs-session for a session builder, who makes the change this lane cannot, and the run ends in a failing pr-unblock/fix-ci status on the head the lane gated.

• Reason: ci-fix-escalated
• Head: 4ae729eb6faeffff54dcae5578196f85d122f5f8
• Run: https://github.com/melodic-software/claude-code-plugins/actions/runs/43018575806

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=15343 -f activity=fix-ci

[melodic-automation-lanes]

The unattended merge path armed native auto-merge at head 4ae729eb6faeffff54dcae5578196f85d122f5f8, the head its arming checks judged.

[melodic-automation-lanes]

Verification evidence for 4ae729e

The verifier passed this head. The capture artifact holds the capture summary, which says whether the capture ran, and any screenshot or video that passed the mask check; this comment uploads no file.

Capture artifact: pr-capture-15343-4ae729eb6faeffff54dcae5578196f85d122f5f8, expires on 2026-12-11.

_{One comment per pull request, updated in place for each new head.}

[melodic-automation-lanes]

The post-merge check verified merge commit 4566ff018e3fcfb5d9df0baf0c7a030c4817db0e; later checks count it verified from this comment.