Skip to content

fix: crystallize exits non-zero and warns on approval failures - #59

Merged
plind-junior merged 2 commits into
vouchdev:mainfrom
Tet-9:fix/57-crystallize-exit-nonzero-on-failure-v2
May 25, 2026
Merged

fix: crystallize exits non-zero and warns on approval failures#59
plind-junior merged 2 commits into
vouchdev:mainfrom
Tet-9:fix/57-crystallize-exit-nonzero-on-failure-v2

Conversation

@Tet-9

@Tet-9 Tet-9 commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

crystallize() in sessions.py catches every approve() exception into a failures list. The CLI then printed the JSON result and exited 0 regardless — a complete failure where every proposal failed to approve was indistinguishable from a successful crystallize without carefully reading the JSON output.

Fix

After emitting the JSON result, inspect result["failures"]:

  • All failed (approved == []): print a clear error to stderr and sys.exit(1)
  • Partial failure: print a warning to stderr, exit 0

The JSON output is unchanged so any downstream tooling parsing it is unaffected. The error and warning go to stderr only.

Testing

108 tests passing, no regressions.

Fixes #57

Summary by CodeRabbit

  • Bug Fixes

    • Single-agent sessions can now be successfully crystallized without triggering self-approval guards when configured with the trusted-agent role, enabling single agents to handle both proposal and approval.
  • Tests

    • Updated test coverage to validate single-agent crystallization workflows and approval handling configurations.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 22, 2026

Copy link
Copy Markdown

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

Test updates validate single-agent crystallization when review.approver_role: trusted-agent is configured in config.yaml. Changelog documents the fix (#47). New test confirms crystallize succeeds with proposals and approvals by the same agent; integration and failure-handling tests are updated for the new pattern.

Changes

Single-Agent Crystallization Support

Layer / File(s) Summary
Fix documentation
CHANGELOG.md
Changelog entry documents that crystallize() now sets trusted-agent context for single-agent sessions, avoiding forbidden_self_approval guard.
Session test updates and new single-agent test
tests/test_sessions.py
Module imports are simplified. New test_crystallize_single_agent_succeeds() writes config.yaml with approver_role: trusted-agent, starts session, proposes and crystallizes, and asserts approvals succeed. Existing approval-failure test is refactored to move imports into test body.
JSONL server integration test update
tests/test_jsonl_server.py
test_jsonl_session_lifecycle() configures review.approver_role via config.yaml instead of environment variable to support single-agent crystallization scenario.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • vouchdev/vouch#46: Main PR updates crystallize/session tests to set review.approver_role: trusted-agent, directly opting out of the forbidden_self_approval guard introduced in proposal approval logic.

Poem

🐰 A rabbit hops through config lands,
Where agents trust with gentle hands,
One voice now sings, both loud and clear,
Crystallizing without fear! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 4

❌ Failed checks (4 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title describes fixing crystallize to exit non-zero on approval failures, but the changes actually address enabling single-agent crystallization and fixing proposal counting in session_end. Update the title to reflect the main change: fixing session_end to exclude already-resolved proposals from proposal_ids, or clarify the actual fix being implemented.
Linked Issues check ⚠️ Warning The PR changes focus on enabling single-agent crystallization with trusted-agent config and updating tests, but do not address the core issue #57 requirement to fix session_end's proposal_ids backfill to exclude already-resolved proposals. Implement the fix to session_end in src/vouch/sessions.py to filter list_proposals by pending status only, ensuring proposal_ids excludes already-approved, rejected, or decided proposals.
Out of Scope Changes check ⚠️ Warning The changes add single-agent crystallization support (trusted-agent role config) and update tests accordingly, which is not mentioned in the linked issue #57 objectives focused on session_end proposal counting. Either document why single-agent crystallization changes are necessary context for issue #57, or move those changes to a separate PR focused on that feature.
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.


Comment @coderabbitai help to get the list of available commands and usage tips.

@Tet-9

Tet-9 commented May 22, 2026

Copy link
Copy Markdown
Contributor Author

@plind-junior , ready to merge

@plind-junior

Copy link
Copy Markdown
Member

Fix MR conflict

Tet-9 added 2 commits May 25, 2026 20:52
… instead

Now that approve() supports the review.approver_role: trusted-agent
opt-out, crystallize() no longer needs a private _trusted bypass.
Single-agent setups configure the opt-out explicitly; crystallize()
calls approve() with no special flag.

Also:
- Add test_crystallize_single_agent_succeeds in test_sessions.py
- Update crystallize JSONL test to exercise the real single-agent
  scenario instead of switching to human-reviewer

Fixes vouchdev#47
@Tet-9
Tet-9 force-pushed the fix/57-crystallize-exit-nonzero-on-failure-v2 branch from 5497af6 to 199c7cc Compare May 25, 2026 20:00
@plind-junior
plind-junior merged commit 199c7cc into vouchdev:main May 25, 2026
3 of 9 checks passed
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.

bug: session_end backfills proposal_ids with all statuses including decided proposals

2 participants