fix(claude-memory): resolve memory_dir from topic-docs seam in orphan-rule-check by kyle-sexton · Pull Request #470 · melodic-software/claude-code-plugins

kyle-sexton · 2026-07-19T09:47:26Z

Summary

orphan-rule-check.sh hardcoded the literal .work/ exclusion in its reference search instead of reading memory_dir from the topic-docs seam (.claude/topic-docs.yaml). A consumer that overrides memory_dir therefore had its real memory tier scanned, so ephemeral files there registered false "references" that could mask an orphan always-loaded rule. This couples the check to the convention's default NAME rather than the externalized seam.

Fix

Resolve memory_dir from ${repo_root}/.claude/topic-docs.yaml using the same sed-extract shape as the fleet precedent plugins/docs-hygiene/skills/audit-noise/scripts/lib/noise-shapes.sh:15-38, and exclude THAT path in the git grep pathspec.
Fall back to the convention default .work/ only when the seam is unset.
Update the three .work/ references (header comment, --help text, inline comment) to describe the resolved tier instead of the hardcoded literal.
Tests add discriminating cases proving the excluded tier moves under an override (the overridden dir is excluded AND .work/ stops being excluded — an additive "exclude both" bug fails these) and that the .work/ fallback still holds when the seam is absent.

Verification

bash plugins/claude-memory/skills/audit/scripts/orphan-rule-check.test.sh — all 12 checks pass (3 new).
shellcheck --rcfile=.shellcheckrc on both changed scripts — clean.
typos --config _typos.toml + markdownlint-cli2 --config .markdownlint-cli2.jsonc on CHANGELOG.md — clean (0 issues).

Part of the work-readiness .work-convention sweep: the always-loaded-rule reference search must resolve the in-repo memory tier from the externalized seam, not the default name.
Mirrors the memory_dir-resolution precedent in docs-hygiene noise-shapes.sh (same sed-extract from .claude/topic-docs.yaml).
Sibling seam item session-flow: retro context snippet assigns literal HANDOFF_DIR=.work/handoffs, bypassing memory_dir resolution if copied as-is #420 (retro resolve-memory-dir) applies the same seam-resolution shape in session-flow; this PR is the claude-memory audit-tier counterpart.
No ADRs or decision-log entries are closed by this change (behavior-preserving default; seam-resolution mechanics only).

Closes #419

…-rule-check orphan-rule-check hardcoded the literal `.work/` exclusion instead of reading `memory_dir` from `.claude/topic-docs.yaml`. A consumer that overrides memory_dir had its real memory tier scanned by the reference search, so ephemeral files there registered false references that could mask an orphan always-loaded rule. Resolve `memory_dir` from `${repo_root}/.claude/topic-docs.yaml` using the same sed-extract shape as docs-hygiene's noise-shapes.sh and exclude that path, falling back to `.work/` only when the seam is unset. Comments and --help updated to match. Tests add discriminating cases proving the excluded tier MOVES under an override (the overridden dir is excluded and `.work/` stops being excluded) and that the `.work/` fallback still holds when the seam is absent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

claude · 2026-07-19T09:47:43Z

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

PR Review: fix(claude-memory): resolve memory_dir from topic-docs seam in orphan-rule-check

Gather context from PR description and changed files list
Read orphan-rule-check.sh — review the sed-extract and pathspec logic
Read orphan-rule-check.test.sh — verify new test cases are discriminating
Read plugin.json — confirm version bump
Read CHANGELOG.md — verify entry quality
Synthesize findings and post review

Review

Overall verdict: Approve. The fix is correct, tightly scoped, and well-tested. The implementation faithfully mirrors the fleet precedent. Notes below are observations, not blockers.

Core fix — `orphan-rule-check.sh` lines 62–69 and 91

The sed-extract block is a faithful port of the noise-shapes.sh precedent (lines 25–27 of that file), adapted for the single-key memory_dir case:

seam=$(sed -n 's/^memory_dir:[[:space:]]*//p' "$topic_docs" | head -1)
seam="${seam%%#*}"; seam="${seam//[[:space:]]/}"; seam="${seam%/}"
seam="${seam#\"}"; seam="${seam%\"}"; seam="${seam#\'}"; seam="${seam%\'}"
[[ -n "$seam" ]] && memory_dir="$seam"

Strip order (inline comment → all-whitespace → trailing slash → outer quotes) matches the precedent exactly. The guard [[ -n "$seam" ]] keeps .work as the default when the key is present but empty — correct behaviour.

The pathspec change from ':(exclude).work/' to ":(exclude)${memory_dir}/" is minimal and correct: one variable substitution, no surrounding logic change.

Test coverage — `orphan-rule-check.test.sh` cases 6 & 7

Case 6 (the discriminating override test) is the key correctness check. It proves exclusion moves rather than being additive by:

Placing a reference to a.md only in .scratch/ (the overridden tier) → expects a.md is still an orphan (.scratch/ is excluded).
Placing a reference to b.md only in .work/ → expects b.md is not orphaned (.work/ is no longer excluded).

The assert_not_contains "... b.md not orphan" assertion is exactly the right probe — it would fail under an "exclude both dirs" bug. Well-constructed.

Case 7 (fallback) is a straight regression guard. No issues.

The make_repo helper's unset GIT_DIR GIT_INDEX_FILE GIT_WORK_TREE GIT_COMMON_DIR before each fixture init is correct hygiene — prevents fixture commits from bleeding into the real repo.

Minor observations (non-blocking)

1. memory_dir not escaped for git pathspec magic characters.
Line 91 embeds $memory_dir directly in the pathspec string. If the value contained *, ?, or [ these would be interpreted by git's pathspec glob. The noise-shapes.sh precedent handles this for its regex context (line 32: sed "s/[.[\\*^\$()+?{|]/\\\\&/g"). In the pathspec context the risk is extremely low (a memory_dir value with glob chars would be broken in many other ways), but it is a behavioural difference from the precedent worth being aware of for future callers that extend this pattern.

2. Whitespace stripping removes embedded spaces from the path value.
seam="${seam//[[:space:]]/}" strips all whitespace, not just leading/trailing. A value like my work would become mywork. This matches the precedent and the YAML convention (paths don't embed spaces), so it's not a defect — just a known limitation of the simple sed-extract over a proper YAML parse.

3. grep -vcxF -- "$file" relies on relative-path parity with git grep -l output.
Both $file (from the for loop over .claude/rules/*.md) and git grep -l output are relative to the same $repo_root CWD, so the whole-line match is reliable. This would silently miscount if one were ever absolute. Not an issue in the current code, but worth a note if this idiom is factored into a shared lib later.

Plugin version and CHANGELOG

Version bump 0.2.0 → 0.2.1 is appropriate (patch: bug fix, no interface change). CHANGELOG entry is clear and accurate.

No issues requiring changes before merge.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25689522b8

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

…ry_dir parse The orphan-rule-check memory_dir extract stripped every whitespace character with ${seam//[[:space:]]/}, collapsing interior spaces in a valid quoted relative root: memory_dir: ".work artifacts" resolved to .workartifacts, so the real memory tier was not excluded from the reference search and a tracked artifact there could still count as a reference and mask an orphan rule — the regression this change prevents. Trim only trailing whitespace (${seam%"${seam##*[![:space:]]}"}), the same minimal fix the sibling retro parse used (fix/420): sed already removes leading whitespace after the colon and the closing quote shields significant trailing space before quote-stripping. Interior spaces in quoted and plain scalars are preserved; unquoted trailing whitespace, inline comments, and trailing slashes still trim. Adds regression Case 8 (quoted ".scratch dir" tier) that fails under the collapsing strip. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

claude · 2026-07-19T10:59:58Z

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

PR Review: fix(claude-memory): resolve memory_dir + preserve interior whitespace in orphan-check

Gather context from PR history and changed files
Read orphan-rule-check.sh — verify whitespace-strip fix (e4bf13c)
Read orphan-rule-check.test.sh — verify Case 8 regression guard
Read plugin.json and CHANGELOG.md — confirm metadata consistency
Synthesize findings and post review

Overall verdict: Approve. The whitespace-strip fix in e4bf13c is correct and tightly targeted; Case 8 is a well-constructed discriminating regression guard. Notes below are non-blocking.

Core whitespace fix — `orphan-rule-check.sh` line 66

The fix replaces the collapsing global strip:

seam="${seam//[[:space:]]/}"   # strips ALL whitespace — collapses interior spaces

with the trailing-trim idiom:

seam="${seam%"${seam##*[![:space:]]}"}"`   # strips only trailing whitespace

This is the minimal correct change: sed already drops leading whitespace after the memory_dir: key, and the outer-quote stripping that follows handles quoted values. Stripping only the tail preserves interior spaces like .scratch dir, which is the bug Codex flagged (P2). The idiom matches the fix applied in the sibling fix/420 (1621a15), making the pattern consistent across the fleet.

Strip-order trace for memory_dir: ".scratch dir":

Step	Expression	Result
`sed`	strips `memory_dir:[[:space:]]*`	`".scratch dir"`
`%%#*`	strips inline comment	`".scratch dir"`
trailing-trim	strips trailing whitespace	`".scratch dir"`
`%/`	strips trailing slash	`".scratch dir"`
`#\"` / `%\"`	strips outer quotes	`.scratch dir`

Result: memory_dir=".scratch dir" → pathspec :(exclude).scratch dir/ — correct.

Under the old collapsing strip step 2 would produce ".scratchdir" → pathspec :(exclude).scratchdir/ → real tier .scratch dir/ included → false reference masks orphan. ✓ fix prevents that.

Case 8 regression guard — `orphan-rule-check.test.sh` lines 127–145

The fixture is correctly constructed:

mkdir -p "$WS/.scratch dir" — creates a tracked directory whose name contains an interior space.
printf 'see d.md\n' >"$WS/.scratch dir/refD.md" — places the only reference to d.md inside that tier.
assert_contains "... d.md orphan" "$OUT" "d.md" — under the OLD code the strip collapses the path, the tier is not excluded, the reference is found, and d.md is not reported as an orphan → assertion fails. Under the fix the tier is excluded and d.md is an orphan → assertion passes.

The test correctly fails under the bug and passes under the fix. Well-constructed.

Note: the test relies on git add -A tracking a path with an interior space inside $TEST_TMPDIR. This is standard git behaviour; the fixture is fine.

Minor observations (non-blocking, carry-over from first review)

1. CHANGELOG 0.2.1 entry is silent on the whitespace-preservation change.
The entry accurately describes the seam-resolution fix but doesn't mention that e4bf13c also corrected the interior-whitespace collapse. Since these are squash-merged as one version, a second bullet (e.g. "Memory tier paths containing interior whitespace are now preserved correctly") would improve completeness, but is not required.

2. memory_dir not escaped for git pathspec magic characters (carry-over observation 1).
Still present; still very low risk given the constraint that memory_dir must be a valid directory path. The noise-shapes.sh precedent escapes for regex context — the pathspec context is different and the exposure is negligible for realistic values. Acceptable as-is.

3. grep -vcxF -- "$file" relies on relative-path parity (carry-over observation 3).
Still present; still correct given both operands are relative to the same $repo_root CWD. Worth noting if this idiom is ever factored into a shared lib.

No issues requiring changes before merge.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e4bf13c18e

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

kyle-sexton · 2026-07-19T11:16:53Z

Re: claude[bot] review pass (run 29684331427) — verdict: Approve, no blockers. Three non-blocking observations, each self-dispositioned by the reviewer; classified below.

#	Finding	Classification	Evidence	Reacted
1	CHANGELOG `0.2.1` entry is silent on the interior-whitespace-preservation change in `e4bf13c`	VALID (defer)	Reviewer marks it "would improve completeness, but is not required". The existing bullet ("reads `memory_dir` … and excludes that path") already covers correct value resolution; interior-whitespace handling is an implementation detail of resolving that value. Not worth a new commit that resets the double-Approve / re-review clock on an otherwise-ready PR.	👍
2	`memory_dir` not escaped for git pathspec magic chars (`*` `?` `[`) at line 91	VALID (defer)	Reviewer marks it "Acceptable as-is … exposure is negligible for realistic values" (a `memory_dir` with glob chars would be broken in many other ways). Same shared hand-rolled-parse hardening class tracked in #474 — belongs in the shared resolver, not an ad-hoc patch of this one copy.	👍
3	`grep -vcxF -- "$file"` relies on relative-path parity with `git grep -l` output	INCORRECT (no defect)	Reviewer confirms the code is correct: both `$file` and `git grep -l` output are relative to the same `$repo_root` CWD, so the whole-line match is reliable. It is a forward-looking note for a future shared-lib refactor, not a defect in this PR. No action.	👍

…ia shared helper (#756) ## Summary The topic-docs `memory_dir` seam was parsed inline at two sites with a naive `${val%%#*}`-first strip — the same naive-string-op class the #469/#470 whitespace fixes closed, left as a review-deferred residual. It truncated a legitimately quoted value containing `#` (`memory_dir: "a#b"` → `a`) because it stripped a "comment" before resolving surrounding quotes, and an absent concern file collapsed straight to `.work` without consulting the contract's rung-2 (a save-point convention declared in `CLAUDE.md` / `.claude/rules`). Per the issue's acceptance criteria, both sites now delegate to **one** shared parser rather than re-forking the duplicated block. ## Fix - **`lib/parse-concern-value.sh`** (new) — the single seam parser. Order is quote-resolution **first**, then a quote-aware comment strip (unquoted ` #…` only; `a#b` / `.work/#topic` preserved), then whitespace-trim, then trailing-slash-normalize. Takes `<concern-file> <key> [fallback]`; the `fallback` is the **caller-resolved** rung-2 location (prose is an inference source, not a machine key — the agent resolves it and passes it in). Emits empty only when nothing resolves, so the caller owns the documented `.work` default. - **Both consumers wired through it:** claude-memory `orphan-rule-check.sh` (real script) and the session-flow retro `HANDOFF_DIR` snippet (`retro/context/session.md`). The non-interactive orphan check degrades to `.work` and notes that inferred/interactive resolution is the calling skill's job, rather than silently collapsing rung 2. - **Materialization + drift gate:** copies are synced from `lib/` into each plugin (installed plugins are cache-isolated, so each must be self-contained — the `lib/hook-utils.sh` precedent). New `scripts/sync-parse-concern-value.sh` (`--check` / `--check-bump`) + a `parse-concern-value-sync` CI job keep the copies byte-identical and enforce version bumps when the lib changes. The two copies live at different relpaths, so the generic cross-plugin drift check does not cluster them — a dedicated gate is required. ## Verification All run locally (Git Bash): - `lib/parse-concern-value.test.sh` — **19/19 pass**, incl. regressions: double/single-quoted `a#b` and `.work/#topic` keep `#`; quoted value with trailing comment drops the comment but keeps the inner `#`; absent/empty key returns the supplied fallback; held behavior (unquoted trailing ` # comment` stripped, whitespace trimmed, interior quoted space preserved, trailing slash normalized). - `orphan-rule-check.test.sh` — **14/14 pass**, incl. new Case 9: a ref inside a quoted `".scratch#dir"` tier is correctly excluded (the old `%%#*` truncated to `.scratch`, masking the orphan). - `scripts/sync-parse-concern-value.sh --check` — copies match source. - `scripts/check-cross-plugin-source-drift.sh --check` — no unregistered/drifted clusters. - `shellcheck` + `shfmt -d` — clean on all new/edited scripts. - `scripts/validate-plugins.sh` — all manifests + catalog valid. Closes #482 ## Related Realizes the shared-helper shape #474 called for, scoped to #482's two sites. #474 (wiring the *remaining* consumers to this helper) stays parked and is unblocked by this PR. --------- Co-authored-by: Claude Opus <noreply@anthropic.com>

…o shared parse-concern-value helper (#847) ## Summary Final wiring of the ratified bounded-widen scope on #474: fold the last two verified hand-rolled `memory_dir`/`contract_dir` copies into #482's already-landed `parse-concern-value.sh` helper (merged via #756). - **`plugins/docs-hygiene/skills/audit-noise/scripts/lib/noise-shapes.sh`** — `audit_noise_convention_roots_pattern` hand-rolled the same naive `${val%%#*}`-first strip #482 fixed at its two hot sites (truncates a quoted value at an interior `#`), plus a defect unique to this copy: `${val//[[:space:]]/}` collapsed *all* interior whitespace, and there was no quote-stripping. Now resolves both keys through the shared helper, materialized to `plugins/docs-hygiene/skills/audit-noise/scripts/lib/parse-concern-value.sh` (byte-identical + mode-identical, `100755`, to the two existing copies) and registered in `scripts/sync-parse-concern-value.sh`'s `copies` array — the same upstream-SSOT + sync-materialization pattern as the two prior sites (installed plugins are cache-isolated and cannot `source` a sibling's file). - **`plugins/session-flow/skills/handoff/context/structure.md`** L116 — replaced the placeholder note (`# the concern file's memory_dir when set — resolve it first`, no mechanism named) with a pointer to the shared helper via the retro skill's Phase 1.1 as the worked call form. Doc note only: the handoff skill has no script of its own to rewire. Per the operator's ratified bounded-widen decision (2026-07-19T23:26Z) and the tower's 2026-07-21 decision comment on #474, the broader tracker-seam-resolution class (#470/#469/#432/#365/#369) stays explicitly OUT of scope. Version bumps stack past two independently-merged sibling PRs that landed while this one was open: docs-hygiene `0.8.0` → `0.8.2` (stacks past #846's `0.8.1`, an exact version collision resolved by re-numbering this PR's entry on top and preserving #846's CHANGELOG entry underneath); session-flow `0.11.0` → `0.12.1` (stacks past a merged `0.12.0` that added the `orient` skill). ## Related - #482 / #756 — landed the shared `parse-concern-value.sh` helper and its first two consumers; this PR extends it to the remaining two per the ratified bounded-widen scope. - #846 — merged concurrently with this PR and independently bumped `docs-hygiene` to `0.8.1`; this PR's version was restacked to `0.8.2` on top, both CHANGELOG entries preserved. - #470 / #469 / #432 / #365 / #369 — the broader tracker-seam-resolution class; explicitly excluded from this PR's scope per the operator's ratified decision. ## Test plan - [x] `bash plugins/docs-hygiene/skills/audit-noise/scripts/detect.test.sh` — 34/34 pass, including a new regression case: a quoted `memory_dir: ".scratch#dir/" # trailing comment` now correctly resolves to `.scratch#dir` (flags `.scratch#dir/foo/` as a concrete slice) instead of the old parser's silent truncation. - [x] Manual before/after demonstration of the two defects the old parser had: - Quoted + interior `#` + trailing comment + trailing slash: old `[.scratch]` (wrong — truncated inside the quotes) vs new `[.scratch#dir]` (correct). - Unquoted value with interior whitespace: old `[mydir]` (wrong — collapsed) vs new `[my dir]` (correct, surrounding whitespace trimmed only). - [x] `scripts/sync-parse-concern-value.sh --check` — all 3 plugin copies match `lib/parse-concern-value.sh`. - [x] `scripts/check-cross-plugin-source-drift.sh --check` — no unregistered/drifted clusters (the three copies have distinct within-plugin relpaths, so the generic drift check doesn't need to cluster them; the dedicated sync gate already covers them). - [x] `scripts/check-changelog-parity.sh --check` and `--check-bump origin/main` — both touched plugins (`docs-hygiene` 0.8.0→0.8.2, `session-flow` 0.11.0→0.12.1) carry matching CHANGELOG entries. - [x] `shellcheck` + `shfmt -d` — clean on all new/edited scripts. - [x] `scripts/validate-plugins.sh` — all manifests + catalog valid. Closes #474 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

chatgpt-codex-connector Bot reviewed Jul 19, 2026

View reviewed changes

Comment thread plugins/claude-memory/skills/audit/scripts/orphan-rule-check.sh Outdated

kyle-sexton mentioned this pull request Jul 19, 2026

claude-memory/session-flow/docs-hygiene: memory_dir/contract_dir config-value parsing is hand-rolled and copy-pasted across ≥4 files, not a shared helper #474

Closed

chatgpt-codex-connector Bot reviewed Jul 19, 2026

View reviewed changes

Comment thread plugins/claude-memory/skills/audit/scripts/orphan-rule-check.sh

kyle-sexton merged commit f4b8d21 into main Jul 19, 2026
15 checks passed

kyle-sexton deleted the fix/419-orphan-check-resolve-memory-dir branch July 19, 2026 16:20

This was referenced Jul 19, 2026

ops: loop-lane telemetry — running per-lane status (babysit / triage / work) #502

Open

fix(claude-memory,session-flow): quote-aware topic-docs seam parser via shared helper #756

Merged

kyle-sexton mentioned this pull request Jul 21, 2026

fix(docs-hygiene,session-flow): wire remaining memory_dir consumers to shared parse-concern-value helper #847

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(claude-memory): resolve memory_dir from topic-docs seam in orphan-rule-check#470

fix(claude-memory): resolve memory_dir from topic-docs seam in orphan-rule-check#470
kyle-sexton merged 2 commits into
mainfrom
fix/419-orphan-check-resolve-memory-dir

kyle-sexton commented Jul 19, 2026

Uh oh!

claude Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

claude Bot commented Jul 19, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

kyle-sexton commented Jul 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

kyle-sexton commented Jul 19, 2026

Summary

Fix

Verification

Related

Uh oh!

claude Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: fix(claude-memory): resolve memory_dir from topic-docs seam in orphan-rule-check

Review

Core fix — orphan-rule-check.sh lines 62–69 and 91

Test coverage — orphan-rule-check.test.sh cases 6 & 7

Minor observations (non-blocking)

Plugin version and CHANGELOG

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

claude Bot commented Jul 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: fix(claude-memory): resolve memory_dir + preserve interior whitespace in orphan-check

Core whitespace fix — orphan-rule-check.sh line 66

Case 8 regression guard — orphan-rule-check.test.sh lines 127–145

Minor observations (non-blocking, carry-over from first review)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

kyle-sexton commented Jul 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Jul 19, 2026 •

edited

Loading

Core fix — `orphan-rule-check.sh` lines 62–69 and 91

Test coverage — `orphan-rule-check.test.sh` cases 6 & 7

claude Bot commented Jul 19, 2026 •

edited

Loading

Core whitespace fix — `orphan-rule-check.sh` line 66

Case 8 regression guard — `orphan-rule-check.test.sh` lines 127–145