feat(sessions): wire up the client flow for session creation response by evanspearman · Pull Request #568 · gominimal/minimal

evanspearman · 2026-06-24T19:42:18Z

Phase 3 of session composition: client-side handler that turns the daemon's
ContributionResponse into a ContributionVerdict.

New client::handler::handle_response: resolves pending vars against the
client env, expands patch sources against the gated + same-batch vars,
runs UserPolicy, and prompts via PolicyHooks only when the policy
can't decide. Emits one wire verdict per pending id; lifecycle hooks
pass through.
Per-item verdicts on the wire: WireVarVerdict / WirePatchVerdict now
carry name / host_path on every variant; daemon correlates by id.
SessionVar is a thin typestate wrapper over ProvenancedVar; new
PendingVar / PendingPatchFile bridge wire ↔ domain on the
verdict-emitting side.
Shared hook plumbing lifted into prompt_var_hook / prompt_patch_hook
so Phase 1 and Phase 3 stop duplicating the abort/contract/install dance.

Summary by CodeRabbit

New Features
- Added client-side composition response handling that produces per-item var and patch verdicts (including approved, denied, and ignored outcomes).
Bug Fixes
- Improved verdict correlation so results match reliably by item id even when emission order differs.
- Enhanced error reporting for missing environment resolution, invalid pending patch destinations, and additional failure cases.
Documentation
- Updated session composition docs to clarify phase-specific gating, verdict ordering, and hook/policy behavior.
Tests
- Added integration coverage for client flow 2, including hook decisions, patch fan-out, and verdict/JSON round-trip validation.

coderabbitai · 2026-06-24T19:42:35Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d0d68537-c5e8-40f1-83e3-555ff0cc7950

📥 Commits

Reviewing files that changed from the base of the PR and between 250f312 and 1fdddbc.

📒 Files selected for processing (1)

crates/sessions/src/core/compose.rs

🚧 Files skipped from review as they are similar to previous changes (1)

crates/sessions/src/core/compose.rs

📝 Walkthrough

Walkthrough

Session composition now gates pending vars and patches on the client side, updates verdict payload shapes and shared compose errors, refactors hook prompting and pending-item handling, and adds integration coverage for verdict composition, batching, and failure cases.

Changes

Session composition gating

Layer / File(s)	Summary
Verdict contracts and ordering `crates/sessions/docs/COMPOSITION.md`, `crates/sessions/src/core/primitives.rs`, `crates/sessions/src/wire/policy.rs`, `crates/sessions/src/wire/request.rs`	`WireVarSpec` now maps to `VarValue`, verdict payloads carry per-item names and host paths, and the composition docs describe the updated phase-specific correlation and ordering rules.
Core composition model and hook helpers `crates/sessions/docs/COMPOSITION.md`, `crates/sessions/src/core/compose.rs`	`ComposeError`, `SessionVar`, pending var/patch wrappers, and hook prompt helpers are refactored around the client-side gating flow, with env, `~`, and failure-case docs updated to match.
Client response handling `crates/sessions/src/client/mod.rs`, `crates/sessions/src/client/handler.rs`	The new client handler gates vars before patches, expands patch sources from approved vars, and returns the final `ContributionVerdict`; the client module exports it.
Flow 2 integration coverage `crates/sessions/tests/client_flow2.rs`	Integration tests cover var and patch verdict composition, hook batching and contract failures, lifecycle-hook dropping, and invalid patch destinations.

Sequence Diagram(s)

sequenceDiagram
  participant HandleResponse as "client::handler::handle_response"
  participant GateVars as "gate_pending_vars"
  participant Hooks as "PolicyHooks"
  participant GatePatches as "gate_pending_patches"
  participant Verdict as "ContributionVerdict"

  HandleResponse->>GateVars: resolve pending vars and collect approved vars
  GateVars->>Hooks: prompt undecidable vars
  Hooks-->>GateVars: ItemDecision / updated policy
  GateVars-->>HandleResponse: wire var verdicts + approved vars
  HandleResponse->>GatePatches: expand patch sources with approved vars
  GatePatches->>Hooks: prompt undecidable patch files
  Hooks-->>GatePatches: ItemDecision / updated policy
  GatePatches-->>HandleResponse: wire patch verdicts
  HandleResponse-->>Verdict: assemble final verdict

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

gominimal/minimal#528: Establishes the client/daemon composition split that this PR extends with client::handler::handle_response and the shared compose helpers.
gominimal/minimal#443: Introduces the ContributionResponse/ContributionVerdict wire flow that this PR extends with client-side gating and updated verdict fields.

Suggested reviewers

twitchyliquid64

Poem

(_/)
(•_•) I hopped through vars and patchy trails,
/ >🥕 and sorted verdicts by their id rails.
Hooks sniffed, then chirped, then danced anew,
One gate, one path, and everything flew.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately describes the new client-side session creation response flow.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

🧹 Nitpick comments (1)

crates/sessions/src/core/compose.rs (1)
461-478: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

unreachable! on non-ResolutionFailure is correct today but brittle.

ResolvedVar::resolve_with currently only ever returns VarError::ResolutionFailure, so the catch-all panic is unreachable in practice. However, this couples from_wire to an implementation detail of resolve_with; if a future change makes it return another VarError variant, this becomes a runtime panic instead of a recoverable error. Consider folding any unexpected variant into a non-panicking ComposeError (e.g. InvalidWireItem) instead.
🤖 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 `@crates/sessions/src/core/compose.rs` around lines 461 - 478, The from_wire
mapping in compose.rs relies on an unreachable! for non-ResolutionFailure
VarError variants, which makes it brittle if ResolvedVar::resolve_with ever
returns something else. Update the error handling in from_wire so the match on
resolve_with maps known ResolutionFailure into ComposeError::VarResolution and
converts any unexpected VarError variant into a recoverable ComposeError such as
InvalidWireItem instead of panicking.

🤖 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.

Nitpick comments:
In `@crates/sessions/src/core/compose.rs`:
- Around line 461-478: The from_wire mapping in compose.rs relies on an
unreachable! for non-ResolutionFailure VarError variants, which makes it brittle
if ResolvedVar::resolve_with ever returns something else. Update the error
handling in from_wire so the match on resolve_with maps known ResolutionFailure
into ComposeError::VarResolution and converts any unexpected VarError variant
into a recoverable ComposeError such as InvalidWireItem instead of panicking.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2011ec67-ef9f-4e3e-8dab-19dff4d06b2e

📥 Commits

Reviewing files that changed from the base of the PR and between f29f917 and 250f312.

📒 Files selected for processing (8)

crates/sessions/docs/COMPOSITION.md
crates/sessions/src/client/handler.rs
crates/sessions/src/client/mod.rs
crates/sessions/src/core/compose.rs
crates/sessions/src/core/primitives.rs
crates/sessions/src/wire/policy.rs
crates/sessions/src/wire/request.rs
crates/sessions/tests/client_flow2.rs

evanspearman added 2 commits June 23, 2026 09:56

feat(sessions): wire up the client flow for session creation response

250f312

fix(sessions): coderabbit fix to squash

1fdddbc

gominimal-aw-bot Bot added the needs-spec label Jun 24, 2026

gominimal deleted a comment from gominimal-aw-bot Bot Jun 24, 2026

coderabbitai Bot reviewed Jun 24, 2026

View reviewed changes

norrietaylor approved these changes Jun 24, 2026

View reviewed changes

gominimal deleted a comment from gominimal-aw-bot Bot Jun 24, 2026

evanspearman merged commit ddc715f into main Jun 25, 2026
26 checks passed

evanspearman deleted the evan/split05 branch June 25, 2026 14:52

This was referenced Jul 2, 2026

feat(sessions): tighten daemon composer pending/ready routing #602

Merged

feat(sessions): SubmitVerdict handler resumes pending sessions #618

Merged

chore(sessions): loadouts flow cleanup #639

Merged

Navigation Menu

Uh oh!

feat(sessions): wire up the client flow for session creation response - #568

feat(sessions): wire up the client flow for session creation response#568
evanspearman merged 2 commits into
mainfrom
evan/split05

evanspearman commented Jun 24, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

evanspearman commented Jun 24, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

evanspearman commented Jun 24, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 24, 2026 •

edited

Loading