feat(schedule): warn at startup when the agent state allowlist resolves to deny-all - #55
Open
arcuru-bot wants to merge 2 commits into
Open
feat(schedule): warn at startup when the agent state allowlist resolves to deny-all#55arcuru-bot wants to merge 2 commits into
arcuru-bot wants to merge 2 commits into
Conversation
…es to deny-all A deny-all entry (agent_state_allowlist.<ext>: []) is invisible at the tool boundary - every lookup fails with the uniform not-found error, so a broken config looks identical to a working one until someone stares at 'not found' errors. deny_all_warning() in agent_state.rs builds the operator-facing message (naming the extension and the config key) and the schedule extension logs it once at its construction site, which for a global-scope extension is once at startup. Also repoints the stale in_scope doc comment that referenced a never-built ExtensionHub::build_agent_state_admin factory, and corrects the CapabilityRequest::AgentStateAdmin variant docs, which claimed the operator injects the agents field - nothing injects it today; the live scoping input is the operator map applied by the extension itself.
…t shipped The Implementation Log claimed resolve_agent_allowlist() intersection tests, a hub-side build_agent_state_admin factory, and a manifest declaration of the cap on the schedule extension - none of which exist. Grep finds zero hits for the function and the factory, all eight extension/mod.rs intersection tests were never written, and every builtin extension ships requested_capabilities: Vec::new(). Rewrite the log, tests table, and status header to match the tree: the operator's agent_state_allowlist map is the only scoping input, and each consuming extension builds its own ScopedAgentStateAdmin from its map entry at instantiate time. The manifest-intersection model stays in the doc as an explicitly-marked future refinement. Also fix the Scoped Wrapper code block, whose pre-implementation draft used HashSet<String> with empty = deny-all - the shipped encoding is Option<HashSet<String>>, where None means unrestricted and Some(empty) means deny-all; the draft collapsed those two meanings into one.
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.
Two commits, one thread: the agent_state_admin design doc claimed a manifest∩operator allowlist model that never shipped.
The finding. The doc's Implementation Log marked three rows done that do not exist in the tree:
resolve_agent_allowlist()(zero hits across crates/), the hub factorybuild_agent_state_admin(only a stale doc-comment referenced it), and a manifest declaration of the cap on the schedule extension (every builtin extension shipsrequested_capabilities: Vec::new()). All eightextension/mod.rsintersection tests listed in the doc were never written.The call. Rather than build the intersection layer, this retracts it as a future refinement: the manifest side has no declarers and nothing enforces it (extensions build their own
ScopedAgentStateAdminfrom the operator map), so its only live behavior would be a pass-through. The operator'sagent_state_allowlistmap is the one real scoping input, and the doc now says exactly that.What ships. The genuinely missing piece the user guide already promises (
configuration.md: "logged at WARN on startup"): a one-time deny-all WARN.deny_all_warning()inagent_state.rsbuilds the message — naming the extension and the config key — and the schedule extension logs it at its construction site (once at startup for a global-scope extension). Two unit tests pin the message shape and the silence of healthy configurations.Also fixed in passing: the doc's Scoped Wrapper draft showed
HashSet<String>with "empty = deny-all", which inverts the shippedOption<HashSet<String>>encoding (None= unrestricted vsSome(empty)= deny-all are distinct meanings); and stale code comments pointing at the never-built factory.Verification:
nix build .#checks.x86_64-linux.{lint,treefmt,test,doc,build} --no-link— all five pass. Targeted:cargo test -p chaz-core --lib agent_state11/11,extensions::schedule7/7.No workflow files touched. Docs:
docs/src/design/agent_state_admin.md.