fix(capture): read the session id off the SessionStart hook payload#493
fix(capture): read the session id off the SessionStart hook payload#493boskodev790 wants to merge 1 commit into
Conversation
|
Warning Review limit reached
Next review available in: 57 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✨ 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 |
finalize-all resolved the current session id from --session-id or VOUCH_SESSION_ID only. the shipped SessionStart hook passes neither, and nothing in the adapter or runtime exports VOUCH_SESSION_ID, so sid was always "" and the sweep returned empty on every session start. that made the vouchdev#304 backstop dead: buffers orphaned by sessions that ended without SessionEnd accumulated in .vouch/captures/ and never rolled up into pending proposals, so the session never reached the review queue. the id now comes off the stdin payload the same way capture finalize reads it at SessionEnd, with --session-id and VOUCH_SESSION_ID kept as fallbacks. a malformed payload still no-ops rather than failing the host's turn. the adapter hook test asserted observe/finalize/banner but never finalize-all, which is why the shipped wiring went unexercised; it now covers it.
ead457d to
7e2030a
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
What changed
vouch capture finalize-allnow resolves the current session id from the SessionStart hook payload on stdin — the same wirecapture finalizealready reads at SessionEnd.--session-idandVOUCH_SESSION_IDare kept as fallbacks, in that order.Why
fixes #492. the command resolved the id from
--session-idorVOUCH_SESSION_IDonly, but the shipped hook is barevouch capture finalize-all || trueand nothing in the adapter or runtime ever exportsVOUCH_SESSION_ID—grepfinds only the cli reader and two tests that set it themselves. sosidwas always""and the sweep returned{"finalized": [], ...}on every session start.that left the #304 backstop dead in the shipped configuration: buffers orphaned by sessions that ended without SessionEnd accumulated in
.vouch/captures/and never rolled up into pending proposals, so those sessions never reached the review queue at all.it went unnoticed because
test_adapter_settings_wires_capture_hooksassertedobserve/finalize/bannerbut neverfinalize-all, and every cli test for the command passed--session-idor the env var explicitly — the shipped path was the one path never exercised. that assertion is added here.What might break
nothing on disk: no file moves, no field changes shape, no
kb.*method behaves differently (capture finalize-allis CLI + SessionStart-hook only).the behaviour change is the intended one: on an existing
.vouch/, the first session start after upgrading will sweep any stale buffers that have been sitting incaptures/— each becomes one pending proposal, so a user with orphaned buffers may see several new items invouch review. nothing is auto-approved and the review gate is untouched. the>1hage guard still protects the current session's buffer, and precedence is unchanged for anyone already passing--session-idexplicitly.a malformed or absent stdin payload still no-ops rather than raising, preserving the hook's never-fail-the-turn contract.
VEP
not a surface change — no
kb.*method, object model, on-disk layout, bundle format or audit-log shape touched. no VEP.Tests
make checkpasses locally (lint + mypy + pytest) — 1379 passed, 49 skipped; ruff clean; mypy clean across 98 source filesNew / changed behaviour has a test
CHANGELOG.mdupdated under## [Unreleased]test_capture_finalize_all_cmd_reads_session_from_stdin— verified failing ontestwithout the src changetest_capture_finalize_all_cmd_survives_malformed_stdin— pins the never-fail-the-turn contracttest_capture_finalize_all_cmd_flag_beats_stdin— pins precedencetest_adapter_settings_wires_capture_hooksnow asserts the SessionStartfinalize-allwiringone thing i left alone deliberately:
observe,finalize,ingest-codex --hookand this command now each parse a stdin payload their own way. consolidating them behind one helper seemed worth doing separately rather than inside a bug fix — happy to send that as a follow-up if you want it.no ui surface touched, so no screenshots.