Repository navigation
effect-session: allow an empty resource for whole-system verbs - #77
Merged
Merged
Conversation
effect_session_open() rejected any empty resource as bad_request:resource, but whole-system verbs (update/upgrade/dist_upgrade/configure) carry an empty resource by contract -- the planner returns resource:"", the broker accepts it, and the durable record stores "". The flat rejection failed every whole-system commit at the effect-session open step, before any intent opened. The native wrapper was stricter than the broker it fronts. Scope the check with the existing runix_verb_takes_packages(): an empty resource is a bad_request only for targeted verbs (install/remove/purge/hold/unhold); an over-length resource is still always rejected. pkgops writes resource:"" to match the reference boundary (rab-exercise), so the fix belongs here, not a pkgops sentinel that would diverge the durable record. Regression tests (test_effect_session_api.R): install with "" still bad_request:resource; update with "" is accepted and proceeds to the transport (runix_broker_unavailable on a dead socket). Found by the Part B disposable-VM gate -- invisible to the hermetic pkgops tests, whose fake open accepts any resource. 358 checks pass in both the production and -DRUNIX_TESTING builds.
The native open_intent frame omitted the broker-required outcome and effect_issued fields, so every native open was schema_invalid at a real broker -- durable-audit-contract.md (two-phase write) has always specified an intent record carries effect_issued=false, outcome="intent". The schemaless fake responder the hermetic suite uses validated nothing and passed the malformed frame; the disposable-VM (Part B) run caught it. Add an at-home regression driving native open + write_outcome against the REAL broker daemon (a must-match negative asserts the daemon rejects a record missing outcome: broker_error/schema_invalid), plus a dedicated CI job (effect-session-native) that builds runix with the testing seam and a post-v0.0.1 broker so the native path is exercised for real. The linux job now runs at_home= TRUE -- test_package() defaults it FALSE, which had been silently skipping the uid-0 pinning regression and the live-broker round-trips its own comment claims.
Contributor
Author
|
Part B authoritative run 4: polkit matrix 23/23, §7 gates 68/68, rc=0. Empty-resource + |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HELD draft — landed pending the Part B VM re-run that proves it end to end.
Found by the Part B disposable-VM gate (pkgops issuer run 1): every whole-system commit died at the effect-session open with
bad_request: resource.effect_session_open()(src/effect_session.c) rejected any emptyresource:But whole-system verbs (
apt.update/upgrade/dist_upgrade/configure) bind no package and carryresource = ""by contract — the planner returnsresource:"", the broker accepts it (rab-exercise passes""straight through and its update gates pass), and the durable record stores"". The native wrapper was stricter than the broker it fronts.Fix: scope the empty check with the existing
runix_verb_takes_packages()— empty is abad_requestonly for the targeted verbs; over-length is still always rejected. The fix belongs in runix, not a pkgops sentinel: a synthesized resource would diverge the durable record from the reference boundary (rab-exercise'sresource:""), failing the §5 record-parity check.Why the hermetic tests missed it: every pkgops commit test uses a fake session-ops whose
openaccepts any resource. Only a real runixopenagainst the real broker exercises this — exactly what Part B is for.Tests (
inst/tinytest/test_effect_session_api.R):install+""stillrunix_broker_bad_requestdetailresource;update+""is accepted and proceeds to the transport (runix_broker_unavailableon a dead socket). 358 checks pass in both the production and-DRUNIX_TESTINGbuilds.Version 0.0.1.13.