fix hosted HTTP response saving - #6959
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
🚅 Deployed to the ironclaw-pr-6959 environment in ironclaw-ci-preview
|
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughHTTP egress assembly now passes a scoped filesystem as its body store. The integration harness creates this filesystem. The HTTP save test uses the real egress pipeline and verifies the persisted response body. ChangesHTTP egress body-store integration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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 |
🔎 Review · PR #6959
Submitted review →Reviewed the complete trusted base-to-head comparison across all three changed files. The hosted production assembly now supplies its existing scoped filesystem through the established HTTP body-store port, preserving authorization, sanitization, network-policy, and response-limit stages. The integration harness mirrors this wiring, and the regression verifies the persisted response bytes through the real egress pipeline. No actionable issues found. Automatic · PR opened · attempt 1 of 3 · completed in 1m 35s Run details
|
There was a problem hiding this comment.
🔍 Review complete · PR #6959
✅ No actionable findings
Reviewed the complete trusted base-to-head comparison across all three changed files. The hosted production assembly now supplies its existing scoped filesystem through the established HTTP body-store port, preserving authorization, sanitization, network-policy, and response-limit stages. The integration harness mirrors this wiring, and the regression verifies the persisted response bytes through the real egress pipeline. No actionable issues found.
Validation and technical details
- Verified trusted comparison refs: base a88bcb2 and head 2e1ffa2.
- Inspected the complete diff and surrounding production assembly, host-runtime builder, body-store authorization/write implementation, egress pipeline, mount resolver, integration harness, and workspace read-back assertion.
- Confirmed the modified files in the checkout match refs/ironloop/head despite the checkout HEAD pointing at a different commit.
- git diff --check refs/ironloop/base..refs/ironloop/head completed without errors.
- Focused cargo test could not run because cargo is unavailable in the review environment (
cargo: command not found). - Base:
main - Head:
codex/fix-hosted-http-saveat2e1ffa2 - Run:
2fabf84a-97b9-4662-b8f1-086c69dd7b4e
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.73% — 323144 / 376937 lines Per-crate breakdown (60 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (18 entry/entries excluded from the accounting above)
|
* fix hosted HTTP response saving * fix changed coverage for HTTP save wiring * align HTTP save wiring with coverage regions
Summary
builtin.http.savecan persist sanitized response bodies./workspace.Change Type
Linked Issue
None. Reproduced from an exported hosted single-tenant run.
Validation
cargo fmt --all -- --checkcargo clippy --all --benches --tests --examples --all-features -- -D warnings— Not run workspace-wide; scoped all-target/all-feature clippy passed forironclaw_reborn_composition.cargo build— Not run separately; targeted integration and clippy builds completed.main; complete 30-testreborn_integration_tool_callsuite before rebase.cargo test --features integration— Not applicable: no database schema or backend-specific persistence behavior changed.A broad
cargo test -p ironclaw_reborn_compositionrun compiled and started 535 tests, but was stopped after unrelated OAuth/extension lifecycle tests exceeded their 60-second warnings. It is not counted as passing evidence.Test Strategy
User behavior:
Given a hosted single-tenant user invokes
builtin.http.save, when the mediated HTTP request succeeds, the sanitized response body is written to the requested scoped/workspacepath instead of returningresponse body store is unavailable.Risk areas:
Tests added or updated:
runs_http_save_tool_call_through_real_egress_and_persists_body.What the tests prove:
The test drives a whole scripted turn through the real capability and host egress path, then reads the actual workspace file and verifies the response bytes. Before the fix, the file was absent; after the fix, it contains the expected response body.
Commands run:
cargo test -p ironclaw_reborn_integration_tests --test reborn_integration_tool_call runs_http_save_tool_call_through_real_egress_and_persists_bodycargo test -p ironclaw_reborn_integration_tests --test reborn_integration_tool_callcargo clippy -p ironclaw_reborn_composition --all-targets --all-features -- -D warningscargo fmt --all -- --checkgit diff --checkSecurity Impact
Restores the declared scoped-filesystem write path for
builtin.http.save. The change does not grant new authority or bypass approval, network policy, mount resolution, sanitization, or response-size enforcement.Reborn Trust-Boundary Checklist
serde(default)fields: N/A; no serialization changed.Database Impact
None. No migrations, schemas, queries, or PostgreSQL/libSQL-specific behavior changed.
Blast Radius
Hosted and production-shaped Reborn runtime composition for
builtin.http.save, includinghosted-single-tenantandhosted-single-tenant-volume. Inlinebuiltin.http, network policy, secrets, approvals, and other filesystem capabilities are unchanged.Rollback Plan
Revert commit
2e1ffa279. This restores the prior fail-closed behavior where saved-body HTTP requests return the body-store-unavailable error and write no file.Review Follow-Through
Reviewer judgment requested on whether a production-profile-specific composition test is desirable beyond the whole-turn real-egress regression. No known compatibility or migration risk.
Review track: C (runtime wiring)