Repository navigation
Agentic guards: goal_met/prove_goal, hint origin, built-in rules for planner Bash (#124 F5, F6, F11) - #190
Merged
Conversation
Audit findings F5, F6 and F11 for issue #124. F5, goal guards: - goal_met refuses while any goal node of the task is open, and when the goals cannot be read. A task without goal nodes keeps the old behavior. - prove_goal accepts a fact as evidence only when its confidence is "observed"; otherwise it refuses and the goal stays open. Findings are still accepted. F6, hint origin: - add_hint stores the calling ToolSet's worker as the hint origin instead of "human", and passes it to the planner trigger (TriggerEvent.HintOrigin). - renderTriggers names the writer: the human via the main agent, a cross-task orchestration, or an agent, with a neutral fallback. F11, built-in rules for the planner and the main agent: - Interceptor.MatchBuiltin evaluates the enabled "[built-in]" deny rules on the intercept-enabled tools. - The target-only guard applies them before the target gate, whether the gate is on or off. User rules and the judge stay out of it. - SetRulesForTest is a test seam for the store-less guard tests; Log is a no-op without a store.
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
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.
Phase B, PR 2 of 2 for #124: audit findings F5 (goal guards), F6 (truthful hint origin) and F11 (built-in deny rules for the planner and the main agent).
What and why
F5, goal guards (
agent/tools.go)The audit (goal drift, memory poisoning) found that
goal_metended a task without any check, and thatprove_goalaccepted any fact as evidence. A planted or inferred fact could therefore close a goal or the whole task.goal_metlists the task's goal nodes. If any goal is notmet, it refuses with the number of open goals and points toprove_goal;GoalMetis not set. If the goals cannot be read, it refuses. A task with no goal node keeps the old behavior. The tool now useswriteExpTool, so a ToolSet without an exploration store gets the standard "requires a task context" refusal instead of a nil dereference.prove_goalwith a fact readsconfidencefrom the payload and requires exactlyobserved(trimmed, case-insensitive). Otherwise it refuses with a clear message and the goal stays open. Findings are still accepted as before.F6, hint origin (
agent/tools.go,agent/planner.go,agent/mainagent.go,server/manager.go,server/orchestration.go)Every
add_hintwas stored with originhuman, and the planner trigger said "The human (main agent)", including for hints written by the cross-task orchestrator.addOneHintstorest.workeras the origin:humanfor the main-agent chat,orchestratorfor cross-task orchestration, otherwise the agent name.SetNotifyHint/notifyHinttake(origin string, texts []string),Task.NotifyHint(origin, texts), andTriggerEventgainsHintOrigin.server/server.goneeds no change (it passes the method value).renderTriggersnames the writer: "The human (via the main agent)", "A cross-task orchestration", "An agent (worker-N)", or "A source that was not recorded" when the origin is empty. Theadd_hintdescription no longer says the hint comes from the human or main agent.F11, built-in rules for the planner and the main agent (
intercept/intercept.go,guard/guard.go)The planner and the main agent run Bash under
NewTargetOnly, which applied no intercept rule at all.Interceptor.MatchBuiltin(tool, input): the loaded, enabled rules whose name starts with[built-in](the v1/v2/v3 seeds) and whose action isdeny, matched with the existingruleMatches, first match in priority order. Without a database and without loaded rules it returns false, so store-less interceptors never touch a store.guard.applyIntercept: for a target-only guard,MatchBuiltinruns first, before the target gate and before the early return when the gate is off. A match is logged asdeniedand blocked with the rule's message. User rules and the judge stay out of the target-only guard. The worker path is unchanged.MatchBuiltinonly considers the intercept-enabled tools (by default Bash, WebFetch, web_search, shell_open, shell_send, Write, Edit, MultiEdit), the same set as the worker path. Without this, atool_inputrule such as "[built-in] Delete-style endpoint paths" would denyadd_hintoradd_intentcalls whose text just names a/deleteendpoint.Limits
confidenceis declared by the model that calledrecord_fact. This PR does not check where the fact came from. The source check (source != target_response) comes with F4 (fact provenance) in 0.7.0.prove_goalon a non-observed fact is a plain refusal and the goal stays open.Interceptor.SetRulesForTest(rules)installs rules (and the default enabled tools) on an interceptor without a database. The guard tests need it because they cannot reach the unexported rule cache.Interceptor.Lognow returns early when there is no store, which is needed by the same store-less tests and changes nothing in production, where the store is always set.toolstable only on first insert, and the model sees the stored text. The newadd_hintandprove_goaldescriptions only reach fresh databases. Existing installs keep the old text until the descriptions are refreshed. This PR does not bumptool_schema_refresh_*, because that would also overwrite edited descriptions of every orchestration and platform tool. The guards themselves are in the handlers and apply everywhere.block(), as it already does for target-gate blocks.Tests
PG-unit (
agent/agentic_guards_test.go, against a real Postgres):TestGoalMetRefusesWhileGoalsOpen: an open goal plusgoal_metreturns an error naming "1 of 2 goals" andprove_goal, andGoalMetstays false. Once all goals are met,goal_metsucceeds.TestGoalMetWithoutGoalNodes: with no goal node,goal_metsucceeds.TestProveGoalRequiresObservedFact: an inferred fact and a fact without confidence are refused, and the goal stays open. An" Observed "fact marks the goal met. A finding marks the last goal met and completes the task.TestAddHintRecordsOrigin:add_hintthrough anorchestratorToolSet stores originorchestratorand passes it to the trigger. Through the main agent'shumanToolSet, it stayshuman.Pure:
TestRenderTriggersHintOrigin: wording forhuman,orchestrator,worker-3, and an empty origin (single and batch), and never "The human (main agent)".TestTargetOnlyGuardAppliesBuiltinDenyRules: a store-less interceptor with rules injected throughSetRulesForTest. With the gate off and on, Bashrm -rf /is blocked by the built-in rule before the gate is reached,ls -lapasses, and a user rule (no[built-in]prefix) is not applied.add_hinttext naming a destructive command is not matched. The full guard still applies the user rule.TestTargetOnlyGuardSkipsRules: unchanged assertions. An interceptor with no database and no loaded rules still passes every call without touching the store. The comment now says that in production the built-in rules apply.Local gate (Kali, Go 1.26.7, throwaway
postgres:16-alpinecontainer):go build ./...: OKgo vet ./...: OKgo test -count=1 -p 1 ./agent ./guard ./intercept ./server: all fourokgofmt -lstill listsagent/tools.go(detectStagnation,stagnation_warning) andserver/manager.go(theTaskstruct) for misformatting that predates this branch; I left it alone to keep the diff surgical.Not verified
go test -racewas not run locally (CI runs it on./agent ./guard ./intercept)../agent ./guard ./intercept ./serverwere tested locally; other packages were only built and vetted../serverand./interceptsuites, which load the real seeds, still pass.