You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Project root resolves via CLAUDE_PROJECT_DIR, falling back to git rev-parse --show-toplevel from CWD (never the script's own location — plugin cache is not the project).
Persistent state moved out of the plugin/source tree to ${CLAUDE_PLUGIN_DATA}: troubleshooting registry.json, check-all output, --write observability reports. Medley's registry data does NOT ship — consumers start empty.
Bundled assets referenced via ${CLAUDE_PLUGIN_ROOT}; no ../ reach-outs; sibling-skill refs stay bare; the one cross-plugin ref (/bug-report:bug-report) is qualified with graceful degradation.
Medley-specific rules/docs/skill references (work-slice paths, /issues, /onboard, rule-file cites, repo-grep tooling, release-tracking issue markers) generalized to consumer-context seams; integrations skip silently when the consumer lacks them.
Tests rewritten self-contained (no external test lib), matching this repo's convention. DuckDB format='nd' alias replaced with canonical 'newline_delimited' (verified equivalent against live duckdb).
No userConfig: variability is covered by existing env vars (CC_OTEL_STORE, retention windows, CHECK_ALL_OUTPUT_DIR) and conventional project-relative defaults — no speculative knobs.
Gate evidence
claude plugin validate --strict plugins/claude-ops → PASS; claude plugin validate --strict . (catalog manifest) → PASS
claude plugin details token cost: always-on ~454 tok (per-component ~150–160 always-on; on-invoke ~3.3k / ~2.4k / ~1.9k)
--plugin-dir smoke test in a clean non-medley repo: /claude-ops:claude-troubleshooting status loaded under the claude-ops namespace, ran registry stats against empty state, fetched status.claude.com, and rendered the health snapshot (graceful empty-state degradation confirmed)
All 8 bundled test suites pass locally (219 cases); shellcheck --rcfile=.shellcheckrc clean; typos clean; markdownlint-cli2 clean; editorconfig-checker clean; exec bits set on shebang scripts
Hooks/MCP/LSP: none shipped. Scripts spawn only local tooling (git, jq, gh, python3, optional duckdb/otelcol-contrib/Docker); no eval, no curl | sh.
Network egress: gh reads of GitHub issues and curl reads of status.claude.com/Marginlab pages (troubleshooting quality/status), WebFetch of the official changelog — read-only; issue creation is draft-first with explicit confirmation.
No secrets/PII; no telemetry emission (this plugin READS local telemetry).
Note
Medium Risk
Large new plugin with many bash scripts that stop/restart otelcol-contrib, prune local telemetry (including prompts/API bodies when captured), and a repo-mutating changelog apply path; network reads via gh/status/changelog are read-only per the PR description.
Overview
Adds the claude-ops plugin (0.1.0) to the marketplace catalog and root README — three repo-agnostic Claude Code operations skills under /claude-ops:*.
Observability is the largest addition: skill docs plus a local OTEL stack (collector config, DuckDB cc-otel.sql, Aspire dashboard starters, JSONL/ccusage query catalogs) and retention via clean / prune-otel-store.sh (hot NDJSON trim, cold Parquet compaction, collector stop/restart with locking). Bundled bash regression tests cover jq pipelines, port probing, prune, and collector startup contracts.
Changelog integration adds /claude-ops:claude-code-changelog with gated actions (fetch/diff/status read-only; apply only on explicit intent) — explore → research → interview → implement, P1/P2/P3 rubric, git-log–based applied-version tracking.
Troubleshooting (per plugin manifest/README; not fully shown in the excerpted diff) covers GitHub bug search, health checks, and a persistent issue registry under ${CLAUDE_PLUGIN_DATA}. Medley-specific paths and shipped registry data are not included; project root resolves via CLAUDE_PROJECT_DIR / git toplevel.
Reviewed by Cursor Bugbot for commit a418c23. Bugbot is set up for automated code reviews on this repo. Configure here.
Solid architecture: plugin-form isolation is correct (${CLAUDE_PLUGIN_ROOT}, ${CLAUDE_PLUGIN_DATA}, ${CLAUDE_PROJECT_DIR} fallbacks), no ../ reach-outs, no secrets, marketplace entry looks clean. The observability prune pipeline is unusually careful (dry-check, sentinel locking, compact-before-trim, verify-before-replace). One blocker before merge, two confirmed Codex P2s, and a few smaller issues below.
Critical — Blocker
1. Python 3 SyntaxError in registry_manager.py (×3 locations)
except ValueError, TypeError: is Python 2 syntax. In Python 3 it is a SyntaxError raised at parse time, so the entire module fails to import and every invocation of /claude-ops:claude-troubleshooting that touches the registry exits with a traceback before running any logic.
Fix: except (ValueError, TypeError): at all three sites. The bundled tests are shell-only (.test.sh), so this escaped the test suite — a Python smoke test (python3 -c "import registry_manager") would catch it.
2. Registry keyed by number only, not (repo, number)
find_by_number at line 125 compares i.get("number") == number without comparing repo. The skill tracks six different repos (anthropics/claude-code, anthropics/anthropic-sdk-python, anthropics/anthropic-sdk-csharp, modelcontextprotocol/python-sdk, microsoft/mcp, …). Issue numbers are only unique within a repo, so anthropics/claude-code#1 and anthropics/anthropic-sdk-python#1 collide in every lookup:
action_add (line 332): rejects the second as a duplicate
action_update / action_remove (lines 364, 395): target whichever one appears first in the list
3. jq exit 5 accepted as success in clean.sh — can silently truncate JSONL
Lines 150–154 treat rc=5 as success with the comment "valid lines still made it to $tmp." This is jq-version-dependent. In jq 1.6 (still common on older Ubuntu/macOS setups), a malformed intermediate line causes jq to stop processing and exit non-zero — events after the malformed record never reach $tmp. Then mv -f "$tmp" "$file" overwrites the original with a truncated file, silently losing all events that followed the bad line.
The flock guard prevents concurrent malformed writes but not lines already in the file from prior aborted writers.
Safer approach: on exit 5, emit a warning and leave the original untouched (return non-zero so the skip path at line 178 fires). Pruning incomplete data is worse than pruning nothing.
On a machine where another application (monitoring agent, sidecar, app-level collector) also runs otelcol-contrib, this kills all of them. Only the plugin's own collector is restarted via cleanup. Any other collectors remain dead until manually restarted.
The collector is spawned without tracking a PID file, so there's no clean way to scope the kill. Minimum viable fix: write the spawned PID to $store_dir/collector.pid in start-collector.sh, then read it here and kill only that PID (falling back to pkill if the file is absent or the process is gone).
save_registry at line 90 uses an atomic temp+os.replace which is safe for the final write, but two concurrent add invocations could both read the same registry (both see 5 issues), both add their entry to their in-memory copy, and the last os.replace wins — one addition is silently lost. An fcntl.flock on the registry file during load-through-save would close the window.
6. No Bash 5+ or Python 3 runtime requirements in plugin.json
prune-otel-store.sh uses EPOCHSECONDS (a Bash 5.0 variable). macOS ships Bash 3.2 as the system shell; consumers without Homebrew bash will get EPOCHSECONDS= (empty string), producing a silent arithmetic error that passes validation but computes cutoff_seconds = -N*86400 — an epoch cutoff in 1969 that keeps everything (prune becomes a no-op). The plugin.json has no runtimeRequirements field advertising the Bash 5 + Python 3.6+ constraints, and the README doesn't call them out as prerequisites.
Low / Informational
7. check-all.test.sh mocks gh but has no test for the happy path where snapshot is created by the skill
check-all.sh exits with ERROR: snapshot not found if the TSV doesn't exist; the snapshot creation responsibility sits in the model-invoked context/action-check-all.md prose. This is documented-by-convention but fragile — if the model skips the snapshot step, the shell script fails with a user-unfriendly error. A guard that creates an empty snapshot (or a distinct exit code) would improve the DX.
8. validate_issue URL check too narrow
Line 177: url.startswith("https://github.com/") is correct for all six listed repos (all on GitHub). Minor concern: if a future tracked repo is on https://github.com/ this is fine, but the error message says "url must start with https://github.com/" which implicitly locks the skill to GitHub-hosted issues. Fine today; worth a comment for future extension.
What looks good
Plugin isolation is correct: ${CLAUDE_PLUGIN_ROOT}, ${CLAUDE_PLUGIN_DATA}, ${CLAUDE_PROJECT_DIR} used consistently; no ../ reach-outs
OTEL prune lifecycle is carefully designed: sentinel locking, dry-check short-circuit, compact-before-trim, verify-before-replace — the failure modes have been thought through
action_add duplicate detection, save_registry atomic write (os.replace), flock in clean.sh — all correct for the single-process case
start-collector.sh sentinel double-check before spawn minimizes TOCTOU window
Marketplace entry category/tags are appropriate; plugin.json version explicit at 0.1.0
clean action requires explicit user confirmation before running (SKILL.md gating)
Network egress is read-only; no eval, no curl | sh
--dry-run flags throughout are correct and tested
Recommendation: request changes — the Python 3 SyntaxError (finding 1) makes /claude-ops:claude-troubleshooting DOA on first run, and the repo-collision bug (finding 2) will silently corrupt the registry once any two tracked repos share an issue number. Both are small, targeted fixes.
Review registry_manager.py for Python 3 syntax issues and registry key bugs
Review clean.sh for jq exit-5 handling
Review prune-collector-lifecycle.sh for pkill scoping
Review prune-otel-store.sh for Bash 5+ EPOCHSECONDS usage
Check data-sources.md for jq query issue
Check start-collector.sh for PID tracking
Check if new commit bc90e4a addresses any prior findings
Post comprehensive review findings
The new commit (bc90e4a) only adds a # gitleaks:allow annotation to a test fixture. None of the previously flagged issues have been addressed. All findings below are confirmed against the current HEAD.
Critical — Blocker
1. Python 3 SyntaxError in registry_manager.py (×3 locations)
except ValueError, TypeError: is Python 2 syntax. In Python 3, it is a SyntaxError raised at parse time — the entire module fails to import before any logic runs. Every invocation of /claude-ops:claude-troubleshooting that touches the registry exits with a traceback.
Fix: except (ValueError, TypeError): at all three sites. The test suite is shell-only (.test.sh), so this escaped the suite — a one-liner Python smoke test (python3 -c "import registry_manager") would catch it.
High — Confirmed P2s (carry-over from prior reviews, still unaddressed)
2. Registry keyed by number only, not (repo, number)
find_by_number at L125 compares i.get("number") == number without comparing repo. The skill tracks issues across six distinct repos. GitHub issue numbers are only unique within a repo, so anthropics/claude-code#1 and anthropics/anthropic-sdk-python#1 collide in every lookup:
action_add L332: rejects the second cross-repo issue as a duplicate
In jq 1.6 (common on older Ubuntu/macOS), a malformed line causes jq to stop processing entirely and exit non-zero — events after the bad line never reach $tmp. Then mv -f "$tmp" "$file" at L164 silently overwrites the original with truncated content, permanently losing all events that followed the malformed record.
Fix: on exit 5, emit a warning and leave the original file untouched (return non-zero so the skip path at L176 fires).
On a machine where another app (monitoring sidecar, CI agent) also runs otelcol-contrib, pruning kills all collectors, and only the plugin's own one is restarted. Meanwhile, start-collector.sh spawns the collector via a subshell (L170–173) and does not capture or persist the PID — so there is currently no way to scope the kill to the plugin-owned process.
Fix: write the spawned PID to $store_dir/collector.pid after the nohup spawn (echo $! > "$store_dir/collector.pid"; note $! refers to the subshell here so the collector itself should be spawned without the outer subshell, or the inner nohup PID captured via nohup ... & echo $! > pid_file). Then in prune-collector-lifecycle.sh, read that PID and kill only that PID, falling back to pkill if the file is absent.
5. closedAt datetime rejected by date.fromisoformat at runtime
validate_issue L180–186 runs date.fromisoformat(val) on the closedAt field. GitHub's closedAt value (from gh issue view --json closedAt) is YYYY-MM-DDTHH:MM:SSZ — an ISO 8601 datetime with a Z suffix. datetime.date.fromisoformat only accepts YYYY-MM-DD in Python ≤ 3.10 and still rejects the Z suffix in 3.11+, raising ValueError. This means a registry updated with --closedAt "$closed" immediately fails validate.
Note: this is also affected by finding #1 — the except ValueError, TypeError: on L185 is Python 2 syntax and causes a SyntaxError at import time regardless.
Fix: normalize to YYYY-MM-DD (e.g., val[:10]) before parsing, or use datetime.fromisoformat(val.replace('Z', '+00:00')) and .date().
L90: The atomic temp+os.replace protects the final write, but two concurrent add invocations could both call load_registry, both see the same 5-issue list, both add their entry in memory, and the last os.replace wins — one addition is silently lost. An fcntl.flock over the registry file during load-through-save would close the window.
EPOCHSECONDS is a Bash 5.0 built-in. macOS ships Bash 3.2 as /bin/bash. Consumers without Homebrew bash will get an empty EPOCHSECONDS, producing a silent arithmetic evaluation of (0 - N*86400) — a cutoff in 1969 that keeps all records, making prune a no-op. No error is emitted. The plugin.json has no runtimeRequirements field declaring Bash 5+ or Python 3.6+, and the README doesn't list them as prerequisites.
Fix: add a runtimeRequirements section to plugin.json and guard EPOCHSECONDS usage with a version check (((BASH_VERSINFO[0] >= 5))) or use $(date +%s) as a portable fallback.
8. jq indexing bug in data-sources.md "failed-then-fixed" example
Inside range(1; length), . is the loop integer, not the sorted array. .[.-1] therefore evaluates to integer[integer-1] = null, so null.hook == null.hook → true and null.exit_code != 0 → true for every record. The select() passes all entries unconditionally, producing garbage output rather than detecting retried hooks.
Fix: bind the sorted array to a named variable before the range:
9. check-all.test.sh mocks gh but has no happy-path snapshot test
check-all.sh errors with ERROR: snapshot not found if the TSV doesn't exist; snapshot creation is delegated to the model via context/action-check-all.md prose. If the model skips the snapshot step, the shell script fails with a user-unfriendly error. A distinct exit code or a graceful empty-state would improve DX.
10. validate_issue URL check implicitly locks the skill to GitHub
L177: url.startswith("https://github.com/") is correct today. Worth a comment noting this is intentional so the constraint is visible if a non-GitHub repo is added later.
What looks good
Plugin isolation is correct: ${CLAUDE_PLUGIN_ROOT}, ${CLAUDE_PLUGIN_DATA}, ${CLAUDE_PROJECT_DIR} used consistently; no ../ reach-outs
action_add duplicate detection and save_registry atomic write (os.replace + fsync) are correct for the single-process case
start-collector.sh double-checks the prune sentinel closest to spawn to minimize the TOCTOU window
clean action requires explicit user confirmation before running (SKILL.md gating)
Network egress is read-only; no eval, no curl | sh; --dry-run flags throughout are correct and tested
Marketplace entry category/tags are appropriate; plugin.json has explicit version: 0.1.0
Recommendation: request changes. Findings 1–5 must be addressed before merge. Finding 1 (Python 3 SyntaxError) makes the troubleshooting skill DOA on first invoke. Finding 2 (repo-key collision) silently corrupts the registry once any two tracked repos share an issue number. Findings 3–5 carry confirmed data-loss or machine-scope risk.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached), so the required automated review did not complete successfully. Human review is needed; no reviewers were assigned because no eligible non-author maintainers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached), so the required automated review did not complete successfully. Human review is needed; no reviewers were assigned because no eligible non-author maintainers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached), so the required automated review did not complete successfully. Human review is needed; no reviewers were assigned because no eligible non-author maintainers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Review data-sources.md and otel-queries.md — jq query fixes
Review action-check-all.md — resolved issue status fix
Check plugin structure and CLAUDE.md compliance
Post comprehensive findings
Summary
All blockers and high-priority findings from the two prior Claude reviews (Python 3 SyntaxError, repo/number composite key, closedAt datetime, jq exit-5 data loss, pkill blast radius, jq indexing in data-sources.md, union aggregate in otel-queries.md, resolved-issue status in action-check-all.md, OTLP port probing in start-dashboard.sh, net-probe TCP classification) are confirmed fixed in the current HEAD. Three items from the Codex P2 round remain open.
EPOCHSECONDS is Bash 5.0+. macOS ships Bash 3.2 as /bin/bash; consumers without Homebrew bash will get EPOCHSECONDS expanding to empty string. Arithmetic on empty string evaluates as 0, so cutoff_seconds = 0 - N*86400, which is a large negative epoch — all records are newer than the cutoff and prune silently becomes a no-op. No error is emitted. The script's own shebang comment says "bash 5+" but this is the only runtime check that's missing.
The plugin.json has no runtimeRequirements field to advertise the constraint, and README.md doesn't list it as a prerequisite.
port_state="$(port_status "$OTLP_HTTP_PORT")"# only 4318
otel-collector.yaml binds both127.0.0.1:4317 (gRPC) and 127.0.0.1:4318 (HTTP). When port 4317 is already occupied by another process, port_state reads free, action becomes would-spawn, and the nohup launch proceeds — but the Collector fails its own gRPC bind at startup and silently exits. The script returns 0 and emits spawned Collector, but telemetry capture is off.
start-dashboard.sh was correctly fixed to probe both its ports; the same pattern applies here.
save_registry at line 90: os.replace makes the final write atomic, but two concurrent add invocations both calling load_registry see the same 5-issue list, each appending their entry to an in-memory copy. The last os.replace wins — one entry is silently lost. An fcntl.flock on the registry file for the load-through-save window closes the race. Low occurrence risk in practice (the skill is invoked interactively), but the failure mode is silent.
What looks good
All six previously flagged correctness/data-loss blockers are confirmed fixed with clean implementations
Plugin isolation is correct throughout: ${CLAUDE_PLUGIN_ROOT}, ${CLAUDE_PLUGIN_DATA}, ${CLAUDE_PROJECT_DIR} used consistently; no ../ reach-outs; no hardcoded project paths
prune-collector-lifecycle.sh config-file pattern matching is a principled fix — significantly better than bare binary name; the residual documented with escape hatch env vars (CC_OTEL_STOP_CMD / CC_OTEL_RUNNING_CMD) is a reasonable trade-off
Cold compaction is protected by the prune sentinel, so the Codex concern about same-second filename collisions is effectively mitigated — a second concurrent prune cannot start while the first holds the sentinel
action_add duplicate detection and action_remove identity-comparison removal are correct
--dry-run flags throughout are consistent and tested
Network egress is read-only; no eval, no curl | sh
Marketplace category/tags are appropriate; plugin.json version is explicit at 0.1.0
Recommendation: Finding 1 (EPOCHSECONDS / silent no-op prune) is a medium-severity correctness issue on a commonly-used macOS baseline; finding 2 (gRPC port not probed) is a small gap in the same probing logic just fixed in start-dashboard.sh. Both are straightforward one-liners. Finding 3 (registry locking) is low-risk in practice. The PR is otherwise in good shape.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached), so the required automated review did not complete successfully. Human review is needed; no reviewers were assigned because no eligible non-author maintainers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Check git log for latest commit (b37abde — "fix: address third Codex review batch")
Verify previously-open items are fixed (EPOCHSECONDS, gRPC port probe, registry locking)
Review new Codex P2s: daemon-path in operator docs, Docker reachability, curl rc=1
Post comprehensive review findings
Summary
The third Codex batch (b37abde) closes two remaining findings from the prior round — the gRPC port probe and cold parquet filename collision. Three medium items and one low item remain open.
EPOCHSECONDS is Bash 5.0+. macOS ships Bash 3.2 as /bin/bash; consumers without Homebrew bash get EPOCHSECONDS expanding to the empty string, so cutoff_seconds = 0 - N*86400 — a negative epoch in 1969 that's older than every record. Prune becomes a silent no-op: nothing is trimmed, no error is emitted, printf 'cutoff_epoch_seconds=%s\n' outputs a large negative number that looks plausibly wrong but is easy to miss.
This has been flagged in every Claude review round and is unaddressed. One-liner fix in the pruning body before line 203:
date +%s is portable on GNU date (Linux) and BSD date (macOS). Also worth adding a runtimeRequirements note in plugin.json / README listing Python 3.6+ as a prerequisite regardless.
After migration to a plugin, otel-collector.yaml lives in the plugin cache under ${CLAUDE_PLUGIN_ROOT}/skills/claude-observability/otel/otel-collector.yaml — not under the consumer project's .claude\skills\. A consumer following this template will register a scheduled task pointing at a file that doesn't exist; the Collector silently never starts.
The macOS recipe at line 88 has the same problem: <repo-root>/${CLAUDE_PLUGIN_ROOT}/skills/... is a mixed-path nonsense string.
The doc already says the command "carries machine-specific absolute paths" and is "generated from your machine's paths" — it just needs to tell consumers how to find the plugin root (e.g. claude plugin details claude-ops exposes the cache location, or the skill can echo "${CLAUDE_PLUGIN_ROOT}" during setup). The template placeholder should be <plugin-root>, not <repo-root>\.claude\skills\.
if docker inspect "$container_name">/dev/null 2>&1;then
...
fi
...
printf'%s\n'"absent"
When the Docker CLI is installed but the daemon is not running (Docker Desktop paused, socket not owned by the user), docker inspect exits non-zero — indistinguishable from "container does not exist." The function returns "absent", which in main() leads to action="would-spawn" (when ports are free), and run_dashboard_create calls docker run. Under set -euo pipefail, docker run exits non-zero → the whole script exits non-zero, which the header explicitly says it won't: "NON-BLOCKING / advisory: skip paths… exit 0."
The docker-absent path at line 74 handles "no docker binary" but not "binary present, daemon unreachable." Adding a docker info >/dev/null 2>&1 || { printf '%s\n' "docker-daemon-absent"; return 0; } guard before the inspect calls closes the gap.
A prior fix (2nd batch) correctly added rc 8/52/56 for connected-but-non-HTTP cases. The Codex P2 identifies one more: when a TCP peer accepts and sends an SSH banner (or HTTP/0.9 response), curl exits 1 (CURLE_UNSUPPORTED_PROTOCOL). This falls to * and is reported as free. With an SSH daemon on port 4318 (or 18888), start-collector.sh/start-dashboard.sh would attempt a bind and fail rather than reporting the conflict.
5. save_registry has no file-level locking (carry-over)
registry_manager.py:90: Two concurrent add invocations both read the same 5-issue list, both append in memory, last os.replace wins — one entry silently lost. fcntl.flock during load-through-save closes the race. Low occurrence risk in the interactive skill use case; documented here for completeness.
What looks good
gRPC port probe fix in start-collector.sh is clean — the order of checks (http-listening → sentinel → binary-absent → grpc-listening → would-spawn) is correct
Cold parquet filename uniquification in prune-compact.sh is correct; sentinel prevents concurrency, serial loop suffices
Plugin isolation (${CLAUDE_PLUGIN_ROOT}, ${CLAUDE_PLUGIN_DATA}, ${CLAUDE_PROJECT_DIR}) correct throughout; no ../ reach-outs
Marketplace entry and plugin.json version (0.1.0) look clean
Recommendation: request changes. Findings 1 and 2 are the priority — EPOCHSECONDS makes prune silently a no-op on the macOS baseline most plugin consumers run, and the wrong scheduled-task path in the operator docs would leave the always-on daemon unconfigured. Findings 3 and 4 are quick one-liner additions to the advisory scripts. All four are targeted fixes with no architectural impact.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached), so the required automated review did not complete successfully. Human review is needed; no reviewers were assigned because no eligible non-author maintainers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
EPOCHSECONDS is a Bash 5.0 built-in. macOS ships Bash 3.2 as /bin/bash; consumers without Homebrew bash get it expanding to an empty string. $(( - N*86400)) evaluates as 0 - N*86400 = a large negative epoch — older than every stored record — so prune becomes a silent no-op (nothing is trimmed, no error is emitted). The cutoff_epoch_seconds=<large-negative> line in the output makes it visible only to someone looking closely.
Docker's -p host:container without a host IP defaults to 0.0.0.0 — all network interfaces. The Aspire dashboard is spawned with ASPIRE_DASHBOARD_UNSECURED_ALLOW_ANONYMOUS=true. On any developer machine reachable from a LAN or VPN, this exposes the live telemetry UI (sessions, tool I/O, traces) and the OTLP ingestion port to anyone on the network.
When CC_OTEL_STORE is unset in a session, the Collector writes session telemetry (prompts, tool I/O, API request/response bodies) to .claude/observability/otel/ relative to the Collector's working directory — typically the consumer repo root for a SessionStart-hook-spawned Collector. The marketplace repo's .gitignore only excludes .claude/settings.local.json. A consumer repo without an explicit .claude/observability/ ignore will see the JSONL files as untracked and can accidentally stage/commit them.
Mitigations to consider (in priority order):
Default to ${CLAUDE_PLUGIN_DATA} — moves the store fully out of the consumer tree when CC_OTEL_STORE is unset; safest for new consumers
Have setup scripts add the .gitignore entry — start-collector.sh or operator-setup.md could echo '.claude/observability/' >> .gitignore as part of setup
At minimum, document prominently — operator-setup.md and SKILL.md should call out the accidental-commit risk in the setup checklist, not just as a buried prerequisite note
stopped) action="would-start";;# no port check — absent path has one, stopped does not
...
would-start)
docker start "$container_name">/dev/null # under set -euo pipefail
The absent branch correctly gates on ui_port_state/otlp_port_state (line 184), but the stopped branch routes directly to would-start without the same check. When the container was stopped externally and another process has claimed its published port, docker start fails with a port-already-allocated error and exits non-zero under set -e. The script's own header says "NON-BLOCKING / advisory: skip paths exit 0" — this violates that contract.
Fix: apply the same port guard to the stopped case:
stopped)
if [[ "$ui_port_state"=="listening"||"$otlp_port_state"=="listening" ]];then
action="skip-port-in-use"else
action="would-start"fi;;
5. save_registry has no file-level locking (carry-over, low severity)
registry_manager.py:90: Two concurrent add invocations both calling load_registry see the same 5-issue list, both append in memory, last os.replace wins — one entry silently lost. The atomic write is correct for the final write; the race is in the load-through-save window. An fcntl.flock over the registry file closes it. Low occurrence risk in the interactive use case.
What looks good
All four blockers and confirmed data-loss issues from prior rounds are fixed and verified present
net-probe.sh curl exit-code mapping is now comprehensive and well-documented (rc 0/1/8/52/56 = listening, bias toward free on timeout = correct for advisory tool)
start-dashboard.sh docker daemon detection is clean: docker info before docker inspect, with correct advisory skip path
operator-setup-collector-daemon.md stable-copy rationale is well-explained (cache path changes on update)
OTEL prune lifecycle (sentinel lock, dry-check, compact-before-trim, verify-before-replace) is well-designed
No eval, no curl | sh, no secrets; network egress remains read-only
Recommendation: request changes. Findings 2 and 3 are the priority — exposing the anonymous Aspire dashboard on all interfaces is a clear local-network security issue for any developer on a LAN or VPN, and the unguarded default store path is a data-exfiltration risk (prompts, API bodies in git history). Finding 1 (EPOCHSECONDS) makes prune a silent no-op on the macOS baseline and has been flagged in every review round. Finding 4 is a one-line fix to extend the existing port guard to the stopped-container path.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached), so the required automated review did not complete successfully. Human review is needed; no reviewers were assigned because no eligible non-author maintainers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached), so the required automated review did not complete successfully. Human review is needed; no reviewers were assigned because no eligible non-author maintainers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Check for any new issues introduced by latest batch
Post comprehensive review findings
Summary
The sixth Codex batch (4742897) closes three items from the prior round. One medium finding has been open across all six review rounds and remains unaddressed. The rest of the PR is in good shape.
Confirmed Fixed in 4742897
Finding
File
Status
noop-already-running reported success for foreign :4318 listener, silently losing CC telemetry
start-collector.sh:74–86, 149–157
✅ Fixed — collector_process_running() mirrors prune lifecycle scoping; foreign listener reports skip-port-conflict with explicit "telemetry NOT captured" advisory
Single quotes in store path broke DuckDB SQL literals (/tmp/O'Neil/...)
prune-compact.sh:15–23
✅ Fixed — sql_path() doubles single quotes via ${p//\'/\'\'} before any SQL interpolation site
Scheduled prune task pointed at defunct in-repo .claude/skills/ path
operator-setup-retention.md:89–96
✅ Fixed — recipes resolve via ${CLAUDE_PLUGIN_ROOT}/skills/claude-observability/otel/prune-otel-store.sh with re-register note
Still Open — Medium (flagged in every review round)
EPOCHSECONDS used without Bash 5+ guard — silent no-op prune on macOS
EPOCHSECONDS is Bash 5.0+. macOS ships Bash 3.2 as /bin/bash; consumers without Homebrew bash get it expanding to the empty string. $(( - N*86400)) evaluates as 0 - N*86400 — a large negative epoch that is older than every record. Prune silently becomes a no-op: nothing is trimmed, no error is emitted, and cutoff_epoch_seconds=<large-negative> in the output is easy to miss. The script's own header comment says "bash 5+" but there is no runtime guard.
registry_manager.py:90: The os.replace atomic write is correct for the final write, but two concurrent add invocations both calling load_registry first see the same in-memory state. Last writer wins — one entry is silently lost. An fcntl.flock over the registry file for the load-through-save window closes the race. Low occurrence risk in the interactive skill use case.
What looks good
All prior critical/high fixes confirmed present: Python 3 SyntaxError, (repo, number) composite key, closedAt ISO datetime, jq exit-5 data loss, pkill blast radius, jq indexing, DuckDB aggregate binder error, action-check-all status enum, dashboard OTLP port probe, net-probe curl exit codes, start-dashboard.sh loopback binding, store .gitignore protection, Docker daemon reachability, stopped-container port guard, gRPC port probe, parquet filename uniquification, operator-setup-collector-daemon path
collector_process_running() in start-collector.sh correctly mirrors the prune lifecycle scoping (config-filename shape match, POSIX + Windows both covered)
sql_path() in prune-compact.sh handles both path conversion (cygpath) and SQL-literal safety (single-quote doubling) in a single well-named helper
Test enum in start-collector.test.sh updated to include skip-port-conflict ✓
Plugin isolation (${CLAUDE_PLUGIN_ROOT}, ${CLAUDE_PLUGIN_DATA}, ${CLAUDE_PROJECT_DIR}) consistent throughout; no ../ reach-outs
No eval, no curl | sh; network egress remains read-only; --dry-run flags throughout
Recommendation: the EPOCHSECONDS one-liner is the only thing between this and a clean bill. It is a two-line change that has been called out every round — fixing it removes the last medium finding and gets the PR to merge-ready.
The reason will be displayed to describe this comment to others. Learn more.
Stale comment
Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached), so the required automated review did not complete successfully. Human review is needed; no reviewers were assigned because no eligible non-author maintainers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
Bundles three Claude Code operations skills migrated from
melodic-software/medley (melodic-software/medley#1288):
- claude-observability: read locally captured telemetry (OTEL DuckDB
store, collector, optional Aspire dashboard, hook-event JSONL,
ccusage) with cross-session trend reports and store pruning
- claude-troubleshooting: search known Claude product GitHub bugs,
check service health, maintain a persistent tracked-issue registry
- claude-code-changelog: ingest Claude Code changelog entries and
integrate them into the current repo (fetch/diff/status/apply)
Repo-agnostic: project root resolves via CLAUDE_PROJECT_DIR with git
fallback; persistent state (issue registry, check-all output, written
reports) lives under CLAUDE_PLUGIN_DATA; bundled assets referenced via
CLAUDE_PLUGIN_ROOT; work-item and quirks-doc integrations degrade
gracefully per consumer context. Tests are self-contained (no external
test lib).
- clean.sh: never promote a partial jq temp — a malformed JSONL line
makes jq stop mid-stream, so accepting exit 5 could silently drop
every valid event after the bad line; any nonzero rc now skips the
file intact
- prune-collector-lifecycle.sh: scope collector stop/running checks to
processes whose command line references this plugin's config file
instead of every otelcol-contrib on the machine (Windows path now
matches via CIM CommandLine; posix via pgrep/pkill -f)
- registry_manager.py: issue identity is (repo, number) — duplicate
detection, get/update/remove disambiguation via --repo; closedAt
accepts GitHub's datetime timestamps; PEP 758 except clauses
parenthesized for Python 3.10 compatibility
- data-sources.md: fix invalid jq in the failed-then-fixed query
(bind sorted array, index by range variable) + add a regression test
mirroring it; drop two leftover relative reach-outs into medley
internals (rules schema link, cc-telemetry-ensure hook links)
- net-probe.sh: classify connected-but-non-HTTP curl exits (8/52/56) as
listening — a non-HTTP process on the port must read occupied or
callers publish a doomed Docker bind onto it; residual ambiguity
(timeout) still leans free per the advisory rationale
- start-dashboard.sh: probe the role's OTLP host port too — docker run
publishes both ports, so either being bound now reports
skip-port-in-use instead of failing the spawn under set -e
- otel-queries.md: aggregate the hot+cold union in a subquery (the
bare UNION ALL form is a DuckDB binder error, verified live)
- action-check-all.md: resolved issues record status=closed +
category=fixed ('fixed' is a category, not a valid status)
- start-collector.sh: probe the gRPC receiver port (4317) too — the
config binds both receivers, so 4318-free-but-4317-taken now reports
skip-grpc-port-in-use instead of spawning a collector doomed to die
on its duplicate bind while claiming success
- prune-compact.sh: uniquify cold parquet filenames when a second prune
of the same store file lands in the same UTC second — mv -f no longer
overwrites already-compacted history (append-only contract)
- operator-setup-collector-daemon.md: daemon recipes now point at a
stable local copy of the plugin's otel-collector.yaml (the in-repo
.claude/skills path no longer exists under plugin install, and the
plugin cache path changes on update)
- start-dashboard.sh: probe docker daemon reachability — CLI-present-
but-daemon-down now reports skip-docker-unreachable instead of dying
on docker run under set -e
- net-probe.sh: curl rc 1 (protocol mismatch after an accepted
connection, e.g. a non-HTTP banner) now classifies as listening
- start-dashboard.sh: publish both dashboard ports on 127.0.0.1 only —
the container runs anonymous-auth and its telemetry can carry
sensitive data, so it must never be reachable beyond localhost
- start-collector.sh: drop an ignore-all .gitignore into the OTEL store
dir on spawn (raw prompts/API bodies must not land as committable
untracked files when the store lives inside a repo); daemon-setup doc
carries the same advice for the manual path
- start-dashboard.sh: the stopped-container path is port-aware — docker
start rebinds published ports, so a foreign listener now reports
skip-port-in-use instead of failing under set -e
- start-collector.sh: a :4318 listener only reports noop-already-running
when an otelcol-contrib process running this config shape exists —
a stale/third-party listener now reports skip-port-conflict (telemetry
NOT captured) instead of silent false success
- prune-compact.sh: sql_path doubles single quotes so paths like
/tmp/O'Neil survive DuckDB SQL literal interpolation (verified live)
- operator-setup-retention.md: scheduled prune recipes point at the
installed plugin's script (resolve via CLAUDE_PLUGIN_ROOT; re-register
after plugin updates) instead of the defunct in-repo path
Verify EPOCHSECONDS finding is addressed (flagged every prior round)
Scan for any new issues introduced by latest batch
Post comprehensive review findings
Confirmed Fixed in a418c23
Finding
File
Status
noop-already-running on foreign :4318 listener — telemetry silently lost
start-collector.sh:68–90, 147–160
✅ Fixed — collector_process_running() uses config-shape match (pgrep/CIM otel-collector.yaml); foreign listener now reports skip-port-conflict with explicit "telemetry NOT captured" advisory
Single quotes in store path broke DuckDB SQL literals
prune-compact.sh:15–23
✅ Fixed — sql_path() doubles single quotes via ${p//\'/\'\'} before any SQL interpolation
Scheduled prune task pointed at defunct in-repo path
operator-setup-retention.md:82–112
✅ Fixed — Windows and macOS/Linux recipes now reference <plugin-prune-script> resolved via ${CLAUDE_PLUGIN_ROOT}, with a re-register-after-update note
Still Open — Medium (flagged in every review round, remains unaddressed)
EPOCHSECONDS used without Bash 5+ guard — silent no-op prune on macOS
EPOCHSECONDS is a Bash 5.0 built-in. macOS ships Bash 3.2 as /bin/bash; consumers without Homebrew bash get EPOCHSECONDS expanding to the empty string. $(( - N*86400)) evaluates as 0 - N*86400 — a large negative epoch older than every stored record — so prune silently becomes a no-op: nothing is trimmed, no error is emitted, and cutoff_epoch_seconds=<large-negative> in the printed output is easy to overlook. The script header comments "bash 5+" but there is no runtime guard.
registry_manager.py:90: The os.replace atomic write is correct for the final write, but two concurrent add invocations both calling load_registry first see the same in-memory state — last writer wins, one entry is silently lost. An fcntl.flock over the registry file for the load-through-save window closes the race. Low occurrence risk given interactive-only skill use.
What looks good
All prior critical/high fixes confirmed present: Python 3 SyntaxError, (repo, number) composite key, closedAt ISO datetime, jq exit-5 data loss, pkill blast radius, jq indexing, DuckDB aggregate binder error, action-check-all status enum, dashboard OTLP port probe, net-probe curl exit codes, start-dashboard loopback binding, store .gitignore protection, Docker daemon reachability, stopped-container port guard, gRPC port probe, parquet filename uniquification, operator-setup-collector-daemon path
collector_process_running() in start-collector.sh correctly mirrors the prune lifecycle scoping — config-filename shape match with POSIX + Windows both handled; enum updated in test suite
sql_path() in prune-compact.sh cleanly handles both MSYS→Windows path conversion and SQL-literal escaping in one helper; all DuckDB interpolation sites flow through it
Plugin isolation (${CLAUDE_PLUGIN_ROOT}, ${CLAUDE_PLUGIN_DATA}, ${CLAUDE_PROJECT_DIR}) consistent throughout; no ../ reach-outs; no hardcoded paths
No eval, no curl | sh; network egress read-only; --dry-run flags throughout
Recommendation: EPOCHSECONDS is the only thing between this PR and a clean bill. It is a two-line, zero-risk change that makes prune work correctly on the macOS baseline most plugin consumers run. Fixing it closes the last open finding and the PR is ready to merge.
The reason will be displayed to describe this comment to others. Learn more.
Risk: medium. Not approving because Cursor Bugbot skipped (usage limit reached), so the required automated review did not complete successfully. Human review is needed; no reviewers were assigned because no eligible non-author maintainers are available in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
The reason will be displayed to describe this comment to others. Learn more.
Classify accepted-connection timeouts as occupied
When a process on one of the probed ports accepts the TCP connection but does not send an HTTP response, curl exits 28 (curl --manual: “Operation timeout”), and this wildcard reports the port as free. I verified the dashboard path with a local listener on 18888 that accepts and sleeps: start-dashboard.sh --dry-run reported port_18888=free and action=would-spawn, so the non-dry run would reach docker run -p 127.0.0.1:18888:... under set -e and exit nonzero instead of taking the advisory skip-port-in-use path.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Closes the publish gate for melodic-software/medley#1288 (EPIC melodic-software/medley#1273).
What
New
claude-opsplugin bundling three Claude Code operations skills migrated from medley:/claude-ops:claude-observabilitycleanprune action/claude-ops:claude-troubleshooting/claude-ops:claude-code-changelogfetch/diff/statusread-only;applygated on explicit user intent)Marketplace entry:
category: monitoring, tagsoperations, observability, otel, telemetry, troubleshooting, changelog, claude-code, skill. Explicitversion: 0.1.0inplugin.jsononly.De-coupling (repo-agnostic)
CLAUDE_PROJECT_DIR, falling back togit rev-parse --show-toplevelfrom CWD (never the script's own location — plugin cache is not the project).${CLAUDE_PLUGIN_DATA}: troubleshootingregistry.json,check-alloutput,--writeobservability reports. Medley's registry data does NOT ship — consumers start empty.${CLAUDE_PLUGIN_ROOT}; no../reach-outs; sibling-skill refs stay bare; the one cross-plugin ref (/bug-report:bug-report) is qualified with graceful degradation./issues,/onboard, rule-file cites, repo-grep tooling, release-tracking issue markers) generalized to consumer-context seams; integrations skip silently when the consumer lacks them.format='nd'alias replaced with canonical'newline_delimited'(verified equivalent against live duckdb).userConfig: variability is covered by existing env vars (CC_OTEL_STORE, retention windows,CHECK_ALL_OUTPUT_DIR) and conventional project-relative defaults — no speculative knobs.Gate evidence
claude plugin validate --strict plugins/claude-ops→ PASS;claude plugin validate --strict .(catalog manifest) → PASSclaude plugin detailstoken cost: always-on ~454 tok (per-component ~150–160 always-on; on-invoke ~3.3k / ~2.4k / ~1.9k)--plugin-dirsmoke test in a clean non-medley repo:/claude-ops:claude-troubleshooting statusloaded under theclaude-opsnamespace, ran registry stats against empty state, fetched status.claude.com, and rendered the health snapshot (graceful empty-state degradation confirmed)shellcheck --rcfile=.shellcheckrcclean;typosclean;markdownlint-cli2clean;editorconfig-checkerclean; exec bits set on shebang scriptsSecurity review (per MIGRATION-PLAYBOOK acceptance)
git,jq,gh,python3, optionalduckdb/otelcol-contrib/Docker); noeval, nocurl | sh.ghreads of GitHub issues andcurlreads of status.claude.com/Marginlab pages (troubleshootingquality/status), WebFetch of the official changelog — read-only; issue creation is draft-first with explicit confirmation.Note
Medium Risk
Large new plugin with many bash scripts that stop/restart
otelcol-contrib, prune local telemetry (including prompts/API bodies when captured), and a repo-mutating changelogapplypath; network reads viagh/status/changelog are read-only per the PR description.Overview
Adds the
claude-opsplugin (0.1.0) to the marketplace catalog and root README — three repo-agnostic Claude Code operations skills under/claude-ops:*.Observability is the largest addition: skill docs plus a local OTEL stack (collector config, DuckDB
cc-otel.sql, Aspire dashboard starters, JSONL/ccusage query catalogs) and retention viaclean/prune-otel-store.sh(hot NDJSON trim, cold Parquet compaction, collector stop/restart with locking). Bundled bash regression tests cover jq pipelines, port probing, prune, and collector startup contracts.Changelog integration adds
/claude-ops:claude-code-changelogwith gated actions (fetch/diff/statusread-only;applyonly on explicit intent) — explore → research → interview → implement, P1/P2/P3 rubric, git-log–based applied-version tracking.Troubleshooting (per plugin manifest/README; not fully shown in the excerpted diff) covers GitHub bug search, health checks, and a persistent issue registry under
${CLAUDE_PLUGIN_DATA}. Medley-specific paths and shipped registry data are not included; project root resolves viaCLAUDE_PROJECT_DIR/ git toplevel.Reviewed by Cursor Bugbot for commit a418c23. Bugbot is set up for automated code reviews on this repo. Configure here.