Skip to content

refactor(architecture): resolve the view medium through the shared resolver - #12991

Closed
melodic-automation-lanes[bot] wants to merge 3 commits into
mainfrom
work-resolve-issue/issue-12672-41723143585-1
Closed

melodic-automation-lanes[bot] wants to merge 3 commits into
mainfrom
work-resolve-issue/issue-12672-41723143585-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #12672

Summary

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

The nine architecture map-* skills and the improve deepening report now resolve their medium through the shared resolver (lib/view-settings.mjs resolve --lane architecture --option ${user_config.medium}) and print its report line. They no longer read the retired ~/.claude/rendered-views.md or .claude/rendered-views*.md files, and no skill or eval text lists medium values. Design choice, with its basis: the resolver CLI requires --option <user_config.medium> (view-settings.mjs:268), and retiredNotices points users at 'the <plugin> plugin's medium option' (view-settings.mjs:158), so the architecture plugin gains a medium userConfig option that mirrors planning's. What each medium does to the deepening report now lives in the shared reference (lib/rendered-view.md, 'The deepening report'), so the skill text names no values. ${user_config.medium} is written in improve/SKILL.md, not actions/deepening.md, because the plugin docs say it is substituted only in skill content.

The run went past its turn budget (turns=118 max-turns=113): the work may be too big for one run.

Not verified

  • Tidy skipped: the change is 33 files, over the 10-file, 300-line and 1-area cutoffs, and it touches the shared path lib/
  • Review finding left as is: the map-* skills say 'its report line is printed' without naming notice lines. Step 3 of the shared reference they link already says to print every resolver line as it stands
  • No model evals were run (the issue says not to); the eval cases are checked only by check-evals-quality
  • CI-only checks left to the PR's CI: evals-schema, plugin-schema, lychee-offline, contract-clause-coverage, portability-lint, cant-fail-scan and the other whole-repo scanners check-affected lists as CI only
  • The demo (run architecture:improve on a sample and see the report line) was not run: this lane runs no skill session or node command. The resolver's report-line format was checked by reading reportLines in lib/view-registry.mjs
  • Untracked files not made by this run are in the checkout root (.bash_profile, .bashrc, .gitconfig, .idea, .mcp.json, .profile, .ripgreprc, .vscode, .zprofile, .zshrc), most likely from the runner environment. Leave them out of the patch
  • Earlier stale text outside this issue's scope: the planning entry in docs/contracts/rendered-views/README.md's 'no medium option' emitter list predates this change and still describes planning as reading the cascade

Fix

  • plugins/architecture/.claude-plugin/plugin.json: new medium userConfig option (default auto), mirroring planning's
  • plugins/architecture/README.md: Interactive views paragraph points at the shared resolver; new generated options-reference block for the medium option (render() format from scripts/sync-plugin-options-docs.py; plugin-options-docs check passes)
  • lib/rendered-view.md: architecture listed as a resolver lane in step 3; Resolver lane: --lane architecture in the Architecture section; new 'The deepening report' subsection (resolve with --medium as --argument, what to do on a usage exit 2, and what each medium does to the report)
  • plugins/{architecture,planning,discovery}/reference/rendered-view.md, plugins/debugging/skills/debug/reference/rendered-view.md: regenerated from lib/rendered-view.md with sync-shared-copies
  • plugins/architecture/skills/map-{landscape,containers,components,dependencies,data,events,flow,context,deployment}/SKILL.md: the view paragraph says the shared medium resolver decides, using this plugin's medium option ${user_config.medium}, and its report line is printed
  • plugins/architecture/skills/map-*/evals/evals.json (9 files): the offers-built-interactive-view case expects the shared resolver and its report line, not the rendered-views cascade with a file default
  • plugins/architecture/skills/improve/SKILL.md: Arguments line states the medium option value and links the reference's 'The deepening report' section
  • plugins/architecture/skills/improve/actions/deepening.md: hand-written cascade walk and per-value bullets replaced with a pointer to the shared resolver and the reference section
  • plugins/architecture/skills/improve/evals/evals.json: case 15 renamed plugin-option-terminal-medium-skips-html-report; the fixture sets the plugin option to terminal and keeps a retired ~/.claude/rendered-views.md; the case expects view: terminal (userConfig) plus the retired-file notice
  • docs/contracts/rendered-views/README.md: architecture:improve moved out of the 'no medium option, reads the cascade' emitter list and described as a resolver-lane emitter
  • .changes/architecture/work-resolve-issue-issue-12672-41723143585-1-6568f049.md: minor fragment
  • .changes/{planning,discovery,debugging}/work-resolve-issue-issue-12672-41723143585-1-*.md: patch fragments for the regenerated shared reference copy

Verification

The runner ran each check from the base copy against 0fd08fb:

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

The run reports:

  • read https: //code.claude.com/docs/en/plugins/manifest-reference: ${user_config.KEY} is substituted in skill and agent content; skill, command and agent content resolve ${...} anywhere in the Markdown body; userConfig options are strict objects (type, title, description, default, options)
  • Grep over plugins/architecture/skills for enumerated medium lists, 'rendered-views cascade', 'medium cascade' and ~/.claude/rendered-views: no enumerated list left; the only hits are the improve eval fixture's retired-file mention
  • check-evals-quality over all 12 plugins/architecture/**/evals.json: PASS, no FAIL (warnings are earlier ones: build-view.mjs fixture Q4, an undefined 'correctly' Q10)
  • check-changelog-fragments: all 5 fragments valid
  • check-changed-skills: 11 skills checked, 0 failed (warnings are earlier size/description notes plus 1-line body growth)
  • check-affected: 11 passed, 7 skipped, 0 failed over 33 changed files (an earlier run failed on missing planning/discovery/debugging fragments, a stale options-docs block and a 315-char option description; all three fixed)
  • sync-shared-copies: regenerated the 4 rendered-view.md copies, exit 0
  • local fresh-context review subagent: 5 findings; fixed 4 (usage exit 2 on an unknown --medium, plugin-root path for improve, stale contract list entry, notice shown once per project in the eval fixture)
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41723143585

Left out: the items under Not verified.

Related

#12672

Lane-Activity: work-resolve-issue/resolve-issue

@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-security-review: 1 inline finding(s) on 0fd08fb5dd188fde6f6a0c04a25ce5e5adbea476; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

Comment on lines +9 to +17
"userConfig": {
"medium": {
"type": "string",
"title": "Preferred delivery medium",
"description": "Where the map-* views and the improve report go. auto (default): a local file; terminal: no page; file: a local file; artifact: a private Artifact (the improve report stays a local file); hosted is read as artifact. A team rendered-views file never publishes.",
"default": "auto",
"options": ["auto", "terminal", "file", "artifact", "hosted"]
}
},

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.

manifest userConfig: flag, a new medium option is added to an existing plugin

This is not on the inert list, so this lane flags it by default. Here is what it does. Every installed user of architecture gets a new medium option on update, with "default": "auto" (line 14). Its value goes into the map-* and improve skill text as ${user_config.medium}. The skill then passes it as --option to the shared resolver, lib/view-settings.mjs resolve --lane architecture.

I read the code that consumes the value, and the default does not widen anything:

  • For this lane, auto resolves to the registry's local default. See lib/view-registry.mjs:49: lane("architecture", [...], false, "file").
  • A publishing value counts only from the argument, from userConfig or from the personal override. See lib/view-settings.mjs:200 (gateExplicit).
  • A team rendered-views file never publishes (lib/view-settings.mjs:241-247).

Under the old per-skill cascade, a tracked team file in a scanned repository could set the map-* views' medium. The resolver now ignores a publishing team value, so this narrows publishing. The value comes from the user's own config on their own machine, so no cross-trust input reaches the command. No code starts running on update: no hooks, MCP servers, monitors or dependencies are added.

severity: IMPORTANT

claude-lane: claude-security-review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

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

Security review: PR #12991 at c371e771

Scope. This is a full review. I read every hunk of all 33 changed files. Three of them (plugins/{debugging/skills/debug,discovery,planning}/reference/rendered-view.md) are byte-identical copies of plugins/architecture/reference/rendered-view.md. Each copy sits at a reference path that only skill text reads, so the path does not change what the content does. The diff at this head is the same as the one this lane reviewed at 0fd08fb5; the head is a merge of main.

What changed. The architecture map-* skills and the improve deepening report no longer walk the rendered-views cascade themselves. They now call the shared resolver, lib/view-settings.mjs resolve --lane architecture, and a new medium plugin option feeds it.

Trust-boundary check. I read the bundled resolver (plugins/architecture/lib/view-settings.mjs):

  • A team file's medium is ignored and never publishes (L272-L283).
  • For the architecture lane the default is the local file medium (view-registry.mjs L54).

The deepening.md text this PR removes had the skill check the overlay's tracked and gitignored state itself. That check now lives in the resolver (L109), so no protected rule is left unenforced. The rule that keeps another repository's files out of the HTML report stays in deepening.md unchanged. The values that reach the resolver (${user_config.medium} and --medium) come from the user's own config and request, not from repository text. The PR narrows what can publish and widens nothing. It touches no trust control, workflow, token or merge hold.

Manifest.

  • manifest userConfig: already posted, https://github.com/melodic-software/claude-code-plugins/pull/12991#discussion_r4461678614. This is the new medium option, "default": "auto", which resolves to a local file. It adds no hooks, MCP servers, monitors or dependencies. It stays IMPORTANT.
  • manifest version: pass. The version is unchanged, and no other manifest field changed.

Already posted: #12991 (comment). The lines are unchanged since it was posted, so I posted no new inline comment.

Embedded text: none in the PR context.

Spend: nothing beyond normal skill runs.

I found no new security issues.

critical-findings: 0

@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: 2 inline finding(s) on 0fd08fb5dd188fde6f6a0c04a25ce5e5adbea476; the summary is in this lane's pull request comment.

claude-lane: claude-review

"title": "Preferred delivery medium",
"description": "Where the map-* views and the improve report go. auto (default): a local file; terminal: no page; file: a local file; artifact: a private Artifact (the improve report stays a local file); hosted is read as artifact. A team rendered-views file never publishes.",
"default": "auto",
"options": ["auto", "terminal", "file", "artifact", "hosted"]

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.

manifest userConfig.medium options: flag. This adds a userConfig field that has an options list. That can raise the lowest Claude Code client version that can load the architecture plugin, but neither the PR description nor the fragment names that version. Planning already uses options, but this plugin hasn't so far, so anyone who installs only architecture now needs the newer client too.

Fix: state the version floor in the PR description, or in the .changes/architecture/... fragment, citing the user configuration reference.

claude-lane: claude-review

@@ -0,0 +1,11 @@
---
bump: minor

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.

manifest version: flag. bump: minor is lower than this change needs.

Architecture 1.4.0 (plugins/architecture/CHANGELOG.md:28) made ~/.claude/rendered-views.md and the team/overlay .claude/rendered-views*.md files a supported way to choose where the deepening report goes. This PR stops reading them.

How it breaks: a user whose ~/.claude/rendered-views.md says medium: terminal gets no HTML report today. After upgrading, they get view: file (plugin default) and a report is written. They only see a one-time notice. A setting that used to work and now silently stops is a breaking change under SemVer.

Planning shipped the same retirement in its 1.0.0, with the files listed under ### Removed (plugins/planning/CHANGELOG.md:119-122).

Fix:

  • Make this bump: major.
  • Move the "no longer read" sentence under a ### Removed heading.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

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

Code review: PR #12991 (full review at c371e771)

I read every hunk of all 33 changed files. Four of them have identical changes: the three copies of reference/rendered-view.md under planning, discovery and debugging/skills/debug match plugins/architecture/reference/rendered-view.md. Following the identical-files rule, I counted that change once, which gives files_read 30. Each copy sits on a reference-doc path, so the copies do nothing different from the original.

To check what the edited text claims, I also read lib/view-settings.mjs, plugins/architecture/lib/view-settings.mjs and plugins/architecture/lib/config-cascade.mjs. I ran nothing. The PR's verification list is the author's claim and I haven't confirmed it.

Findings

Earlier findings that are still open (the wrapper marks both lines changed since posted: no):

  • manifest userConfig.medium options: already posted, https://github.com/melodic-software/claude-code-plugins/pull/12991#discussion_r4461698076
  • manifest version: already posted, https://github.com/melodic-software/claude-code-plugins/pull/12991#discussion_r4461698086
    • Correction to that post's example: the resolver still honors a terminal or file value in a retired file through retiredLocal (view-settings.mjs#L175). So a ~/.claude/rendered-views.md that sets medium: terminal still suppresses the report. The setting that really stops working is a publishing value (artifact/hosted) in a retired file: the map-* views used to publish an Artifact for it and now fall back to a local file. The bump question still stands on that basis.

New findings (inline):

  1. plugins/architecture/skills/improve/evals/evals.json, case 15: the fixture sets medium: file in the retired user file. The resolver's notice for that case says the file "is retired but still keeps the view local", not that it "is no longer read" as the case expects.
  2. .changes/architecture/...6568f049.md:17: the changelog says the retired files "are no longer read". Their terminal/file values still decide the medium when no current scope sets one.

Manifest

  • manifest userConfig.medium default: pass: this is a new field. Its default auto resolves to the lane's file default, and all nine map-* skills plus improve/SKILL.md read it through ${user_config.medium}.
  • manifest userConfig.medium options: already posted, https://github.com/melodic-software/claude-code-plugins/pull/12991#discussion_r4461698076
  • manifest version: already posted, https://github.com/melodic-software/claude-code-plugins/pull/12991#discussion_r4461698086
  • manifest description/keywords: pass: the change adds an option, not a new capability that the description or keywords leave out.

Closing issue #12672

  • Done-when (AC4/AC7/AC15): the architecture skills' prose no longer lists medium values. The one inline value list in the reference is in the shared reference/rendered-view.md, not in the skills or evals. The author reports check-evals-quality passing over every architecture evals file; I haven't verified that. No model evals are run, as the issue asks.

Round 3 of 3, so the round cap didn't apply. I found no embedded instructions in the PR context or the prior-findings file.

Spend: none. This change adds no API, Actions or billing spend.

critical-findings: 4

Findings not posted inline (their lines are not in the diff):

  • plugins/architecture/skills/improve/evals/evals.json:220 (RIGHT): The fixture contradicts the notice this case expects. The fixture puts medium: file in ~/.claude/rendered-views.md. For that value the shipped resolver doesn't say the file "is no longer read". In plugins/architecture/lib/view-settings.mjs:217, a claude-user file whose value is local (terminal/file, see isLocal at L163) gets the status is retired but still keeps the view local.

So a correct run prints ~/.claude/rendered-views.md is retired but still keeps the view local (it sets medium: file); set it in docs/conventions/rendered-views.local.yaml instead; per user, the architecture plugin's medium option also works. A grader holding to expected_output would mark that run wrong. A model trying to match the case would also be pushed to rephrase a line the shared reference says to print as it stands.

Fix: pick one:

  • Change the fixture's retired value to a publishing one (e.g. medium: artifact). That value really "is no longer read", and the userConfig terminal still decides.
  • Keep file and change the expected notice to "is retired but still keeps the view local".
  • .changes/architecture/work-resolve-issue-issue-12672-41723143585-1-6568f049.md:17 (RIGHT): The changelog misstates what still works. "~/.claude/rendered-views.md and the .claude/rendered-views*.md files are no longer read" doesn't match the resolver. In plugins/architecture/lib/view-settings.mjs:278-279, retiredLocal still applies a terminal or file value from any of those files when no personal override, team or userConfig scope sets a local value. The decision then reads retired <path>.

A user who reads this entry and deletes their medium: terminal file to "clean up" will start getting HTML reports. A user with medium: artifact there loses Artifact publishing from the map-* views without this entry saying so.

Fix: say what changed, e.g. "A publishing value (artifact, hosted) in those files is no longer read; a terminal or file value there still keeps the view local until a current scope sets one, and the resolver prints a notice either way."

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 4, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

The pr-unblock lane stopped address-feedback on this pull request: the pull request has an unresolved review thread the lane does not take (a reviewer it does not act on, a review of an older head, or a thread it already answered), so no lane resolves it and a session builder takes it. 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/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=12991 -f activity=address-feedback

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

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

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

Lane-Activity: pr-unblock/resolve-conflicts
@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Nov 4, 2026
@kyle-sexton

Copy link
Copy Markdown
Member

Hold: the fix lane escalated this PR (needs-session), so PR Lane Automation is building an owner-login replacement from main that merges this head, resolves its conflicts and fixes its open threads. Releases when: the replacement merges, and then this PR is closed as superseded.

@melodic-automation-lanes melodic-automation-lanes Bot removed the needs-session A pr-unblock stop a session builder takes: a change the lane cannot make. Not an owner stop. label Nov 4, 2026
@kyle-sexton

Copy link
Copy Markdown
Member

Superseded by #13027, which carries the same change for #12672 (Closes #12672 stays on #13027). Closing this duplicate.

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

Labels

do-not-merge Hard merge gate: do not merge while applied.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(architecture): resolve the view medium through the shared resolver

1 participant