Skip to content

feat: add diagnose plugin#44

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

feat: add diagnose plugin#44
kyle-sexton merged 8 commits into
mainfrom
feat/plugin-diagnose

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Contributor

Migrates the diagnose skill into a repo-agnostic plugin. Six-phase debugging loop (feedback-loop -> reproduce -> hypothesise -> instrument -> fix+regression -> cleanup); Phase 1 feedback loop is a hard gate. Cross-skill refs degrade to prose; consumer conventions from consumer CLAUDE.md; hitl-loop shipped as a fill-in template. Clean-tier wave Phase 2.


Note

Low Risk
Adds documentation and skill metadata only; no hooks or executable automation in consumer repos beyond an optional user-run shell template.

Overview
Adds a new diagnose marketplace skill plugin that migrates a repo-agnostic six-phase debugging workflow into the catalog alongside bug-report.

The skill (/diagnose:diagnose) enforces Phase 1 as a hard gate: build a fast, deterministic pass/fail reproduction loop before reproduce → hypothesise → instrument → fix + regression test → cleanup. It documents graceful degradation for optional adjacent tools, consumer conventions from project CLAUDE.md, and bundled assets via ${CLAUDE_PLUGIN_ROOT}: a phase checklist template, ecosystem-debugging reference (.NET/Python/TS logging and perf notes), and a user-run HITL bash loop template for interactive repro.

Catalog wiring: plugins/diagnose/ manifest + README, and new entries in .claude-plugin/marketplace.json and the root README plugin table.

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

Migrated from medley .claude/skills/diagnose. Six-phase debug loop
(feedback-loop -> reproduce -> hypothesise -> instrument -> fix+regression
-> cleanup). Repo-agnostic: cross-skill refs degrade to prose; consumer
conventions read from consumer CLAUDE.md/.claude/rules; hitl-loop shipped
as a fill-in template.
@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_a2424582-5359-45a4-a397-dda1f6d0656f)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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


PR Review: feat/plugin-diagnose

Tasks

  • Load review tools
  • Read changed files
  • Check alignment with CLAUDE.md and plugin design rules
  • Security review
  • Final review comment

Summary

Well-structured migration. The six-phase debugging methodology is coherent, Phase 1 as a hard gate is enforced consistently across SKILL.md and README, and the graceful-degrade pattern is applied correctly throughout. The plugin is repo-agnostic, cache-isolation-safe, and carries a clean security profile (no hooks, no MCP servers, no egress). Ready to merge with the findings below addressed.


Security Review (plugin-acceptance checklist)

Surface Status
Code execution (hooks/scripts) ✅ No hooks/hooks.json. The only shell artifact is hitl-loop.template.sh; it is a fill-in template that users copy manually — not wired as a hook, never auto-executed.
Remote MCP servers ✅ None.
Consumer config (userConfig) ✅ No userConfig. README documents this correctly.
Cache isolation ✅ All internal references use ${CLAUDE_PLUGIN_ROOT}. No ../ reach-outs.
Data egress ✅ No outbound network calls in any component.
Provenance / third-party trust ✅ Author field matches submitter. No third-party SaaS wired.

Design Rules Check

Rule Status
Repo-agnostic ✅ No hardcoded paths. Consumer conventions read from consumer CLAUDE.md / .claude/rules.
Configurable without editing ✅ No configuration needed; documented explicitly in README.
Plugin-form-safe ${CLAUDE_PLUGIN_ROOT} used for all bundled-asset references.
No PII / secrets ✅ Clean.
Versioned "version": "0.1.0" in plugin.json.
Marketplace entry ✅ Correct ./-prefixed source path and category: "debugging".

Findings

1. ecosystem-debugging.md is functionally incomplete for Python and TypeScript stacks

Severity: Actionable

reference/ecosystem-debugging.md

SKILL.md Phase 1 directs consumers to this file for timing-injection ("see the timing-injection row for your stack") and Phase 4 directs them for logging, banned-output, and perf-tooling. The Python and TypeScript sections only supply a logging row. A consumer on either stack who follows the Phase 4 or Phase 1 cross-references will find nothing for the other three rows.

The .NET section shows the complete pattern. Python and TypeScript need the same coverage, or at minimum a "not yet documented" placeholder so the gap is explicit rather than silent.

Fix this →


2. Redundant disable-model-invocation: false in SKILL.md frontmatter

Severity: Minor

SKILL.md line 6

disable-model-invocation: false is the default value — explicitly setting it to its default adds noise without effect. Other skills in the repo omit it. Remove the line.


3. Specific third-party plugin reference by full ID

Severity: Informational

SKILL.md line 37

The Phase 1 pre-investigation block names andrej-karpathy-skills:karpathy-guidelines by full slash-command ID. Graceful degrade is documented inline and the prose fallback is solid, so this doesn't violate the "cross-skill refs degrade to prose" rule. However, the ID is brittle — if that plugin is renamed or the marketplace changes its slug, this becomes stale prose. Consider describing the capability ("a behavioral-guidelines skill that primes think-before-code / simplicity-first discipline") with the specific ID as an example, rather than routing model invocation through it.


4. Checklist Phase 6 skip criteria is looser than SKILL.md Phase 6

Severity: Informational

templates/checklist.md line 17

SKILL.md Phase 6 requires all [DEBUG-...] tags removed ("grep -r "[DEBUG-" returns nothing in source"). The checklist adds a skip: "instrumentation cleanup SKIPPED when the instrumentation is a desirable permanent observability addition." The tagged [DEBUG-<hex>] probes are defined as temporary by design; a permanent observability addition would be promoted to a real, untagged log statement, not left with a debug tag. The skip criteria as written could allow debug-tagged probes to survive review. Tighten: if the probe is permanent, promote it to untagged structured logging before Phase 6 — that is what "promotion" means, and cleanup still removes the tag.


hitl-loop.template.sh — clean

set -euo pipefail, quoted variables, printf -v for indirect assignment, shellcheck disable=SC2154 on RESULT is appropriate (the macro sets the variable via printf -v, which shellcheck can't follow statically). Cross-platform note is accurate. No issues.


@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 no other collaborators are available 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: ccb4018854

ℹ️ 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/diagnose/skills/diagnose/scripts/hitl-loop.template.sh Outdated
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_85072c6e-f654-4f16-94d0-ab9c1e46561d)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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


Claude finished @kyle-sexton's task


PR Review: feat/plugin-diagnose

Tasks

  • Load review tools
  • Fetch PR diff and changed files
  • Read changed files at HEAD
  • Check alignment with CLAUDE.md and plugin design rules
  • Security review (plugin-acceptance checklist)
  • Review new commits since prior review (fix(diagnose) commits)
  • Final review comment

Summary

The latest commit (2d7dde6) correctly resolves the Codex finding on hitl-loop.template.sh — the RESULT-specific footer is replaced with a generic CAPTURED_VARS loop, and printf -v / ${!var} indirect expansion handles any custom-named capture variable correctly under set -u. That fix is clean.

Three findings from the prior Claude review remain open. One is actionable (broken cross-references for Python/TS consumers); two are minor/informational and could ship as a follow-up.


Security Review

Surface Status
Code execution (hooks/scripts) hitl-loop.template.sh is a manually-copied fill-in, never auto-executed
Remote MCP servers ✅ None
Consumer config (userConfig) ✅ None, documented correctly
Cache isolation ✅ All references use ${CLAUDE_PLUGIN_ROOT}; no ../ reach-outs
Data egress ✅ None
Provenance / third-party trust ✅ Author field matches submitter

Design Rules Check

Rule Status
Repo-agnostic ✅ No hardcoded paths
Configurable without editing ✅ No config needed; documented
Plugin-form-safe ${CLAUDE_PLUGIN_ROOT} throughout
No PII / secrets ✅ Clean
Versioned "version": "0.1.0"
Marketplace entry ✅ Correct ./-prefixed source, category: "debugging"

Codex Finding — Resolved ✅

The hitl-loop.template.sh footer was changed from a hardcoded printf 'RESULT=%s\n' "$RESULT" to:

CAPTURED_VARS=()
# in capture():
CAPTURED_VARS+=("$var")
# footer:
for var in "${CAPTURED_VARS[@]}"; do
  printf '%s=%s\n' "$var" "${!var}"
done

This correctly handles any number of renamed capture variables (OBSERVED, DETAIL, etc.) under set -u. The shellcheck disable=SC2154 comment is updated to reflect the new mechanism. No issues.


Open Findings (carried from prior review)

1. ecosystem-debugging.md Python and TypeScript sections are incomplete — Actionable

reference/ecosystem-debugging.md lines 14-20

SKILL.md Phase 1 sends consumers to the timing-injection row; Phase 4 sends them to logging, banned-output, and perf-tooling. The Python and TypeScript sections each only supply a single logging row. A consumer on either stack who follows Phase 1 or the Phase 4 perf/banned-output cross-references reaches a silent gap. The .NET section demonstrates the complete pattern; Python and TypeScript need either the same coverage or explicit "not yet documented — defer to your project conventions" placeholders.

Fix this →


2. Redundant disable-model-invocation: false in SKILL.md — Minor

SKILL.md line 6

false is the default value; the line has no effect and no other skill in the repo sets it explicitly. Remove it.


3. Hard-coded third-party skill slug — Informational

SKILL.md line 37

The andrej-karpathy-skills:karpathy-guidelines full ID is referenced in Phase 1. Graceful degrade is correctly documented inline, so this doesn't break anything. Consider referencing the capability by description rather than by slug to decouple from a slug rename. Low priority.


4. Checklist Phase 6 skip criteria is looser than SKILL.md — Informational

templates/checklist.md line 17

The skip for "a desirable permanent observability addition" could leave [DEBUG-<hex>] tagged probes in place. SKILL.md defines the tag as temporary by design; a permanent addition is promoted to untagged structured logging, at which point Phase 6 cleanup has nothing left to do. Tightening: "SKIPPED when all debug-tagged probes have been promoted to untagged structured logging" avoids the ambiguity.


Verdict

Actionable item before merge: Finding #1 (ecosystem-debugging.md Python/TypeScript gap). All other findings are minor or informational and could be addressed in a follow-up.

@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 no other collaborators are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

Resolve marketplace.json and README conflicts to include both plugins.

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_5a9c5576-7794-4b12-9ef8-63cd1583f995)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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


PR Review: feat/plugin-diagnose

Tasks

  • Load review tools
  • Read changed files at HEAD
  • Check status of open findings from prior reviews
  • Review merge commit changes
  • Final review comment

Summary

The latest commit (3df0831) is a clean merge from origin/main pulling in the powershell-format plugin. It touches no diagnose plugin files. No new issues introduced.

The one actionable blocking finding from prior reviews — the incomplete Python and TypeScript sections in ecosystem-debugging.md — remains unresolved. Three minor/informational findings also remain open.


Merge Commit — Clean ✅

3df0831 Merge origin/main (powershell-format) into diagnose plugin branch.

No conflicts. No diagnose-plugin files changed. Safe to proceed.


Security Review

Surface Status
Code execution (hooks/scripts) hitl-loop.template.sh is a manually-copied fill-in; never auto-executed
Remote MCP servers ✅ None
Consumer config (userConfig) ✅ None, documented correctly
Cache isolation ✅ All references use ${CLAUDE_PLUGIN_ROOT}; no ../ reach-outs
Data egress ✅ None
Provenance / third-party trust ✅ Author field matches submitter

Design Rules

Rule Status
Repo-agnostic ✅ No hardcoded paths
Configurable without editing ✅ No config needed; documented
Plugin-form-safe ${CLAUDE_PLUGIN_ROOT} throughout
No PII / secrets ✅ Clean
Versioned "version": "0.1.0"
Marketplace entry ✅ Correct ./-prefixed source, category: "debugging"

Open Findings

1. ecosystem-debugging.md Python and TypeScript sections are incomplete — Actionable (blocking)

reference/ecosystem-debugging.md lines 14–20

SKILL.md Phase 1 routes consumers to the timing-injection row; Phase 4 routes them to logging, banned-output, and perf-tooling. The Python section has only logging; TypeScript has only logging. A consumer on either stack who follows any of the Phase 1 or Phase 4 cross-references beyond logging arrives at a silent gap. The .NET section shows the complete four-row pattern. Python and TypeScript need at minimum:

Row Python TypeScript
perf-tooling cProfile / timeit / pytest-benchmark performance.now() / console.time / vitest bench
timing-injection unittest.mock.patch('time.time') or freezegun jest.useFakeTimers() / vi.useFakeTimers()
banned-output no default — defer to project CLAUDE.md use structured logger if project configures one

Fix this →


2. Redundant disable-model-invocation: false in SKILL.md — Minor

SKILL.md line 6

false is the default; the field adds noise without effect. No other skill in the repo sets it explicitly. Safe to remove.


3. Hard-coded third-party skill slug — Informational

SKILL.md line 37

andrej-karpathy-skills:karpathy-guidelines is referenced by full ID in Phase 1. Graceful degrade is correctly documented inline so this doesn't break behavior, but the slug is brittle to a marketplace rename. Low priority — could describe the capability generically with the ID as an example.


4. Phase 6 skip criteria in checklist looser than SKILL.md — Informational

templates/checklist.md line 17

The skip "SKIPPED when the instrumentation is a desirable permanent observability addition" could allow [DEBUG-<hex>] tagged probes to survive Phase 6. SKILL.md defines the tag as temporary by design; a genuine permanent addition would be promoted to untagged structured logging first, at which point there is nothing left to clean up. Suggested tightening: "SKIPPED when all debug-tagged probes have been promoted to untagged structured logging."


Verdict

Actionable item before merge: Finding #1 (ecosystem-debugging.md Python and TypeScript gap). All other findings are minor/informational and can follow in a separate commit.

@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 no other collaborators are available 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 diagnose.

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_c262408a-6deb-4371-a52c-7b666920b804)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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


PR Review: feat/plugin-diagnose

Tasks

  • Load review tools
  • Read changed files at HEAD
  • Check status of open findings from prior reviews
  • Fix blocking issue: ecosystem-debugging.md Python/TypeScript incomplete
  • Fix minor issue: redundant disable-model-invocation: false in SKILL.md
  • Fix informational issue: checklist Phase 6 skip criteria too loose
  • Commit fixes (push blocked — 403, see note below)
  • Final review comment

Summary

The latest merge commit (8534748) pulls in the actionlint plugin from main — no diagnose plugin files changed, no new issues introduced.

The one actionable blocking finding from prior reviews (incomplete Python and TypeScript rows in ecosystem-debugging.md) remains unresolved in the current branch. I've prepared and committed a fix locally; push is blocked because this runner lacks write access to the fork. The proposed diff is below — apply it directly to resolve all open findings.


Security Review

Surface Status
Code execution (hooks/scripts) hitl-loop.template.sh is a manually-copied fill-in; never auto-executed
Remote MCP servers ✅ None
Consumer config (userConfig) ✅ None, documented correctly
Cache isolation ✅ All references use ${CLAUDE_PLUGIN_ROOT}; no ../ reach-outs
Data egress ✅ None
Provenance / third-party trust ✅ Author field matches submitter

Design Rules

Rule Status
Repo-agnostic ✅ No hardcoded paths
Configurable without editing ✅ No config needed; documented
Plugin-form-safe ${CLAUDE_PLUGIN_ROOT} throughout
No PII / secrets ✅ Clean
Versioned "version": "0.1.0"
Marketplace entry ✅ Correct ./-prefixed source, category: "debugging"

Proposed Fix (apply to unblock merge)

Three files need changes. The diff below covers all three:

plugins/diagnose/skills/diagnose/reference/ecosystem-debugging.md — add perf-tooling, timing-injection, and banned-output rows to the Python and TypeScript sections:

 ## Python (`python`)

 - **logging** — Prefix `logger.debug()` or `print()` with the `[DEBUG-<hex>]` tag.
+- **perf-tooling** — Establish a baseline with `cProfile` (whole-function profile), `timeit` (micro-bench), or `pytest-benchmark` (benchmark harness inside the test suite). Bisect against the baseline.
+- **timing-injection** — Patch `time.time` / `time.monotonic` via `unittest.mock.patch('time.time', return_value=...)` or use `freezegun` to freeze `datetime.now()`. Apply the same principle to other I/O sources — wrap them at the seam where they enter the code so the loop can swap a deterministic stand-in.
+- **banned-output** — No universal default; defer to your project's `CLAUDE.md` / `.claude/rules`. If your project requires a structured logger (e.g. `structlog`, `loguru`), route every probe through it rather than raw `print()` — raw prints bypass any telemetry pipeline and may be flagged by a linter rule.

 ## TypeScript (`typescript`)

 - **logging** — Prefix `console.log()` with the `[DEBUG-<hex>]` tag.
+- **perf-tooling** — Establish a baseline with `performance.now()` brackets, `console.time` / `console.timeEnd`, or Vitest's `bench()` API. Bisect against the baseline.
+- **timing-injection** — Use `jest.useFakeTimers()` (Jest) or `vi.useFakeTimers()` (Vitest) to control `Date`, `setTimeout`, and related APIs. For hand-rolled clocks, inject a `Clock` interface at the seam so tests swap a deterministic stand-in.
+- **banned-output** — No universal default; defer to your project's `CLAUDE.md` / `.claude/rules`. If the project configures a structured logger (e.g. `pino`, `winston`), route probes through it — raw `console.log` bypasses any transport/sink pipeline.

Fix this →


plugins/diagnose/skills/diagnose/SKILL.md — remove redundant disable-model-invocation: false (line 6):

 user-invocable: true
-disable-model-invocation: false
 ---

plugins/diagnose/skills/diagnose/templates/checklist.md — tighten Phase 6 skip criteria (line 17):

-- Phase 6 instrumentation cleanup SKIPPED when the instrumentation is a desirable permanent observability addition
+- Phase 6 instrumentation cleanup SKIPPED when all debug-tagged probes have been promoted to untagged structured logging (at which point the `[DEBUG-<hex>]` tag no longer exists to clean up)

Remaining Informational Finding

Hard-coded third-party skill slugSKILL.md line 36: andrej-karpathy-skills:karpathy-guidelines is referenced by full ID in Phase 1. Graceful degrade is correctly documented inline, so this doesn't break behavior. Consider describing the capability generically with the ID as an example to decouple from a slug rename. Low priority, no action required before merge.


Verdict

Apply the three-file diff above (or run the Fix-this links) and this PR is clear to merge. All other findings are informational.

Note: Push to feat/plugin-diagnose was blocked (HTTP 403 — runner lacks write access to the fork). The fix is committed locally in this runner but could not be pushed. Applying the diff above directly is the fastest path to unblocking.

@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 no other collaborators are available 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: 8534748baa

ℹ️ 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/diagnose/skills/diagnose/scripts/hitl-loop.template.sh Outdated
Document that the bundled hitl-loop template must be run by the user in
a TTY and stdout pasted back; the Bash tool cannot satisfy interactive
read prompts.

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_12b01630-b6e6-42f6-a5d9-5f0456675b2b)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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


PR Review: feat/plugin-diagnose

Tasks

  • Load review tools
  • Read changed files at HEAD
  • Review latest commit (adf5ba9 — fix HITL loop user-run) ✅ Clean
  • Fix blocking finding: ecosystem-debugging.md Python/TS incomplete ✅ Prepared
  • Fix minor finding: remove redundant disable-model-invocation: false ✅ Prepared
  • Fix informational finding: tighten checklist Phase 6 skip criteria ✅ Prepared
  • Commit and push fixes — ⚠️ push blocked (403, fork write access)
  • Final review comment

Summary

Latest commit adf5ba9 correctly resolves the Codex finding: the HITL loop header and SKILL.md strategy #10 now both make clear that the user runs the script in their terminal and pastes back the captured output — the Bash tool cannot satisfy interactive read prompts. That fix is clean.

All three remaining open findings are resolved in a prepared commit (ce2fc83 — can't push due to fork write restriction). The diff is reproduced below; @kyle-sexton please apply these three changes to unblock merge.


Latest Commit Review — adf5ba9

hitl-loop.template.sh header: "The human runs the script in their terminal; paste the captured stdout back to the agent" — correct.
Usage comment: "user-run — do NOT invoke via the agent Bash tool; interactive read needs a TTY" — correct.
SKILL.md strategy #10: now explicitly instructs the model to ask the user to run it and paste back --- Captured --- KEY=VALUE stdout — correct.
No issues.


Security Review

Surface Status
Code execution (hooks/scripts) hitl-loop.template.sh is a manually-copied fill-in the user runs; never auto-executed
Remote MCP servers ✅ None
Consumer config (userConfig) ✅ None, documented correctly
Cache isolation ✅ All references use ${CLAUDE_PLUGIN_ROOT}; no ../ reach-outs
Data egress ✅ None
Provenance / third-party trust ✅ Author field matches submitter

Design Rules

Rule Status
Repo-agnostic ✅ No hardcoded paths
Configurable without editing ✅ No config needed; documented
Plugin-form-safe ${CLAUDE_PLUGIN_ROOT} throughout
No PII / secrets ✅ Clean
Versioned "version": "0.1.0"
Marketplace entry ✅ Correct ./-prefixed source, category: "debugging"

Prepared Fixes (apply to unblock merge)

Push was blocked — 403, runner has no write access to the fork. The diff below covers all three open findings. Apply these changes to feat/plugin-diagnose to resolve everything:

plugins/diagnose/skills/diagnose/reference/ecosystem-debugging.md — add three missing rows to Python and TypeScript:

 ## Python (`python`)
 
 - **logging** — Prefix `logger.debug()` or `print()` with the `[DEBUG-<hex>]` tag.
+- **perf-tooling** — Establish a baseline with `cProfile` (whole-function profile), `timeit` (micro-bench), or `pytest-benchmark` (benchmark harness inside the test suite). Bisect against the baseline.
+- **timing-injection** — Patch `time.time` / `time.monotonic` via `unittest.mock.patch('time.time', return_value=...)` or use `freezegun` to freeze `datetime.now()`. Apply the same principle to other I/O sources — wrap them at the seam where they enter the code so the loop can swap a deterministic stand-in.
+- **banned-output** — No universal default; defer to your project's `CLAUDE.md` / `.claude/rules`. If your project requires a structured logger (e.g. `structlog`, `loguru`), route every probe through it rather than raw `print()` — raw prints bypass any telemetry pipeline and may be flagged by a linter rule.
 
 ## TypeScript (`typescript`)
 
 - **logging** — Prefix `console.log()` with the `[DEBUG-<hex>]` tag.
+- **perf-tooling** — Establish a baseline with `performance.now()` brackets, `console.time` / `console.timeEnd`, or Vitest's `bench()` API. Bisect against the baseline.
+- **timing-injection** — Use `jest.useFakeTimers()` (Jest) or `vi.useFakeTimers()` (Vitest) to control `Date`, `setTimeout`, and related APIs. For hand-rolled clocks, inject a `Clock` interface at the seam so tests swap a deterministic stand-in.
+- **banned-output** — No universal default; defer to your project's `CLAUDE.md` / `.claude/rules`. If the project configures a structured logger (e.g. `pino`, `winston`), route probes through it — raw `console.log` bypasses any transport/sink pipeline.

plugins/diagnose/skills/diagnose/SKILL.md — remove redundant default field (line 6):

 user-invocable: true
-disable-model-invocation: false
 ---

plugins/diagnose/skills/diagnose/templates/checklist.md — tighten Phase 6 skip criteria (line 17):

-- Phase 6 instrumentation cleanup SKIPPED when the instrumentation is a desirable permanent observability addition
+- Phase 6 instrumentation cleanup SKIPPED when all debug-tagged probes have been promoted to untagged structured logging (at which point the `[DEBUG-<hex>]` tag no longer exists to clean up)

Remaining Informational Finding (no action required before merge)

SKILL.md line 37andrej-karpathy-skills:karpathy-guidelines is referenced by full ID in Phase 1. Graceful degrade is correctly documented inline; no behavior break. Consider describing the capability generically with the ID as an example to decouple from a slug rename. Low priority.


Verdict

Apply the three-change diff above and this PR is clear to merge. The plugin methodology is sound, the security surface is clean, and all structural design rules pass.

@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 no other collaborators are available 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: adf5ba9c3e

ℹ️ 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/diagnose/skills/diagnose/reference/ecosystem-debugging.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_61d19ba2-095f-40e1-8f9d-74fc6b7a9994)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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


PR Review: feat/plugin-diagnose

Tasks

  • Load review tools
  • Read changed files at HEAD
  • Review latest commit (392a244 — FakeTimeProvider namespace fix)
  • Check status of all open findings from prior reviews
  • Final review comment

Summary

Latest commit 392a244 resolves the Codex finding: the .NET timing-injection row now correctly uses the Microsoft.Extensions.Time.Testing.FakeTimeProvider namespace and documents the NuGet package name (Microsoft.Extensions.TimeProvider.Testing) separately. That fix is accurate and clean.

The one actionable blocking finding from all prior Claude reviews — incomplete Python and TypeScript sections in ecosystem-debugging.md — is still unresolved. Three minor/informational findings also remain open.


Latest Commit Review — 392a244

Change: .NET timing-injection row corrected from Microsoft.Extensions.TimeProvider.Testing.FakeTimeProvider (wrong namespace) to Microsoft.Extensions.Time.Testing.FakeTimeProvider (correct namespace, per Microsoft Learn), with the NuGet package name Microsoft.Extensions.TimeProvider.Testing now also explicitly documented inline. The distinction matters: the wrong namespace would produce a compile error in any consumer test that copied the using directive verbatim.


Security Review

Surface Status
Code execution (hooks/scripts) hitl-loop.template.sh is a manually-copied fill-in the user runs; never auto-executed
Remote MCP servers ✅ None
Consumer config (userConfig) ✅ None, documented correctly
Cache isolation ✅ All references use ${CLAUDE_PLUGIN_ROOT}; no ../ reach-outs
Data egress ✅ None
Provenance / third-party trust ✅ Author field matches submitter

Design Rules

Rule Status
Repo-agnostic ✅ No hardcoded paths
Configurable without editing ✅ No config needed; documented
Plugin-form-safe ${CLAUDE_PLUGIN_ROOT} throughout
No PII / secrets ✅ Clean
Versioned "version": "0.1.0"
Marketplace entry ✅ Correct ./-prefixed source, category: "debugging"

Open Findings

1. ecosystem-debugging.md Python and TypeScript sections are incomplete — Actionable (blocking)

reference/ecosystem-debugging.md lines 14–20

SKILL.md Phase 1 sends consumers to the timing-injection row; Phase 4 sends them to logging, banned-output, and perf-tooling. The Python section has only logging; TypeScript has only logging. A consumer on either stack who follows any Phase 1 or Phase 4 cross-reference beyond logging hits a silent gap. The .NET section demonstrates the complete four-row pattern; Python and TypeScript need at minimum:

Row Python TypeScript
perf-tooling cProfile / timeit / pytest-benchmark performance.now() / console.time / Vitest bench
timing-injection unittest.mock.patch('time.time') or freezegun jest.useFakeTimers() / vi.useFakeTimers()
banned-output no default — defer to project CLAUDE.md use structured logger if project configures one

Fix this →


2. Redundant disable-model-invocation: false in SKILL.md — Minor

SKILL.md line 6

false is the default value; the field has no effect and no other skill in the repo sets it explicitly. Remove the line.


3. Hard-coded third-party skill slug — Informational

SKILL.md line 37

andrej-karpathy-skills:karpathy-guidelines is referenced by full ID in Phase 1. Graceful degrade is correctly documented inline, so behavior is not broken. Consider describing the capability generically with the ID as an example to decouple from a slug rename. No action required before merge.


4. Phase 6 skip criteria in checklist looser than SKILL.md — Informational

templates/checklist.md line 17

The skip "SKIPPED when the instrumentation is a desirable permanent observability addition" could leave [DEBUG-<hex>] tagged probes in place. SKILL.md defines the tag as temporary by design; a genuine permanent addition would be promoted to untagged structured logging first, leaving nothing to clean up. Suggested tightening: "SKIPPED when all debug-tagged probes have been promoted to untagged structured logging." No action required before merge.


Verdict

One actionable item before merge: Finding #1 — add perf-tooling, timing-injection, and banned-output rows to the Python and TypeScript sections in ecosystem-debugging.md. All other findings are minor or informational and can follow in a separate commit.

@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 no other collaborators are available 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_2d53d1cf-8dc9-4490-a81d-2b1d9c4219e6)

@claude

claude Bot commented Jul 10, 2026

Copy link
Copy Markdown

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


PR Review: feat/plugin-diagnose

Tasks

  • Load review tools
  • Read changed files at HEAD
  • Check status of open findings from prior reviews
  • Apply fixes for blocking and minor findings
  • Commit prepared (push blocked — 403, runner lacks write access to fork)
  • Final review comment

Summary

The latest merge commit (f0b3883) pulls in the bug-report plugin from main — no diagnose plugin files changed, no new issues introduced.

All three findings from prior reviews have been prepared and committed locally (09a202c) — push is blocked with the same 403 error all prior runs hit (runner has no write access to the fork). The diff is below; @kyle-sexton please apply these three changes to unblock merge.


Security Review

Surface Status
Code execution (hooks/scripts) hitl-loop.template.sh is a manually-copied fill-in the user runs; never auto-executed
Remote MCP servers ✅ None
Consumer config (userConfig) ✅ None, documented correctly
Cache isolation ✅ All references use ${CLAUDE_PLUGIN_ROOT}; no ../ reach-outs
Data egress ✅ None
Provenance / third-party trust ✅ Author field matches submitter

Design Rules

Rule Status
Repo-agnostic ✅ No hardcoded paths
Configurable without editing ✅ No config needed; documented
Plugin-form-safe ${CLAUDE_PLUGIN_ROOT} throughout
No PII / secrets ✅ Clean
Versioned "version": "0.1.0"
Marketplace entry ✅ Correct ./-prefixed source, category: "debugging"

Fix to apply — three files

plugins/diagnose/skills/diagnose/reference/ecosystem-debugging.md — add three missing rows to Python and TypeScript:

 ## Python (`python`)
 
 - **logging** — Prefix `logger.debug()` or `print()` with the `[DEBUG-<hex>]` tag.
+- **perf-tooling** — Establish a baseline with `cProfile` (whole-function profile), `timeit` (micro-bench), or `pytest-benchmark` (benchmark harness inside the test suite). Bisect against the baseline.
+- **timing-injection** — Patch `time.time` / `time.monotonic` via `unittest.mock.patch('time.time', return_value=...)` or use `freezegun` to freeze `datetime.now()`. Apply the same principle to other I/O sources — wrap them at the seam where they enter the code so the loop can swap a deterministic stand-in.
+- **banned-output** — No universal default; defer to your project's `CLAUDE.md` / `.claude/rules`. If your project requires a structured logger (e.g. `structlog`, `loguru`), route every probe through it rather than raw `print()` — raw prints bypass any telemetry pipeline and may be flagged by a linter rule.
 
 ## TypeScript (`typescript`)
 
 - **logging** — Prefix `console.log()` with the `[DEBUG-<hex>]` tag.
+- **perf-tooling** — Establish a baseline with `performance.now()` brackets, `console.time` / `console.timeEnd`, or Vitest's `bench()` API. Bisect against the baseline.
+- **timing-injection** — Use `jest.useFakeTimers()` (Jest) or `vi.useFakeTimers()` (Vitest) to control `Date`, `setTimeout`, and related APIs. For hand-rolled clocks, inject a `Clock` interface at the seam so tests swap a deterministic stand-in.
+- **banned-output** — No universal default; defer to your project's `CLAUDE.md` / `.claude/rules`. If the project configures a structured logger (e.g. `pino`, `winston`), route probes through it — raw `console.log` bypasses any transport/sink pipeline.

Fix this →


plugins/diagnose/skills/diagnose/SKILL.md — remove redundant default field:

 user-invocable: true
-disable-model-invocation: false
 ---

plugins/diagnose/skills/diagnose/templates/checklist.md — tighten Phase 6 skip criteria:

-- Phase 6 instrumentation cleanup SKIPPED when the instrumentation is a desirable permanent observability addition
+- Phase 6 instrumentation cleanup SKIPPED when all debug-tagged probes have been promoted to untagged structured logging (at which point the `[DEBUG-<hex>]` tag no longer exists to clean up)

Remaining Informational Finding (no action required before merge)

SKILL.md line 37andrej-karpathy-skills:karpathy-guidelines is referenced by full ID in Phase 1. Graceful degrade is correctly documented inline; no behavior break. Consider describing the capability generically with the ID as an example to decouple from a slug rename. Low priority.


Verdict

Apply the three-file diff above and this PR is clear to merge. All other findings are informational.

Note: Push was blocked (HTTP 403 — runner lacks write access to the fork). This has blocked every automated fix attempt across multiple review runs. The fix is committed locally but could not be pushed; the diff above is ready to apply directly.

@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 no other collaborators are available on this repository.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@kyle-sexton
kyle-sexton merged commit b286745 into main Jul 10, 2026
22 checks passed
@kyle-sexton
kyle-sexton deleted the feat/plugin-diagnose branch July 10, 2026 17:42

@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: f0b38834c6

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


- Original repro no longer reproduces (re-run the Phase 1 loop)
- Regression test passes (or absence of correct seam is documented as an architectural finding)
- All `[DEBUG-...]` instrumentation removed (`grep -r "\[DEBUG-` returns nothing in source)

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 Make the cleanup grep runnable

When Phase 6 runs after adding [DEBUG-*] probes, copying this required cleanup check into Bash leaves the double quote unterminated, so the command waits for continuation or syntax-errors instead of verifying that instrumentation was removed. Close the pattern and include a search root, e.g. grep -r "\[DEBUG-" ., so the skill can collect the required cleanup evidence.

Useful? React with 👍 / 👎.

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