feat(agent): put inline completion through the admission gates - #56
Merged
Conversation
Closes #22. `_garrison/complete` resolved the session that owns it and then made a paid model call. Ownership proves the client holds the session; it says nothing about whether this install may spend. An install refused from running a turn kept spending on every typing pause, and the number of completions in flight against the model was bounded only by a two-second deadline. Running completions off the session actor was a deliberate latency decision and is still the right one. Losing admission along with it was not a decision at all, and that is what this fixes. Every gate but one now answers a completion exactly as it answers a turn. The request carries an `admission::Work` saying which kind of work it is, so each gate decides for itself what its own rule means. The alternative was a second, shorter gate list for the completion path, and it was rejected: it puts the judgment about completions in the code that assembles the list rather than in the gate that holds the rule, and a gate added later would join that list by being forgotten rather than by being considered. Policy is the gate that matters most here, against first appearances. The issue called it unclear on the grounds that a completion runs no tools, but this gate does not gate tools at all: it refuses when there is no policy state, when the install is Ungoverned, and when the configured provider is not an approved endpoint in the bundle. Its own comment says refusing late means an operator "has already sent it their code" — which is precisely what a completion does, on every typing pause, to whatever endpoint the daemon points at. It was the least gated path to the least approved provider. The session keeper is the exception and admits a completion without asking. Both of its rules are about a session's stored record, and a completion writes none; refusing one because an earlier turn was left open would stop a developer's editor for a reason that is not about them. It is also the only gate that reaches the store to answer, so skipping that read is what keeps the gates inside a completion's budget. `session_persistence` proves it with a real daemon and a real trail rather than by assertion: with the carve-out removed the test fails, having sealed a second `turn_interrupted` refusal. A refused completion is sealed, an admitted one is not. The refusal is the governance-relevant event, and a debounce timer decides how often this path is entered — a trail written by a timer rather than by a person is noise wearing the shape of evidence. `audit::seal_refusal` is the sealing `thread.rs` already did, lifted so both paths record the same shape. Two smaller decisions the issue raised: The gates get 250ms on this path rather than the turn path's five seconds. Every gate a completion crosses answers from state it already holds, so one that has not replied in that long is wedged rather than working, and a completion is abandoned two seconds from now regardless. Waiting the full five would spend the budget on the gates and leave nothing for the model. Concurrency is capped at eight in flight daemon-wide, which the issue filed under "Related" and which nothing bounded before. A completion that cannot get a permit is dropped rather than queued, for the same reason this path does not go through the session actor: a suggestion that waits for a slot arrives after the keystroke that made it interesting. A refusal stays silent to the editor. Every other failure on this path answers "no suggestion" and this is no different; raising an error per keystroke would bury a developer in dialogs over a state one glance at `_garrison/status` explains. The trail is where a refusal is not silent.
Merged
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.
Closes #22, the known gap named in both the 1.1 and 1.2 release notes.
_garrison/completeresolved the session that owns it and then made a paid model call. Ownership proves the client holds the session; it says nothing about whether this install may spend.One finding changed the issue's framing
The issue called policy "unclear" for completions because a completion runs no tools. But
policy/agent.rsdoesn't gate tools at all — it refuses when there's no policy state, when the install is Ungoverned, and when the configured provider isn't an approved endpoint in the bundle. Its own comment is that refusing late means an operator "has already sent it their code", which is exactly what a completion does on every typing pause. Policy turned out to be the gate that mattered most here, not the least.What this does
AdmitTurncarries anadmission::Work, so each gate decides for itself rather than a caller filtering opaque handles. A gate added later cannot compile without answering for both.audit::seal_refusalis the sealingthread.rsalready did, lifted so both paths record the same shape.Verification
The
session_persistenceassertion has teeth, checked rather than assumed: with the keeper carve-out removed the test fails, having sealed a secondturn_interruptedrefusal (left: 2, right: 1). That matters because the keeper is last in gate order, so the assertion could otherwise have passed vacuously.Workspace clippy clean at
-D warnings; 1297/1297 tests passing.