fix(session-flow): derive retro HANDOFF_DIR from resolved memory_dir

[kyle-sexton]

Summary

The retro Phase 1.1 multi-session snippet (plugins/session-flow/skills/retro/context/session.md) hard-coded HANDOFF_DIR=.work/handoffs directly beneath an instruction to resolve it from the concern file's memory_dir. A copy-as-is execution encoded the bare default and bypassed the memory_dir seam — missing the handoff chain in any repo that relocates its memory tier via .claude/topic-docs.yaml.

The snippet now derives HANDOFF_DIR from the resolved memory_dir: it reads the .claude/topic-docs.yaml concern file's memory_dir key and falls back to .work, matching the topic-docs.yaml parse shape already used elsewhere in the fleet. Comment prose trimmed to describe what the snippet actually performs, pointing to the handoff skill for the full resolution order.

Version bumped 0.10.1 -> 0.10.2 with a matching CHANGELOG entry.

Closes #420

Related

• Work-readiness .work-convention sweep: this is one instance of a broader pattern where copy-as-is snippets encode the bare .work default instead of resolving the memory_dir seam.
• Sibling memory_dir-seam item claude-memory: orphan-rule-check hardcodes .work/ exclusion instead of resolving memory_dir from topic-docs seam #419 in the claude-memory plugin tracks the same seam from the auto-memory side.
• No ADRs or decision-log entries are closed by this change.

[claude]

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

---

PR Review

• Gather context and read changed files
• Review session.md — core logic change
• Review plugin.json — version bump
• Review CHANGELOG.md — entry quality
• Post findings

---

Summary

The fix is correct and well-scoped. The hard-coded HANDOFF_DIR=.work/handoffs was a genuine footgun — any repo using a non-default memory_dir in .claude/topic-docs.yaml would silently find no handoff chain, making multi-session retro aggregation fall back to single-session silently. The new bash snippet resolves the seam properly.

---

session.md — core logic

The fix itself is sound. The sed + parameter-expansion parsing handles the common YAML scalar cases correctly:

• ^memory_dir: is anchored to line start, so nested/indented YAML keys are correctly ignored.
• head -1 takes the first match only.
• %%#* strips inline comments.
• Trailing-slash normalisation prevents //handoffs double-slash.
• Quote stripping handles both "path" and 'path' quoting styles.

One subtlety worth noting: ${VAL//[[:space:]]/} removes all whitespace, not just leading/trailing. For the overwhelmingly common case of ASCII paths this is fine, but a path containing an internal space (e.g. my work dir) would have that space collapsed. This matches how the handoff skill's structure.md snippet comments "resolve it first" without providing YAML-parse code, so it's at least consistent. Raising as an observation rather than a blocker.

Scope gap vs. SKILL.md resolution order. SKILL.md specifies that a CLAUDE.md/.claude/rules/ save-point convention also wins over the default. The new snippet only checks topic-docs.yaml. This is the same gap that existed before (the old code didn't consult either), and the PR description explicitly calls it out as part of a broader sweep tracked in #419. Not a regression introduced here.

Repo-root guard absent. structure.md validates that MEMORY_ROOT doesn't resolve to the repo root before using it. The retro snippet doesn't — but it's read-only (ls + parser invocation), so a bad value yields a harmless empty NEWEST and the parser call fails gracefully. Not a concern at this scope.

Comment quality: improved. Old comment described the resolution intent but not the actual (missing) behaviour; new comment is honest about what the snippet does and explicitly delegates the full order to the handoff skill. ✓

---

plugin.json

Patch bump 0.10.1 → 0.10.2 is the correct semver category for a bug fix with no API/schema change. ✓

---

CHANGELOG.md

Entry is clear, accurate, and mentions both the incorrect prior behaviour and the failure scenario (repos that relocate the memory tier). The Fixed: heading follows the established changelog style. ✓

---

Verdict

Approve. No blocking issues. The bash parsing is robust enough for the input space; the one whitespace-in-path edge case is unlikely and consistent with the rest of the fleet. The narrower scope vs. the full SKILL.md resolution order is intentional and tracked separately.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3042a8e7e9

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

[claude]

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

---

PR Review (synchronize — new commit 1621a15)

• Gather context
• Read changed files
• Review session.md — whitespace fix
• Post findings

---

What changed

The new commit addresses the Codex P2 finding: memory_dir values containing interior whitespace (e.g. .agent work) were silently collapsed by the previous ${VAL//[[:space:]]/} global substitution. The single-line change:

-  VAL="${VAL%%#*}"; VAL="${VAL//[[:space:]]/}"; VAL="${VAL%/}"
+  VAL="${VAL%%#*}"; VAL="${VAL%"${VAL##*[![:space:]]}"}"; VAL="${VAL%/}"

replaces the global whitespace-removal with the standard bash trailing-whitespace-trim idiom:

Step	Expression	Effect
1	${VAL##*[![:space:]]}	Remove the longest prefix ending in a non-space; leaves only trailing spaces
2	${VAL%"<above>"}	Strip that trailing-space suffix from VAL

Leading whitespace was already handled by the sed pattern (s/^memory_dir:[[:space:]]*//p). The only missing piece was the trailing trim, which this idiom provides precisely.

---

Correctness

Edge cases all pass:

• memory_dir: .agent work → VAL=".agent work" → interior space preserved, trailing newline trimmed ✓
• memory_dir: ".work" → quote-stripped to .work ✓
• memory_dir: (empty) → VAL="" → [[ -n "$VAL" ]] guard keeps MEMORY_DIR=.work ✓
• memory_dir: (only spaces) → strips to "" → guard fires ✓
• Nested/indented key ( memory_dir: ...) → ^memory_dir: anchor still ignores it ✓

Scope: No other files changed in this commit. The version bump (0.10.1 → 0.10.2) and CHANGELOG entry are already in place from the prior commit.

Existing observations from the earlier review remain unchanged — the CLAUDE.md/.claude/rules/ save-point resolution gap vs. SKILL.md is tracked in #419 and not a regression here; the repo-root guard absence is still harmless (read-only path, empty NEWEST on bad value).

---

Verdict

No new issues. The whitespace fix is correct, minimal, and exactly addresses the Codex P2 finding. Ready to merge.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1621a1598a

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