Skip to content

feat(lib): sync the view-settings copies into adhd and ai-briefing - #12917

Merged
kyle-sexton merged 4 commits into
mainfrom
feat/12667-shared-copies-a
Nov 3, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
feat/12667-shared-copies-a

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Refs: #12667

Summary

First of three batches split out of #12792 (S8 of epic #12646). #12792 adds the shared view-settings copies to 9 new carrier plugins, about 72 KB each, and its review lanes come back partial every time (read 61 of 73 files). The lanes count a synced copy as read only when scripts/shared-copies.txt declares it at the merge base, so a PR can't use the copies it declares itself. Landing the carriers in batches small enough for one review pass gets them declared on main, and #12792 then carries only the rest.

This batch: the lib/ changes, the registry update in every plugin that already carries it, and new carriers adhd and ai-briefing.

Fix

  • lib/view-registry.mjs registers the remaining view lanes; the copy in each existing carrier (harness-ops, planning, review, source-control, visualization, work-items) is re-synced.
  • lib/rendered-view.md: the canonical rendered-view reference.
  • adhd and ai-briefing get their own lib/ copies of view-settings, view-registry, config-cascade, yaml-subset and state-key, declared in scripts/shared-copies.txt.
  • One changelog fragment per plugin touched.
  • lib/view-settings.test.mjs stays at main's version; chore(shared-copies): ship the medium resolver and one rendered-view reference to every view lane #12792 brings the 9-carrier version with its last batch.

Verification

  • node --test lib/view-settings.test.mjs: 34 pass, 0 fail.
  • scripts/sync-shared-copies.sh --check: all 606 registered copies match their canonical sources.
  • scripts/sync-shared-copies.sh --check-bump origin/main and scripts/check-changelog-fragments.sh --check: exit 0.
  • node -e "import('./plugins/<plugin>/lib/view-settings.mjs')" exits 0 for adhd and ai-briefing.
  • Size: 27 files, 180526 bytes of diff against main, under one review pass's 300000-byte budget.
  • Every file's content is chore(shared-copies): ship the medium resolver and one rendered-view reference to every view lane #12792's at 7899a74; only the two manifest files and the fragments are cut to this batch.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01MWmASPrNZkCGEUBcZsKuvx

kyle-sexton and others added 2 commits November 3, 2026 14:22
Register the remaining view lanes in lib/view-registry.mjs, sync that
registry into every plugin that already carries it, add the canonical
lib/rendered-view.md, and give adhd and ai-briefing their own copies of
the medium resolver modules.

Refs: #12667

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MWmASPrNZkCGEUBcZsKuvx
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 3, 2026 19:34
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T19:37:00.833210Z 0e93ecc Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 3c9d091fbbb71a584e8773a8ae017392be440177: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #12917, incremental (0e93ecc → 3c9d091)

No security issues found.

critical-findings: 0

Scope

I read the one file this push changed, lib/view-settings.test.mjs, and every hunk of it (1 of 1 files).

What I checked

  • New test: "every registered copy imports its sibling modules and resolves". It reads scripts/shared-copies.txt and picks out each lib/view-settings.mjs <copy> entry. For each copy it runs node on <repo>/<copy> resolve --lane planning --option "".
    • The child is started with spawnSync and an argument array, not a shell, so no command injection is possible.
    • The child's environment points HOME, USERPROFILE and CLAUDE_PROJECT_DIR at an empty home and clears CI and CLAUDE_PLUGIN_DATA, so it reads no real user settings.
    • The copy paths come from a file inside the repository, and changing that file already takes a pull request. The test runs only when a developer or CI runs the test suite.
    • The test doesn't touch any trust control, workflow, manifest or instruction file.
  • Trust controls: this push changes no trusted-path file.
  • Plugin manifests: this push changes none, so there are no manifest items.
  • Instruction-surface deletions: none.
  • Spend: the new test starts one extra Node process per registered copy during test runs. It spends no prepaid credit and creates nothing billable.
  • Prior findings: the earlier round reported 0 findings, so nothing carries over. The round cap was not reached (0 of 3).

@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 0e93ecc3c8afe46ec2a0ac178405de7175d34d84; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread scripts/shared-copies.txt Resolved
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 3c9d091fbbb71a584e8773a8ae017392be440177: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Code review (incremental): PR #12917, changes since 0e93ecc3

No findings. This push changes one file, lib/view-settings.test.mjs, and I read its whole change.

What changed: a new CLI test, "every registered copy imports its sibling modules and resolves" (L255–264). The test does three things:

  • It reads every lib/view-settings.mjs <copy> line from scripts/shared-copies.txt. Today that is 8 copies, including adhd and ai-briefing at L500–501.
  • It asserts that at least one copy is registered, so a manifest the regex can't parse fails the test instead of passing with nothing checked.
  • It runs resolve --lane planning --option "" on each copy as a separate process, with the same isolated environment the existing run helper uses (L235). It checks for exit 0 and a view: report line.

Prior finding: my earlier advisory on scripts/shared-copies.txt L500–501 (thread) said no test would catch a copy failing to import. This test fixes that.

  • The failure case from that thread is now caught: if plugins/adhd/lib/yaml-subset.mjs were deleted, the adhd copy would fail to import and exit non-zero.
  • Later carriers are covered automatically once they are registered.
  • The regex matches LF lines only. That is safe: .gitattributes sets * text=auto eol=lf, so the manifest checks out with LF line endings on every platform.

Closing issue #12667: the "for each new carrier the view-settings copy imports" criterion now has a test that would fail if it broke. I read the test but did not run it; I have no shell in this lane.

No manifest, path-scoped rule or paved-path checks applied: this push changes none of the files they cover.

Prior findings: 1 round so far, cap 3, not past the cap.

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

The pr-verify verifier failed head 0e93ecc3c8afe46ec2a0ac178405de7175d34d84. What it wrote, as data:

What it wrote (29 lines)
Summary: This PR closes #12667 but delivers only part of it. The body's "Related" line says "#12792
  (S8; closes #12667 after the batches land)". GitHub reads "closes #12667" as a closing keyword
  wherever it appears, so the spec links #12667 as `closes`. Merging this PR would therefore close
  the issue. Judged against the whole issue, the diff falls short. It adds the view-settings,
  view-registry, config-cascade, yaml-subset and state-key copies only to adhd and ai-briefing, plus
  the registry re-sync in existing carriers. It does not cover every view-lane plugin: architecture,
  debugging, discovery, codebase-health, education, overengineering and prototype are missing. It
  also does not sync the rendered-view reference into planning, architecture, discovery or
  debugging, so the issue's Outcome is unmet. The code itself is clean. Each copy matches its
  canonical source, and all CI jobs passed (test-bash and test-node:
  https://github.com/melodic-software/claude-code-plugins/actions/runs/41704557292/job/126157164296).
  The blocker is the closing link.
- PR body (Related section): the words "closes #12667" in "#12792 (S8; closes #12667 after the
  batches land)" make GitHub link #12667 as closed by this PR, so merging this batch would close the
  issue too early. Reword the line so it has no closing keyword, for example "#12792 (S8; it will
  resolve #12667 once all batches land)". Then `Refs: #12667` is the only link, and this PR can be
  judged as the partial delivery it says it is.
- Spec #12667 Outcome: as a whole delivery, the diff gives the shared copies to adhd and ai-briefing
  only. It does not add them to the other view-lane plugins (architecture, debugging,
  codebase-health, discovery, education, overengineering, prototype). Fixing the closing keyword
  above is enough, because the body already names the remaining batches.
- lib/rendered-view.md: added as a canonical source, but scripts/shared-copies.txt registers no copy
  of it, so planning, architecture, discovery and debugging do not yet share one synced reference.
  The body leaves this to the next batch, which is fine once the PR is judged as a Refs partial
  delivery.
- lib/view-registry.mjs:49: registers 13 new lanes; this is additive. The re-synced copies in
  harness-ops, planning, review, source-control, visualization and work-items match the canonical
  source (blob 5b33fb87a), and the new adhd and ai-briefing copies match their canonical sources. I
  found no correctness or safety issue in the copied code.

This pull request leaves the merge queue, and ci-status holds it out while it carries pr-verify-failed. A pr-verify pass at a new commit removes the label.

@github-actions github-actions Bot added the pr-verify-failed pr-verify failed this head; a pass at a new commit removes it. label Nov 3, 2026
kyle-sexton and others added 2 commits November 3, 2026 14:50
…nical one

Reads each `lib/view-settings.mjs <copy>` line from scripts/shared-copies.txt
and runs that copy's resolve CLI, so a carrier whose copy cannot import its
sibling modules fails the suite. Covers each carrier as its batch registers it.

Refs: #12667

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MWmASPrNZkCGEUBcZsKuvx
@github-actions github-actions Bot removed the pr-verify-failed pr-verify failed this head; a pass at a new commit removes it. label Nov 3, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 3, 2026
Merged via the queue into main with commit 28509a0 Nov 3, 2026
82 checks passed
@kyle-sexton
kyle-sexton deleted the feat/12667-shared-copies-a branch November 3, 2026 20:25
kyle-sexton added a commit that referenced this pull request Nov 3, 2026
Batches #12917 and #12937 landed the lib changes, the existing-carrier
copies, adhd, ai-briefing, architecture, debugging and every
rendered-view copy. This branch now carries codebase-health, discovery,
education, overengineering and prototype.

- scripts/shared-copies.txt: main's file plus only the 23 lines for the
  remaining carriers (the auto-merge had registered 24 copies twice).
- .changes/planning fragment removed: the release on main shipped it.
- .changes/discovery fragment trimmed to the lib copies; batch B shipped
  the rendered-view part.
- lib/view-settings.test.mjs: drop the hardcoded 9-carrier import test.
  The registered-copy test from #12917 runs every view-settings copy in
  scripts/shared-copies.txt, these nine included.

Refs: #12667

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MWmASPrNZkCGEUBcZsKuvx
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.

1 participant