feat: claude code session auto-capture + approved-knowledge recall#303
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds passive Claude Code session auto-capture (observe/finalize/banner) that files review-gated PENDING page proposals, a session-start recall digest of approved knowledge, JSON-merge-based adapter install support, explicit UTF-8 encoding across KB storage, adapter hook wiring, smoke test scripts, and related documentation/changelog updates. ChangesSession Auto-Capture and Recall
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ClaudeCode as Claude Code Hooks
participant CLI as vouch CLI
participant Capture as capture.py
participant Store as KBStore/Proposals
ClaudeCode->>CLI: PostToolUse -> capture observe (stdin payload)
CLI->>Capture: summarize_tool() + observe()
Capture->>Capture: append deduped observation to JSONL buffer
ClaudeCode->>CLI: SessionEnd -> capture finalize
CLI->>Capture: finalize(session_id)
Capture->>Capture: read buffer, collect git diff backstop
Capture->>Capture: build_summary_body()
Capture->>Store: propose_page(PENDING, proposed_by=vouch-capture)
Store-->>Capture: proposal id
Capture->>Capture: delete buffer file
Capture-->>CLI: capture summary + proposal id
ClaudeCode->>CLI: SessionStart -> capture banner
CLI->>Capture: pending_count()
Capture->>Store: count PENDING proposals by actor
Store-->>CLI: pending count
CLI-->>ClaudeCode: awaiting-review banner text
ClaudeCode->>CLI: SessionStart -> vouch recall
CLI->>Store: build_digest(approved claims/pages)
Store-->>CLI: digest text
CLI-->>ClaudeCode: injected <vouch-approved-knowledge> block
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
docs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
src/vouch/capture.py (1)
278-282: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
pending_countinherits an O(all proposals ever decided) scan on every SessionStart banner check.
store.list_proposals(ProposalStatus.PENDING)(storage.py) unconditionally globs and parses every YAML file under bothproposed/anddecided/before filtering by status — decided proposals can never bePENDING, so that half of the scan is pure waste. Sincepending_countis now invoked from thecapture bannerhook, likely once per Claude Code session start, this cost grows unbounded with KB history size.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/capture.py` around lines 278 - 282, The pending_count check is doing unnecessary work because it calls store.list_proposals(ProposalStatus.PENDING), which still scans and parses decided proposals even though they can never be pending. Update pending_count in capture.py to use a more targeted storage path or listing helper from KBStore/storage.py that only reads proposed items, and keep the CAPTURE_ACTOR filter there so the SessionStart banner check avoids the full historical scan.src/vouch/recall.py (1)
19-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImporting a private symbol across module boundary.
_RETRACTED_CLAIM_STATUSESis underscore-prefixed in.context, signaling it's internal to that module. Reaching across the module boundary to consume it here couplesrecall.pytocontext.py's private implementation detail and can silently break ifcontext.pyrenames/removes it without treating it as a public contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/recall.py` at line 19, The recall.py import is reaching into a private module detail by using _RETRACTED_CLAIM_STATUSES from context; replace that cross-module dependency with a public constant or helper exposed by context.py, and update the recall logic to consume the public symbol instead of the underscore-prefixed one.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vouch/capture.py`:
- Around line 40-57: `load_config` in `capture.py` only catches YAML/read
errors, so invalid `capture` field types can still crash instead of using
defaults. Update `load_config` to handle malformed `enabled`,
`min_observations`, and `dedup_window_seconds` values safely by wrapping their
conversions or validating them before constructing `CaptureConfig`, and ensure
any bad `capture` section falls back to `CaptureConfig()` just like the other
early-return paths. Use the existing `load_config` and `CaptureConfig` symbols
as the place to centralize this fallback behavior so callers like
`capture_finalize_cmd` don’t see uncaught `ValueError` or `TypeError`.
- Around line 246-259: The finalize flow in capture should clean up the session
buffer file even when cfg.enabled is false. Update finalize in capture.py so the
disabled branch mirrors the below-min branch by deleting the
buffer_path(session_id) file if it exists before returning, using the existing
path variable and _read_observations/load_config flow. Keep the return payload
the same, just ensure the scratch buffer is not orphaned when capture is turned
off mid-session.
- Line 43: The config load in vouch capture is missing an explicit UTF-8
encoding, unlike recall.load_config, so update the store.config_path.read_text()
call in the capture config-loading path to read with encoding="utf-8". Keep the
change localized around the yaml.safe_load(...) call so the config parsing
behavior stays the same while matching the UTF-8 hardening used elsewhere.
In `@src/vouch/cli.py`:
- Around line 1393-1419: The capture_banner_cmd and recall_cmd paths are missing
error handling around storage reads, so malformed KB YAML can abort the
SessionStart hook. Update the logic around capture_mod.pending_count(store) and
recall_mod.build_digest(store, max_chars=cfg.max_chars) to catch read/parse
failures from the underlying store access and return quietly or emit a non-fatal
warning instead of raising. Keep the guard localized in capture_banner_cmd and
recall_cmd, using the existing _capture_store, capture_mod, and recall_mod entry
points.
- Around line 1365-1391: The finalize hook path in capture_finalize_cmd lacks
the same crash guard used by capture_observe_cmd, so exceptions from
capture_mod.finalize can abort the host session. Wrap the body of
capture_finalize_cmd (from payload parsing through _emit_json(result)) in a
broad exception handler that returns on failure, preserving the hook contract
that hooks must never abort the host. Keep the existing session_id,
_capture_store, and capture_mod.finalize flow intact, just make the SessionEnd
hook resilient to any unexpected error.
In `@src/vouch/install_adapter.py`:
- Around line 309-310: Update the comment near the hooks grouping logic in
install_adapter so it removes first-person wording like "we don't fan out
groups" and uses neutral, guideline-compliant phrasing instead. Keep the intent
tied to the existing hook-deduplication behavior in the nearby comment block,
but rewrite it to explain the grouping rationale without using "we" or similar
marketing-style tone.
- Around line 340-341: The merge in _install_json_merge currently assumes
target_group["hooks"] is a list, so a malformed existing group can make
target_group.setdefault("hooks", []).extend(fresh) raise AttributeError and
abort install(). Update the hook-group merge logic to validate or coerce the
existing hooks value before extending it, and if it is not a list, replace it
with a list or skip merging into that group so the function keeps the same
tolerant behavior as the rest of install_adapter.py. Use the target_group and
_install_json_merge symbols to locate the fix.
In `@src/vouch/recall.py`:
- Around line 35-49: The `load_config` function in `src/vouch/recall.py` has the
same defaults-fallback gap as `capture.load_config`: `recall.max_chars` is
converted with `int(...)` outside the existing `try/except`, so invalid config
values can raise instead of returning `RecallConfig()` as promised. Update
`load_config` to catch malformed `max_chars` values when building the
`RecallConfig`, and fall back to defaults for any parsing/conversion error while
keeping the current behavior for missing or non-dict `recall` blocks.
---
Nitpick comments:
In `@src/vouch/capture.py`:
- Around line 278-282: The pending_count check is doing unnecessary work because
it calls store.list_proposals(ProposalStatus.PENDING), which still scans and
parses decided proposals even though they can never be pending. Update
pending_count in capture.py to use a more targeted storage path or listing
helper from KBStore/storage.py that only reads proposed items, and keep the
CAPTURE_ACTOR filter there so the SessionStart banner check avoids the full
historical scan.
In `@src/vouch/recall.py`:
- Line 19: The recall.py import is reaching into a private module detail by
using _RETRACTED_CLAIM_STATUSES from context; replace that cross-module
dependency with a public constant or helper exposed by context.py, and update
the recall logic to consume the public symbol instead of the underscore-prefixed
one.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 71ad285d-2d27-4180-855b-f56218de0825
📒 Files selected for processing (18)
CHANGELOG.mdMakefileREADME.mdadapters/claude-code/.claude/settings.jsonadapters/claude-code/install.yamldocs/superpowers/plans/2026-07-01-vouch-session-autocapture.mddocs/superpowers/specs/2026-07-01-vouch-session-autocapture-design.mdscripts/smoke-capture.shscripts/smoke-recall.shsrc/vouch/__main__.pysrc/vouch/capture.pysrc/vouch/cli.pysrc/vouch/install_adapter.pysrc/vouch/recall.pysrc/vouch/storage.pytests/test_capture.pytests/test_install_adapter.pytests/test_recall.py
| def load_config(store: KBStore) -> CaptureConfig: | ||
| """Read ``capture:`` from config.yaml; fall back to defaults.""" | ||
| try: | ||
| loaded = yaml.safe_load(store.config_path.read_text()) | ||
| except (OSError, yaml.YAMLError): | ||
| return CaptureConfig() | ||
| if not isinstance(loaded, dict): | ||
| return CaptureConfig() | ||
| raw = loaded.get("capture") | ||
| if not isinstance(raw, dict): | ||
| return CaptureConfig() | ||
| return CaptureConfig( | ||
| enabled=bool(raw.get("enabled", DEFAULT_ENABLED)), | ||
| min_observations=int(raw.get("min_observations", DEFAULT_MIN_OBSERVATIONS)), | ||
| dedup_window_seconds=float( | ||
| raw.get("dedup_window_seconds", DEFAULT_DEDUP_WINDOW_SECONDS) | ||
| ), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
load_config doesn't actually fall back to defaults on malformed values.
The docstring promises "fall back to defaults" on read failure, and the try/except only guards yaml.safe_load against (OSError, yaml.YAMLError). The int(...)/float(...) conversions on lines 53-56 run outside that guard, so a config like capture:\n min_observations: "many" raises an uncaught ValueError/TypeError instead of falling back. Since capture_finalize_cmd (cli.py) has no exception guard around capture_mod.finalize(), this would crash the SessionEnd hook.
🩹 Proposed fix
raw = loaded.get("capture")
if not isinstance(raw, dict):
return CaptureConfig()
- return CaptureConfig(
- enabled=bool(raw.get("enabled", DEFAULT_ENABLED)),
- min_observations=int(raw.get("min_observations", DEFAULT_MIN_OBSERVATIONS)),
- dedup_window_seconds=float(
- raw.get("dedup_window_seconds", DEFAULT_DEDUP_WINDOW_SECONDS)
- ),
- )
+ try:
+ return CaptureConfig(
+ enabled=bool(raw.get("enabled", DEFAULT_ENABLED)),
+ min_observations=int(raw.get("min_observations", DEFAULT_MIN_OBSERVATIONS)),
+ dedup_window_seconds=float(
+ raw.get("dedup_window_seconds", DEFAULT_DEDUP_WINDOW_SECONDS)
+ ),
+ )
+ except (TypeError, ValueError):
+ return CaptureConfig()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def load_config(store: KBStore) -> CaptureConfig: | |
| """Read ``capture:`` from config.yaml; fall back to defaults.""" | |
| try: | |
| loaded = yaml.safe_load(store.config_path.read_text()) | |
| except (OSError, yaml.YAMLError): | |
| return CaptureConfig() | |
| if not isinstance(loaded, dict): | |
| return CaptureConfig() | |
| raw = loaded.get("capture") | |
| if not isinstance(raw, dict): | |
| return CaptureConfig() | |
| return CaptureConfig( | |
| enabled=bool(raw.get("enabled", DEFAULT_ENABLED)), | |
| min_observations=int(raw.get("min_observations", DEFAULT_MIN_OBSERVATIONS)), | |
| dedup_window_seconds=float( | |
| raw.get("dedup_window_seconds", DEFAULT_DEDUP_WINDOW_SECONDS) | |
| ), | |
| ) | |
| def load_config(store: KBStore) -> CaptureConfig: | |
| """Read ``capture:`` from config.yaml; fall back to defaults.""" | |
| try: | |
| loaded = yaml.safe_load(store.config_path.read_text()) | |
| except (OSError, yaml.YAMLError): | |
| return CaptureConfig() | |
| if not isinstance(loaded, dict): | |
| return CaptureConfig() | |
| raw = loaded.get("capture") | |
| if not isinstance(raw, dict): | |
| return CaptureConfig() | |
| try: | |
| return CaptureConfig( | |
| enabled=bool(raw.get("enabled", DEFAULT_ENABLED)), | |
| min_observations=int(raw.get("min_observations", DEFAULT_MIN_OBSERVATIONS)), | |
| dedup_window_seconds=float( | |
| raw.get("dedup_window_seconds", DEFAULT_DEDUP_WINDOW_SECONDS) | |
| ), | |
| ) | |
| except (TypeError, ValueError): | |
| return CaptureConfig() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/capture.py` around lines 40 - 57, `load_config` in `capture.py`
only catches YAML/read errors, so invalid `capture` field types can still crash
instead of using defaults. Update `load_config` to handle malformed `enabled`,
`min_observations`, and `dedup_window_seconds` values safely by wrapping their
conversions or validating them before constructing `CaptureConfig`, and ensure
any bad `capture` section falls back to `CaptureConfig()` just like the other
early-return paths. Use the existing `load_config` and `CaptureConfig` symbols
as the place to centralize this fallback behavior so callers like
`capture_finalize_cmd` don’t see uncaught `ValueError` or `TypeError`.
| def load_config(store: KBStore) -> CaptureConfig: | ||
| """Read ``capture:`` from config.yaml; fall back to defaults.""" | ||
| try: | ||
| loaded = yaml.safe_load(store.config_path.read_text()) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Missing explicit UTF-8 encoding, inconsistent with recall.py.
recall.load_config reads config with read_text(encoding="utf-8") (recall.py:38), but this reads store.config_path.read_text() without an encoding. Given the PR's own rationale for UTF-8 hardening elsewhere in storage.py (locale-dependent default encoding mangles non-ASCII on bare Linux containers), this same class of bug can resurface here if config.yaml contains non-ASCII (e.g. comments).
🩹 Proposed fix
- loaded = yaml.safe_load(store.config_path.read_text())
+ loaded = yaml.safe_load(store.config_path.read_text(encoding="utf-8"))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| loaded = yaml.safe_load(store.config_path.read_text()) | |
| loaded = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/capture.py` at line 43, The config load in vouch capture is missing
an explicit UTF-8 encoding, unlike recall.load_config, so update the
store.config_path.read_text() call in the capture config-loading path to read
with encoding="utf-8". Keep the change localized around the yaml.safe_load(...)
call so the config parsing behavior stays the same while matching the UTF-8
hardening used elsewhere.
| """Roll a session buffer into one PENDING summary proposal. No approve().""" | ||
| cfg = config or load_config(store) | ||
| path = buffer_path(store, session_id) | ||
| observations = _read_observations(path) | ||
| if not cfg.enabled: | ||
| return {"captured": len(observations), "summary_proposal_id": None, | ||
| "skipped": "disabled"} | ||
| changed_files, git_stat = _git_changes(cwd or Path.cwd()) | ||
| total = len(observations) + len(changed_files) | ||
| if total < cfg.min_observations: | ||
| if path.exists(): | ||
| path.unlink() | ||
| return {"captured": total, "summary_proposal_id": None, | ||
| "skipped": "below-min"} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Buffer file is left behind when capture is disabled mid-session.
When cfg.enabled is False, finalize returns immediately (lines 250-252) without deleting path, unlike the below-min branch (line 257) which explicitly cleans up. If capture was enabled long enough to accumulate observations and then disabled before SessionEnd, the gitignored scratch buffer for that session is orphaned forever.
🩹 Proposed fix
if not cfg.enabled:
+ if path.exists():
+ path.unlink()
return {"captured": len(observations), "summary_proposal_id": None,
"skipped": "disabled"}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| """Roll a session buffer into one PENDING summary proposal. No approve().""" | |
| cfg = config or load_config(store) | |
| path = buffer_path(store, session_id) | |
| observations = _read_observations(path) | |
| if not cfg.enabled: | |
| return {"captured": len(observations), "summary_proposal_id": None, | |
| "skipped": "disabled"} | |
| changed_files, git_stat = _git_changes(cwd or Path.cwd()) | |
| total = len(observations) + len(changed_files) | |
| if total < cfg.min_observations: | |
| if path.exists(): | |
| path.unlink() | |
| return {"captured": total, "summary_proposal_id": None, | |
| "skipped": "below-min"} | |
| """Roll a session buffer into one PENDING summary proposal. No approve().""" | |
| cfg = config or load_config(store) | |
| path = buffer_path(store, session_id) | |
| observations = _read_observations(path) | |
| if not cfg.enabled: | |
| if path.exists(): | |
| path.unlink() | |
| return {"captured": len(observations), "summary_proposal_id": None, | |
| "skipped": "disabled"} | |
| changed_files, git_stat = _git_changes(cwd or Path.cwd()) | |
| total = len(observations) + len(changed_files) | |
| if total < cfg.min_observations: | |
| if path.exists(): | |
| path.unlink() | |
| return {"captured": total, "summary_proposal_id": None, | |
| "skipped": "below-min"} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/capture.py` around lines 246 - 259, The finalize flow in capture
should clean up the session buffer file even when cfg.enabled is false. Update
finalize in capture.py so the disabled branch mirrors the below-min branch by
deleting the buffer_path(session_id) file if it exists before returning, using
the existing path variable and _read_observations/load_config flow. Keep the
return payload the same, just ensure the scratch buffer is not orphaned when
capture is turned off mid-session.
| @capture.command("finalize") | ||
| @click.option("--session-id", default=None, help="Session id (else read from stdin payload).") | ||
| def capture_finalize_cmd(session_id: str | None) -> None: | ||
| """Roll a session buffer into a PENDING summary (SessionEnd hook payload on stdin).""" | ||
| payload: dict[str, Any] = {} | ||
| if not sys.stdin.isatty(): | ||
| raw = sys.stdin.read() | ||
| if raw.strip(): | ||
| try: | ||
| loaded = json.loads(raw) | ||
| if isinstance(loaded, dict): | ||
| payload = loaded | ||
| except json.JSONDecodeError: | ||
| payload = {} | ||
| sid = session_id or str(payload.get("session_id") or "") | ||
| if not sid: | ||
| return | ||
| store = _capture_store() | ||
| if store is None: | ||
| return | ||
| cwd = Path(str(payload.get("cwd") or ".")).resolve() | ||
| result = capture_mod.finalize( | ||
| store, sid, cwd=cwd, project=cwd.name, | ||
| generated_at=datetime.now(UTC).isoformat(), | ||
| ) | ||
| _emit_json(result) | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
capture finalize has no crash guard, unlike capture observe.
capture_observe_cmd wraps its entire body in except Exception: return specifically because, per _capture_store()'s docstring, "hooks must never abort the host." capture_finalize_cmd makes the same trust assumption (it's invoked from the SessionEnd hook) but has no equivalent guard around capture_mod.finalize(...). Any exception there — a malformed capture.yaml value (see companion comment in capture.py), a propose_page validation error, or an unexpected storage failure — propagates out of the hook, which can abort session teardown and silently lose the pending session summary this feature exists to capture.
🩹 Proposed fix
cwd = Path(str(payload.get("cwd") or ".")).resolve()
- result = capture_mod.finalize(
- store, sid, cwd=cwd, project=cwd.name,
- generated_at=datetime.now(UTC).isoformat(),
- )
- _emit_json(result)
+ try:
+ result = capture_mod.finalize(
+ store, sid, cwd=cwd, project=cwd.name,
+ generated_at=datetime.now(UTC).isoformat(),
+ )
+ except Exception:
+ # a capture failure must never break the SessionEnd hook / host.
+ return
+ _emit_json(result)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @capture.command("finalize") | |
| @click.option("--session-id", default=None, help="Session id (else read from stdin payload).") | |
| def capture_finalize_cmd(session_id: str | None) -> None: | |
| """Roll a session buffer into a PENDING summary (SessionEnd hook payload on stdin).""" | |
| payload: dict[str, Any] = {} | |
| if not sys.stdin.isatty(): | |
| raw = sys.stdin.read() | |
| if raw.strip(): | |
| try: | |
| loaded = json.loads(raw) | |
| if isinstance(loaded, dict): | |
| payload = loaded | |
| except json.JSONDecodeError: | |
| payload = {} | |
| sid = session_id or str(payload.get("session_id") or "") | |
| if not sid: | |
| return | |
| store = _capture_store() | |
| if store is None: | |
| return | |
| cwd = Path(str(payload.get("cwd") or ".")).resolve() | |
| result = capture_mod.finalize( | |
| store, sid, cwd=cwd, project=cwd.name, | |
| generated_at=datetime.now(UTC).isoformat(), | |
| ) | |
| _emit_json(result) | |
| `@capture.command`("finalize") | |
| `@click.option`("--session-id", default=None, help="Session id (else read from stdin payload).") | |
| def capture_finalize_cmd(session_id: str | None) -> None: | |
| """Roll a session buffer into a PENDING summary (SessionEnd hook payload on stdin).""" | |
| payload: dict[str, Any] = {} | |
| if not sys.stdin.isatty(): | |
| raw = sys.stdin.read() | |
| if raw.strip(): | |
| try: | |
| loaded = json.loads(raw) | |
| if isinstance(loaded, dict): | |
| payload = loaded | |
| except json.JSONDecodeError: | |
| payload = {} | |
| sid = session_id or str(payload.get("session_id") or "") | |
| if not sid: | |
| return | |
| store = _capture_store() | |
| if store is None: | |
| return | |
| cwd = Path(str(payload.get("cwd") or ".")).resolve() | |
| try: | |
| result = capture_mod.finalize( | |
| store, sid, cwd=cwd, project=cwd.name, | |
| generated_at=datetime.now(UTC).isoformat(), | |
| ) | |
| except Exception: | |
| # A capture failure must never break the SessionEnd hook or host. | |
| return | |
| _emit_json(result) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/cli.py` around lines 1365 - 1391, The finalize hook path in
capture_finalize_cmd lacks the same crash guard used by capture_observe_cmd, so
exceptions from capture_mod.finalize can abort the host session. Wrap the body
of capture_finalize_cmd (from payload parsing through _emit_json(result)) in a
broad exception handler that returns on failure, preserving the hook contract
that hooks must never abort the host. Keep the existing session_id,
_capture_store, and capture_mod.finalize flow intact, just make the SessionEnd
hook resilient to any unexpected error.
| @capture.command("banner") | ||
| def capture_banner_cmd() -> None: | ||
| """Emit a SessionStart nudge if captured summaries await review.""" | ||
| store = _capture_store() | ||
| if store is None: | ||
| return | ||
| n = capture_mod.pending_count(store) | ||
| if n: | ||
| click.echo( | ||
| f"🔔 {n} auto-captured session summary(ies) awaiting review — " | ||
| f"run `vouch review`." | ||
| ) | ||
|
|
||
|
|
||
| @cli.command(name="recall") | ||
| def recall_cmd() -> None: | ||
| """Emit a digest of all approved knowledge for session-start injection.""" | ||
| store = _capture_store() | ||
| if store is None: | ||
| return | ||
| cfg = recall_mod.load_config(store) | ||
| if not cfg.enabled: | ||
| return | ||
| digest = recall_mod.build_digest(store, max_chars=cfg.max_chars) | ||
| if digest.strip(): | ||
| click.echo(digest) | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
capture banner and recall are similarly unguarded against storage read failures.
capture_mod.pending_count(store) and recall_mod.build_digest(store, ...) both read/parse KB files (list_proposals, list_claims, list_pages); a single malformed YAML artifact would raise and abort the SessionStart hook instead of degrading gracefully, same root cause as the capture finalize guard gap above.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/cli.py` around lines 1393 - 1419, The capture_banner_cmd and
recall_cmd paths are missing error handling around storage reads, so malformed
KB YAML can abort the SessionStart hook. Update the logic around
capture_mod.pending_count(store) and recall_mod.build_digest(store,
max_chars=cfg.max_chars) to catch read/parse failures from the underlying store
access and return quietly or emit a non-fatal warning instead of raising. Keep
the guard localized in capture_banner_cmd and recall_cmd, using the existing
_capture_store, capture_mod, and recall_mod entry points.
| # hooks — per event, add only commands not already present. Prefer folding | ||
| # into an existing group with the same matcher so we don't fan out groups. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Comment uses first-person "we".
"so we don't fan out groups" uses the disallowed first-person tone. As per coding guidelines: "Do not use marketing tone like 'we' or 'let's' in code comments; comments should explain why, not what."
✏️ Suggested wording
- # hooks — per event, add only commands not already present. Prefer folding
- # into an existing group with the same matcher so we don't fan out groups.
+ # hooks — per event, add only commands not already present. Fold into an
+ # existing group with the same matcher to avoid fanning out groups.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # hooks — per event, add only commands not already present. Prefer folding | |
| # into an existing group with the same matcher so we don't fan out groups. | |
| # hooks — per event, add only commands not already present. Fold into an | |
| # existing group with the same matcher to avoid fanning out groups. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/install_adapter.py` around lines 309 - 310, Update the comment near
the hooks grouping logic in install_adapter so it removes first-person wording
like "we don't fan out groups" and uses neutral, guideline-compliant phrasing
instead. Keep the intent tied to the existing hook-deduplication behavior in the
nearby comment block, but rewrite it to explain the grouping rationale without
using "we" or similar marketing-style tone.
Source: Coding guidelines
| if target_group is not None: | ||
| target_group.setdefault("hooks", []).extend(fresh) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Non-list hooks on an existing group crashes the merge.
If a user's settings.json has a hook group whose hooks value is not a list (e.g. a hand-edited "hooks": "..."), setdefault("hooks", []) returns the existing non-list value and .extend(fresh) raises AttributeError. That escapes _install_json_merge (its try/except only wraps json.loads), so install() aborts with a traceback rather than the intended clean skip. The rest of this function is careful to coerce malformed shapes, so this is an inconsistent gap.
🛡️ Proposed guard
if target_group is not None:
- target_group.setdefault("hooks", []).extend(fresh)
+ group_hooks = target_group.get("hooks")
+ if not isinstance(group_hooks, list):
+ group_hooks = []
+ target_group["hooks"] = group_hooks
+ group_hooks.extend(fresh)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if target_group is not None: | |
| target_group.setdefault("hooks", []).extend(fresh) | |
| if target_group is not None: | |
| group_hooks = target_group.get("hooks") | |
| if not isinstance(group_hooks, list): | |
| group_hooks = [] | |
| target_group["hooks"] = group_hooks | |
| group_hooks.extend(fresh) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/install_adapter.py` around lines 340 - 341, The merge in
_install_json_merge currently assumes target_group["hooks"] is a list, so a
malformed existing group can make target_group.setdefault("hooks",
[]).extend(fresh) raise AttributeError and abort install(). Update the
hook-group merge logic to validate or coerce the existing hooks value before
extending it, and if it is not a list, replace it with a list or skip merging
into that group so the function keeps the same tolerant behavior as the rest of
install_adapter.py. Use the target_group and _install_json_merge symbols to
locate the fix.
| def load_config(store: KBStore) -> RecallConfig: | ||
| """Read ``recall:`` from config.yaml; fall back to defaults.""" | ||
| try: | ||
| loaded = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) | ||
| except (OSError, yaml.YAMLError): | ||
| return RecallConfig() | ||
| if not isinstance(loaded, dict): | ||
| return RecallConfig() | ||
| raw = loaded.get("recall") | ||
| if not isinstance(raw, dict): | ||
| return RecallConfig() | ||
| return RecallConfig( | ||
| enabled=bool(raw.get("enabled", DEFAULT_ENABLED)), | ||
| max_chars=int(raw.get("max_chars", DEFAULT_MAX_CHARS)), | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Same defaults-fallback gap as capture.load_config.
int(raw.get("max_chars", DEFAULT_MAX_CHARS)) on line 48 runs outside the try/except (OSError, yaml.YAMLError) guard, so a malformed recall.max_chars value in config.yaml raises instead of falling back to defaults as the docstring promises.
🩹 Proposed fix
raw = loaded.get("recall")
if not isinstance(raw, dict):
return RecallConfig()
- return RecallConfig(
- enabled=bool(raw.get("enabled", DEFAULT_ENABLED)),
- max_chars=int(raw.get("max_chars", DEFAULT_MAX_CHARS)),
- )
+ try:
+ return RecallConfig(
+ enabled=bool(raw.get("enabled", DEFAULT_ENABLED)),
+ max_chars=int(raw.get("max_chars", DEFAULT_MAX_CHARS)),
+ )
+ except (TypeError, ValueError):
+ return RecallConfig()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def load_config(store: KBStore) -> RecallConfig: | |
| """Read ``recall:`` from config.yaml; fall back to defaults.""" | |
| try: | |
| loaded = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) | |
| except (OSError, yaml.YAMLError): | |
| return RecallConfig() | |
| if not isinstance(loaded, dict): | |
| return RecallConfig() | |
| raw = loaded.get("recall") | |
| if not isinstance(raw, dict): | |
| return RecallConfig() | |
| return RecallConfig( | |
| enabled=bool(raw.get("enabled", DEFAULT_ENABLED)), | |
| max_chars=int(raw.get("max_chars", DEFAULT_MAX_CHARS)), | |
| ) | |
| def load_config(store: KBStore) -> RecallConfig: | |
| """Read ``recall:`` from config.yaml; fall back to defaults.""" | |
| try: | |
| loaded = yaml.safe_load(store.config_path.read_text(encoding="utf-8")) | |
| except (OSError, yaml.YAMLError): | |
| return RecallConfig() | |
| if not isinstance(loaded, dict): | |
| return RecallConfig() | |
| raw = loaded.get("recall") | |
| if not isinstance(raw, dict): | |
| return RecallConfig() | |
| try: | |
| return RecallConfig( | |
| enabled=bool(raw.get("enabled", DEFAULT_ENABLED)), | |
| max_chars=int(raw.get("max_chars", DEFAULT_MAX_CHARS)), | |
| ) | |
| except (TypeError, ValueError): | |
| return RecallConfig() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vouch/recall.py` around lines 35 - 49, The `load_config` function in
`src/vouch/recall.py` has the same defaults-fallback gap as
`capture.load_config`: `recall.max_chars` is converted with `int(...)` outside
the existing `try/except`, so invalid config values can raise instead of
returning `RecallConfig()` as promised. Update `load_config` to catch malformed
`max_chars` values when building the `RecallConfig`, and fall back to defaults
for any parsing/conversion error while keeping the current behavior for missing
or non-dict `recall` blocks.
1 participant
What changed
adds two session-lifecycle features for the claude code adapter, plus the
plumbing to make them install cleanly. capture: a
PostToolUsehookharvests tool-use into a gitignored scratch buffer and a
SessionEndhookrolls it into one pending session-summary page proposal (mechanical, no llm).
recall: a
SessionStarthook injects a digest of every live approvedclaim + page title into a new session's context, so it starts knowing the
reviewed KB. both are cli-only (
vouch capture …,vouch recall) — no newkb.*method. also teachesinstall-mcpto merge hooks into an existing.claude/settings.json, and fixes storage to read/write KB files as utf-8.Why
sessions lose their context when the window closes, and a fresh session starts
blind to what the team already agreed. capture preserves the what happened
(gated behind human review, never auto-written); recall surfaces the what we
know at the top of every session. the review gate stays load-bearing: capture
lands as
PENDING, and recall only ever emits approved knowledge. the utf-8fix is a prerequisite — on a non-utf-8 locale,
vouch pendingcrashed reading aproposal containing an em-dash (and it clears two pre-existing test failures
with the same root cause).
What might break
install-mcp claude-codenow merges into an existing.claude/settings.json(newjson_mergestrategy) instead of skipping it —it adds vouch's hooks + read-only permission allowlist without touching your
own entries, and is idempotent. fresh installs are unchanged.
capture.*andrecall.*config namespaces are added to the starterconfig; existing KBs read them defensively and fall back to defaults, so no
migration is needed. opt out with
capture.enabled: false/recall.enabled: false..vouch/captures/scratch dir. no durable on-disk format,object-model, bundle, or audit-log shape changes.
encoding="utf-8"on all KB file I/O — locale-independentand correct; only affects boxes whose locale wasn't utf-8 (where it was
already broken).
VEP
not a surface change — no new
kb.*method, no object-model / on-disk-layout /bundle / audit-log change. no VEP required.
Tests
make checkpasses locally (lint + mypy + pytest) — 790 passed, 0 failedtests/test_capture.py,tests/test_recall.py,tests/test_install_adapter.py) plus end-to-endmake smoke-capture/make smoke-recallCHANGELOG.mdupdated under## [Unreleased]new code coverage:
capture.py100%,recall.py100%, thejson_mergeinstall strategy 100%.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation