Skip to content

feat(claude-ops): OTEL store size cap and retention visibility - #5425

Merged
kyle-sexton merged 17 commits into
mainfrom
feat/5236-otel-retention-visibility
Sep 30, 2026
Merged

kyle-sexton merged 17 commits into
mainfrom
feat/5236-otel-retention-visibility

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Refs: #5236

Summary

The OTEL store had no size bound and its status output hid cold size and prune age. This adds a hot-file size cap, reports cold size and last-prune age, and corrects the retention docs.

Fix

  • prune-otel-store.sh: CC_OTEL_HOT_MAX_MB caps each hot store file as a fallback after age pruning (the file exporter appends and cannot rotate).
  • probe-observability-state.sh --otel-store: prints cold size and time since the last prune.
  • operator-setup-retention.md and the observability SKILL: measured sizes, the cap, and the file exporter limit.
  • claude-ops bumped to 0.71.0 with a CHANGELOG entry.

Verification

  • prune-otel-store.test.sh: 176 passed, 0 failed
  • probe-observability-state.test.sh: 64 cases passed
  • scripts/check-changelog-parity.sh --check --check-order: pass
  • scripts/validate-plugins.sh: all manifests and catalog validated

Related

Refs #5236. The ADR-0009 acceptance criterion lives in melodic-software/medley and stays with the owner, so this PR does not close the issue. #5274 (stable prune entry point) is related and not addressed here.

🤖 Generated with Claude Code

kyle-sexton and others added 5 commits September 29, 2026 15:39
A hot file over the cap drops its oldest lines even when every line is inside
the age windows. The extra drops raise that file's structure cutoff, so they
compact to cold before the trim like age drops. --dry-run reports the cap and
the files it would size-prune.

Refs #5236

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…re status

prune-otel-store.sh stamps <store>/.last-prune after every successful
non-dry run, including the nothing-to-prune short-circuit. The
--otel-store probe keeps its three per-file lines and appends the cold
tier size and the last-prune time and age; the observability skill tells
a report to flag a missing or stale (about 2 days) prune.

Refs #5236

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…p and the fileexporter limit

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ntry

Refs #5236

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

This PR body conforms to the issue-linkage contract. Nothing to do.

@kyle-sexton
kyle-sexton marked this pull request as ready for review September 29, 2026 20:05
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-29T20:08:39.061896Z b205438 Draft marked ready
ℹ️ About Codex in GitHub

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

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

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

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

ℹ️ 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/claude-ops/skills/observability/otel/prune-otel-store.sh
Comment thread plugins/claude-ops/skills/observability/otel/prune-otel-store.sh Outdated
Recompute the size-cap cutoff after the Collector stops and write the
last-prune stamp only after a clean restart.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review — no issues found

  • Gather context (PR diff, changed files)
  • Review changed shell scripts for injection / unsafe handling
  • Check instruction-surface deletions lens (docs/SKILL.md changes)
  • Report findings

Scope: all 9 changed files (prune-otel-store.sh, probe-observability-state.sh, prune-collector-lifecycle.sh, their test files, SKILL.md, operator-setup-retention.md, CHANGELOG.md, plugin.json), diffed against origin/main via gh pr diff 5425.

What I checked:

  • Env-var → arithmetic/command injection. CC_OTEL_HOT_MAX_MB is validated with ^[0-9]+$ (prune-otel-store.sh#L235) before it ever reaches $((hot_max_mb * BYTES_PER_MB)) arithmetic expansion, so unsanitized input can't reach bash's $(( )) evaluator (a known vector when arithmetic expansion runs on unvalidated strings).
  • New effective_cutoff() awk invocation (prune-otel-store.sh#L161-L181): time_field is set from a fixed two-value set, not attacker input; the size/cutoff arithmetic operates on values already numeric-validated. No injection surface.
  • New .last-prune stamp read/parse in the probe (probe-observability-state.sh#L221-L235): the stamp is passed quoted as the argument to -d/-f, so it can't be reinterpreted as a flag, and date -d doesn't execute shell content — worst case is a parse failure that already falls back to last-prune:never. The store itself is local, same-trust-boundary state the script already treats as trusted (same pattern as the pre-existing hot-file reads).
  • Instruction-surface deletions lens (per this skill's docs-deletion check): the doc edits in operator-setup-retention.md/SKILL.md are additive (measured sizes, new cap knob, new verify step) or replace a stale estimate with a measured one — no removal of a standing guardrail, retention rule, or check. Lens not triggered.
  • No secrets/token handling, no new network-facing surface, no new untrusted-input parsing path (the store files are the collector's own output, not external input).

The two Codex review comments on this PR (recompute cutoff after collector stop; write .last-prune only after a successful restart) are correctness/staleness bugs in the new logic, not security findings — out of scope for this lane.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review (/review:code-review)

  • Gather context (PR diff, comments, review comments)
  • Run /review:code-review per instructions
  • Post findings as inline PR comments
  • Update summary comment

Scope: the 9 changed files (plugin manifest/CHANGELOG bump, SKILL.md and operator-setup-retention.md docs, prune-otel-store.sh + lifecycle helper + tests, probe-observability-state.sh + tests), diffed against origin/main.

On the existing Codex findings

Codex's two P2 findings (recompute the size-cap cutoff after stopping the Collector; write .last-prune only after a successful restart) are already fixed at HEAD (c91e18d) — the merge commit c91e18d itself carries the fix (commit message: "Recompute the size-cap cutoff after the Collector stops and write the last-prune stamp only after a clean restart"). I verified both in the current code:

  • prune-otel-store.sh:354 recomputes effective_cutoff per file inside the trim loop, which runs after stop_collector/wait_collector_gone (line 321), so late-appended batches count toward the cap.
  • prune-collector-lifecycle.sh:132 only calls stamp_last_prune from cleanup() when original_rc == 0, and original_rc is forced to 1 if start_collector fails — so a failed restart no longer produces a fresh stamp.

Both are covered by new tests (prune-otel-store.test.sh cases 26/#"sizecap-late", and the "start failure writes no last-prune stamp" assertion). No action needed on these.

New finding (posted inline)

  • plugins/claude-ops/skills/observability/context/operator-setup-retention.md:27-29 — the new hot-tier size paragraph (7 days of logs held 790 MB, cc-logs.json reached 2.8 GB, cc-metrics.json 361 MB) states measured numbers with no basis/as-of date/recheck trigger, where .claude/rules/skill-bodies-state-current-rules.md requires that four-part record for a volatile specific a skill body restates. The same file gets this right immediately above it (cold-tier measurement) and below it (fileexporter README claim) — this paragraph is the one spot that omits it.

I did not independently re-run the author's claimed test/lint output (prune-otel-store.test.sh, probe-observability-state.test.sh, check-changelog-parity.sh, validate-plugins.sh) — that's author-claimed and unverified by me.

No other correctness, error-handling, or maintainability issues found in the size-cap logic (effective_cutoff), the probe's cold-size/last-prune reporting, or the test coverage.

Branch: feat/5236-otel-retention-visibility

Comment thread plugins/claude-ops/skills/observability/context/operator-setup-retention.md Outdated
kyle-sexton and others added 2 commits September 29, 2026 16:34
…on tests

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Security review (/review:security-review)

  • Gather context (PR diff, comments, review comments)
  • Run /review:security-review per instructions
  • Post findings
  • Update summary comment

Scope: the full cumulative diff of this PR against origin/main at HEAD 55a81f2 (9 changed files: prune-otel-store.sh + its test, prune-collector-lifecycle.sh, probe-observability-state.sh + its test, SKILL.md, operator-setup-retention.md, CHANGELOG.md, plugin.json), via gh pr diff 5425.

Result: no security issues found

This is functionally the same change surface two earlier reviews on this PR already covered (at c91e18d3e); this pass re-verified against the current HEAD rather than relying on the prior verdict.

What was checked:

  • CC_OTEL_HOT_MAX_MB → arithmetic expansion. Validated with ^[0-9]+$ before reaching $((hot_max_mb * BYTES_PER_MB)) at prune-otel-store.sh#L245-L248, so unsanitized input can't reach bash's $(( )) evaluator.
  • effective_cutoff() awk invocation (prune-otel-store.sh#L205-L225): time_field is passed via awk -v tf= (a bound variable, not interpolated into the program text), and comes from a fixed two-value set (timeUnixNano / startTimeUnixNano) based on filename, not file content. The size/cutoff math operates on already-validated numeric values. No injection surface, and store content (the collector's own output) is the same trust boundary already treated as trusted for the existing hot-file reads.
  • New .last-prune stamp, write and read paths. stamp_last_prune() writes a fixed date -u +%Y-%m-%dT%H:%M:%SZ format (prune-otel-store.sh#L235). The probe reads it back and passes it quoted as the -d/-f argument to date (probe-observability-state.sh#L497-L503), so it can't be reinterpreted as a flag and date -d doesn't execute shell content; worst case on a garbled stamp is the already-handled last-prune:never fallback.
  • Windows path normalization (STORE="${STORE//\\//}", probe-observability-state.sh#L476): plain string substitution, mirrors the existing pattern in prune-otel-store.sh.
  • Instruction-surface deletions lens (per this skill's docs-deletion check, checked against docs/conventions/instruction-exception-register/README.md, present in this checkout): the operator-setup-retention.md edits replace an estimate with a measured figure and add new documentation (the size cap, the file-exporter append-only limit); the standing guardrail itself (recheck if cold/ exceeds ~2 GB) is retained verbatim, not narrowed or removed. Lens not triggered.
  • No secrets/token handling, no new network-facing surface, no new externally-reachable input path — the OTEL store files are the collector's own local output, not external/untrusted input, and the GitHub Actions surface is unchanged in this diff (deferred to zizmor's lane per this skill's scope).

The two Codex P2 findings on this PR (recompute the size-cap cutoff after the Collector stops; write .last-prune only after a successful restart) are correctness/staleness issues, not security findings, and are already fixed at HEAD per the author's replies and the new regression tests (prune-otel-store.test.sh case 26 "sizecap-late", and the "start failure writes no last-prune stamp" assertion) — out of scope for this lane either way.
· branch feat/5236-otel-retention-visibility

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


/review:code-review — CI code-review lane

  • Gather context (diff since the previous review, comments, review comments)
  • Apply the lane's skip gate
  • Report findings

Skip gate: triggered. This lane's prior review already covers this head. b205438/c91e18d (SHA c91e18d3e178fc363dc57a1eb04c0176202cced5) got a full code-review pass from this lane at 2026-09-29T20:28:34Z, and the only commits since then are:

  • 8eb887b — shellcheck/portability lint-only touch-ups (an if/fi in place of &&/|| chaining, a trailing comment, EPOCHSECONDS/printf '%(...)T' in place of GNU-only date -d) to the three test files already reviewed. No behavior change.
  • 55a81f2 — a merge of origin/main, carrying no new content for this PR's own files.

Per this repo's REVIEW.md/skip-gate rule 4 ("this head already has a successful review from this lane that still applies"), I'm not re-deriving findings from scratch. Restating that prior review's outcome for continuity:

  • Both Codex P2 findings (recompute the size cap cutoff after the Collector stops; write .last-prune only after a successful restart) are fixed and covered by dedicated tests — confirmed in code at prune-otel-store.sh:354 and prune-collector-lifecycle.sh:132.
  • One finding from that review is still open at this head: operator-setup-retention.md:27-29 states measured hot-tier sizes (790 MB/2.8 GB/361 MB) with no basis, as-of date, or recheck trigger — the four-part record .claude/rules/skill-bodies-state-current-rules.md requires for a volatile specific a skill body restates. The paragraph immediately above (cold-tier measurement) and the one below (fileexporter README claim) both carry that record; this one doesn't. Not re-posted as a new inline comment since it's unchanged from the prior review — see that review's inline comment on this PR for the original flag.

No new findings from the lint-only commit; it doesn't touch executable logic.
· Branch: feat/5236-otel-retention-visibility

…sizes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


Code review (/review:code-review)

  • Gather context (diff since the previous review, comments, review comments)
  • Apply the lane's skip gate
  • Report findings

Scope: the only commit since this lane's last full pass (at 55a81f2, 2026-09-29T20:52:42Z) is 5984a00 ("docs(claude-ops): record basis and recheck trigger for the hot-store sizes"), a single-file, 4-line docs diff to operator-setup-retention.md.

Prior open finding — now fixed. This lane's previous pass flagged operator-setup-retention.md:27-29 for stating measured hot-tier sizes (790 MB / 2.8 GB / 361 MB) with no basis, as-of date, or recheck trigger, per .claude/rules/skill-bodies-state-current-rules.md's four-part record requirement. Commit 5984a00 adds all three:

"Measured on melo-lap-001 on 2026-09-28: 7 days of logs held 790 MB; unpruned, cc-logs.json reached 2.8 GB and cc-metrics.json 361 MB. Recheck if the store traffic mix changes (for example body capture turned off)."

This matches the pattern already used by the cold-tier measurement two paragraphs up (Measured: 72 MB over about 5 weeks (...); recheck if cold/ exceeds ~2 GB) and the fileexporter claim below it (Verified 2026-09-29 ...; recheck when the exporter README drops the restriction). Finding resolved — no re-post needed.

No new findings. The commit is a documentation-only wording change to an already-reviewed paragraph; it doesn't touch any executable logic (prune-otel-store.sh, prune-collector-lifecycle.sh, probe-observability-state.sh) or their tests, all of which retain the outcome from the prior passes on this PR:

  • Both Codex P2 findings (recompute the size-cap cutoff after the Collector stops; write .last-prune only after a successful restart) remain fixed and test-covered (prune-otel-store.sh:354, prune-collector-lifecycle.sh:132).
  • No correctness, error-handling, or maintainability issues in the size-cap logic, the probe's cold-size/last-prune reporting, or test coverage.

I did not independently re-run the author's claimed test/lint output (prune-otel-store.test.sh, probe-observability-state.test.sh, check-changelog-parity.sh, validate-plugins.sh) — that's author-claimed and unverified by me.
· Branch: feat/5236-otel-retention-visibility

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

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


/review:security-review — CI security-review lane

  • Gather context (PR diff, comments, review comments)
  • Apply the lane's skip gate
  • Report findings

Skip gate: triggered. This head already has a successful security review from this lane that still applies. The prior run at 2026-09-29T20:46:26Z covered the full cumulative diff against origin/main at HEAD 55a81f2 (all 9 changed files) and found no security issues.

The only commit since then is 5984a00 — a docs-only change to operator-setup-retention.md, adding the basis/as-of-date/recheck-trigger record (Measured on melo-lap-001 on 2026-09-28; Recheck if the store traffic mix changes) to the hot-tier size paragraph flagged by the code-review lane. I diffed 55a81f2..5984a00 directly: it only adds provenance prose to an already-reviewed measured-figures paragraph — no code, no logic, no new executable surface. Per the instruction-surface deletions lens, this is additive/corrective (fills in a missing verification record), not a removal of a standing guardrail, so that lens isn't triggered either.

No security-relevant surface changed since the last successful review. Restating that review's outcome for continuity:

  • CC_OTEL_HOT_MAX_MB is validated with ^[0-9]+$ before reaching $(( )) arithmetic expansion — no injection surface.
  • effective_cutoff()'s awk invocation uses a bound -v variable from a fixed two-value set, not interpolated input.
  • The new .last-prune stamp is written in a fixed date -u format and read back quoted as a date -d/-f argument — can't be reinterpreted as a flag or executed.
  • No secrets/token handling, no new network-facing surface; the OTEL store files are the collector's own local output, not external/untrusted input.
    · Branch: feat/5236-otel-retention-visibility

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

Bump claude-ops to 0.66.0, one step above main's 0.65.0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

Bump claude-ops to 0.67.0, one step above main's 0.66.0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 0s —— View job


I'll analyze this and get back to you.

Re-bump claude-ops to 0.71.0, the next free version after main's 0.70.0.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


/review:security-review — CI security-review lane

  • Gather context (PR diff, comments, review comments)
  • Apply the lane's skip gate
  • Report findings

Skip gate: triggered. This head already has a successful security review from this lane that still applies.

The prior run at 2026-09-29T21:13:31Z reviewed the full cumulative diff against origin/main (all 9 changed files) at head 5984a00 and found no security issues. I re-pulled the PR's diff at the current head (5edbd24) via gh pr diff 5425 and it is byte-for-byte identical to what that review covered: same 9 files, same hunks in prune-otel-store.sh, prune-collector-lifecycle.sh, probe-observability-state.sh, their tests, SKILL.md, operator-setup-retention.md, CHANGELOG.md, and plugin.json.

Everything committed between 5984a00 and 5edbd24 (17 commits, including several chore: merge origin/main into feat/5236-otel-retention-visibility merges) only brings in unrelated origin/main history — the version/description bumps visible in a raw git diff against the old commit (e.g. plugin.json 0.65.0 → 0.71.0, extra CHANGELOG.md entries) come from other PRs merged into main in the interim, not from this PR's own content. gh pr diff, which computes the diff against the actual merge base, confirms no net change to any file this PR owns.

No security-relevant surface changed since the last successful review. Restating that review's outcome for continuity:

  • CC_OTEL_HOT_MAX_MB is validated with ^[0-9]+$ before reaching $(( )) arithmetic expansion — no injection surface (prune-otel-store.sh#L245-L249).
  • effective_cutoff()'s awk invocation uses a bound -v variable from a fixed two-value set, not interpolated input (prune-otel-store.sh#L206-L226).
  • The .last-prune stamp is written in a fixed date -u format and read back quoted as a date -d/-f argument — can't be reinterpreted as a flag or executed (probe-observability-state.sh#L498-L512, prune-otel-store.sh#L236), and it's only written after a clean restart (prune-collector-lifecycle.sh#L131-L134).
  • No secrets/token handling, no new network-facing surface; the OTEL store files are the collector's own local output, not external/untrusted input.
  • Instruction-surface deletions lens: not triggered — the doc edits add or correct measured figures and a new knob, and don't remove a standing guardrail.

No new findings.
· Branch: feat/5236-otel-retention-visibility

@claude

claude Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor

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


/review:code-review — CI code-review lane

  • Gather context (diff since the previous review, comments, review comments)
  • Apply the lane's skip gate
  • Report findings

Skip gate: triggered. This head already has a successful review from this lane that still applies. The prior full pass at 5984a00 (2026-09-29T21:12:30Z) covered all 9 changed files and found no open issues (the one prior finding on operator-setup-retention.md's hot-tier sizes paragraph was resolved by that same commit).

What changed since 5984a00 (this PR's own commits only, excluding unrelated content pulled in by merging origin/main):

  • 7dc91c8 — operator-setup-retention.md's Windows scheduled-task verify line now reads bash <skill-dir>/scripts/probe-observability-state.sh instead of bash "${CLAUDE_PLUGIN_ROOT}"/skills/observability/scripts/probe-observability-state.sh. <skill-dir> is an established repo-wide placeholder (see plugins/claude-ops/skills/changelog/SKILL.md:180: "<skill-dir>, which is ${CLAUDE_SKILL_DIR}"), and this section already relies on manual path substitution for a task that runs outside a live Claude Code session (see the neighboring <plugin-prune-script> placeholder two lines above). Correct, no issue.
  • Several chore: merge origin/main commits, none of which touch this PR's own executable files (prune-otel-store.sh, prune-collector-lifecycle.sh, probe-observability-state.sh, or their tests — all byte-identical to 5984a00). They do bring in unrelated claude-ops version churn (plugin.json 0.65.0→0.71.0, five interleaving CHANGELOG.md entries from other PRs) — this PR's own changelog entry and version content are unchanged, just renumbered by the shared-file merge. They also bring in an unrelated SKILL.md "Boundary, the bundled explain-usage skill" section from PR feat(claude-ops): learn from dismissed native-overlap pairs and triage every candidate #5466 — not authored by this PR, already on main independent of this branch.

No new findings. Nothing in this PR's own diff changed beyond the placeholder correction above, which is doc-only, non-behavioral, and consistent with existing repo convention. Restating the prior review's still-current outcome for continuity:

  • Both Codex P2 findings (recompute the size-cap cutoff after the Collector stops; write .last-prune only after a successful restart) are fixed and test-covered (prune-otel-store.sh:354, prune-collector-lifecycle.sh:132).
  • No correctness, error-handling, or maintainability issues in the size-cap logic, the probe's cold-size/last-prune reporting, or test coverage.

I did not independently re-run the author's claimed test/lint output (prune-otel-store.test.sh, probe-observability-state.test.sh, check-changelog-parity.sh, validate-plugins.sh) — that's author-claimed and unverified by me.

Branch: feat/5236-otel-retention-visibility

@kyle-sexton
kyle-sexton merged commit 7708cba into main Sep 30, 2026
32 of 33 checks passed
@kyle-sexton
kyle-sexton deleted the feat/5236-otel-retention-visibility branch September 30, 2026 03:25
kyle-sexton added a commit that referenced this pull request Sep 30, 2026
Closes #5222

## Summary

Adds `/session-flow:tidy-work` (session-flow 0.41.0) and
`scripts/tidy_work.py`, an opt-in lifecycle for the gitignored `.work`
tiers in the current repo (the concern file's `memory_dir`, else
`.work`) and `~/.work`. Nothing runs unless the skill is invoked.

- `report` (read-only): per-item age, size, kind (handoff,
running-retro, checklist, slice, scratch, concern, unknown) and
in-flight status. A scratch item also shows the issue or PR its name
carries and that item's state.
- `normalize`: moves misplaced handoffs and running-retro ledgers (a
handoff's `.slots.json` sidecar with it) into the standard layout; never
deletes, never overwrites.
- `clean`: removes an item only when it is a known kind, is not in
flight, and names at least one issue or PR, every one closed or merged.
A scratch item goes only once its issue is closed or its PR merged.
- Unknown items (for example `.work/drain/status/*.json`) are always
kept and reported.

Issue body step 4 (entries that cannot be attributed are listed and
never deleted) holds for every kind. An item that names no issue or PR
is reported as `keep: names no issue or PR` and kept however old it is:
a handoff or running retro whose text names none, and every slice and
checklist, which have no attribution source (no writer records an issue
or PR in them), so `clean` never removes one. The owner comment's
`clean` (Expected 3) is read as acting on the attributed items that are
not in flight.

A handoff or running-retro file is that kind wherever it sits (the root,
`handoffs/`, `running-retros/`), so `report` and `normalize` agree on
it, with its `.slots.json` sidecar part of it.

Attribution (issue body steps 2 to 4, owner comment's `scratch` kind):

- A top-level entry is `scratch` when its name holds exactly one
all-digit token of 3 to 7 digits, optionally prefixed `pr`, `issue`, or
`gh` (`lint-5371.log`, `measure-4608`, `pr4120.md`, `reverify-4186`,
`scratch-4586-d2cc1ea4d`). A year-like token (1900 to 2099) needs the
prefix: `pr2026.md` is attributed, `backup-2026.tar` and `notes-2025.md`
are not. A name with no such token, with several (a version such as
`native-surfaces-2-1-284`, a date such as `triage-2026-09-28`), or
starting with a writer's `<TS>Z-` timestamp is not attributed: it stays
unknown and is never removed.
- The number is read as an issue or PR of the repository holding the
memory root, and its state is looked up: `gh api` lists the open items
once per repository, then reads each other number once. A number that is
no issue or PR of the repository is unknown, so it keeps the item. A PR
closed without merging is `closed-unmerged` and keeps the item.
- `report` and each `clean` dry-run path show every state looked up, for
example `stale [#4608 closed]` or `would remove: <path> [#5371 merged]`,
so the confirmation covers the issue or PR each path was matched to
(handoffs and retros included).
- The issue lists the name, a marker file, or the producing branch as
attribution sources. Only the name is read for scratch: the repo defines
no marker-file convention, and no `.work` writer records a branch
(handoff frontmatter is
`type`/`handoff_shape`/`date`/`topic`/`session_id`/`transcript`/`previous_handoff`/`chain`).
Step 3 offers delete or consolidate; `clean` deletes.
- In `~/.work` a bare number has no repository, so a scratch item there
is unknown-state and always kept.

In flight, so kept:

- a slice whose `INDEX.md` `status:` is not `done`, or that holds a
child slice whose status is not `done` (the topic-docs contract makes
that frontmatter field the slice's state; missing and unrecognized
values keep it too)
- a workflow checklist with an unfinished stage
- anything with a `.git` file or directory under it: a clone or worktree
can hold commits that exist nowhere else, and the tracked-path guard
only asks the outer repository
- a change inside `--days` (default 14)
- a later handoff that is itself kept and names the item. A stale
handoff does not keep what it names.
- a handoff or running retro whose text names an issue or PR that is not
closed or merged (a `github.com` URL, `owner/repo#N`, or `#N`), or one
whose state could not be read
- a scratch item whose number is open, a PR closed unmerged, or
unreadable

No writer records an issue or PR in frontmatter, so handoff and retro
references are read from the text. A bare `#N` means the repository
holding the memory root; in `~/.work` it counts as unknown. A link is
looked up only for an item nothing cheaper already keeps, except a
scratch item's, whose state is the attribution the report shows. The
`.git` check likewise runs only on an item nothing cheaper keeps.

The contract designates no top-level `.work` directory as scratch by
name, so the entries of `reviews/`, `exports/`, `overengineering/`,
`enforceability/`, `docs-hygiene/`, and `lanes/` are kind `concern`:
reported, never removed, because their owning skills read them back.
Other tools' free-form output without an issue or PR number (for example
`~/.work/local-otel-claude-code`) is unknown and kept.

## Fix

Placement is session-flow, not repo-hygiene as the issue title says: the
issue body proposes `/session-flow:tidy-work`, session-flow owns the
`.work` layout and the `save_point.py` frontmatter parsers, repo-hygiene
cannot import them without a cross-plugin dependency, and `~/.work` is
not a repo. The "call disk-hygiene's protection list" line is a proposed
direction, not an expected item, and disk-hygiene has no public entry
point for it (only its skill-private `baseline-policy.json`), so reading
it would break skill encapsulation. It is not read.

`normalize` and `clean` are dry runs that print exact absolute paths
until `--apply`. They never modify content git tracks, following the
topic-docs runtime guards:

- a memory root whose `.gitignore` lacks a line `*` is refused
(`~/.work` excepted, which has no fixed self-ignore file)
- every command, `report` included, rejects an existing memory root
outside the repository whose `.gitignore` lacks a line `*` (exit 2,
nothing listed). `memory_dir` comes from the repo-controlled
`.claude/topic-docs.yaml` and `report` is pre-approved, so `memory_dir:
/etc` or `../outside` is refused before anything is enumerated. This is
the guard the handoff writer requires of every memory root; a root below
the repository is unaffected, and a root that does not exist yet reports
as empty
- an item with a tracked path under it is refused
- every command rejects a memory root that is the repository root
(`--memory-dir .`)
- a relative `--memory-dir` resolves against the repository top level
- `--apply` also refuses paths outside the resolved roots and symlinks
leaving them

The skill resolves `memory_dir` through `parse-concern-value.sh` as
handoff does and passes `--memory-dir`. It pre-approves only the
read-only `report` invocation and the read-only parse helper; `--apply`
stays behind the permission flow. Version bump 0.40.2 to 0.41.0 (minor),
changelog entry above main's 0.40.x entries, README, `topic-docs.md`,
catalog and cheat sheet updated.

## Verification

- `uvx pytest -q plugins/session-flow/scripts/tests/test_tidy_work.py`:
45 passed (186 with `test_save_point.py`). Nine cases fail against the
previous `tidy_work.py`: the unattributed handoff, running retro, `done`
slice and finished checklist are kept and absent from the dry run and
`--apply` (tree listing unchanged); a kept handoff that names no issue
keeps the scratch directory it names; the year names (`_name_refs`
directly, and `backup-2026.tar` and `notes-2025.md` beside a closed
`#2026` and a merged `#2025` in the link table) stay unknown while
`pr2026.md` is attributed and removed once `#2026` is closed. The
symlink-escape, tracked-content and missing-self-ignore cases now remove
an attributed scratch directory or handoff (`measure-4608` with `#4608`
merged, a handoff naming a closed `#7`) instead of an unattributed one,
and each fails when its guard is disabled. The stale-handoff-chain case
runs on attributed handoffs and a scratch directory they name. The
memory-root cases still cover an outside root (absolute and
`../outside`) rejected by all three commands with nothing listed and
reported once its `.gitignore` holds `*`, an absent outside root, every
file `normalize` would move classified as a known kind, and `clean
--apply` on a stale root handoff with its sidecar. The scratch cases use
real `.work` names: six attribute (`lint-5371.log`, `measure-4608`,
`pr4120.md`, `reverify-4186`, `scratch-4586-d2cc1ea4d`, `pr2026.md`) and
eleven do not; dry-run and apply remove only closed or merged scratch; a
number that is no issue or PR keeps its item; an item holding a `.git`
directory or file survives even with a merged number.
- `scripts/run-ruff.sh check plugins/session-flow/scripts`: all checks
passed; `format --check` clean on `tidy_work.py` and its test.
- Read-only `tidy_work.py report --days 0 --memory-dir <this repo's
.work>` with live `gh` (`--days 0` turns off the recency window; at the
default 14 days every entry there is kept): 87 items, 6 removable, 81
kept. Removable: `lint-5371.log` `[#5371 merged]`, `measure-4608`
`[#4608 closed]`, `pr4120.md` `[#4120 closed]`, `pr5425-body.md` `[#5425
merged]`, `scratch-4586-d2cc1ea4d` `[#4586 closed]`, and one handoff
whose text names only merged and closed items, with its sidecar.
`reverify-4186` is kept because it holds five git clones and #4186 is
open; `native-surfaces-2-1-284` and `triage-2026-09-28` are unknown and
kept; each of the 14 handoffs names an issue or PR, so none is kept for
lacking one. `clean --days 0` without `--apply` listed those paths with
their `[#N state]`; `--apply` was not run.
- `scripts/check-changelog-parity.sh --check --check-order`: pass;
`scripts/validate-plugins.sh`: pass; `generate-catalog.mjs --check` and
`generate-cheatsheet.mjs --check`: in sync
- `CHECK_SKILL_SKILLS_ROOT=plugins/session-flow/skills bash
plugins/skill-quality/scripts/check-skill.sh tidy-work`: PASS, 0
warnings; `check-evals-quality.sh` on the skill's `evals.json`: 0
warnings; `check-purged-em-dashes.sh`: no em dashes; `markdownlint-cli2`
on the touched markdown: 0 issues
- `bash scripts/affected-tests.sh --run --jobs 4` (shell suites, rerun
on this head): all pass except `scripts/check-script-contract.test.sh`,
whose two `check-html-assets.sh` cases need `npm ci` (htmlhint absent),
unrelated to this change. The selection also names two Python suites the
shell runner does not execute, `test_tidy_work.py` (run above with `uvx
pytest`) and `test_audit_skill_visibility.py` (untouched by this branch)
- `git ls-files -s plugins/session-flow/scripts/tidy_work.py`: mode
100755, as the lint exec-bit step requires of a shebang file
(`save_point.py` is 100755)
- `bash plugins/session-flow/scripts/tidy_work.test.sh`: SKIP locally
(system python3 lacks pytest); the pytest run above covers it

## Related

Related: #3555, #4295, #4228.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant