Skip to content

feat: add improve-architecture plugin#45

Merged
kyle-sexton merged 8 commits into
mainfrom
feat/plugin-improve
Jul 10, 2026
Merged

feat: add improve-architecture plugin#45
kyle-sexton merged 8 commits into
mainfrom
feat/plugin-improve

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Migrates the improve skill into a repo-agnostic plugin (named improve-architecture — bare improve was ambiguous). Ousterhout-method architecture-improvement scanning; one skill with an internal deepening lens (progressive disclosure for future lenses). Self-contained HTML report (no remote fetch; inline SVG only). No userConfig. Clean-tier wave Phase 2.


Note

Low Risk
Adds documentation, skill playbooks, and catalog entries only; no executable hooks or changes to existing plugin behavior.

Overview
Adds improve-architecture to the marketplace and root catalog as a read-only skill that proactively scans existing code for module-level friction (default deepening lens: shallow modules, seam leaks, locality).

The skill is wired as explore → HTML report → interview → planning handoff: SKILL.md routes empty args to deepening, loads actions/deepening.md plus progressive research/deepening/*, and documents how future lenses extend via new actions/ + research/ without changing existing contracts. Phase 2 mandates a self-contained temp HTML report (inline CSS/SVG, HTML-escape repo-derived text, no remote assets) and a durable candidate markdown artifact (default ${CLAUDE_PLUGIN_DATA}, or the consumer’s work-artifact path). Phase 3 records agreed-shape for downstream planning and optional glossary/ADR side effects.

Ships plugin manifest, README, and evals/evals.json expectations for routing, shallow-cluster classification, and not over-flagging already-deep modules. No userConfig and no changes to existing plugins’ runtime behavior.

Reviewed by Cursor Bugbot for commit 10903a5. Bugbot is set up for automated code reviews on this repo. Configure here.

Migrate medley's in-repo `improve` skill into a repo-agnostic skill plugin.
Scans an existing codebase for module-level friction (shallow modules, seam
leaks, locality gaps) via Ousterhout's deep-module lens, presents candidates as
a self-contained HTML report, and interviews the selected candidate before
handoff for planning.

- One skill with an internal action-router (deepening lens; open for more lenses)
- Namespaced /improve-architecture:improve-architecture
- No userConfig; durable candidate artifact defaults to ${CLAUDE_PLUGIN_DATA},
  honoring a consumer work-artifact convention when present
- Medley rule/path couplings stripped; cross-skill refs degrade gracefully
- Ports evals as a local gate
…cripts in inline SVG

- README license: replace ../../LICENSE link (breaks under plugin cache
  isolation) with inline MIT SPDX reference
- html-report.md: forbid <script> elements inside inline SVG alongside the
  existing no-remote-fetch rules
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_2c7f8671-6efa-413d-9019-5e72ae0f3cfe)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 45s —— View job


PR Review: feat/plugin-improve

  • Gather context and read changed files
  • Review plugin.json schema and marketplace.json registration
  • Review skill definition (SKILL.md, actions, research docs)
  • Check CLAUDE.md / migration playbook alignment
  • Security review
  • Post findings

Summary

Clean addition. This is the marketplace's first skill-type plugin (all prior plugins are hooks/scripts), and the design is solid — vocabulary discipline, progressive-disclosure lens architecture, self-contained HTML output, and correct ${CLAUDE_PLUGIN_DATA} usage. One medium concern about plugin-cache path resolution for skill cross-references, one minor operational note, and a handful of nits.

Security review: PASS — no hooks, no MCP servers, no code execution surface, no data egress, no credentials. Self-contained skill only.


Issue 1 — Medium: Skill cross-references don't use ${CLAUDE_PLUGIN_ROOT}

The migration playbook's plugin-form caveats section states:

"Fix: bundle dependencies inside the plugin and reference them via ${CLAUDE_PLUGIN_ROOT}"

In actions/deepening.md, research files are referenced as bare relative paths:

Full vocabulary in [../research/deepening/vocabulary.md](../research/deepening/vocabulary.md)

And in SKILL.md the action is referenced without an absolute base:

Full process: `actions/deepening.md`

When installed, the plugin lives in ~/.claude/plugins/cache/<id>-<sha>/. If Claude uses the Read tool to load these files (rather than the skill system pre-resolving them), it needs an absolute path — and without ${CLAUDE_PLUGIN_ROOT} it would have to infer the cache location, which is fragile.

This is the first skill plugin in this marketplace, so there's no prior in-repo precedent for how the runtime resolves skill markdown cross-references. If the Claude Code skill system pre-loads referenced markdown inline (similar to how #include works), relative paths are fine. But if it instructs the model to read the file via tool, ${CLAUDE_PLUGIN_ROOT} anchoring is required.

Suggested fix — use ${CLAUDE_PLUGIN_ROOT} as the base wherever a bundled file is referenced from a skill:

Full vocabulary in `${CLAUDE_PLUGIN_ROOT}/skills/improve-architecture/research/deepening/vocabulary.md`

This is safe either way: if the skill system pre-loads the file, the variable in the text is a no-op hint; if the model reads via tool, it has the absolute path.

Fix this →


Issue 2 — Minor: xdg-open / open / start depend on Bash tool access

In actions/deepening.md Phase 2:

Open for user: `start <path>` on Windows, `open <path>` on macOS, `xdg-open <path>` on Linux.

Since this is a pure skill (not a hook script), Claude would execute these via the Bash tool. If the consumer's Claude Code settings restrict Bash access, the browser-open step silently fails — the HTML file is still written and the absolute path is reported, so this degrades gracefully. No action required, but worth noting in the README or skill description so consumers know to allow Bash if they want auto-open.


Nit: vocabulary.md self-reference uses a relative link

In dependencies.md line 1:

Uses vocabulary from [vocabulary.md](vocabulary.md)

Same relative-path concern as Issue 1 — a sibling-file reference with no ${CLAUDE_PLUGIN_ROOT} base. Minor since both files live in the same directory and the model would likely navigate by context, but it's inconsistent with the playbook's stated fix.


Verification checklist — things that look correct

  • version: "0.1.0" in plugin.json
  • source: "./plugins/improve-architecture" has the required ./ prefix per playbook step 10 ✅
  • Naming: improve-architecture is a domain-noun plugin; skill name reads as a verb-phrase; /plugin:skill doubling is idiomatic per playbook ✅
  • ${CLAUDE_PLUGIN_DATA} used correctly for the durable candidate artifact ✅
  • No userConfig — adapts via consumer CLAUDE.md/rules; no speculative knobs ✅
  • HTML report: self-contained, inline <style> only, no CDN, no remote scripts, <script> inside SVG explicitly prohibited ✅
  • Progressive disclosure: research files loaded only when their lens runs — good for token efficiency ✅
  • Evals: three well-scoped behavioral cases — default routing, positive identification, and false-positive prevention ✅
  • Graceful degradation: glossary/ADR optional; planning-skill fallback documented; non-git environments handled ✅
  • Extensibility: adding a lens is a pure ADD (open/closed) — documented pattern is clean ✅
  • Security surfaces: no hooks, no MCP servers, no code execution, no egress, no third-party trust delegation ✅
  • Authorship: author in plugin.json matches marketplace owner ✅
  • README catalog entry: one-sentence description matches plugin scope ✅

Overall

Approve with Issue 1 as the one thing worth resolving before merge — it's the question of whether relative skill cross-references survive plugin cache installation. If the Claude Code runtime pre-resolves those paths, this is clear to merge as-is; if it doesn't, the skill will silently lose its research context after install. Given this is the first skill plugin here, confirming the runtime behavior (or defensively adding ${CLAUDE_PLUGIN_ROOT}) would close the loop cleanly.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale comment

Risk: medium. Cursor Bugbot did not complete successfully (check skipped; usage limit reached), so the required automated review signal is missing. Human review is needed before merge; no reviewers were assigned because the author is the only human collaborator on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1987d06ae0

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…eedback

Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_5d0c8ca8-c956-40fa-bcc5-4f8675b7a460)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 3s —— View job


PR Review: feat/plugin-improve

  • Gather context and identify changed files
  • Read all changed files
  • Check whether prior review feedback was addressed
  • Fresh security and correctness review
  • Post findings

Status of Prior Feedback

Codex P2 (HTML escaping) — Addressed ✅

The latest commit (f85806c) added the required escaping requirement at the top of html-report.md:

Escape all codebase-derived text before writing the report. Paths, glossary terms, ADR excerpts, repo names, module labels, and any other string taken from the scanned repository must be HTML-escaped (&, <, >, ", ') before embedding in element text or attributes.

This directly addresses the injection vector Codex flagged. ✅


Previous Claude review Issue 1 (Medium) — Still Open ⚠️

The ${CLAUDE_PLUGIN_ROOT} path anchoring for skill cross-file references was not addressed. The relative paths remain unchanged across all skill markdown files:

actions/deepening.md lines 20–22, 32, 34, 66:

Full vocabulary in [../research/deepening/vocabulary.md](../research/deepening/vocabulary.md)
Classify each candidate's dependencies per [../research/deepening/dependencies.md](../research/deepening/dependencies.md)
Full scaffold and diagram patterns in [../research/deepening/html-report.md](../research/deepening/html-report.md)
See [../research/deepening/interface-design.md](../research/deepening/interface-design.md)

SKILL.md line 32 (backtick-quoted, not a link):

Full process: `actions/deepening.md`

dependencies.md line 1:

Uses vocabulary from [vocabulary.md](vocabulary.md)

Also: interface-design.md has the same pattern (not flagged in prior review):

interface-design.md lines 4, 28:

Dependencies and their category (per [dependencies.md](dependencies.md))
Include both [vocabulary.md](vocabulary.md) terms...

The risk profile remains: if the Claude Code runtime passes skill markdown text to the model without pre-resolving relative links, and the model attempts to use the Read tool to fetch the referenced research files, it cannot construct an absolute path without the plugin cache root. Since this is the first skill plugin in this marketplace, the runtime behavior is unconfirmed. Using ${CLAUDE_PLUGIN_ROOT} is the safe defensive choice regardless.

Suggested fix for actions/deepening.md:

Full vocabulary in `${CLAUDE_PLUGIN_ROOT}/skills/improve-architecture/research/deepening/vocabulary.md`

And in SKILL.md:

Full process: `${CLAUDE_PLUGIN_ROOT}/skills/improve-architecture/actions/deepening.md`

Fix this →


Fresh Review Findings

Everything verified in the previous review holds. No regressions introduced by the merge commit. A few additional observations:

SKILL.md pre-computed context — The ! command expansions at skill load time are well-guarded with 2>/dev/null || echo "..." fallbacks for non-git environments. ✅

plugin.json field coverage — Consistent with other plugins in this marketplace (markdown-formatter, etc.): $schema, name, version, description, author, keywords — no license field, which matches the rest of the repo's convention. ✅

Eval coverage — Three cases cover the essential behavioral contract: empty-args routing, strong positive classification, and false-positive prevention on an already-deep module. ✅

deepening.md Phase 3 glossary side-effects — The inline-update-on-crystallization pattern (lines 61–66) is idiomatic and the graceful-degradation note ("add the term immediately rather than batching it to the end") is good operational discipline. ✅

SVG script prohibitionhtml-report.md line 3 explicitly forbids <script> elements inside SVG. ✅

Security surface — No hooks, no MCP servers, no code execution path beyond the optional open/xdg-open/start browser-open step (which degrades gracefully if Bash is restricted). No egress, no third-party trust, no secrets. ✅


Verdict

One item remains before merge. The ${CLAUDE_PLUGIN_ROOT} anchoring for skill cross-file references (prior Issue 1, now extended to interface-design.md) is the only open concern. The Codex P2 HTML escaping feedback was addressed cleanly. All other design, security, and schema correctness points pass.

If the author has confirmed via testing or documentation that the Claude Code runtime pre-resolves relative links in skill markdown before passing them to the model, that would clear this — a comment here explaining the confirmed behavior would close the loop. Otherwise, the defensive ${CLAUDE_PLUGIN_ROOT} fix is the right call.
| Branch: feat/plugin-improve

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale comment

Risk: medium. Cursor Bugbot did not complete successfully (check skipped; usage limit reached), so the required automated review signal is missing. Human review is needed before merge; no reviewers were assigned because the author is the only human contributor on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f85806c003

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/improve-architecture/skills/improve-architecture/actions/deepening.md Outdated
Include powershell-format in marketplace.json and README. Use mktemp for
secure, unpredictable HTML report paths on shared hosts.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_5cc560a0-ca6e-4a2e-90a6-ea86a4d3bc90)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

PR Review: feat/plugin-improve

  • Gather context and read changed files
  • Fetch current plugin docs (fresh-docs mandate)
  • Check prior review feedback status
  • Review all changed files for correctness, security, and CLAUDE.md alignment
  • Post findings

Status of Prior Feedback

Codex P2 (HTML escaping) — Addressed ✅

html-report.md now opens with an explicit escaping requirement covering &, <, >, ", ' for all codebase-derived text before embedding in element text or attributes.

Codex P2 (temp file security) — Addressed ✅

deepening.md Phase 2 now specifies mktemp (with examples for both GNU and BSD variants) and user-scoped temp on Windows, replacing the prior predictable path. The security surface is closed.

Prior Claude review — Medium issue on ${CLAUDE_PLUGIN_ROOT} — Closed ✅ (Not a real concern)

This was raised twice by previous reviews, but fresh docs confirm it was never a real issue. The official Skills documentation explicitly shows supporting-file references using relative paths:

## Additional resources
- For complete API details, see [reference.md](reference.md)
- For usage examples, see [examples.md](examples.md)

This is exactly the pattern the PR uses. More importantly, ${CLAUDE_PLUGIN_ROOT} is not a documented variable for skill markdown content at all — it is only available in hooks, MCP server configs, and monitor commands. The documented skill path variable is ${CLAUDE_SKILL_DIR} (available for bash injection, not markdown links). Applying ${CLAUDE_PLUGIN_ROOT} to skill markdown would render it as a literal string, not a resolved path — the suggested fix from prior reviews would have been wrong. The PR's relative-path approach is correct.


Fresh Review Findings

Informational: ${CLAUDE_SKILL_DIR} is the right variable for future bash injection

If a future lens adds bash injection (!`command`) that needs to reference a bundled file from the skill directory, the correct variable is ${CLAUDE_SKILL_DIR} — documented as "the directory containing the skill's SKILL.md file; for plugin skills, this is the skill's subdirectory within the plugin." The current skill's bash injection in the pre-computed context block (!git ...``) doesn't reference plugin files, so this is informational only for future lens authors.

plugin.json — No source field required in the manifest itself ✅

The source field belongs in marketplace.json (which correctly has "source": "./plugins/improve-architecture"). The plugin.json manifest itself correctly omits it.

Skill frontmatter is complete and correct ✅

user-invocable: true and disable-model-invocation: false are correctly set — both Claude and the user can invoke this, which is appropriate for a scan skill with no destructive side effects.

Progressive disclosure is correctly structured ✅

Research files (vocabulary.md, dependencies.md, html-report.md, interface-design.md) are referenced from SKILL.md / actions only when their lens runs. This costs zero tokens until the relevant action is invoked.

Evals coverage is well-scoped ✅

Three cases cover the behavioral contract: default routing (eval 1), positive-case Strong classification (eval 2), and false-positive prevention on a genuinely deep module (eval 3). The expectations are specific enough to be meaningful as regression guards.

mktemp guidance is cross-platform ✅

mktemp --tmpdir deepening-review-XXXXXX.html covers Linux; mktemp -t deepening-review.XXXXXX.html covers macOS BSD mktemp; Windows gets %LOCALAPPDATA%\Temp. File permissions are restrictive by default via mktemp (0600). This is solid.

Security surface remains clean ✅

No hooks, no MCP servers, no code execution beyond the optional open/xdg-open/start browser-open (which degrades gracefully if Bash is restricted). No egress, no third-party trust delegation, no credentials.


Verification Checklist

Item Status
version: "0.1.0" in plugin.json
source: "./plugins/improve-architecture" has ./ prefix
Marketplace category: "architecture"
${CLAUDE_PLUGIN_DATA} for durable candidate artifact
No userConfig — adapts via consumer CLAUDE.md
HTML report: inline <style> only, no CDN, no remote scripts, no <script> in SVG
All codebase-derived text HTML-escaped before write
Temp file created via mktemp (unpredictable path, 0600 perms)
Relative paths for skill supporting files (documented pattern)
Graceful degradation: glossary/ADR optional, non-git environments handled
Evals cover routing, positive ID, false-positive prevention
Extensibility: new lens is pure ADD (open/closed principle documented)
README catalog row description matches plugin scope
Author in plugin.json matches marketplace owner

Verdict

Ready to merge. All prior feedback has been addressed. The outstanding "medium" concern from two previous reviews is resolved by current documentation — the relative-path approach is the documented and correct pattern; ${CLAUDE_PLUGIN_ROOT} is not available in skill markdown content. No open issues remain.

View job run | Branch: feat/plugin-improve

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale comment

Risk: medium. Cursor Bugbot did not complete successfully (check skipped), so the required automated review signal is missing. Human review is needed before merge; no reviewers were assigned because the author is the only human contributor on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

Resolve marketplace.json and README to include actionlint alongside
powershell-format and improve-architecture.

Co-authored-by: Cursor <cursoragent@cursor.com>
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_fc56bbb8-c842-402e-a060-b486e7991150)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 59s —— View job


PR Review: feat/plugin-improve

  • Gather context and read changed files
  • Fetch current plugin docs (fresh-docs mandate)
  • Check prior review feedback status
  • Review all changed files for correctness, security, and CLAUDE.md alignment
  • Post findings

What triggered this review

The latest push (3299420) is a merge-from-main that incorporated the actionlint plugin. The merge commit touched only .claude-plugin/marketplace.json, README.md, and the new plugins/actionlint/ files. No improve-architecture plugin files were modified. The scope of this pass is therefore: confirm the merge introduced no regressions in the files that matter to this PR, and independently verify the current state of all improve-architecture files.


Status of prior feedback

Issue Status
HTML injection — escape codebase-derived text (Codex P2) Addressed ✅html-report.md opens with a hard escaping rule for &, <, >, ", ' before embedding in element text or attributes
Predictable temp file path (Codex P2) Addressed ✅deepening.md specifies mktemp --tmpdir deepening-review-XXXXXX.html (GNU) and mktemp -t deepening-review.XXXXXX.html (BSD macOS), plus user-scoped %LOCALAPPDATA%\Temp on Windows
Skill cross-reference ${CLAUDE_PLUGIN_ROOT} anchoring (prior Claude — medium) Correctly dismissed ✅ — the official Skills reference documents relative paths as the correct pattern for skill supporting files. ${CLAUDE_PLUGIN_ROOT} is only defined for hooks, MCP server configs, and monitor commands. Applying it to skill markdown would render it as a literal string, not a resolved path. The PR's relative-path approach is correct.

Fresh review of the current state

plugin.json

Schema, name, version, description, author, keywords all present. Consistent with other plugins in this repo (no license field — idiomatic for this marketplace). name: "improve-architecture" is correctly kebab-case. ✅

marketplace.json

Entry includes source: "./plugins/improve-architecture" with the required ./ prefix, category: "architecture", and matching tags. The merge commit correctly resolved this file to include both actionlint and improve-architecture entries. ✅

SKILL.md

Frontmatter is complete: name, description, argument-hint, user-invocable: true, disable-model-invocation: false. Pre-computed context bash injections all carry 2>/dev/null || echo "..." fallbacks — handles non-git environments cleanly. Boundary ("does not") section clearly separates this skill from planning, enforcement, and review tools. Lens extensibility ("Adding a lens") is a pure ADD — open/closed principle documented correctly. ✅

actions/deepening.md

Three phases with hard gates. mktemp on both GNU and macOS BSD variants, Windows user-scoped temp — unpredictable path, 0600 permissions by default. ${CLAUDE_PLUGIN_DATA} used correctly for the durable candidate artifact. Phase 3 glossary side-effects (add-term-immediately pattern) are good operational discipline. ✅

research/deepening/html-report.md

Opens with escaping requirement. Scaffold uses inline <style> only — no CDN, no remote runtime. <script> inside SVG explicitly prohibited at line 3. All diagram patterns are inline SVG or hand-built HTML/CSS — no Mermaid or other remote renderer. ✅

research/deepening/vocabulary.md, dependencies.md, interface-design.md

Vocabulary: source-attributed, "Rejected framings" section prevents common misapplications. Dependencies: four categories with clear testing strategies and seam-discipline rules. Interface design: "Design It Twice" with parallel subagents, well-scoped prompts per agent. All cross-references are relative paths — the documented and correct pattern for skill supporting files. ✅

evals/evals.json

Three cases cover the behavioral contract: empty-args routing, strong positive classification (shallow wrapper chain), and false-positive prevention (already-deep module). Expectations are specific enough to serve as regression guards. skill_name: "improve-architecture" matches the skill's frontmatter name. ✅


Merge regression check

The merge commit (3299420) only adds the actionlint plugin and resolves marketplace.json / README.md to include both new plugins. No improve-architecture files were modified. No regressions. ✅


Security review

No hooks, no MCP servers, no code execution surface beyond the optional open/xdg-open/start browser-open step (which degrades gracefully if Bash is restricted). No data egress, no third-party trust delegation, no credentials. The one realistic attack vector — codebase-derived text embedded in the HTML report — is addressed by the explicit escaping requirement in html-report.md. PASS


Verdict

Ready to merge. All prior feedback is addressed. The merge commit introduced no regressions. Fresh review of all improve-architecture files confirms correctness, security, and alignment with the CLAUDE.md playbook. No open issues.
| Branch: feat/plugin-improve

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale comment

Risk: medium. Cursor Bugbot did not complete successfully (check skipped), so the required automated review signal is missing. Human review is needed before merge; no reviewers were assigned because the author is the only human contributor on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3299420887

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/improve-architecture/skills/improve-architecture/SKILL.md Outdated
Set disable-model-invocation so broad prompts cannot auto-trigger HTML
report generation, glossary edits, or other side-effecting phases.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_83426a3a-62e9-40d3-99c3-06201a059180)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 56s —— View job


PR Review: feat/plugin-improve

  • Gather context and identify latest commit
  • Fetch current plugin docs (fresh-docs mandate)
  • Read changed files
  • Check status of all prior review feedback
  • Fresh correctness, security, and CLAUDE.md alignment review
  • Post findings

What triggered this review

The latest commit d7f6218 is a single-line change:

-disable-model-invocation: false
+disable-model-invocation: true

This directly addresses the Codex P2 from the prior review pass.


Status of all prior feedback

Issue Status
Codex P2 — HTML injection (escape codebase-derived text) Addressed ✅html-report.md line 5 opens with a hard rule to HTML-escape &, <, >, ", ' before embedding in element text or attributes
Codex P2 — predictable temp path (use mktemp) Addressed ✅deepening.md Phase 2 specifies mktemp --tmpdir deepening-review-XXXXXX.html (GNU), mktemp -t deepening-review.XXXXXX.html (BSD/macOS), and %LOCALAPPDATA%\Temp (Windows)
Codex P2 — disable-model-invocation (prevent auto-invocation of side-effecting skill) Addressed ✅ (this commit)disable-model-invocation: true is now set; confirmed against docs: this prevents Claude from loading the skill in response to broad prompts, requiring explicit /improve-architecture:improve-architecture invocation
Prior Claude reviews — ${CLAUDE_PLUGIN_ROOT} in skill markdown Correctly dismissed ✅ — fresh docs confirm ${CLAUDE_PLUGIN_ROOT} is not a documented variable for skill markdown; the documented variables are ${CLAUDE_SKILL_DIR}, ${CLAUDE_PROJECT_DIR}, ${CLAUDE_SESSION_ID}, etc. Relative paths for supporting files ([vocabulary.md](vocabulary.md), [../research/deepening/vocabulary.md](../research/deepening/vocabulary.md)) are the documented pattern. Prior reviews' suggested fix would have rendered as a literal string, not a resolved path.

Fresh correctness and security review

SKILL.md frontmatter — All required fields present: name, description, argument-hint, user-invocable: true, disable-model-invocation: true. Setting both user-invocable: true and disable-model-invocation: true is the correct combination for a side-effecting skill that should be user-initiated only. ✅

Pre-computed context bash injections — All carry 2>/dev/null || echo "..." fallbacks for non-git environments. ✅

deepening.md Phase 2 temp filemktemp produces an unpredictable path with 0600 permissions by default. Cross-platform coverage is complete. ✅

html-report.md — Self-contained: inline <style> only, no CDN, no remote runtime, no remote fetch, no <script> inside SVG. Escaping requirement is the first content element. ✅

${CLAUDE_PLUGIN_DATA} usage — Correctly used for the durable candidate artifact in deepening.md Phase 2. ✅

Evals — Three cases in evals.json: empty-args routing → deepening, strong positive classification (shallow wrapper chain), false-positive prevention (already-deep module). skill_name: "improve-architecture" matches frontmatter. ✅

Security surface — No hooks, no MCP servers, no code execution beyond optional open/xdg-open/start browser-open (graceful degradation documented). No data egress, no third-party trust delegation, no credentials. PASS

CLAUDE.md alignment — Repo-agnostic (no hardcoded paths), no userConfig (adapts via consumer CLAUDE.md/rules), plugin-cache-safe file references, ${CLAUDE_PLUGIN_DATA} for durable artifacts, explicit semver version: "0.1.0", security review criteria met. ✅


Verdict

Ready to merge. All Codex P2 findings and all prior Claude review issues are now addressed. The single-line change in this commit (disable-model-invocation: falsetrue) closes the last open concern cleanly and is correctly motivated — broad prompts cannot now auto-trigger HTML report generation, glossary edits, or other side-effecting phases without explicit user invocation.
| Branch: feat/plugin-improve

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale comment

Risk: medium. Cursor Bugbot did not complete successfully (check skipped), so the required automated review signal is missing. Human review is needed before merge; no reviewers were assigned because the author is the only human contributor on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d7f6218d6a

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/improve-architecture/skills/improve-architecture/actions/deepening.md Outdated
Co-authored-by: Cursor <cursoragent@cursor.com>
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_e5237b9d-a565-4f6c-a4c2-8d1eb27099c6)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 49s —— View job


PR Review: feat/plugin-improve

  • Gather context and identify latest commit
  • Read changed files (latest commit)
  • Fetch plugin docs (fresh-docs mandate)
  • Check status of all prior review feedback
  • Fresh correctness and security review
  • Post findings

What triggered this review

The latest commit dae2f83 is a single-line change in actions/deepening.md:

-- deepening: <one sentence — the proposed deep interface>
+- deepening: <one sentence — the shallow-module friction signal, not an interface proposal>

This directly addresses the last open Codex P2 finding.


Status of all prior feedback

Issue Status
Codex P2 — HTML injection (escape codebase-derived text) Addressed ✅html-report.md line 5 requires HTML-escaping &, <, >, ", ' before embedding in element text or attributes
Codex P2 — predictable temp path (use mktemp) Addressed ✅deepening.md Phase 2 specifies mktemp --tmpdir deepening-review-XXXXXX.html (GNU), mktemp -t deepening-review.XXXXXX.html (BSD/macOS), and %LOCALAPPDATA%\Temp (Windows)
Codex P2 — disable-model-invocation (prevent auto-invocation of side-effecting skill) Addressed ✅disable-model-invocation: true set in d7f6218; confirmed by fresh docs: prevents Claude from loading the skill automatically, requires explicit /improve-architecture:improve-architecture invocation
Codex P2 — interface proposals in Phase 2 (field description conflicted with Phase 2 gate) Addressed ✅ (this commit)deepening field now describes "the shallow-module friction signal, not an interface proposal," consistent with the Phase 2 gate Do NOT propose interfaces yet and correctly scoped to discovery rather than design
Prior Claude reviews — ${CLAUDE_PLUGIN_ROOT} in skill markdown Correctly dismissed ✅ — fresh docs confirm ${CLAUDE_PLUGIN_ROOT} is not available in skill markdown content; ${CLAUDE_SKILL_DIR} is the documented variable for that context, and relative paths for supporting files are the documented pattern

Assessment of the fix

The change is precise and well-targeted. The deepening field's prior description ("proposed deep interface") introduced a structural contradiction: Phase 2 is the discovery phase that explicitly gates on Do NOT propose interfaces yet, while interface design belongs in Phase 3 via agreed-shape. The new description ("shallow-module friction signal, not an interface proposal") is:

  • Semantically correct — it captures what the field should hold at write-time (the observed friction, not the prescribed remedy)
  • Consistent with the Phase 2 gateDo NOT propose interfaces yet on line 55 and the field description now say the same thing
  • Consistent with the artifact's role — the durable candidate file is a Phase 2 discovery handoff; agreed-shape (Phase 3) is where the design lives
  • Non-breaking — the field name is unchanged; only the description/hint changes

No collateral changes required.


Verification checklist — current state

Item Status
version: "0.1.0" in plugin.json
source: "./plugins/improve-architecture" has ./ prefix
category: "architecture" in marketplace.json
disable-model-invocation: true (side-effecting, user-initiated only)
${CLAUDE_PLUGIN_DATA} for durable candidate artifact
No userConfig — adapts via consumer CLAUDE.md/rules
HTML: self-contained, inline <style> only, no CDN, no remote runtime
All codebase-derived text HTML-escaped before write
Temp file via mktemp (unpredictable path, 0600 permissions)
Phase 2 artifact: discovery only, no interface proposals (this commit)
Phase 3 agreed-shape: where interface design lives
Relative paths for skill supporting files (documented pattern)
Evals cover routing, positive ID, false-positive prevention
Extensibility: new lens is pure ADD (open/closed)
Security surface: no hooks, no MCP, no egress, no third-party trust

Verdict

Ready to merge. All Codex P2 findings across all review passes have been addressed. The latest commit closes the last open concern cleanly — the deepening field description now correctly scopes Phase 2 artifact content to discovery signals rather than interface proposals. No open issues remain.
| Branch: feat/plugin-improve

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale comment

Risk: medium. Cursor Bugbot did not complete successfully (check skipped; usage limit reached), so the required automated review signal is missing. Human review is needed before merge; no reviewers were assigned because the author is the only human contributor on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

# Conflicts:
#	.claude-plugin/marketplace.json
#	README.md
@cursor

cursor Bot commented Jul 10, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit.

A user or team admin can review and increase usage limits in the Cursor dashboard.

(requestId: serverGenReqId_9b145612-455d-45ec-aca4-bc122ad7bf6d)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 27s —— View job


PR Review: feat/plugin-improve

  • Check what changed in the latest merge commit
  • Verify improve-architecture plugin files are unaffected
  • Confirm all prior feedback is still addressed
  • Post updated verdict

What triggered this review

The latest commit 10903a5 is a merge from origin/main that brought in three new plugins (guardrails, bug-report, diagnose) plus various CI/tooling updates. Conflicts were resolved in .claude-plugin/marketplace.json and README.md only. No improve-architecture plugin files were modified by this merge commit.


Merge regression check

The conflict resolution in marketplace.json correctly preserves the improve-architecture entry alongside the new plugins from main:

{ "name": "improve-architecture", "source": "./plugins/improve-architecture",
  "category": "architecture",
  "tags": ["architecture", "refactoring", "deep-modules", "ousterhout", "module-design", "testability", "skill"] }

source: "./plugins/improve-architecture" retains the required ./ prefix. ✅


Status of all prior feedback

Issue Status
Codex P2 — HTML injection (escape codebase-derived text) Addressed ✅html-report.md opens with a hard rule requiring HTML-escape of &, <, >, ", ' for all codebase-derived text
Codex P2 — predictable temp path (use mktemp) Addressed ✅deepening.md Phase 2 specifies mktemp --tmpdir deepening-review-XXXXXX.html (GNU), mktemp -t deepening-review.XXXXXX.html (BSD/macOS), and %LOCALAPPDATA%\Temp (Windows)
Codex P2 — disable-model-invocation (prevent auto-invocation) Addressed ✅SKILL.md line 6 has disable-model-invocation: true; confirmed intact after merge
Codex P2 — interface proposals in Phase 2 Addressed ✅deepening.md line 46 reads the shallow-module friction signal, not an interface proposal; intact after merge
Prior Claude reviews — ${CLAUDE_PLUGIN_ROOT} in skill markdown Correctly dismissed ✅ — relative paths are the documented pattern for skill supporting files; ${CLAUDE_PLUGIN_ROOT} is not a documented variable in skill markdown context

Verification checklist — current state

Item Status
version: "0.1.0" in plugin.json
source: "./plugins/improve-architecture" with ./ prefix in marketplace.json
category: "architecture" in marketplace entry
disable-model-invocation: true (side-effecting, user-initiated only)
${CLAUDE_PLUGIN_DATA} for durable candidate artifact
No userConfig — adapts via consumer CLAUDE.md/rules
HTML: self-contained, inline <style> only, no CDN, no remote assets
All codebase-derived text HTML-escaped before write
Temp file via mktemp (unpredictable path, 0600 permissions)
Phase 2 artifact: discovery only, no interface proposals
Relative paths for skill supporting files (documented pattern)
Evals cover routing, positive ID, false-positive prevention
Extensibility: new lens is pure ADD (open/closed principle)
Security surface: no hooks, no MCP, no egress, no third-party trust
Merge conflict resolution preserves improve-architecture entry correctly

Verdict

Ready to merge. The latest merge from origin/main introduced no regressions. All prior Codex and Claude review findings are addressed. The improve-architecture plugin files are unchanged by the merge; only the marketplace conflict resolution touched this PR's files, and that resolution is correct.
| Branch: feat/plugin-improve

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Risk: medium. Cursor Bugbot did not complete successfully (check skipped; usage limit reached), so the required automated review signal is missing. Human review is needed before merge; no reviewers were assigned because the author is the only human contributor on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton merged commit 30cf034 into main Jul 10, 2026
22 checks passed
@kyle-sexton
kyle-sexton deleted the feat/plugin-improve branch July 10, 2026 17:48

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 10903a5634

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

## <candidate title>

- status: proposed | selected | agreed-shape | rejected
- files: <comma-separated paths>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Quote paths in the durable handoff

When the scanned repo contains a path with a comma or newline (valid Git filenames), this machine-readable handoff can be parsed as extra paths or even extra Markdown fields/candidates by the later planning step, because paths are emitted as one raw comma-separated scalar. The HTML report now has escaping rules, but this durable artifact is the one subsequent automation consumes; require a structured representation such as a JSON/YAML array, or explicit escaping/quoting of every codebase-derived scalar before writing it.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 10903a5634

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


Use the project's domain glossary vocabulary for the domain, and [../research/deepening/vocabulary.md](../research/deepening/vocabulary.md) vocabulary for architecture.

**Durable candidate artifact.** Alongside the HTML (the human-readable companion, ephemeral in the temp dir), write a machine-readable candidate list that survives the session. Default location: `${CLAUDE_PLUGIN_DATA}/deepening-candidates-<timestamp>.md`. If the consuming project maintains its own per-task work-artifact convention (e.g. a tracked slice/working directory documented in its `CLAUDE.md` or rules), honor that instead and write the file there. Tell the user the path. This file — not the HTML — is the durable handoff a planning step consumes. One entry per candidate:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep candidate artifacts scoped to the project

When this plugin is used in more than one repository, this default writes every repo’s architecture candidate list into the same plugin-level data directory; the Claude Code reference says ${CLAUDE_PLUGIN_DATA} resolves to ~/.claude/plugins/data/{id}/, so it is keyed by plugin rather than by project (https://code.claude.com/docs/en/plugins-reference#environment-variables). These artifacts contain repo paths and design notes, so a later planning handoff or cleanup in another project can see or stale-use unrelated candidates; put them under a ${CLAUDE_PROJECT_DIR}-derived slug, or require a project-local convention before writing.

Useful? React with 👍 / 👎.

kyle-sexton added a commit that referenced this pull request Jul 14, 2026
## What

A new `comment-residue` skill in the **code-tidying** plugin: a
read-only classifier that flags **out-of-context residue** in code
comments and presents Tier 1/2 findings for author-applied deletion.
Mirrors `declutter`'s architecture (`detect.sh` + sourceable shape
library + self-contained `detect.test.sh`), but scans code files and
detects only on the **comment portion** of a line.

**Residue shapes:** `history-narration` ("used to… now…"),
`plan-reference` ("Task 2 replaces the old…", "in this PR"),
`conversational-antecedent` ("per your request") — all Tier 1;
`ticket-pr-residue` ("see PR #45", "JIRA-123") — Tier 2. `TODO(#issue)`
is the sanctioned back-reference and is never flagged.

Bumps code-tidying `0.2.0 → 0.3.0`; documents the skill in the manifest
+ README catalog.

## Why

Agents leak the producing conversation/plan/history into comments — text
that only makes sense inside the chat thread that wrote it. No existing
skill targets this: `declutter` is markdown-only, `compress` excludes
code comments, and `tidy`'s Beck-#15 handles generic restatement, not
the out-of-context residue class. This is the clean-up-afterward
companion to the source-side comment rule (a separate dotfiles-repo
change).

## Design notes

- **Comment-scoped detection** — residue-shaped words in identifiers or
string literals (`previously = load_cache()`, `# encode as UTF-8 /
SHA-256`) are **not** flagged; only the comment text is classified. This
keeps false positives low; anything uncertain lands in Tier 2 (review),
and the skill is read-only so the author confirms every deletion.
- **Placement** — filed under `code-tidying` (subject = code) per the
plan. It's a read-only *classifier* like `declutter` (which lives in
`docs-hygiene`), so a mode-based argument for `docs-hygiene` exists;
flagging for reviewer preference.

## Security review (plugin-acceptance)

Touches **only surface 1** — a read-only, advisory `detect.sh` (spawns
git/find/awk, never mutates, no network, no MCP server, no
`userConfig`/secrets, reads only files under the invoking repo via
`${CLAUDE_PLUGIN_ROOT}` / `git rev-parse`). No surface 2/5/6 trust
decision required.

## Test plan

- [x] `detect.test.sh` — 19/19 (shapes, tiers, opt-out, markdown-skip,
dir + paths-file, and the false-positive guards: code identifier,
`UTF-8`/`SHA-256`, sanctioned `TODO(#123)`)
- [x] shellcheck clean
- [x] shfmt clean (editorconfig-aware)
- [x] markdownlint-cli2 clean (repo config)
- [x] `claude plugin validate --strict .` passed

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Read-only local bash/git scanning with no mutations, secrets, or
network; CI change is a targeted hygiene exclude for test/detector
content.
> 
> **Overview**
> Adds **`/code-tidying:comment-residue`**, a read-only skill that
audits **code** comments for four residue shapes (history narration,
plan/session refs, conversational antecedents, ticket/PR back-refs) and
reports **Tier 1/2** findings with treatment guidance—authors apply
deletions; no edits from the skill.
> 
> Implementation follows the `declutter`-style split: **`detect.sh`**
(targets, git default, deterministic output),
**`lib/comment-shapes.sh`** (comment-only extraction with string-aware
leaders, shape regexes, sanctioned `TODO(#issue)`,
`comment-residue-ignore`), plus **`detect.test.sh`**, eval fixtures, and
**`evals.json`**. **code-tidying** bumps to **0.3.0**; README and
manifest copy describe comment hygiene alongside `tidy` /
`batch-simplify`.
> 
> CI **comment-hygiene** now **excludes** `comment-residue` **scripts**
and **evals** so intentional TODO/PR-shaped test corpus and detector
patterns do not trip the repo ban policy.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
43f7bfa. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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