From e603b95b7190e92bfb7a4c8c41c04865d9c907e3 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sun, 16 Aug 2026 09:57:18 +0530 Subject: [PATCH 01/28] fix(agent): the incompletion detector keeps its allowances from swallowing admissions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split out of #829 as an independent fix, per @Vasanthdev2004's review asking for the self-contained pieces to arrive separately. The detector marks a run incomplete when the model admits it could not finish. Its allowance list exists so a finder reporting an absence — "I could not find where X is set in production code" — is not marked incomplete for doing its job; that case cost a real audit, which spent 53 tool calls proving a negative and was called incomplete for saying so. But the stems the allowance matches on ("find the", "reproduce ", "confirm any", "observe any") also head the most ordinary way of admitting defeat, and the allowance fired on the tail prefix alone regardless of how the sentence ended. Measured against eleven genuine admissions, TEN passed the detector undetected: "I could not reproduce the crash, so the fix is unverified." "I could not find the root cause; someone else will need to pick this up." "I could not locate the source of the regression and have run out of ideas." That is the guard's whole purpose defeated: it is the last thing between a stalled run and a report that reads like success. The allowance now yields when the sentence ALSO says the work is blocked (blockedWorkMarkers). Re-measured: 3 of 11 still pass, all single-clause sentences carrying no blocked-work signal, and false positives on legitimate findings stayed at zero. Deliberately not tuned further: the remaining three are genuinely ambiguous without more context, and fitting the list to them would be over-fitting to the eleven examples rather than measuring. Independent of the remaining #829 work; builds and tests against current main on its own. Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 118 +++++++++- .../agent/guardrails_false_admission_test.go | 208 ++++++++++++++++++ internal/agent/guardrails_test.go | 38 ++++ 3 files changed, 362 insertions(+), 2 deletions(-) create mode 100644 internal/agent/guardrails_false_admission_test.go diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index ccd9d26e9..376f9eb7f 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -168,7 +168,14 @@ var selfReportPhrases = []string{ var inabilityStems = []string{ "i cannot ", "i can't ", "i can not ", "i could not ", "i couldn't ", "i am unable to", "i'm unable to", "i was unable to", "i wasn't able to", - "i was not able to", "i do not have", "i don't have", "unable to ", + "i was not able to", "i do not have", "i don't have", + // "unable to " WITHOUT A SUBJECT WAS REMOVED. It is the only stem here that + // does not name who was unable, and it fired on a report's own section + // heading — "**Unable to verify (1):** - MCP #3 claim was truncated" — in a + // verification task that had completed and was categorising its findings. + // The first-person forms above still catch every genuine admission; a + // heading is not one. + "we are unable to", "we were unable to", "without being able to", } @@ -179,6 +186,19 @@ var inabilityStems = []string{ var successNegationTails = []string{ "find any", "found any", "find a ", "see any", "detect any", "identify any", "reproduce", "spot any", "locate any", + // A NEGATIVE SEARCH RESULT IS THE ANSWER, not a failure to produce one. + // + // The list above already encodes this — "I could not find any remaining + // issues" is success — but only for the "any" phrasings. A finder reporting + // "I could NOT find where AllowManifestToolAutoApproval is set to true in + // production code" was marked INCOMPLETE after 53 tool calls and a 19,145 + // character audit, for doing precisely the job it was given: establishing + // that something is not there. + "find where", "find the", "find it", "find that", "find this", + "found where", "found the", + "locate where", "locate the", "locate it", + "determine where", "identify where", "see where", + "reproduce ", "confirm any", "observe any", } // narrativeMarkers flag a sentence as RETELLING a past exchange rather than @@ -247,11 +267,101 @@ func admissionSentences(lower string) []string { // retells a past exchange (narrativeMarkers) is skipped entirely — an admission // must be the model's own report about the CURRENT objective, not general // language that merely resembles one. +// toolGrantMarkers flag a sentence as reporting WHICH TOOLS this run was given, +// not whether the work was done. +// +// A read-only plan task is SUPPOSED to say this. One wrote "I don't have an +// update_plan tool available in this specialist context (only read-only +// exploration tools were provided)" and then delivered the complete answer — +// helper name, file, line 214, full source — and was marked INCOMPLETE on the +// "i don't have" stem. The prompt asks tasks to name their limits plainly; a +// detector that punishes exactly that teaches the opposite. +// +// NARROW ON PURPOSE: the sentence must name a TOOL or a GRANT. "I do not have +// enough evidence" is still an admission and still fires. +// NARROWED AFTER AN AUDIT OF THIS VERY FIX. The first version listed a bare +// " tool", which exempts any inability sentence that merely mentions one — +// measured at 5/5 on ordinary phrasings: +// +// "I cannot run the build tool, so the change is unverified" +// "I could not use the migration tool and the data is untouched" +// "I was unable to invoke the formatting tool on the output" +// +// Those are genuine admissions, and silently exempting them is the WORSE +// direction: a false positive costs a re-run, a false negative reports +// unfinished work as done. The markers now have to be about what the run WAS +// GIVEN, not about a tool being mentioned at all. +var toolGrantMarkers = []string{ + "tool available", "tools available", "no such tool", "not available in this", + "read-only tools", "read only tools", "only read-only", "only read only", + "tools were provided", "tools were given", "toolset provided", + "in this specialist context", "in this context only", + "is not in my toolset", "not in my toolset", "not in this toolset", +} + +// objectiveFailureMarkers name the OBJECTIVE rather than a capability. A +// sentence carrying one is about whether the job got done, so the tool-grant +// exemption above does not apply to it however many tools it mentions. +// VERB-ANCHORED, not bare nouns. "this task" alone was too crude: a task that +// finished wrote "so i could not record a plan; the task is a single +// read-and-report step and is now complete" — it names the task in order to +// report SUCCESS, and a bare-noun override read that as failure. The marker has +// to be the objective NOT BEING DONE, which needs the verb. +var objectiveFailureMarkers = []string{ + "complete this task", "complete the task", "completing this task", "completing the task", + "finish this task", "finish the task", "finishing this task", + "complete it", "completing it", "finish it", "finishing it", + "the objective", "the assignment", "as requested", "what was asked", + "do this task", "perform this task", "carry out this task", +} + +// blockedWorkMarkers are what turns an absence-establishing sentence back into +// an admission of failure. +// +// MEASURED, NOT ARGUED. The successNegationTails list above exists so a finder +// reporting "I could not find where X is set" is not marked incomplete for doing +// its job. But the stems it added — "find the", "reproduce ", "confirm any", +// "observe any" — also head the most ordinary way of admitting defeat, and the +// allowance fired on the whole sentence regardless of how it ended. Measured +// against eleven genuine admissions, TEN passed the detector undetected: +// +// "I could not reproduce the crash, so the fix is unverified." +// "I could not find the root cause; someone else will need to pick this up." +// "I could not locate the source of the regression and have run out of ideas." +// +// That is the guard's entire purpose defeated — it is the last thing between a +// stalled run and a report that reads like success. +// +// So the allowance now yields when the sentence ALSO says the work is blocked. +// "I could not find where X is set in production code" still passes, because +// nothing in it claims the objective was left undone; the three above do not. +var blockedWorkMarkers = []string{ + "unverified", "not verified", "cannot verify", "could not verify", + "someone else", "will need to", "needs someone", "handed off", "hand off", + "ran out of", "run out of", "out of time", "in the time available", + "stopped there", "stopping here", "gave up", "giving up", + "nothing was modified", "no changes were made", "left unchanged", "left undone", + "may be inert", "is unresolved", "remains unresolved", "still broken", + "so i cannot", "so i could not", +} + func selfReportedIncompletion(text string) string { for _, sentence := range admissionSentences(strings.ToLower(stripQuoted(text))) { if containsAny(sentence, narrativeMarkers) { continue } + // A sentence about the tool grant is about CAPABILITY, not about the + // objective — UNLESS it also says the task itself could not be done. + // + // THE OVERRIDE IS NOT OPTIONAL. Without it the exemption swallowed a + // genuine failure: "I am unable to complete this task with the current + // tool set … Only write_file is enabled … so I cannot inspect the + // codebase." That task really did fail, and it mentions tools, so a bare + // tool-marker check waved it through. Naming the task is what separates + // "I lack a tool I did not need" from "I lack the tools this needed". + if containsAny(sentence, toolGrantMarkers) && !containsAny(sentence, objectiveFailureMarkers) { + continue + } for _, phrase := range selfReportPhrases { if strings.Contains(sentence, phrase) { return selfReportReason(phrase) @@ -268,7 +378,11 @@ func selfReportedIncompletion(text string) string { } abs := start + rel tail := strings.TrimSpace(sentence[abs+len(stem):]) - if !hasAnyPrefix(tail, successNegationTails) { + // The allowance yields to a sentence that also says the work is + // blocked: "could not reproduce the crash" is a finding, "could not + // reproduce the crash, so the fix is unverified" is an admission, + // and the tail prefix alone cannot tell them apart. + if !hasAnyPrefix(tail, successNegationTails) || containsAny(sentence, blockedWorkMarkers) { return selfReportReason(strings.TrimSpace(stem) + " …") } start = abs + len(stem) diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go new file mode 100644 index 000000000..8419da8db --- /dev/null +++ b/internal/agent/guardrails_false_admission_test.go @@ -0,0 +1,208 @@ +package agent + +import "testing" + +// FOUR COMPLETED TASKS WERE MARKED INCOMPLETE, and the sentences below are +// verbatim from those sessions. +// +// The detector reads message text — the one place in this feature that does — +// and it is supposed to catch a model admitting it did not do the job. Instead +// it caught four tasks that HAD done the job and were honestly naming a limit, +// which is exactly what the plan-task prompt asks them to do. Two of them had +// made 53 and 60 tool calls; one had written files. +// +// The cost of a false positive here is not cosmetic: the task is reported +// failed, retried on another model, and rendered red in the plan panel. +func TestAnHonestCaveatInsideDeliveredWorkIsNotAnAdmission(t *testing.T) { + for _, tc := range []struct { + name string + text string + }{ + { + // 53 tool calls, a 19,145-character audit. Establishing that + // something is NOT there is the finding, not a failure to find one. + name: "a negative search result is the answer", + text: "I traced every code path where repo-committed files influence tool approval. " + + "I could NOT find where `AllowManifestToolAutoApproval` is set to true in production code paths.", + }, + { + // A completed verification task categorising its own findings. The + // bare "unable to" stem had no first-person subject and fired on a + // markdown heading. + name: "a report section heading", + text: "## Verification Table\n\n**Verified (4):** all confirmed against source.\n" + + "**Unable to verify (1):** - MCP #3 claim was truncated in the input.", + }, + { + // Delivered helper name, file, line 214 and full source. The task was + // read-only by design and said so. + name: "a statement about the tool grant", + text: "I don't have an `update_plan` tool available in this specialist context " + + "(only read-only exploration tools were provided). " + + "The task is a single read-only assessment and I've already gathered all the needed evidence.", + }, + { + name: "read-only tools named plainly", + text: "I did not run the tests because my tools are read-only, " + + "so I can report only what the code and tests say statically.", + }, + } { + t.Run(tc.name, func(t *testing.T) { + if reason := selfReportedIncompletion(tc.text); reason != "" { + t.Fatalf("a completed task was marked incomplete: %s", reason) + } + }) + } +} + +// AND THE DETECTOR MUST STILL BITE. Widening it to stop punishing honesty must +// not turn it off — these are the admissions it exists for, several of them +// verbatim from the same set of sessions. +func TestGenuineAdmissionsAreStillCaught(t *testing.T) { + for _, tc := range []struct { + name string + text string + }{ + { + name: "the measured workspace failure", + text: "I cannot complete this task. The target directory `/Users/kratos/zm-lab/pkg/execprofile` " + + "is outside the workspace boundary.", + }, + { + name: "an empty worktree", + text: "This task cannot be completed in the current workspace because the relevant source tree is absent. " + + "I cannot find BuildFinalResult anywhere.", + }, + { + name: "guessing", + text: "I guessed at the retry semantics because the code was not reachable.", + }, + { + name: "fabrication", + text: "I fabricated the line numbers to fill the table.", + }, + { + name: "stated doubt about the result", + text: "The patch may not be correct — I did not run it.", + }, + { + name: "not enough evidence is still an admission", + text: "I do not have enough evidence to answer the question.", + }, + { + name: "first-person unable, with a subject", + text: "I was unable to determine which branch actually runs.", + }, + } { + t.Run(tc.name, func(t *testing.T) { + if reason := selfReportedIncompletion(tc.text); reason == "" { + t.Fatalf("a real admission was missed: %q", tc.text) + } + }) + } +} + +// The tool-grant exemption is NARROW. A sentence has to be about tools or the +// grant; "not enough evidence" is not, and must keep firing. +func TestTheToolGrantExemptionDoesNotSwallowRealAdmissions(t *testing.T) { + if reason := selfReportedIncompletion("I could not verify the claim and I do not have enough evidence."); reason == "" { + t.Fatal("the tool-grant exemption swallowed an evidence admission") + } + if reason := selfReportedIncompletion("I don't have the file contents, so the answer is a guess."); reason == "" { + t.Fatal("an admission about missing content was exempted as a tool statement") + } +} + +// THE EXEMPTION MUST NOT SWALLOW A TASK THAT REALLY COULD NOT BE DONE. +// +// Verbatim from a session the first version of this fix regressed: the task +// mentions tools, so a bare tool-marker check waved it through — but it names +// the TASK as the thing that failed, which is the whole distinction. +func TestATaskThatCouldNotBeDoneForLackOfToolsStillFails(t *testing.T) { + for _, tc := range []struct { + name string + text string + }{ + { + name: "the measured regression", + text: "I am unable to complete this task with the current tool set. " + + "Only `write_file` is enabled, so I cannot inspect the codebase to collect scan findings.", + }, + { + name: "named differently", + text: "I could not finish the objective because no read tools were provided.", + }, + { + name: "cannot complete it", + text: "I don't have the tools to complete it as requested.", + }, + } { + t.Run(tc.name, func(t *testing.T) { + if reason := selfReportedIncompletion(tc.text); reason == "" { + t.Fatalf("a task that genuinely could not be done was passed as complete: %q", tc.text) + } + }) + } + + // And the exemption must still work for the case it exists for: a tool that + // was not needed, alongside delivered work. + exempt := "I don't have an `update_plan` tool available in this specialist context " + + "(only read-only exploration tools were provided)." + if reason := selfReportedIncompletion(exempt); reason != "" { + t.Fatalf("the override broke the exemption it guards: %s", reason) + } +} + +// NAMING THE TASK TO REPORT SUCCESS IS NOT NAMING IT TO REPORT FAILURE. +// +// Verbatim from the session that the first override regressed. A task that had +// finished wrote a footnote about a tool it did not need, in the same sentence +// as its own completion statement — and a bare-noun override read "the task" as +// an admission. The marker has to be the objective NOT being done. +func TestNamingTheTaskWhileReportingSuccessIsNotAnAdmission(t *testing.T) { + done := "- **note:** the update_plan tool is not available in my current toolset " + + "(only read-only file tools), so i could not record a plan; " + + "the task is a single read-and-report step and is now complete" + if reason := selfReportedIncompletion(done); reason != "" { + t.Fatalf("a finished task was marked incomplete for saying so: %s", reason) + } + // The failure form, which must still fire, differs only in the verb. + failed := "the update_plan tool is not available, so i could not complete this task" + if reason := selfReportedIncompletion(failed); reason == "" { + t.Fatal("a task that could not be completed was passed as complete") + } +} + +// AN INABILITY THAT MERELY MENTIONS A TOOL IS STILL AN ADMISSION. +// +// THE AUDIT FINDING THIS PINS. The first version of the exemption listed a bare +// " tool", so any inability sentence mentioning one escaped — measured at 5/5 on +// ordinary phrasings. That is the worse direction: a false positive costs a +// re-run, a false negative reports unfinished work as done. +func TestMentioningAToolDoesNotExemptAnAdmission(t *testing.T) { + for _, text := range []string{ + "I cannot run the build tool, so the change is unverified", + "I could not use the migration tool and the data is untouched", + "I was unable to invoke the formatting tool on the output", + "I don't have a working compiler tool here", + "I cannot finish because the deploy tools are broken", + } { + if reason := selfReportedIncompletion(text); reason == "" { + t.Errorf("a genuine admission was silently exempted: %q", text) + } + } +} + +// And the exemption still covers what it was built for: a run stating which +// tools it was GIVEN, alongside delivered work. +func TestTheGrantExemptionStillCoversWhatItWasBuiltFor(t *testing.T) { + for _, text := range []string{ + "I don't have an `update_plan` tool available in this specialist context (only read-only exploration tools were provided).", + "I did not run the tests because only read-only tools were provided, so I report what the code says statically.", + "update_plan is not in my toolset, so I could not record a plan.", + } { + if reason := selfReportedIncompletion(text); reason != "" { + t.Errorf("a run naming its own grant was marked incomplete: %q -> %s", text, reason) + } + } +} diff --git a/internal/agent/guardrails_test.go b/internal/agent/guardrails_test.go index 5ae974eb4..d25a530a6 100644 --- a/internal/agent/guardrails_test.go +++ b/internal/agent/guardrails_test.go @@ -554,3 +554,41 @@ func TestRunInjectsToolFailureHintWithSchema(t *testing.T) { t.Fatalf("expected a tool-failure hint on the 3rd turn, messages: %+v", provider.requests[2].Messages) } } + +// An absence-establishing sentence is a finding; the same sentence that also +// says the work is blocked is an admission. The allowance stems ("find the", +// "reproduce ", "confirm any" …) head both, so the tail prefix alone cannot +// separate them — measured against real phrasings, TEN of eleven genuine +// admissions passed the detector before blockedWorkMarkers existed. +func TestIncompletionAllowanceYieldsToBlockedWork(t *testing.T) { + // Must FIRE: the inability is reported as leaving work undone. + for _, admission := range []string{ + "I could not reproduce the crash, so the fix is unverified.", + "I was unable to reproduce the reported behaviour and stopped there.", + "I could not find the root cause; someone else will need to pick this up.", + "I could not find the bug in the time available.", + "I did not manage to reproduce the failure, so I cannot confirm the patch works.", + "I could not locate the source of the regression and have run out of ideas.", + "I could not find where to apply the change, so nothing was modified.", + "I could not observe any effect, so the change may be inert.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("admission passed the detector: %q", admission) + } + } + + // Must NOT fire: establishing that something is absent is the job, and the + // motivating case is the audit that spent 53 tool calls proving a negative. + for _, finding := range []string{ + "I could not find any remaining issues.", + "I could NOT find where AllowManifestToolAutoApproval is set to true in production code.", + "I could not find the flag being set anywhere outside tests, so the concern does not apply.", + "I could not reproduce the reported exploit, which confirms the guard holds.", + "I could not observe any regression across the suite.", + "I could not confirm any leak; every path is bounded.", + } { + if reason := selfReportedIncompletion(finding); reason != "" { + t.Errorf("finding wrongly flagged as incomplete: %q -> %q", finding, reason) + } + } +} From b6ad1fcddf2f29b6393de302515d3ac2b22290d3 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sun, 16 Aug 2026 10:53:13 +0530 Subject: [PATCH 02/28] fix(agent): keep the subjectless stem, and stop blocked-work markers overriding an explicit "any" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects a self-review of this branch found, both introduced by the previous commit on it. THE SUBJECTLESS STEM IS BACK. Deleting "unable to " silenced one completed audit's section heading and lost every admission with no first-person subject: "Unable to complete the task; the build never succeeded." "The agent was unable to finish the migration." "Unable to verify the fix, so the change is unverified." None of these names "i" or "we", so no other stem sees them — one false positive traded for three false negatives, in the direction this guard exists to prevent. The heading is now recognised by its own shape instead: countedLabelSentence skips a sentence that BEGINS with the inability phrase and carries a parenthesised count, which is what "**Unable to verify (1):**" is and what an admission never is. BLOCKED-WORK MARKERS NO LONGER OVERRIDE AN EXPLICIT "ANY". "someone else" and "will need to" describe somebody else's future work, which a successful report says as often as a blocked one, and they flipped two findings into false incompletions: "I could not find any remaining issues, though a follow-up will need to cover the Windows path." "I could not find any blockers; someone else can take the release from here." An explicit "any" is the model asserting it looked and found nothing, so strongAbsenceTails now exempts those tails from the blocked-work override while the weaker ones ("find the", "reproduce ") still yield to it — which keeps "I could not find the root cause; someone else will need to pick this up" firing. Re-measured: 4 of 11 admissions still pass, 0 of 8 findings wrongly flagged. The four are recorded in the test rather than hidden — they are "any"-phrased or single-clause with no blocked-work signal, and every attempt to reach them re-broke the findings. Origin-Session: local-0484ba | Claude Code | 6 prompts Origin-Snapshot: e6d90ff3ce46 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 67 +++++++++++++++++++++++++++---- internal/agent/guardrails_test.go | 52 +++++++++++++++++++++++- 2 files changed, 111 insertions(+), 8 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 376f9eb7f..09b1b44ef 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -1,6 +1,7 @@ package agent import ( + "regexp" "strconv" "strings" @@ -169,13 +170,23 @@ var inabilityStems = []string{ "i cannot ", "i can't ", "i can not ", "i could not ", "i couldn't ", "i am unable to", "i'm unable to", "i was unable to", "i wasn't able to", "i was not able to", "i do not have", "i don't have", - // "unable to " WITHOUT A SUBJECT WAS REMOVED. It is the only stem here that - // does not name who was unable, and it fired on a report's own section - // heading — "**Unable to verify (1):** - MCP #3 claim was truncated" — in a - // verification task that had completed and was categorising its findings. - // The first-person forms above still catch every genuine admission; a - // heading is not one. "we are unable to", "we were unable to", + // THE SUBJECTLESS STEM IS KEPT, and the heading it fired on is handled where + // the heading is, not by deleting the stem. + // + // It was removed once because a completed audit's own section heading — + // "**Unable to verify (1):** - MCP #3 claim was truncated" — reads as an + // admission. But it is the ONLY stem that catches an admission with no + // first-person subject, and removing it lost every one of those: + // + // "Unable to complete the task; the build never succeeded." + // "The agent was unable to finish the migration." + // "Unable to verify the fix, so the change is unverified." + // + // None name "i" or "we", so no other stem sees them. That traded one false + // positive for three false negatives, in the direction this guard exists to + // prevent. countedLabelSentence drops the heading shape instead. + "unable to ", "without being able to", } @@ -335,9 +346,23 @@ var objectiveFailureMarkers = []string{ // So the allowance now yields when the sentence ALSO says the work is blocked. // "I could not find where X is set in production code" still passes, because // nothing in it claims the objective was left undone; the three above do not. +// strongAbsenceTails are the allowance tails carrying an explicit "any": the +// model asserting it looked and found NOTHING. That is a finding whatever the +// rest of the sentence says, so blockedWorkMarkers does not override them. +var strongAbsenceTails = []string{ + "find any", "found any", "see any", "detect any", "identify any", + "spot any", "locate any", "confirm any", "observe any", +} + var blockedWorkMarkers = []string{ "unverified", "not verified", "cannot verify", "could not verify", "someone else", "will need to", "needs someone", "handed off", "hand off", + // "someone else" and "will need to" are the two that also appear in + // SUCCESSFUL reports, describing somebody else's future work — "I could not + // find any remaining issues, though a follow-up will need to cover the + // Windows path" is a finding. They are kept because they do carry a genuine + // handoff-because-blocked ("someone else will need to pick this up"), and the + // strongAbsenceTails exemption above is what stops them flipping a finding. "ran out of", "run out of", "out of time", "in the time available", "stopped there", "stopping here", "gave up", "giving up", "nothing was modified", "no changes were made", "left unchanged", "left undone", @@ -345,11 +370,38 @@ var blockedWorkMarkers = []string{ "so i cannot", "so i could not", } +// countedLabelSentence reports whether a sentence is a markdown LABEL that +// introduces and counts a bucket of findings, rather than a claim about the +// objective. +// +// "**Unable to verify (1):** - MCP #3 claim was truncated" is a section heading +// in a COMPLETED audit. Read as a sentence it is indistinguishable from an +// admission, which is why the subjectless stem was once deleted to silence it. +// +// NARROW ON PURPOSE: the sentence must BEGIN with the inability phrase, after +// markdown emphasis, AND carry a parenthesised count. "Unable to complete the +// task; the build never succeeded" begins the same way and has no count, so it +// still fires — which is the whole reason this is preferable to dropping the +// stem. +func countedLabelSentence(sentence string) bool { + trimmed := strings.TrimLeft(strings.TrimSpace(sentence), "-*#> \t") + if !strings.HasPrefix(trimmed, "unable to ") { + return false + } + return countedLabelSuffix.MatchString(sentence) +} + +var countedLabelSuffix = regexp.MustCompile(`\(\s*\d+\s*\)`) + func selfReportedIncompletion(text string) string { for _, sentence := range admissionSentences(strings.ToLower(stripQuoted(text))) { if containsAny(sentence, narrativeMarkers) { continue } + // A counted label is a heading, not a claim about the objective. + if countedLabelSentence(sentence) { + continue + } // A sentence about the tool grant is about CAPABILITY, not about the // objective — UNLESS it also says the task itself could not be done. // @@ -382,7 +434,8 @@ func selfReportedIncompletion(text string) string { // blocked: "could not reproduce the crash" is a finding, "could not // reproduce the crash, so the fix is unverified" is an admission, // and the tail prefix alone cannot tell them apart. - if !hasAnyPrefix(tail, successNegationTails) || containsAny(sentence, blockedWorkMarkers) { + strong := hasAnyPrefix(tail, strongAbsenceTails) + if !hasAnyPrefix(tail, successNegationTails) || (!strong && containsAny(sentence, blockedWorkMarkers)) { return selfReportReason(strings.TrimSpace(stem) + " …") } start = abs + len(stem) diff --git a/internal/agent/guardrails_test.go b/internal/agent/guardrails_test.go index d25a530a6..691331a57 100644 --- a/internal/agent/guardrails_test.go +++ b/internal/agent/guardrails_test.go @@ -570,7 +570,6 @@ func TestIncompletionAllowanceYieldsToBlockedWork(t *testing.T) { "I did not manage to reproduce the failure, so I cannot confirm the patch works.", "I could not locate the source of the regression and have run out of ideas.", "I could not find where to apply the change, so nothing was modified.", - "I could not observe any effect, so the change may be inert.", } { if selfReportedIncompletion(admission) == "" { t.Errorf("admission passed the detector: %q", admission) @@ -586,9 +585,60 @@ func TestIncompletionAllowanceYieldsToBlockedWork(t *testing.T) { "I could not reproduce the reported exploit, which confirms the guard holds.", "I could not observe any regression across the suite.", "I could not confirm any leak; every path is bounded.", + // Next steps and ownership belong in successful reports too. Both of + // these were flagged when blockedWorkMarkers could override an explicit + // "any", which is the model asserting exhaustive absence. + "I could not find any remaining issues, though a follow-up will need to cover the Windows path.", + "I could not find any blockers; someone else can take the release from here.", } { if reason := selfReportedIncompletion(finding); reason != "" { t.Errorf("finding wrongly flagged as incomplete: %q -> %q", finding, reason) } } + + // NOT CAUGHT, and recorded rather than hidden. An explicit "any" is treated + // as exhaustive absence, so an admission phrased that way passes: + // + // "I could not observe any effect, so the change may be inert." + // + // Measured across eleven admissions, four still pass, all of them either + // "any"-phrased or single-clause with no blocked-work signal ("I failed to + // reproduce it locally"). Catching those needs a different signal than + // substring matching; tightening this list to reach them re-broke the + // findings above every way I tried. +} + +// An admission with NO first-person subject must still fire. The subjectless +// "unable to " stem was once deleted to silence a completed audit's section +// heading, which also lost every impersonal admission — none of these names "i" +// or "we", so no other stem sees them. +func TestImpersonalAdmissionsAreStillCaught(t *testing.T) { + for _, admission := range []string{ + "Unable to complete the task; the build never succeeded.", + "The agent was unable to finish the migration.", + "Unable to verify the fix, so the change is unverified.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("an impersonal admission passed the detector: %q", admission) + } + } +} + +// The heading that motivated deleting the stem must stay exempt. It is a label +// counting a bucket of findings in a COMPLETED audit, not a claim about the +// objective — which is why it is recognised by shape rather than by removing a +// stem that catches real admissions. +func TestACountedHeadingIsNotAnAdmission(t *testing.T) { + for _, heading := range []string{ + "**Unable to verify (1):** - MCP #3 claim was truncated", + "Unable to reproduce (3):", + } { + if reason := selfReportedIncompletion(heading); reason != "" { + t.Errorf("a counted heading was read as an admission: %q -> %q", heading, reason) + } + } + // But the same opening WITHOUT a count is a real admission. + if selfReportedIncompletion("Unable to verify the deployment; it never started.") == "" { + t.Error("a subjectless admission with no count was exempted as a heading") + } } From 5ad73446e44b5485ba623fd4dfdac00191cadaaf Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sun, 16 Aug 2026 15:57:16 +0530 Subject: [PATCH 03/28] fix(agent): verb-anchor the objective markers, and stop a tool caveat excusing blocked work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both raised by CodeRabbit, and both are the detector reading a sentence as the opposite of what it says. BARE NOUNS READ SUCCESS AS FAILURE. "this task" had already been verb-anchored for this exact reason, but "the objective" and "the assignment" were still bare, so a message that mentions a tool it lacked and then reports the objective IS met was read as saying it was not: "I do not have write tools available, but the objective is met: the config already sets the flag." -> the final message admits the objective was not met A finished answer told it had not finished is the worst thing this detector does, and it is the failure mode the verb-anchoring exists to prevent. Measured across success and failure framings of all four bare entries: 2 of 9 successes wrongly flagged before, 1 after, with no genuine failure newly missed. The one that remains is a compound sentence whose first clause is a real admission ("I could not patch it, but the objective is satisfied"), which is arguable rather than plainly wrong. A TOOL CAVEAT IS THE REASON WORK IS BLOCKED, NOT A REASON TO STOP READING. The exemption asked only whether the sentence named the objective, so an admission that named none was waved through for mentioning tools: "No write tools available, so I could not verify the change." -> nothing "There is no edit tool available here, so I could not verify the fix." -> nothing It now also breaks on a blocked STATE. Deliberately NOT on the whole blockedWorkMarkers list: that includes the bare stems "so i cannot" and "so i could not", and applying the list whole regressed a verbatim real-session case — "so i could not record a plan; the task is a single read-and-report step and is now complete" is a FINISHED task, and it started being flagged. The state markers are derived from the list rather than copied so the two cannot drift. Still missed, and left honest rather than patched over: a blocked state with no inability stem at all ("so I ran out of ways to check it") never reaches a trigger. Closing that needs a standalone blocked-state detector, which is a wider change than these findings. All three changes mutation-checked: restoring the bare nouns, restoring the old exemption, and using the whole blocked list each fail the test that covers them. Origin-Session: local-abff1c | Claude Code | 2 prompts Origin-Snapshot: d2f269b81f33 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 57 ++++++++++++++++- .../agent/guardrails_false_admission_test.go | 62 +++++++++++++++++++ 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 09b1b44ef..edc17c017 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -322,7 +322,17 @@ var objectiveFailureMarkers = []string{ "complete this task", "complete the task", "completing this task", "completing the task", "finish this task", "finish the task", "finishing this task", "complete it", "completing it", "finish it", "finishing it", - "the objective", "the assignment", "as requested", "what was asked", + // The objective and the assignment need the verb too, for the same reason + // "this task" did. "the objective is met" and "the assignment is complete" + // name the objective in order to report SUCCESS, and a bare noun read both + // as failure — so a finished answer carrying a tool caveat was told it had + // not finished, which is the worst thing this detector can do. + "complete the objective", "completing the objective", + "finish the objective", "finishing the objective", + "meet the objective", "meeting the objective", "achieve the objective", + "complete the assignment", "completing the assignment", + "finish the assignment", "finishing the assignment", + "as requested", "what was asked", "do this task", "perform this task", "carry out this task", } @@ -370,6 +380,34 @@ var blockedWorkMarkers = []string{ "so i cannot", "so i could not", } +// bareInabilityStems are the two entries above that are STEMS rather than +// descriptions of a blocked state. The stem loop below already scans them with +// its own tail logic, and a sentence can carry one on its way to reporting +// success: "so i could not record a plan; the task is a single read-and-report +// step and is now complete" is a finished task that did not need the plan. +var bareInabilityStems = []string{"so i cannot", "so i could not"} + +// blockedStateMarkers describe WORK LEFT BLOCKED — unverified, unresolved, +// handed off, abandoned for time. Only these break the tool-grant exemption: +// they say something about the state the work is in, which a tool caveat cannot +// excuse, whereas a bare stem says only that one step did not happen. +// +// DERIVED, not copied. Two hand-maintained lists that must stay in step drift, +// and the drift here would be silent in both directions. +var blockedStateMarkers = func() []string { + bare := make(map[string]bool, len(bareInabilityStems)) + for _, stem := range bareInabilityStems { + bare[stem] = true + } + out := make([]string, 0, len(blockedWorkMarkers)) + for _, marker := range blockedWorkMarkers { + if !bare[marker] { + out = append(out, marker) + } + } + return out +}() + // countedLabelSentence reports whether a sentence is a markdown LABEL that // introduces and counts a bucket of findings, rather than a claim about the // objective. @@ -411,7 +449,22 @@ func selfReportedIncompletion(text string) string { // codebase." That task really did fail, and it mentions tools, so a bare // tool-marker check waved it through. Naming the task is what separates // "I lack a tool I did not need" from "I lack the tools this needed". - if containsAny(sentence, toolGrantMarkers) && !containsAny(sentence, objectiveFailureMarkers) { + // BLOCKED WORK BREAKS THE EXEMPTION TOO, not just a named objective. The + // override above asks whether the sentence names the task, which a whole + // class of real admissions never does: + // + // "No write tool is available in this context, so the fix is unverified." + // "There is no edit tool available here, so the change remains unapplied." + // "Write tools are not available in this setup, so the tests were never run." + // + // Every one mentions tools, none names the objective, and all four were + // waved through. The tool caveat is the REASON the work is blocked, not a + // reason to stop reading — so a sentence that also says something is + // unverified, unapplied or untested goes on to the blocked-work handling + // below instead of being exempted here. + if containsAny(sentence, toolGrantMarkers) && + !containsAny(sentence, objectiveFailureMarkers) && + !containsAny(sentence, blockedStateMarkers) { continue } for _, phrase := range selfReportPhrases { diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 8419da8db..cf2fba4db 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -206,3 +206,65 @@ func TestTheGrantExemptionStillCoversWhatItWasBuiltFor(t *testing.T) { } } } + +// NAMING THE OBJECTIVE TO REPORT SUCCESS, same trap as naming the task. +// +// "the objective" and "the assignment" were bare nouns in objectiveFailureMarkers +// while "this task" had already been verb-anchored for exactly this reason. A +// message that mentions a tool it lacked and then says the objective IS met was +// read as saying the objective was not met — a finished answer told it had not +// finished, which is the worst outcome this detector has. +func TestNamingTheObjectiveWhileReportingSuccessIsNotAnAdmission(t *testing.T) { + for _, done := range []string{ + "I do not have write tools available, but the objective is met: the config already sets the flag.", + "No write tool is available to me, so the objective was achieved by reading alone.", + "The assignment is complete; no shell was needed.", + "I have no browser tool available here, yet the assignment is complete.", + } { + if reason := selfReportedIncompletion(done); reason != "" { + t.Errorf("a finished answer was marked incomplete for naming its objective: %q -> %s", done, reason) + } + } + // The failure forms differ only in the verb, and must still fire. + for _, failed := range []string{ + "I could not complete the objective because the build never succeeded.", + "I was unable to finish the assignment.", + } { + if reason := selfReportedIncompletion(failed); reason == "" { + t.Errorf("an objective that was not met was passed as complete: %q", failed) + } + } +} + +// A TOOL CAVEAT IS THE REASON WORK IS BLOCKED, NOT A REASON TO STOP READING. +// +// The tool-grant exemption asked only whether the sentence named the objective, +// so an admission that named none was waved through on the strength of +// mentioning tools: +// +// "No write tools available, so I could not verify the change." +// +// It breaks now on a blocked STATE — unverified, unresolved, handed off — but +// deliberately not on the two bare inability stems in that list, because a +// sentence carrying one can still be on its way to reporting success (see +// TestNamingTheTaskWhileReportingSuccessIsNotAnAdmission, which this broke when +// the whole list was used). +func TestAToolCaveatDoesNotExcuseBlockedWork(t *testing.T) { + for _, admission := range []string{ + "No write tools available, so I could not verify the change.", + "There is no edit tool available here, so I could not verify the fix.", + } { + if reason := selfReportedIncompletion(admission); reason == "" { + t.Errorf("a tool caveat excused blocked work: %q", admission) + } + } + // The exemption still covers what it exists for: a tool that was not needed. + for _, exempt := range []string{ + "No update_plan tool available in this specialist context, so I proceeded directly.", + "There is no update_plan tool available in this specialist context (only read-only tools), so I have written the plan into this answer instead.", + } { + if reason := selfReportedIncompletion(exempt); reason != "" { + t.Errorf("the blocked-state check broke the exemption it guards: %q -> %s", exempt, reason) + } + } +} From 732c760df2eccca98b0b20e2b540f9a5cb99c90f Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sun, 16 Aug 2026 16:30:18 +0530 Subject: [PATCH 04/28] fix(agent): an absence is a finding only when the object makes it one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @anandh8x, second of his two on this PR. The first — a tool caveat excusing blocked work, "I don't have the deploy tool available in this context, so the release remains unresolved" — is already fixed on this branch; his review predates that commit, and the sentence is caught on the current head. WHAT FOLLOWS "any" DECIDES. The "any"-family was read as a finding whatever came after it, so an admission wearing the same words walked straight through: "I could not find any remaining issues" -> a finding, the search succeeded "I could not find any solution" -> an admission, the work did not Both carry the explicit "any". Only the object separates them, so only the object can classify them. Absence is now the result for a list of things you go looking for in order to report there are none — issues, regressions, evidence, races, blockers — and everything else falls through to the ordinary blocked-work handling rather than being exempted. THE OBJECT LIST IS AN ALLOW-LIST, deliberately. A deny-list of deliverables (solution, fix, workaround, approach…) would have to anticipate every noun a model might reach for, and each one forgotten would be waved through as success — which is the direction this detector must not fail in. An unrecognised object is simply not strong; it is not flagged outright either, it just stops being exempt. Measured on both sides: four admissions that previously passed are now caught, and five findings — including the ones carrying "someone else will need to" about somebody else's future work, which is exactly what the allowance exists for — are untouched. Writing the object list revealed "blockers" was missing, caught by an existing test rather than by inspection. Mutation-checked: restoring the unconditional "any" prefix lets three of the four admissions through again. Origin-Session: local-abff1c | Claude Code | 3 prompts Origin-Snapshot: 07900397c62e Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 88 ++++++++++++++++++- .../agent/guardrails_false_admission_test.go | 38 ++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index edc17c017..2fd8eda36 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -364,6 +364,92 @@ var strongAbsenceTails = []string{ "spot any", "locate any", "confirm any", "observe any", } +// strongAbsenceObjects are the things whose ABSENCE IS THE RESULT: you go +// looking for them precisely so you can report there are none, and finding none +// is the work succeeding. +// +// WHAT FOLLOWS "any" DECIDES, and treating every "find any" as success was too +// broad: +// +// "I could not find any remaining issues" -> a finding, the search succeeded +// "I could not find any solution" -> an admission, the work did not +// +// Both carry the explicit "any". Only the object separates them, so only the +// object can classify them. +// +// AN ALLOW-LIST, because this grants the exemption. A deny-list of deliverables +// would have to anticipate every noun a model might reach for, and everything +// forgotten would be waved through as success — the failure direction this +// detector exists to prevent. An unrecognised object is simply not strong, which +// leaves the sentence to the ordinary blocked-work handling rather than +// flagging it outright. +var strongAbsenceObjects = []string{ + "issue", "issues", "problem", "problems", "bug", "bugs", "defect", "defects", + "error", "errors", "failure", "failures", "regression", "regressions", + "evidence", "example", "examples", "occurrence", "occurrences", + "instance", "instances", "reference", "references", "match", "matches", + "caller", "callers", "usage", "usages", "use", "uses", "case", "cases", + "vulnerability", "vulnerabilities", "leak", "leaks", "race", "races", + "sign", "signs", "trace", "traces", "mention", "mentions", "difference", "differences", + "blocker", "blockers", "gap", "gaps", "omission", "omissions", "discrepancy", "discrepancies", + "conflict", "conflicts", "violation", "violations", "warning", "warnings", +} + +// absenceQualifiers sit between "any" and the object without changing it. +var absenceQualifiers = []string{ + "remaining", "other", "further", "more", "additional", "obvious", "such", + "outstanding", "new", "existing", "actual", "real", "clear", "direct", +} + +// strongAbsence reports whether tail is an "any"-family absence whose OBJECT +// makes finding nothing the result rather than the shortfall. +func strongAbsence(tail string) bool { + for _, prefix := range strongAbsenceTails { + if !strings.HasPrefix(tail, prefix) { + continue + } + rest := strings.TrimSpace(tail[len(prefix):]) + for trimmed := true; trimmed; { + trimmed = false + for _, qualifier := range absenceQualifiers { + if word, remainder, ok := cutFirstWord(rest); ok && word == qualifier { + rest, trimmed = remainder, true + break + } + } + } + word, _, ok := cutFirstWord(rest) + if !ok { + continue + } + for _, object := range strongAbsenceObjects { + if word == object { + return true + } + } + } + return false +} + +// cutFirstWord returns the first bare word of text, lowercased by the caller's +// own normalisation, with surrounding punctuation removed. +func cutFirstWord(text string) (word string, rest string, ok bool) { + text = strings.TrimLeft(text, " \t") + end := 0 + for end < len(text) { + c := text[end] + if (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' { + end++ + continue + } + break + } + if end == 0 { + return "", text, false + } + return text[:end], text[end:], true +} + var blockedWorkMarkers = []string{ "unverified", "not verified", "cannot verify", "could not verify", "someone else", "will need to", "needs someone", "handed off", "hand off", @@ -487,7 +573,7 @@ func selfReportedIncompletion(text string) string { // blocked: "could not reproduce the crash" is a finding, "could not // reproduce the crash, so the fix is unverified" is an admission, // and the tail prefix alone cannot tell them apart. - strong := hasAnyPrefix(tail, strongAbsenceTails) + strong := strongAbsence(tail) if !hasAnyPrefix(tail, successNegationTails) || (!strong && containsAny(sentence, blockedWorkMarkers)) { return selfReportReason(strings.TrimSpace(stem) + " …") } diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index cf2fba4db..e8d273a0b 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -268,3 +268,41 @@ func TestAToolCaveatDoesNotExcuseBlockedWork(t *testing.T) { } } } + +// WHAT FOLLOWS "any" DECIDES WHETHER FINDING NOTHING IS THE RESULT. +// +// The "any"-family was treated as a finding whatever the object, so an admission +// wearing the same words walked through: +// +// "I could not find any remaining issues" -> a finding, the search succeeded +// "I could not find any solution" -> an admission, the work did not +// +// Both carry the explicit "any"; only the object separates them. The object list +// is an ALLOW-LIST because it grants the exemption — a deny-list of deliverables +// would wave through every noun nobody thought of, which is the wrong direction +// for this detector to fail in. +func TestAnAbsenceIsAFindingOnlyWhenTheObjectMakesItOne(t *testing.T) { + for _, admission := range []string{ + "I could not find any solution, so the migration remains unresolved.", + "I could not find any fix, so the build is still broken.", + "I could not find any workaround; someone else will need to pick this up.", + "I could not identify any approach, so this is unverified.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("an admission wearing the words of a finding passed as complete: %q", admission) + } + } + // The findings the allowance exists for are untouched, including the ones + // carrying a blocked-work marker about somebody else's future work. + for _, finding := range []string{ + "I could not find any remaining issues, though a follow-up will need to cover the Windows path.", + "I could not find any blockers; someone else can take the release from here.", + "I could not find any evidence of a leak, so nothing was modified.", + "I did not see any further regressions, so someone else can ship it.", + "I could not detect any races, and nothing was modified.", + } { + if reason := selfReportedIncompletion(finding); reason != "" { + t.Errorf("a finding was reported as an admission: %q -> %s", finding, reason) + } + } +} From 2fbb5ff5e217c788c6781fcd62c67717819081a3 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sun, 16 Aug 2026 19:56:46 +0530 Subject: [PATCH 05/28] fix(agent): the consequence may be the next sentence, and "blocked" may be said outright MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @Vasanthdev2004 measured 4 of 10 ordinary admissions missed at bd3887b7, and the pattern in what escaped was a full stop. Reproduced exactly: same 4, same 0 false positives on his five honest negative results. A FULL STOP IS NOT A CLAIM THAT THE WORK FINISHED. The blocked-work override only ever saw the sentence the allowance fired in, so the same admission was caught or missed on its punctuation alone: "I could not reproduce the crash, so the fix is unverified." caught "I could not reproduce the crash. The fix is unverified." missed Writing the consequence as its own sentence is how most people write. The blocked-work question now spans the sentence AND the one after it; everything else is still decided on the sentence alone, so a stem in one sentence still cannot be paired with an allowance tail in another. "THE WORK IS BLOCKED" WAS NOT A MARKER. His hardest case — an explicit statement of blockage, in the SAME sentence, still passing — was simply a gap: every marker in the list named a symptom of being blocked and none named the thing itself. THE LOOKAHEAD HAS A COST AND IT IS GUARDED. Reading the next sentence can read another subject's blocked state as this result's consequence. A sentence that announces the change of subject, or disclaims the thing as out of scope, is taken at its word. That does not catch every unrelated follow-on and deliberately errs toward reading ahead, because an admission reported as success is the failure this guard exists to prevent. MEASURED TWICE, THE SECOND TIME HONESTLY. After fixing the topic-shift list against four adversarial cases of my own, that corpus certified the list against itself — his exact objection to building a corpus from the patterns. A second corpus written AFTER the tuning, avoiding every word in the list, found a real false positive: "I could not reproduce any failure in the parser" was not a strong absence, because the "any"-family carried only the SEARCH verbs and not the OBSERVATION ones. Looking for a failure and not producing one is the same kind of result as looking for an issue and not finding one. Final, both corpora: his 10 admissions 0 missed and 5 findings 0 wrongly flagged; my 5 fresh admissions 0 missed and 4 fresh findings 0 wrongly flagged. Mutation-checked in both directions: removing the lookahead lets 3 admissions escape, and removing the topic-shift guard wrongly flags a finding. Origin-Session: local-abff1c | Claude Code | 5 prompts Origin-Snapshot: dddd3415c4e0 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 62 ++++++++++++++++++- .../agent/guardrails_false_admission_test.go | 47 ++++++++++++++ 2 files changed, 107 insertions(+), 2 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 2fd8eda36..6ade86fdd 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -362,6 +362,13 @@ var objectiveFailureMarkers = []string{ var strongAbsenceTails = []string{ "find any", "found any", "see any", "detect any", "identify any", "spot any", "locate any", "confirm any", "observe any", + // The OBSERVATION family. Looking for a failure and not producing one is the + // same kind of result as looking for an issue and not finding one, and + // leaving these out meant "I could not reproduce any failure in the parser" + // was not a strong absence — so an unrelated blocked statement in the next + // sentence flipped a clean negative result into an admission. + "reproduce any", "trigger any", "produce any", "hit any", + "encounter any", "provoke any", "surface any", "measure any", } // strongAbsenceObjects are the things whose ABSENCE IS THE RESULT: you go @@ -393,6 +400,8 @@ var strongAbsenceObjects = []string{ "sign", "signs", "trace", "traces", "mention", "mentions", "difference", "differences", "blocker", "blockers", "gap", "gaps", "omission", "omissions", "discrepancy", "discrepancies", "conflict", "conflicts", "violation", "violations", "warning", "warnings", + "call", "calls", "site", "sites", "callsite", "callsites", "consumer", "consumers", + "dependency", "dependencies", "user", "users", "path", "paths", } // absenceQualifiers sit between "any" and the object without changing it. @@ -464,6 +473,14 @@ var blockedWorkMarkers = []string{ "nothing was modified", "no changes were made", "left unchanged", "left undone", "may be inert", "is unresolved", "remains unresolved", "still broken", "so i cannot", "so i could not", + // Saying the work is BLOCKED, in as many words. "I could not find the root + // cause, so the work is blocked" carries the statement in the same sentence + // and still passed, because every marker above names a symptom of being + // blocked and none named the thing itself. + "is blocked", "are blocked", "remains blocked", "stays blocked", + "cannot proceed", "could not proceed", "can not proceed", "unable to proceed", + "is unfinished", "remains unfinished", "left unfinished", + "is incomplete", "remains incomplete", } // bareInabilityStems are the two entries above that are STEMS rather than @@ -494,6 +511,30 @@ var blockedStateMarkers = func() []string { return out }() +// carriesTheConsequence reports whether the sentence after an allowance should +// be read as that allowance's consequence. +// +// The lookahead exists because the consequence usually IS the next sentence, but +// "usually" is not "always" — a message can turn to something else, and reading +// a blocked statement about a different subject as this one's consequence is the +// cost of the lookahead. A sentence that announces the change of subject, or +// disclaims the thing as out of scope, is taken at its word. +// +// This does not catch every unrelated follow-on, and deliberately errs toward +// reading the next sentence: an admission reported as success is the failure this +// guard exists to prevent, and a message that says something is unverified has +// said it whether or not it is the same something. +func carriesTheConsequence(next string) bool { + return !containsAny(next, topicShiftMarkers) +} + +// topicShiftMarkers say the message has moved on to something else. +var topicShiftMarkers = []string{ + "separately", "unrelatedly", "unrelated", "as an aside", "aside from", + "out of scope", "outside the scope", "not in scope", "for a different", + "in a different", "on another", "elsewhere in", "in other news", +} + // countedLabelSentence reports whether a sentence is a markdown LABEL that // introduces and counts a bucket of findings, rather than a claim about the // objective. @@ -518,7 +559,24 @@ func countedLabelSentence(sentence string) bool { var countedLabelSuffix = regexp.MustCompile(`\(\s*\d+\s*\)`) func selfReportedIncompletion(text string) string { - for _, sentence := range admissionSentences(strings.ToLower(stripQuoted(text))) { + sentences := admissionSentences(strings.ToLower(stripQuoted(text))) + for index, sentence := range sentences { + // THE CONSEQUENCE IS OFTEN THE NEXT SENTENCE. The blocked-work override + // only ever saw the sentence the allowance fired in, so the same + // admission escaped or was caught purely on its punctuation: + // + // "I could not reproduce the crash, so the fix is unverified." caught + // "I could not reproduce the crash. The fix is unverified." missed + // + // A full stop is not a claim that the work finished, and writing the + // consequence as its own sentence is how most people write. The + // blocked-work question is asked of this sentence AND the one after it; + // everything else is still decided on the sentence alone, so a stem in + // one sentence cannot be paired with an allowance tail in another. + blockedContext := sentence + if index+1 < len(sentences) && carriesTheConsequence(sentences[index+1]) { + blockedContext += " " + sentences[index+1] + } if containsAny(sentence, narrativeMarkers) { continue } @@ -574,7 +632,7 @@ func selfReportedIncompletion(text string) string { // reproduce the crash, so the fix is unverified" is an admission, // and the tail prefix alone cannot tell them apart. strong := strongAbsence(tail) - if !hasAnyPrefix(tail, successNegationTails) || (!strong && containsAny(sentence, blockedWorkMarkers)) { + if !hasAnyPrefix(tail, successNegationTails) || (!strong && containsAny(blockedContext, blockedWorkMarkers)) { return selfReportReason(strings.TrimSpace(stem) + " …") } start = abs + len(stem) diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index e8d273a0b..68d04e349 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -306,3 +306,50 @@ func TestAnAbsenceIsAFindingOnlyWhenTheObjectMakesItOne(t *testing.T) { } } } + +// A FULL STOP IS NOT A CLAIM THAT THE WORK FINISHED. +// +// The blocked-work override only ever saw the sentence the allowance fired in, +// so the SAME admission was caught or missed on its punctuation alone: +// +// "I could not reproduce the crash, so the fix is unverified." caught +// "I could not reproduce the crash. The fix is unverified." missed +// +// Writing the consequence as its own sentence is how most people write, and half +// of a reviewer's corpus of ordinary admissions escaped through it. +// +// Only the blocked-work question looks ahead. A stem in one sentence is still +// never paired with an allowance tail in another. +func TestTheConsequenceMayBeTheNextSentence(t *testing.T) { + for _, admission := range []string{ + "I could not reproduce the crash, so the fix is unverified.", + "I could not reproduce the crash. The fix is unverified.", + "I could not locate the source of the regression and have run out of ideas.", + "I could not locate the source of the regression. I have run out of ideas.", + // "the work is blocked" says it in as many words, and every marker was a + // SYMPTOM of being blocked rather than the thing itself. + "I could not find the root cause, so the work is blocked.", + "I could not find the root cause. The work is blocked.", + "I could not get the integration test to run. The behaviour is therefore unverified.", + "I could not apply the patch cleanly. Nothing was modified.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("an admission escaped by putting its consequence in a second sentence: %q", admission) + } + } + + // THE LOOKAHEAD'S OWN COST, guarded. A message that turns to other work must + // not have that work's blocked state read as this result's consequence. + for _, finding := range []string{ + "I searched the tree and could not find any other call sites. The rename is complete.", + "I could not find any remaining usages of the deprecated helper. A future cleanup is blocked until the API freeze lifts.", + "I could not reproduce any failure in the parser. The CI flake in the network suite remains unresolved and belongs to another team.", + "I could not find any issues in the diff. Documentation for the new flag is unfinished, which was never part of this request.", + "I could not reproduce the crash. Separately, the deploy step is unverified because it needs prod access.", + "I could not find any other call sites. The follow-up work is blocked on a design decision, which is out of scope here.", + } { + if reason := selfReportedIncompletion(finding); reason != "" { + t.Errorf("the lookahead read another subject's blocked state as this result's: %q -> %s", finding, reason) + } + } +} From a257b7f1fd0751c3a6442ac907aebbf03fb205e7 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sun, 16 Aug 2026 22:44:46 +0530 Subject: [PATCH 06/28] fix(agent): an explicit failure state outranks a recognised absence object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @anandh8x's P1, which @Vasanthdev2004 verified and corrected his own approval over. Reproduced before changing anything: "I could not find any evidence supporting the fix, so it remains unverified." "I could not find any evidence for the cause, so the bug is unresolved." strongAbsence returns true for "evidence", and that suppressed every blocked-work marker in the sentence — so a message saying in as many words that the work is unverified reported success. The absence protection exists for ownership and follow-up wording, where "I could not find any remaining issues, though a follow-up will need to cover the Windows path" really is a finding. It was never meant to cover a sentence that states the outcome. So the states that outrank it are the SHORT list: "unverified" and "still broken" have one reading, while "someone else", "will need to" and "nothing was modified" have two and stay ambiguous. Same-sentence only — a state in the next sentence may belong to another subject, which is what the lookahead's topic-shift guard is for, and it is why "I could not reproduce any failure in the parser. The CI flake … remains unresolved and belongs to another team." stays silent. TWO HAND-MAINTAINED LISTS THAT MUST AGREE, so the agreement is asserted rather than remembered. unambiguousFailureStates only decides whether the absence stops protecting the sentence; blockedWorkMarkers is what actually fires. I added "still blocked" to the first and not the second, and the case looked handled because the phrase appeared in the code — it did nothing. The new invariant test then found a second dead entry I had already shipped, "is still broken", which "still broken" already covered. Final on both corpora: 6 admissions caught including the four from this review, 0 of 11 findings wrongly flagged. Mutation-checked: removing the override lets three admissions escape, and adding a state that is not a marker fails the invariant test. Origin-Session: local-abff1c | Claude Code | 7 prompts Origin-Snapshot: b7d0806d49f9 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 40 +++++++++++- .../agent/guardrails_false_admission_test.go | 65 +++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 6ade86fdd..50d922611 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -477,10 +477,11 @@ var blockedWorkMarkers = []string{ // cause, so the work is blocked" carries the statement in the same sentence // and still passed, because every marker above names a symptom of being // blocked and none named the thing itself. - "is blocked", "are blocked", "remains blocked", "stays blocked", + "is blocked", "are blocked", "remains blocked", "stays blocked", "still blocked", "cannot proceed", "could not proceed", "can not proceed", "unable to proceed", - "is unfinished", "remains unfinished", "left unfinished", + "is unfinished", "remains unfinished", "left unfinished", "still unfinished", "is incomplete", "remains incomplete", + "still unresolved", "still unverified", } // bareInabilityStems are the two entries above that are STEMS rather than @@ -490,6 +491,35 @@ var blockedWorkMarkers = []string{ // step and is now complete" is a finished task that did not need the plan. var bareInabilityStems = []string{"so i cannot", "so i could not"} +// unambiguousFailureStates say the work is in a bad state, with no reading on +// which it is a result. +// +// AN OBJECT CANNOT OUTRANK AN EXPLICIT STATE. strongAbsence returns true for +// "evidence", and that suppressed every blocked-work marker in the sentence — so +// "I could not find any evidence supporting the fix, so it remains unverified" +// reported success while saying in as many words that the work is unverified. +// The absence protection exists for ownership and follow-up wording, where "I +// could not find any remaining issues, though a follow-up will need to cover the +// Windows path" really is a finding. It was never meant to cover a sentence that +// states the outcome. +// +// So this is the SHORT list: "unverified" and "still broken" have one reading, +// while "someone else", "will need to" and "nothing was modified" have two and +// stay ambiguous. Same-sentence only — a state in the NEXT sentence may belong +// to another subject, which is what the lookahead's topic-shift guard is for. +var unambiguousFailureStates = []string{ + "unverified", "not verified", + "is unresolved", "remains unresolved", + "still broken", + "is blocked", "are blocked", "remains blocked", "stays blocked", "still blocked", + "is unfinished", "remains unfinished", "left unfinished", "still unfinished", + "still unresolved", "still unverified", + "is incomplete", "remains incomplete", + "cannot proceed", "could not proceed", "unable to proceed", + "gave up", "giving up", "ran out of", "run out of", + "left undone", +} + // blockedStateMarkers describe WORK LEFT BLOCKED — unverified, unresolved, // handed off, abandoned for time. Only these break the tool-grant exemption: // they say something about the state the work is in, which a tool caveat cannot @@ -632,6 +662,12 @@ func selfReportedIncompletion(text string) string { // reproduce the crash, so the fix is unverified" is an admission, // and the tail prefix alone cannot tell them apart. strong := strongAbsence(tail) + // The object decides whether finding nothing is a result; an + // explicit failure state in the SAME sentence decides that it is + // not, whatever the object. + if strong && containsAny(sentence, unambiguousFailureStates) { + strong = false + } if !hasAnyPrefix(tail, successNegationTails) || (!strong && containsAny(blockedContext, blockedWorkMarkers)) { return selfReportReason(strings.TrimSpace(stem) + " …") } diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 68d04e349..cd5a5a114 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -353,3 +353,68 @@ func TestTheConsequenceMayBeTheNextSentence(t *testing.T) { } } } + +// AN UNAMBIGUOUS STATE THAT IS NOT ALSO A MARKER IS DEAD. +// +// unambiguousFailureStates only decides whether a recognised absence object +// stops protecting the sentence; something still has to FIRE, and that is +// blockedWorkMarkers. Adding "still blocked" to the first list and not the +// second left it doing nothing at all, and the case looked handled because the +// phrase appeared in the code. +// +// Two hand-maintained lists that must agree is the shape that drifts, so the +// agreement is asserted rather than remembered. +func TestEveryUnambiguousStateIsAlsoABlockedWorkMarker(t *testing.T) { + for _, state := range unambiguousFailureStates { + found := false + for _, marker := range blockedWorkMarkers { + if state == marker { + found = true + break + } + } + if !found { + t.Errorf("%q outranks a strong absence but is not a blockedWorkMarker, so nothing fires on it", state) + } + } +} + +// EXPLICIT FAILURE STATES OUTRANK A RECOGNISED ABSENCE OBJECT. +// +// strongAbsence returns true for objects like "evidence", and that suppressed +// every blocked-work marker in the sentence — so a message saying in as many +// words that the work is unverified reported success. The absence protection is +// for ownership and follow-up wording; it was never meant to cover a sentence +// that states the outcome. +func TestAnExplicitFailureStateOutranksTheAbsenceObject(t *testing.T) { + for _, admission := range []string{ + "I could not find any evidence supporting the fix, so it remains unverified.", + "I could not find any evidence for the cause, so the bug is unresolved.", + "I could not find any way to make it work, so I gave up.", + "I could not find any working approach; it is still broken.", + "I could not find any issues, but the migration is still blocked.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("an explicit failure state was overruled by the absence object: %q", admission) + } + } + + // AMBIGUOUS wording still yields to the absence, which is the whole reason + // the protection exists: ownership and follow-up read two ways, an explicit + // state reads one. + for _, finding := range []string{ + "I could not find any evidence that the flag is read in production.", + "I could not find any remaining issues, though a follow-up will need to cover the Windows path.", + "I could not find any blockers; someone else can take the release from here.", + "I could not find any evidence of a leak, so nothing was modified.", + "I could not detect any races, and nothing was modified.", + // A state in the NEXT sentence may belong to another subject, so + // same-sentence only. + "I could not reproduce any failure in the parser. The CI flake in the network suite remains unresolved and belongs to another team.", + "I could not find any issues in the diff. Documentation for the new flag is unfinished, which was never part of this request.", + } { + if reason := selfReportedIncompletion(finding); reason != "" { + t.Errorf("a finding was flagged by ambiguous or another subject's wording: %q -> %s", finding, reason) + } + } +} From b486a88181ef9d9f64b0ff1f1461c540c542a5dc Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Mon, 17 Aug 2026 11:49:42 +0530 Subject: [PATCH 07/28] fix(agent): a failure state inside the negated proposition is not the outcome MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @anandh8x's P1, and it is the opposite polarity of the case the override was added for one commit earlier. Reproduced before changing anything: "I could not find any evidence that the issue is unresolved." -> INCOMPLETE That is a successful negative finding — there is no evidence the issue remains unresolved — and the override marked it incomplete, because "is unresolved" matched anywhere in the sentence, disabled the strong-absence exemption, and then the same substring fired the blocked-work marker. What separates the two is POSITION. After a consequence boundary the state is being asserted; inside a "that…" clause it is the thing being denied. The override now looks only at the reported consequence — the part after ", so ", "; ", ", but " and their kin — and a sentence that never turns to a consequence has no outcome to read. Both polarities are tested together, because fixing one of them in isolation just moves the error: four negated propositions must pass and five stated outcomes must fire. 0 of 11 findings wrongly flagged, 0 of 5 admissions missed. Mutation-checked: matching the whole sentence again wrongly flags all four negated propositions. Origin-Session: local-ae96ee | Claude Code | 4 prompts Origin-Snapshot: 701aaeb8081a Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 34 ++++++++++++++++- .../agent/guardrails_false_admission_test.go | 38 +++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 50d922611..2301d384c 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -520,6 +520,38 @@ var unambiguousFailureStates = []string{ "left undone", } +// consequenceBoundaries separate what was looked for from what followed. +// +// THE STATE HAS TO BE THE OUTCOME, NOT PART OF WHAT WAS NEGATED. Matching +// "is unresolved" anywhere in the sentence read the negated PROPOSITION as the +// reported result: +// +// "I could not find any evidence that the issue is unresolved." +// +// That is a successful negative finding — there is no evidence the issue remains +// unresolved — and it was marked incomplete, which is the opposite polarity of +// the case the override was added for. What separates them is position: after a +// consequence boundary the state is being asserted, inside a "that…" clause it is +// the thing being denied. +var consequenceBoundaries = []string{", so ", "; ", ", but ", ", therefore", ", leaving ", ", which "} + +// reportedConsequence returns the part of a sentence that states the OUTCOME, +// or "" when the sentence never turns to one. Everything before the first +// boundary is what was searched for. +func reportedConsequence(sentence string) string { + earliest := -1 + width := 0 + for _, boundary := range consequenceBoundaries { + if index := strings.Index(sentence, boundary); index >= 0 && (earliest < 0 || index < earliest) { + earliest, width = index, len(boundary) + } + } + if earliest < 0 { + return "" + } + return sentence[earliest+width:] +} + // blockedStateMarkers describe WORK LEFT BLOCKED — unverified, unresolved, // handed off, abandoned for time. Only these break the tool-grant exemption: // they say something about the state the work is in, which a tool caveat cannot @@ -665,7 +697,7 @@ func selfReportedIncompletion(text string) string { // The object decides whether finding nothing is a result; an // explicit failure state in the SAME sentence decides that it is // not, whatever the object. - if strong && containsAny(sentence, unambiguousFailureStates) { + if strong && containsAny(reportedConsequence(sentence), unambiguousFailureStates) { strong = false } if !hasAnyPrefix(tail, successNegationTails) || (!strong && containsAny(blockedContext, blockedWorkMarkers)) { diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index cd5a5a114..91502668a 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -418,3 +418,41 @@ func TestAnExplicitFailureStateOutranksTheAbsenceObject(t *testing.T) { } } } + +// THE STATE MUST BE THE OUTCOME, NOT PART OF WHAT WAS NEGATED. +// +// The override matched a failure state anywhere in the sentence, which read the +// negated PROPOSITION as the reported result: +// +// "I could not find any evidence that the issue is unresolved." +// +// That is a successful negative finding — there is no evidence the issue remains +// unresolved — and it was marked incomplete. Exactly the opposite polarity of the +// case the override was added for, and the two have to be tested together or +// fixing one just moves the error. +func TestAStateInsideTheNegatedPropositionIsNotTheOutcome(t *testing.T) { + for _, finding := range []string{ + "I could not find any evidence that the issue is unresolved.", + "I could not find any evidence that the build is still broken.", + "I could not find any evidence that the migration is incomplete.", + "I could not find any sign that the deploy is blocked.", + } { + if reason := selfReportedIncompletion(finding); reason != "" { + t.Errorf("a negated proposition was read as the reported outcome: %q -> %s", finding, reason) + } + } + + // Both polarities together: after a consequence boundary the state IS the + // outcome and must still fire. + for _, admission := range []string{ + "I could not find any evidence supporting the fix, so it remains unverified.", + "I could not find any evidence for the cause, so the bug is unresolved.", + "I could not find any way to make it work, so I gave up.", + "I could not find any working approach; it is still broken.", + "I could not find any issues, but the migration is still blocked.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("an outcome stated after a consequence boundary was missed: %q", admission) + } + } +} From a5b0a31cf82801e413f065b134db6ab5e38646e0 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Tue, 18 Aug 2026 00:29:25 +0530 Subject: [PATCH 08/28] fix(agent): recognise a tool caveat in its copula forms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From CodeRabbit's review of the current head — though not the finding it wrote, and the difference is worth recording. IT REPORTED that blockedStateMarkers must drop the ambiguous handoff wording ("someone else", "will need to") so a tool grant followed by ownership wording stays exempt. Every case it described already behaves correctly at this head, and adding an inability stem to make the broken exemption observable turned up a different cause: the exemption never fired at all, because toolGrantMarkers recognised "no update_plan tool available" but not "no update_plan tool IS available". So this was reported incomplete despite having delivered the work: "I could not record a plan because no update_plan tool is available, so I wrote it into this answer instead." Which verb a sentence uses to say a tool was absent is not a distinction this detector should be drawing, so the copula forms are recognised too. I did NOT apply the change as written, and the test says why. Dropping "someone else" and "will need to" from the markers that break this exemption would exempt these: "…no update_plan tool is available, so someone else can pick it up." "…no such tool is available here; a follow-up will need to cover it." A run that could not do the thing and passed it on has not finished it, tool caveat or not. The exemption is for a tool that was NOT NEEDED, and both directions are now pinned so the distinction survives the next reader. Measured: 0 of 4 delivered-work caveats wrongly flagged, 0 of 5 genuine admissions missed. Mutation-checked — removing the copula forms reports the delivered-work case as incomplete again. Origin-Session: local-79d7a0 | Claude Code | 5 prompts Origin-Snapshot: c175cabb9d50 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 3 ++ .../agent/guardrails_false_admission_test.go | 38 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 2301d384c..aacb819ac 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -304,6 +304,9 @@ func admissionSentences(lower string) []string { // GIVEN, not about a tool being mentioned at all. var toolGrantMarkers = []string{ "tool available", "tools available", "no such tool", "not available in this", + "tool is available", "tools are available", "tool was available", "tools were available", + "tool is not available", "tools are not available", + "tool isn'''t available", "tools aren'''t available", "read-only tools", "read only tools", "only read-only", "only read only", "tools were provided", "tools were given", "toolset provided", "in this specialist context", "in this context only", diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 91502668a..752d17d09 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -456,3 +456,41 @@ func TestAStateInsideTheNegatedPropositionIsNotTheOutcome(t *testing.T) { } } } + +// THE COPULA FORMS OF A TOOL CAVEAT. +// +// toolGrantMarkers recognised "no update_plan tool available" but not "no +// update_plan tool IS available" — the same statement with a verb in it. So a +// message that named a tool it was not given AND delivered the work anyway was +// reported as incomplete: +// +// "I could not record a plan because no update_plan tool is available, +// so I wrote it into this answer instead." +// +// Which verb a sentence uses to say a tool was absent is not a distinction this +// detector should be drawing. +func TestATooolCaveatIsRecognisedInItsCopulaForms(t *testing.T) { + for _, exempt := range []string{ + "I could not record a plan because no update_plan tool is available, so I wrote it into this answer instead.", + "I could not run the formatter as no such tool is available, so I checked the style by hand.", + "No update_plan tool available in this specialist context, so I proceeded directly.", + } { + if reason := selfReportedIncompletion(exempt); reason != "" { + t.Errorf("a tool caveat with delivered work was reported incomplete: %q -> %s", exempt, reason) + } + } + + // A HANDOFF IS STILL AN ADMISSION. CodeRabbit's suggestion was to drop + // "someone else" and "will need to" from the markers that break this + // exemption. Measured, that would exempt these — and a run that could not do + // the thing and passed it to someone else has not finished it, tool caveat or + // not. The exemption is for a tool that was NOT NEEDED. + for _, admission := range []string{ + "I could not record a plan because no update_plan tool is available, so someone else can pick it up.", + "I could not run the linter as no such tool is available here; a follow-up will need to cover it.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("a handoff was excused by the tool caveat: %q", admission) + } + } +} From f2d98e7aa46e695c3c32a9d1b96dcdcbb27e03f8 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Tue, 18 Aug 2026 12:33:09 +0530 Subject: [PATCH 09/28] fix(agent): repair two dead marker lists this PR shipped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both from CodeRabbit's review of the current head, and both are entries I added that could never match anything. They compiled, so CI stayed green. THE CONTRACTED FORMS WERE MANGLED BY MY OWN SHELL QUOTING. The commit that added them wrote them through a heredoc, and the escaping leaked into the Go source: "tool isn'''t available", "tools aren'''t available" Valid Go strings, and no message on earth matches them. So the copula fix landed half-done: the plain forms worked and the contracted ones silently did not, which is the case a person is most likely to type. SEVEN OF EIGHT OBSERVATION VERBS WERE UNREACHABLE. strongAbsence is consulted only after hasAnyPrefix(tail, successNegationTails) has already matched, so a strong tail whose verb is absent from THAT list is never asked about. Of the observation family added a round earlier — trigger, produce, hit, encounter, provoke, surface, measure — only "reproduce" ever fired, because it was already a success-negation stem. The corpus that was supposed to cover the other seven passed on the strength of that one. Both are now pinned rather than remembered: an invariant test asserts every strongAbsenceTails entry has a successNegationTails prefix, which is the same shape as the unambiguous-state invariant added earlier for the same reason. That is now three lists in this file whose agreement is asserted, and each was found broken the round after it was written. Also the test-name typo CodeRabbit spotted: TestATooolCaveat -> TestAToolCaveat. Mutation-checked: restoring the mangled apostrophes wrongly flags two contracted caveats, and dropping the observation verbs makes the invariant report all seven. Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 14 ++++- .../agent/guardrails_false_admission_test.go | 62 ++++++++++++++++++- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index aacb819ac..31ffa5769 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -197,6 +197,11 @@ var inabilityStems = []string{ var successNegationTails = []string{ "find any", "found any", "find a ", "see any", "detect any", "identify any", "reproduce", "spot any", "locate any", + // The OBSERVATION verbs. strongAbsence is only consulted after this list has + // already matched, so the observation-family strong tails added a round + // earlier were unreachable without these — seven of eight did nothing, and + // only "reproduce" worked because it was already here. + "trigger", "produce", "hit any", "encounter", "provoke", "surface", "measure", // A NEGATIVE SEARCH RESULT IS THE ANSWER, not a failure to produce one. // // The list above already encodes this — "I could not find any remaining @@ -306,7 +311,7 @@ var toolGrantMarkers = []string{ "tool available", "tools available", "no such tool", "not available in this", "tool is available", "tools are available", "tool was available", "tools were available", "tool is not available", "tools are not available", - "tool isn'''t available", "tools aren'''t available", + "tool isn't available", "tools aren't available", "read-only tools", "read only tools", "only read-only", "only read only", "tools were provided", "tools were given", "toolset provided", "in this specialist context", "in this context only", @@ -374,6 +379,13 @@ var strongAbsenceTails = []string{ "encounter any", "provoke any", "surface any", "measure any", } +// EVERY STRONG TAIL MUST BE REACHABLE. strongAbsence is consulted only after +// hasAnyPrefix(tail, successNegationTails) has already matched, so a strong tail +// whose verb is missing from that list is never asked about. Seven of the eight +// observation verbs added a round earlier were dead on arrival that way — only +// "reproduce" was already a success-negation stem, which is why the corpus that +// was supposed to cover them passed. + // strongAbsenceObjects are the things whose ABSENCE IS THE RESULT: you go // looking for them precisely so you can report there are none, and finding none // is the work succeeding. diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 752d17d09..261e96688 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -1,6 +1,9 @@ package agent -import "testing" +import ( + "strings" + "testing" +) // FOUR COMPLETED TASKS WERE MARKED INCOMPLETE, and the sentences below are // verbatim from those sessions. @@ -469,7 +472,7 @@ func TestAStateInsideTheNegatedPropositionIsNotTheOutcome(t *testing.T) { // // Which verb a sentence uses to say a tool was absent is not a distinction this // detector should be drawing. -func TestATooolCaveatIsRecognisedInItsCopulaForms(t *testing.T) { +func TestAToolCaveatIsRecognisedInItsCopulaForms(t *testing.T) { for _, exempt := range []string{ "I could not record a plan because no update_plan tool is available, so I wrote it into this answer instead.", "I could not run the formatter as no such tool is available, so I checked the style by hand.", @@ -494,3 +497,58 @@ func TestATooolCaveatIsRecognisedInItsCopulaForms(t *testing.T) { } } } + +// EVERY STRONG ABSENCE TAIL MUST BE REACHABLE. +// +// strongAbsence is consulted only after hasAnyPrefix(tail, successNegationTails) +// has already matched, so a strong tail whose verb is missing from that list is +// never asked about. Seven of the eight observation verbs added a round earlier +// were dead that way — only "reproduce" worked, because it was already a +// success-negation stem, and the corpus meant to cover the other seven passed +// on the strength of that one. +// +// Same shape as the unambiguous-state invariant below it: two lists that must +// agree, asserted rather than remembered. +func TestEveryStrongAbsenceTailIsReachable(t *testing.T) { + for _, tail := range strongAbsenceTails { + reachable := false + for _, negation := range successNegationTails { + if strings.HasPrefix(tail, negation) || strings.HasPrefix(negation, tail) { + reachable = true + break + } + } + if !reachable { + t.Errorf("strongAbsenceTails entry %q has no successNegationTails prefix, so strongAbsence never sees it", tail) + } + } +} + +// A CONTRACTED TOOL CAVEAT IS THE SAME CAVEAT. +// +// The contracted markers shipped as "tool isn\'\'\'t available" — shell quoting +// from the commit that added them leaked into the Go source. Valid Go, so it +// compiled and CI stayed green, and no message on earth matches it. The +// observation-family entries above were dead for a different reason in the same +// commit, which is why both are pinned now. +func TestAContractedToolCaveatIsRecognised(t *testing.T) { + for _, exempt := range []string{ + "I could not record a plan because the update_plan tool isn't available, so I wrote it into this answer instead.", + "I could not run the formatter because those tools aren't available, so I checked the style by hand.", + } { + if reason := selfReportedIncompletion(exempt); reason != "" { + t.Errorf("a contracted tool caveat with delivered work was reported incomplete: %q -> %s", exempt, reason) + } + } + + // The observation family, now that its verbs reach strongAbsence at all. + for _, finding := range []string{ + "I could not trigger any crash, so it looks resolved.", + "I could not surface any regressions, so nothing was modified.", + "I could not encounter any failures, and nothing was modified.", + } { + if reason := selfReportedIncompletion(finding); reason != "" { + t.Errorf("a negative observation result was reported as an admission: %q -> %s", finding, reason) + } + } +} From a5b2ed6b446427551ba933a927250eca83f54385 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Wed, 19 Aug 2026 08:11:30 +0530 Subject: [PATCH 10/28] fix(agent): only the "any" forms make an observation verb a successful absence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit's finding on the current head, and a regression I introduced in the commit directly before it. Making the observation-family strong tails reachable meant adding their verbs to successNegationTails — and I added them BARE, which turned every "could not " into a successful negative result: "I could not produce the requested report." went silent "I could not measure the throughput, so the number is unknown." went silent "I could not trigger the migration, so it never ran." went silent Five ordinary admissions, all missed. Looking for something and finding none of it is a result; failing to produce a thing you were asked for is not, and the "any" is the whole difference. The entries now match strongAbsenceTails exactly, which is also what stops the two lists drifting again — the reachability invariant added a commit earlier asserts they agree, and matching forms is how they stay that way rather than by being separately maintained. Both directions are pinned in one test, because the fix for one of them is what broke the other: five admissions must fire, five negative observation results must stay silent. Mutation-checked: restoring the bare verbs silences all five admissions again. Origin-Session: local-ce5b65 | Claude Code | 1 prompt Origin-Snapshot: aadace3f8a64 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 21 +++++++--- .../agent/guardrails_false_admission_test.go | 39 +++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 31ffa5769..194d6d2dc 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -197,11 +197,22 @@ var inabilityStems = []string{ var successNegationTails = []string{ "find any", "found any", "find a ", "see any", "detect any", "identify any", "reproduce", "spot any", "locate any", - // The OBSERVATION verbs. strongAbsence is only consulted after this list has - // already matched, so the observation-family strong tails added a round - // earlier were unreachable without these — seven of eight did nothing, and - // only "reproduce" worked because it was already here. - "trigger", "produce", "hit any", "encounter", "provoke", "surface", "measure", + // The OBSERVATION verbs, in their "... any" forms ONLY. strongAbsence is + // consulted after this list has matched, so the observation-family strong + // tails were unreachable without these — but adding the verbs BARE made every + // "could not " a successful negative result, and these are ordinary + // admissions: + // + // "I could not produce the requested report." + // "I could not measure the throughput, so the number is unknown." + // "I could not trigger the migration, so it never ran." + // + // All five went silent. Looking for something and finding none of it is a + // result; failing to produce a thing you were asked for is not, and the "any" + // is what separates them. Matching the strong tails exactly is also what keeps + // the two lists from drifting apart again. + "trigger any", "produce any", "hit any", "encounter any", + "provoke any", "surface any", "measure any", // A NEGATIVE SEARCH RESULT IS THE ANSWER, not a failure to produce one. // // The list above already encodes this — "I could not find any remaining diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 261e96688..2a8b2dbf7 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -552,3 +552,42 @@ func TestAContractedToolCaveatIsRecognised(t *testing.T) { } } } + +// "ANY" IS WHAT MAKES A NEGATIVE RESULT A RESULT. +// +// The observation verbs were added to successNegationTails BARE, to make the +// observation-family strong tails reachable. That made every "could not " +// a successful negative finding, and these are ordinary admissions that went +// silent: +// +// "I could not produce the requested report." +// "I could not measure the throughput, so the number is unknown." +// +// Looking for something and finding none of it is a result. Failing to produce a +// thing you were asked for is not. Both directions are asserted here because the +// fix for one of them is what broke the other. +func TestABareObservationVerbIsNotASuccessfulAbsence(t *testing.T) { + for _, admission := range []string{ + "I could not produce the requested report.", + "I could not measure the throughput, so the number is unknown.", + "I could not trigger the migration, so it never ran.", + "I could not surface the config value.", + "I could not encounter the documented behaviour.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("an admission was read as a successful negative result: %q", admission) + } + } + + for _, finding := range []string{ + "I could not trigger any crash, so it looks resolved.", + "I could not surface any regressions, so nothing was modified.", + "I could not reproduce any failure after the fix, so it looks resolved.", + "I could not encounter any failures, and nothing was modified.", + "I could not measure any slowdown, so the change is neutral.", + } { + if reason := selfReportedIncompletion(finding); reason != "" { + t.Errorf("a negative observation result was reported as an admission: %q -> %s", finding, reason) + } + } +} From d035884e74ecd238d51bc40583c72c2ca81391cc Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Wed, 19 Aug 2026 10:25:51 +0530 Subject: [PATCH 11/28] test(agent): check tail reachability in the direction the runtime uses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit's finding, about the invariant test added two commits ago rather than about the detector. The reachability check accepted a match in either direction: strings.HasPrefix(tail, negation) || strings.HasPrefix(negation, tail) The runtime only ever asks the first. The reverse clause admits a successNegationTails entry LONGER than the strong tail, which can never match a real message beginning with that tail — so the invariant would report the tail as reachable while it was in fact dead. No entry is longer than its strong tail today, which is precisely why the loose form looked fine. This is the third time in this file that a check passed because the case it could not see did not happen to exist yet, so tightening it to the runtime's own question is worth doing while nothing depends on the difference. Still catches what it was written for: removing the observation tails reports all seven as unreachable. Origin-Session: local-ce5b65 | Claude Code | 2 prompts Origin-Snapshot: ee26025f1ad8 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails_false_admission_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 2a8b2dbf7..6eda20ead 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -513,7 +513,13 @@ func TestEveryStrongAbsenceTailIsReachable(t *testing.T) { for _, tail := range strongAbsenceTails { reachable := false for _, negation := range successNegationTails { - if strings.HasPrefix(tail, negation) || strings.HasPrefix(negation, tail) { + // ONE DIRECTION, the one the runtime uses. hasAnyPrefix asks + // HasPrefix(tail, negation); the reverse clause this used to carry + // accepted a negation entry LONGER than the strong tail, which can + // never match a real message beginning with that tail — so the + // invariant would pass while the tail stayed dead. No entry is longer + // today, which is exactly why the loose form looked fine. + if strings.HasPrefix(tail, negation) { reachable = true break } From 44381804fdaa449ff86a40218d2fdbd9399d2929 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Wed, 19 Aug 2026 18:37:12 +0530 Subject: [PATCH 12/28] fix(agent): a tool named as an excuse does not exempt the failure it explains MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @anandh8x's P1, and a regression from the copula broadening earlier in this PR. Reproduced before changing anything: "I could not run the migration because no migration tool is available." "…because the migration tool is available only on Windows." Both returned no reason. The migration did not run, nothing took its place, and the sentence merely explains WHY — but the broadened matcher recognised the tool phrase and the exemption waved it through. Naming an absent tool does not establish that the tool was unnecessary, which is exactly the claim the exemption makes on the sentence's behalf. HIS PRESCRIPTION, APPLIED LITERALLY, BREAKS THE CASE THE EXEMPTION EXISTS FOR. Requiring a delivered-work alternative outright fails six existing tests, including this, which is verbatim from a session this detector wrongly flagged: "I don't have an `update_plan` tool available in this specialist context (only read-only exploration tools were provided)." That names no failed action at all. It is a capability footnote, and there is nothing for it to have delivered instead. A CAUSAL CONNECTIVE is what separates the two: "because", "since", "due to" mark the tool as the reason some action did not happen, while the footnote has none. The connective then YIELDS to a delivered alternative — "so I wrote it into this answer instead" — because at that point the tool really was unnecessary, which is the premise of the exemption rather than an exception to it. Both marker lists are allow-lists: an unrecognised phrasing does not exempt, so the sentence goes on to the ordinary handling rather than being waved through. Mutation-checked in both directions: dropping the causal condition excuses three admissions again, and requiring the alternative unconditionally breaks six tests. Origin-Session: local-76c8d7 | Claude Code | 6 prompts Origin-Snapshot: 259b715cf0fd Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 53 +++++++++++++++++++ .../agent/guardrails_false_admission_test.go | 48 +++++++++++++++++ 2 files changed, 101 insertions(+) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 194d6d2dc..03696ae69 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -329,6 +329,44 @@ var toolGrantMarkers = []string{ "is not in my toolset", "not in my toolset", "not in this toolset", } +// causalExcuseMarkers introduce the tool as the REASON some action did not +// happen, rather than as a footnote about this run's capabilities. +// +// THE TOOL AS EXCUSE IS NOT THE TOOL AS FOOTNOTE. Naming an absent tool does not +// establish it was unnecessary, and the copula broadening let these through: +// +// "I could not run the migration because no migration tool is available." +// "…because the migration tool is available only on Windows." +// +// The migration did not run and nothing took its place; the sentence explains +// WHY. Whereas the case the exemption was built for names no failed action at +// all — "I don't have an update_plan tool available in this specialist context +// (only read-only exploration tools were provided)" is a capability footnote, +// verbatim from a session this detector wrongly flagged. +// +// A causal connective is what separates them, and it yields when the sentence +// goes on to say what was done instead: the tool really was unnecessary then, +// which is the whole premise of the exemption. +var causalExcuseMarkers = []string{ + "because", "since ", " as no ", " as the ", " as there ", "due to", "owing to", +} + +// deliveredAlternativeMarkers say the work was done another way, which is what +// makes an absent tool harmless. +// +// AN ALLOW-LIST, because it grants the exemption. An unrecognised phrasing +// simply does not exempt — the sentence goes on to the ordinary handling rather +// than being waved through — which is the direction this detector should fail +// in when it cannot tell. +var deliveredAlternativeMarkers = []string{ + "instead", "by hand", "manually", "directly", + "proceeded", "went ahead", "carried on", + "in this answer", "into this answer", "in the answer", + "was not needed", "were not needed", "not needed here", "did not need", + "which was unnecessary", "so i read", "so i wrote", "so i checked", + "so i listed", "so i summarised", "so i summarized", "so i used", +} + // objectiveFailureMarkers name the OBJECTIVE rather than a capability. A // sentence carrying one is about whether the job got done, so the tool-grant // exemption above does not apply to it however many tools it mentions. @@ -694,7 +732,22 @@ func selfReportedIncompletion(text string) string { // reason to stop reading — so a sentence that also says something is // unverified, unapplied or untested goes on to the blocked-work handling // below instead of being exempted here. + // AND SOMETHING WAS DELIVERED INSTEAD. Naming an absent tool does not by + // itself establish that the tool was unnecessary — that is the claim the + // exemption makes on the sentence's behalf, and these do not support it: + // + // "I could not run the migration because no migration tool is available." + // "…because the migration tool is available only on Windows." + // + // The migration did not run and nothing took its place; the sentence + // merely explains WHY it did not. Broadening the matcher to the copula + // forms is what let these through, so the same commit that recognised a + // harmless caveat also started excusing an ordinary failure. + // + // The exemption exists for a tool that was NOT NEEDED, and what shows it + // was not needed is the alternative the sentence goes on to describe. if containsAny(sentence, toolGrantMarkers) && + (!containsAny(sentence, causalExcuseMarkers) || containsAny(sentence, deliveredAlternativeMarkers)) && !containsAny(sentence, objectiveFailureMarkers) && !containsAny(sentence, blockedStateMarkers) { continue diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 6eda20ead..60ec9db69 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -597,3 +597,51 @@ func TestABareObservationVerbIsNotASuccessfulAbsence(t *testing.T) { } } } + +// THE TOOL AS EXCUSE IS NOT THE TOOL AS FOOTNOTE. +// +// Naming an absent tool does not establish that the tool was unnecessary — that +// is the claim the exemption makes on the sentence's behalf. Broadening the +// matcher to the copula forms let these through: +// +// "I could not run the migration because no migration tool is available." +// +// The migration did not run, nothing took its place, and the sentence merely +// explains why. A causal connective separates that from the case the exemption +// was built for, which names no failed action at all — and the connective yields +// when the sentence goes on to say what was done instead, because then the tool +// really was unnecessary. +func TestAToolNamedAsAnExcuseDoesNotExemptTheFailure(t *testing.T) { + for _, admission := range []string{ + "I could not run the migration because no migration tool is available.", + "I could not run the migration because the migration tool is available only on Windows.", + "I could not build the image since no docker tool is available.", + "I could not publish it due to no release tool being available.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("a tool named as the reason for a failed action excused it: %q", admission) + } + } + + // A causal connective YIELDS to a delivered alternative: the tool was + // genuinely unnecessary, which is the premise of the exemption. + for _, exempt := range []string{ + "I could not record a plan because the update_plan tool isn't available, so I wrote it into this answer instead.", + "I could not run the formatter as no such tool is available, so I checked the style by hand.", + } { + if reason := selfReportedIncompletion(exempt); reason != "" { + t.Errorf("a caveat with delivered work was reported incomplete: %q -> %s", exempt, reason) + } + } + + // And the capability FOOTNOTE, which names no failed action, still passes. + // Verbatim from a session this detector wrongly flagged. + for _, footnote := range []string{ + "I don't have an `update_plan` tool available in this specialist context (only read-only exploration tools were provided).", + "No update_plan tool available in this specialist context, so I proceeded directly.", + } { + if reason := selfReportedIncompletion(footnote); reason != "" { + t.Errorf("a capability footnote was reported incomplete: %q -> %s", footnote, reason) + } + } +} From d232e8c505c0d965875f71f18adfadfdbcfe484d Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Thu, 20 Aug 2026 20:58:24 +0530 Subject: [PATCH 13/28] fix(agent): a tool caveat exempts a failure by position, not by connective Reported by @anandh8x: detect a tool excuse by the relationship between the failed action and the capability statement, not by a connector vocabulary. The exemption matched a list of causal words, so only the punctuation had to change to walk past it. Five of six ordinary spellings of the same excuse were wrongly exempted on 149c6f47: EXEMPT "I could not run the migration; no migration tool is available." EXEMPT "I could not run the migration: no migration tool is available." EXEMPT "I could not run the migration - no migration tool is available." EXEMPT "I could not run the migration, no migration tool is available." flagged "I could not run the migration. No migration tool is available." EXEMPT "I could not run the migration (no migration tool is available)." Only the full stop was caught, and only because a sentence split had already separated the two halves. That is a deny-list of connectives wearing a different coat: every mark or phrase nobody listed is a way through. toolCaveatIsTheStatement asks the structural question instead. A tool mention exempts a sentence only when no clause boundary sits between the inability stem and the tool, i.e. when the caveat IS the statement rather than the excuse appended to a separate admission of failure. "No migration tool is available" still passes; "I could not run the migration no migration tool is available" does not, whatever the anything. Measured on the fixture corpus: 0/9 missed, 0/5 wrongly flagged. Mutation-checked twice, each against the new cases: exempting on the tool marker alone, and treating the whole sentence as a single clause, each let 8 admissions through. Pre-existing on this branch and unrelated to it, in this environment only: TestRunDoctorFormatsRedactedProviderDiagnostics and TestRunDoctorConnectivityProbesProvider both exit 3. Origin-Session: local-8cd239 | Claude Code | 11 prompts Origin-Snapshot: 365efe3045f2 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b --- internal/agent/guardrails.go | 89 +++++++++++++++---- .../agent/guardrails_false_admission_test.go | 15 ++++ 2 files changed, 87 insertions(+), 17 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 03696ae69..e5efcecf5 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -329,26 +329,81 @@ var toolGrantMarkers = []string{ "is not in my toolset", "not in my toolset", "not in this toolset", } -// causalExcuseMarkers introduce the tool as the REASON some action did not -// happen, rather than as a footnote about this run's capabilities. +// toolCaveatIsTheStatement reports whether the tool mention IS what the inability +// is about, rather than an excuse offered for a separate action that did not +// happen. // -// THE TOOL AS EXCUSE IS NOT THE TOOL AS FOOTNOTE. Naming an absent tool does not -// establish it was unnecessary, and the copula broadening let these through: +// POSITION, NOT VOCABULARY. This was an enumeration of causal connectives — +// because, since, due to — and enumerating is how this class stays open. Only +// the punctuation had to change to walk straight past it: // -// "I could not run the migration because no migration tool is available." -// "…because the migration tool is available only on Windows." +// "I could not run the migration; no migration tool is available." +// "I could not run the migration: no migration tool is available." +// "I could not run the migration — no migration tool is available." // -// The migration did not run and nothing took its place; the sentence explains -// WHY. Whereas the case the exemption was built for names no failed action at -// all — "I don't have an update_plan tool available in this specialist context -// (only read-only exploration tools were provided)" is a capability footnote, -// verbatim from a session this detector wrongly flagged. +// Five of six ordinary forms were still exempted. Listing every connective and +// every mark a person might type is the deny-list shape this repo has been bitten +// by repeatedly; the relationship is the thing to test. // -// A causal connective is what separates them, and it yields when the sentence -// goes on to say what was done instead: the tool really was unnecessary then, -// which is the whole premise of the exemption. -var causalExcuseMarkers = []string{ - "because", "since ", " as no ", " as the ", " as there ", "due to", "owing to", +// The relationship is simple. If the inability's own clause is the tool statement +// — "I don't have an update_plan tool available in this specialist context" — +// then the caveat IS the statement and there is no failed action to excuse. If a +// clause boundary of ANY kind sits between the inability and the tool mention, +// something else was attempted first and the tool is being offered as the reason +// it did not happen. +// +// A sentence with no inability before the tool mention is a bare capability note +// and stays exempt. +func toolCaveatIsTheStatement(sentence string) bool { + toolAt := firstIndexOfAny(sentence, toolGrantMarkers) + if toolAt < 0 { + return false + } + stemAt, stemLen := firstStemBefore(sentence, toolAt) + if stemAt < 0 { + return true + } + return !containsClauseBoundary(sentence[stemAt+stemLen : toolAt]) +} + +// firstIndexOfAny returns the earliest offset at which any marker occurs, or -1. +func firstIndexOfAny(s string, markers []string) int { + best := -1 + for _, marker := range markers { + if at := strings.Index(s, marker); at >= 0 && (best < 0 || at < best) { + best = at + } + } + return best +} + +// firstStemBefore returns the offset and length of the earliest inability stem +// that starts before limit, or -1. +func firstStemBefore(s string, limit int) (int, int) { + bestAt, bestLen := -1, 0 + for _, stem := range inabilityStems { + at := strings.Index(s, stem) + if at < 0 || at >= limit { + continue + } + if bestAt < 0 || at < bestAt { + bestAt, bestLen = at, len(stem) + } + } + return bestAt, bestLen +} + +// clauseBoundaries end the clause an inability was stated in. Punctuation and +// connectives together, because a person separating two statements reaches for +// either and the detector should not care which. +var clauseBoundaries = []string{ + ";", ":", ",", ".", "(", ")", "|", "\u2014", "\u2013", " - ", " -- ", + "because", "since ", " as ", "due to", "owing to", "given that", + " so ", " but ", " and ", " while ", " though ", " although ", +} + +func containsClauseBoundary(between string) bool { + return containsAny(between, clauseBoundaries) } // deliveredAlternativeMarkers say the work was done another way, which is what @@ -747,7 +802,7 @@ func selfReportedIncompletion(text string) string { // The exemption exists for a tool that was NOT NEEDED, and what shows it // was not needed is the alternative the sentence goes on to describe. if containsAny(sentence, toolGrantMarkers) && - (!containsAny(sentence, causalExcuseMarkers) || containsAny(sentence, deliveredAlternativeMarkers)) && + (toolCaveatIsTheStatement(sentence) || containsAny(sentence, deliveredAlternativeMarkers)) && !containsAny(sentence, objectiveFailureMarkers) && !containsAny(sentence, blockedStateMarkers) { continue diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 60ec9db69..58f30c969 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -612,6 +612,21 @@ func TestABareObservationVerbIsNotASuccessfulAbsence(t *testing.T) { // when the sentence goes on to say what was done instead, because then the tool // really was unnecessary. func TestAToolNamedAsAnExcuseDoesNotExemptTheFailure(t *testing.T) { + // PUNCTUATION IS NOT THE POINT, the relationship is. The first version of + // this fix enumerated causal connectives, so only the mark had to change to + // walk past it — five of six ordinary forms were still exempted: + for _, admission := range []string{ + "I could not run the migration; no migration tool is available.", + "I could not run the migration: no migration tool is available.", + "I could not run the migration — no migration tool is available.", + "I could not run the migration, no migration tool is available.", + "I could not run the migration (no migration tool is available).", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("a tool offered as the reason for a failed action excused it: %q", admission) + } + } + for _, admission := range []string{ "I could not run the migration because no migration tool is available.", "I could not run the migration because the migration tool is available only on Windows.", From 84b5d10f26d2cd35664f8e635cf3210f488c24df Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Fri, 28 Aug 2026 17:25:03 +0530 Subject: [PATCH 14/28] fix(agent): keep tool caveats from hiding admissions --- internal/agent/guardrails.go | 20 ++++++++----- internal/agent/guardrails_test.go | 47 ++++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 17 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index e5efcecf5..82cb7c7f1 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -207,7 +207,7 @@ var successNegationTails = []string{ // "I could not measure the throughput, so the number is unknown." // "I could not trigger the migration, so it never ran." // - // All five went silent. Looking for something and finding none of it is a + // These cases went silent. Looking for something and finding none of it is a // result; failing to produce a thing you were asked for is not, and the "any" // is what separates them. Matching the strong tails exactly is also what keeps // the two lists from drifting apart again. @@ -444,7 +444,9 @@ var objectiveFailureMarkers = []string{ "meet the objective", "meeting the objective", "achieve the objective", "complete the assignment", "completing the assignment", "finish the assignment", "finishing the assignment", - "as requested", "what was asked", + "do what was asked", "doing what was asked", + "complete what was asked", "completing what was asked", + "finish what was asked", "finishing what was asked", "do this task", "perform this task", "carry out this task", } @@ -765,6 +767,15 @@ func selfReportedIncompletion(text string) string { if countedLabelSentence(sentence) { continue } + // Guessing and fabrication are high-signal admissions about the output, + // independent of which tools were granted. Check them before the tool + // caveat exemption so a capability note cannot hide a later admission in + // the same sentence. + for _, phrase := range selfReportPhrases { + if strings.Contains(sentence, phrase) { + return selfReportReason(phrase) + } + } // A sentence about the tool grant is about CAPABILITY, not about the // objective — UNLESS it also says the task itself could not be done. // @@ -807,11 +818,6 @@ func selfReportedIncompletion(text string) string { !containsAny(sentence, blockedStateMarkers) { continue } - for _, phrase := range selfReportPhrases { - if strings.Contains(sentence, phrase) { - return selfReportReason(phrase) - } - } for _, stem := range inabilityStems { // Scan EVERY occurrence of the stem, not just the first: an earlier // success-negation use ("I could not find any examples, so I could not diff --git a/internal/agent/guardrails_test.go b/internal/agent/guardrails_test.go index 691331a57..ad62cf4fa 100644 --- a/internal/agent/guardrails_test.go +++ b/internal/agent/guardrails_test.go @@ -596,16 +596,43 @@ func TestIncompletionAllowanceYieldsToBlockedWork(t *testing.T) { } } - // NOT CAUGHT, and recorded rather than hidden. An explicit "any" is treated - // as exhaustive absence, so an admission phrased that way passes: - // - // "I could not observe any effect, so the change may be inert." - // - // Measured across eleven admissions, four still pass, all of them either - // "any"-phrased or single-clause with no blocked-work signal ("I failed to - // reproduce it locally"). Catching those needs a different signal than - // substring matching; tightening this list to reach them re-broke the - // findings above every way I tried. + // A strong-absence phrase still yields when its consequence explicitly says + // the work is unresolved. + if selfReportedIncompletion("I could not observe any effect, so the change may be inert.") == "" { + t.Error("an unresolved strong-absence report passed the detector") + } +} + +func TestToolCaveatDoesNotHideGuessingOrFabrication(t *testing.T) { + for _, admission := range []string{ + "I don't have a write tool available in this specialist context, so I guessed the line numbers.", + "No update_plan tool is available, so I fabricated the plan section.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("tool caveat hid a high-signal admission: %q", admission) + } + } +} + +func TestToolCaveatAllowsCompletedObjectiveLanguage(t *testing.T) { + for _, completed := range []string{ + "I don't have a write tool available in this specialist context, and the objective is complete.", + "I don't have a write tool available in this specialist context, but I completed the analysis as requested.", + "I don't have an update_plan tool available in this specialist context; here is what was asked for.", + } { + if reason := selfReportedIncompletion(completed); reason != "" { + t.Errorf("completed tool caveat was flagged: %q -> %q", completed, reason) + } + } + + for _, admission := range []string{ + "I could not do what was asked because no write tool is available.", + "I am unable to complete what was asked with the tools available.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("verb-anchored objective failure passed the detector: %q", admission) + } + } } // An admission with NO first-person subject must still fire. The subjectless From b7828a001695c58c1fab57721944242e0548f57b Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Fri, 28 Aug 2026 20:54:39 +0530 Subject: [PATCH 15/28] fix(agent): make completion admission checks clause-aware --- internal/agent/guardrails.go | 136 +++++++++++++++--- .../agent/guardrails_false_admission_test.go | 36 +++++ 2 files changed, 156 insertions(+), 16 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 82cb7c7f1..8aa04257b 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -319,7 +319,7 @@ func admissionSentences(lower string) []string { // unfinished work as done. The markers now have to be about what the run WAS // GIVEN, not about a tool being mentioned at all. var toolGrantMarkers = []string{ - "tool available", "tools available", "no such tool", "not available in this", + "tool available", "tools available", "no such tool", "tool is available", "tools are available", "tool was available", "tools were available", "tool is not available", "tools are not available", "tool isn't available", "tools aren't available", @@ -329,6 +329,15 @@ var toolGrantMarkers = []string{ "is not in my toolset", "not in my toolset", "not in this toolset", } +func hasToolGrantContext(sentence string) bool { + if !containsAny(sentence, toolGrantMarkers) { + return false + } + return containsAny(sentence, []string{ + "tool", "tools", "toolset", "provided", "were given", "specialist context", + }) +} + // toolCaveatIsTheStatement reports whether the tool mention IS what the inability // is about, rather than an excuse offered for a separate action that did not // happen. @@ -355,6 +364,9 @@ var toolGrantMarkers = []string{ // A sentence with no inability before the tool mention is a bare capability note // and stays exempt. func toolCaveatIsTheStatement(sentence string) bool { + if !hasToolGrantContext(sentence) { + return false + } toolAt := firstIndexOfAny(sentence, toolGrantMarkers) if toolAt < 0 { return false @@ -396,10 +408,31 @@ func firstStemBefore(s string, limit int) (int, int) { // clauseBoundaries end the clause an inability was stated in. Punctuation and // connectives together, because a person separating two statements reaches for // either and the detector should not care which. -var clauseBoundaries = []string{ - ";", ":", ",", ".", "(", ")", "|", "\u2014", "\u2013", " - ", " -- ", - "because", "since ", " as ", "due to", "owing to", "given that", +var structuralClauseBoundaries = []string{ + "; ", ": ", ", so ", ", but ", ", therefore", ", leaving ", ", which ", " so ", " but ", " and ", " while ", " though ", " although ", + " - ", " -- ", +} + +var clauseBoundaries = append([]string{ + ";", ":", ",", ".", "(", ")", "|", + "because", "since ", " as ", "due to", "owing to", "given that", +}, structuralClauseBoundaries...) + +func clauseContaining(sentence string, at int) string { + start, end := 0, len(sentence) + for _, boundary := range structuralClauseBoundaries { + if before := strings.LastIndex(sentence[:at], boundary); before >= 0 { + candidate := before + len(boundary) + if candidate > start { + start = candidate + } + } + if after := strings.Index(sentence[at:], boundary); after >= 0 && at+after < end { + end = at + after + } + } + return strings.TrimSpace(sentence[start:end]) } func containsClauseBoundary(between string) bool { @@ -414,14 +447,50 @@ func containsClauseBoundary(between string) bool { // than being waved through — which is the direction this detector should fail // in when it cannot tell. var deliveredAlternativeMarkers = []string{ - "instead", "by hand", "manually", "directly", - "proceeded", "went ahead", "carried on", + "instead", "by hand", "manually", "directly", "in this answer", "into this answer", +} + +var deliveredAlternativeVerbs = []string{ + "i wrote", "i have written", "i've written", "i checked", "i read", "i listed", + "i summarised", "i summarized", "i provided", "i completed", "i finished", + "i did", "i performed", "i used", "i report", "i reported", + "i proceeded", "i went ahead", "i carried on", +} + +var attemptedAlternativeMarkers = []string{ + "i tried", "i attempted", "i planned", "i intend", "i would", "i could try", +} + +var deliveredAlternativeOutcomes = []string{ "in this answer", "into this answer", "in the answer", "was not needed", "were not needed", "not needed here", "did not need", "which was unnecessary", "so i read", "so i wrote", "so i checked", "so i listed", "so i summarised", "so i summarized", "so i used", } +func deliveredAlternative(sentence string) bool { + if containsAny(sentence, attemptedAlternativeMarkers) { + return false + } + if containsAny(sentence, deliveredAlternativeOutcomes) { + return true + } + for _, marker := range deliveredAlternativeMarkers { + for start := 0; ; { + rel := strings.Index(sentence[start:], marker) + if rel < 0 { + break + } + at := start + rel + if containsAny(clauseContaining(sentence, at), deliveredAlternativeVerbs) { + return true + } + start = at + len(marker) + } + } + return false +} + // objectiveFailureMarkers name the OBJECTIVE rather than a capability. A // sentence carrying one is about whether the job got done, so the tool-grant // exemption above does not apply to it however many tools it mentions. @@ -450,6 +519,24 @@ var objectiveFailureMarkers = []string{ "do this task", "perform this task", "carry out this task", } +func hasObjectiveFailure(sentence string) bool { + for _, marker := range objectiveFailureMarkers { + for start := 0; ; { + rel := strings.Index(sentence[start:], marker) + if rel < 0 { + break + } + at := start + rel + stemAt, stemLen := firstStemBefore(sentence, at) + if stemAt >= 0 && !containsClauseBoundary(sentence[stemAt+stemLen:at]) { + return true + } + start = at + len(marker) + } + } + return false +} + // blockedWorkMarkers are what turns an absence-establishing sentence back into // an admission of failure. // @@ -582,6 +669,8 @@ func cutFirstWord(text string) (word string, rest string, ok bool) { var blockedWorkMarkers = []string{ "unverified", "not verified", "cannot verify", "could not verify", + "unapplied", "not applied", "untested", "was never run", "were never run", + "was not run", "were not run", "tests not run", "someone else", "will need to", "needs someone", "handed off", "hand off", // "someone else" and "will need to" are the two that also appear in // SUCCESSFUL reports, describing somebody else's future work — "I could not @@ -630,6 +719,8 @@ var bareInabilityStems = []string{"so i cannot", "so i could not"} // to another subject, which is what the lookahead's topic-shift guard is for. var unambiguousFailureStates = []string{ "unverified", "not verified", + "unapplied", "not applied", "untested", "was never run", "were never run", + "was not run", "were not run", "tests not run", "is unresolved", "remains unresolved", "still broken", "is blocked", "are blocked", "remains blocked", "stays blocked", "still blocked", @@ -654,7 +745,7 @@ var unambiguousFailureStates = []string{ // the case the override was added for. What separates them is position: after a // consequence boundary the state is being asserted, inside a "that…" clause it is // the thing being denied. -var consequenceBoundaries = []string{", so ", "; ", ", but ", ", therefore", ", leaving ", ", which "} +var consequenceBoundaries = structuralClauseBoundaries // reportedConsequence returns the part of a sentence that states the OUTCOME, // or "" when the sentence never turns to one. Everything before the first @@ -733,16 +824,20 @@ var topicShiftMarkers = []string{ // stem. func countedLabelSentence(sentence string) bool { trimmed := strings.TrimLeft(strings.TrimSpace(sentence), "-*#> \t") - if !strings.HasPrefix(trimmed, "unable to ") { - return false - } - return countedLabelSuffix.MatchString(sentence) + return countedLabelHeading.MatchString(trimmed) } -var countedLabelSuffix = regexp.MustCompile(`\(\s*\d+\s*\)`) +var countedLabelHeading = regexp.MustCompile(`^unable to [^:()]*\(\s*\d+\s*\)\s*:\s*(?:\*\*)?(?:\s|$)`) + +func normalizeAdmissionText(text string) string { + return strings.NewReplacer( + "\u2019", "'", "\u2018", "'", "\u02bc", "'", + "\u2014", " - ", "\u2013", " - ", + ).Replace(text) +} func selfReportedIncompletion(text string) string { - sentences := admissionSentences(strings.ToLower(stripQuoted(text))) + sentences := admissionSentences(strings.ToLower(normalizeAdmissionText(stripQuoted(text)))) for index, sentence := range sentences { // THE CONSEQUENCE IS OFTEN THE NEXT SENTENCE. The blocked-work override // only ever saw the sentence the allowance fired in, so the same @@ -812,12 +907,21 @@ func selfReportedIncompletion(text string) string { // // The exemption exists for a tool that was NOT NEEDED, and what shows it // was not needed is the alternative the sentence goes on to describe. - if containsAny(sentence, toolGrantMarkers) && - (toolCaveatIsTheStatement(sentence) || containsAny(sentence, deliveredAlternativeMarkers)) && - !containsAny(sentence, objectiveFailureMarkers) && + if hasToolGrantContext(sentence) && + (toolCaveatIsTheStatement(sentence) || deliveredAlternative(sentence)) && + !hasObjectiveFailure(sentence) && !containsAny(sentence, blockedStateMarkers) { continue } + // A tool limitation can report blocked work without using a first-person + // inability stem: "No edit tool is available, so the change remains + // unapplied." The explicit bad state is the admission in that shape. Keep + // this narrow to a tool-grant statement plus an unambiguous consequence so + // ordinary discussion of an unavailable fixture or platform is not enough. + if hasToolGrantContext(sentence) && + containsAny(reportedConsequence(sentence), unambiguousFailureStates) { + return selfReportReason("tool limitation left work blocked") + } for _, stem := range inabilityStems { // Scan EVERY occurrence of the stem, not just the first: an earlier // success-negation use ("I could not find any examples, so I could not diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 58f30c969..a27732b98 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -660,3 +660,39 @@ func TestAToolNamedAsAnExcuseDoesNotExemptTheFailure(t *testing.T) { } } } + +// The classifier's exemptions must be decided from the relationship between +// clauses, not from one permissive keyword found anywhere in the sentence. This +// table pins the complete review matrix: headings versus prose, negative +// findings versus blocked outcomes, tool footnotes versus failed work, and a +// delivered fallback versus an attempted one. +func TestCompletionAdmissionClauseAndPolarityMatrix(t *testing.T) { + for _, admission := range []string{ + "Unable to verify the deployment after retry (3); it never started.", + "I could not find any evidence supporting the fix and it remains unverified.", + "I could not find any evidence supporting the fix: it remains unverified.", + "I could not find any evidence supporting the fix — it remains unverified.", + "There is no edit tool available here, so the change remains unapplied.", + "Write tools are not available in this setup, so the tests were never run.", + "I could not run the migration because no migration tool is available, although I tried to do it manually.", + "The fixture is not available in this checkout; I cannot run the tests.", + "I could not run the formatter because no formatter tool is available, but I planned to check it manually.", + } { + if selfReportedIncompletion(admission) == "" { + t.Errorf("a genuine incomplete outcome was exempted: %q", admission) + } + } + + for _, complete := range []string{ + "**Unable to verify (1):** - MCP #3 claim was truncated.", + "I could not find any evidence that the issue is unresolved.", + "I could not record a plan because the update_plan tool isn’t available, so I wrote it into this answer instead.", + "I could not run the formatter because no formatter tool is available, so I checked it by hand.", + "I don't have write tools available, but I was able to complete the task by providing the requested review.", + "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided.", + } { + if reason := selfReportedIncompletion(complete); reason != "" { + t.Errorf("a completed outcome was reported incomplete: %q -> %s", complete, reason) + } + } +} From a02d82907e3e69a0147d7ca1e1689e89c2125ab1 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Fri, 28 Aug 2026 21:57:09 +0530 Subject: [PATCH 16/28] fix(agent): scope completion evidence to clauses --- internal/agent/completion_policy_test.go | 32 ++++ internal/agent/guardrails.go | 188 +++++++++++++++-------- 2 files changed, 156 insertions(+), 64 deletions(-) diff --git a/internal/agent/completion_policy_test.go b/internal/agent/completion_policy_test.go index 220961684..519e358da 100644 --- a/internal/agent/completion_policy_test.go +++ b/internal/agent/completion_policy_test.go @@ -19,6 +19,38 @@ func TestCompletionPolicyLocalEvidenceDecidesWithoutSemanticCheck(t *testing.T) } } +func TestCompletionPolicyClauseLocalAdmissionMatrix(t *testing.T) { + for _, text := range []string{ + "I don't have a browser tool available, so I could not inspect the page.", + "No update_plan tool is available, so I wrote the plan manually, but I could not complete the requested analysis.", + "I could not run the migration because no migration tool is available; the error is quoted in this answer.", + "Unable to complete the migration (1): production deployment failed.", + "**Unable to verify (1):** I could not complete the audit; the review is unfinished.", + "I could not find any issues because I ran out of time before inspecting the code.", + "I could not find any issues since I ran out of time before inspecting the code.", + } { + got := newCompletionPolicy(false).evaluate(text, completionContext{}) + if got.Decision != CompletionIncomplete { + t.Errorf("incomplete report decided %q: %q", got.Decision, text) + } + } + + for _, text := range []string{ + "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided.", + "I could not record a plan because the update_plan tool isn't available, so I wrote it into this answer instead.", + "I tried the automated route first; I could not run the formatter because no formatter tool is available, so I checked it by hand.", + "**Unable to verify (1):** - MCP #3 claim was truncated.", + "From the source: I could not find any evidence that the issue is unresolved.", + "I could not find any evidence that the issue is unresolved and the fix is still unverified.", + "I could not find any remaining issues; separately, the documentation is outdated.", + } { + got := newCompletionPolicy(false).evaluate(text, completionContext{}) + if got.Decision != CompletionComplete { + t.Errorf("complete report decided %q: %q (%s)", got.Decision, text, got.Reason) + } + } +} + func TestCompletionPolicyPreservesBoundedPlanStallProtection(t *testing.T) { policy := newCompletionPolicy(false) for attempt := 0; attempt < maxContinueNudges; attempt++ { diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 8aa04257b..503c676c0 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -338,46 +338,6 @@ func hasToolGrantContext(sentence string) bool { }) } -// toolCaveatIsTheStatement reports whether the tool mention IS what the inability -// is about, rather than an excuse offered for a separate action that did not -// happen. -// -// POSITION, NOT VOCABULARY. This was an enumeration of causal connectives — -// because, since, due to — and enumerating is how this class stays open. Only -// the punctuation had to change to walk straight past it: -// -// "I could not run the migration; no migration tool is available." -// "I could not run the migration: no migration tool is available." -// "I could not run the migration — no migration tool is available." -// -// Five of six ordinary forms were still exempted. Listing every connective and -// every mark a person might type is the deny-list shape this repo has been bitten -// by repeatedly; the relationship is the thing to test. -// -// The relationship is simple. If the inability's own clause is the tool statement -// — "I don't have an update_plan tool available in this specialist context" — -// then the caveat IS the statement and there is no failed action to excuse. If a -// clause boundary of ANY kind sits between the inability and the tool mention, -// something else was attempted first and the tool is being offered as the reason -// it did not happen. -// -// A sentence with no inability before the tool mention is a bare capability note -// and stays exempt. -func toolCaveatIsTheStatement(sentence string) bool { - if !hasToolGrantContext(sentence) { - return false - } - toolAt := firstIndexOfAny(sentence, toolGrantMarkers) - if toolAt < 0 { - return false - } - stemAt, stemLen := firstStemBefore(sentence, toolAt) - if stemAt < 0 { - return true - } - return !containsClauseBoundary(sentence[stemAt+stemLen : toolAt]) -} - // firstIndexOfAny returns the earliest offset at which any marker occurs, or -1. func firstIndexOfAny(s string, markers []string) int { best := -1 @@ -457,21 +417,17 @@ var deliveredAlternativeVerbs = []string{ "i proceeded", "i went ahead", "i carried on", } -var attemptedAlternativeMarkers = []string{ - "i tried", "i attempted", "i planned", "i intend", "i would", "i could try", -} - var deliveredAlternativeOutcomes = []string{ - "in this answer", "into this answer", "in the answer", "was not needed", "were not needed", "not needed here", "did not need", "which was unnecessary", "so i read", "so i wrote", "so i checked", "so i listed", "so i summarised", "so i summarized", "so i used", } func deliveredAlternative(sentence string) bool { - if containsAny(sentence, attemptedAlternativeMarkers) { - return false - } + // A prior attempt does not erase a later completed fallback. Order matters: + // the relevant clause must itself contain a completion verb and an + // alternative marker. Bare location phrases such as "the error is in this + // answer" never prove that substitute work was delivered. if containsAny(sentence, deliveredAlternativeOutcomes) { return true } @@ -491,6 +447,75 @@ func deliveredAlternative(sentence string) bool { return false } +func nextInability(sentence string, after int) int { + best := -1 + for _, stem := range inabilityStems { + if rel := strings.Index(sentence[after:], stem); rel >= 0 { + at := after + rel + if best < 0 || at < best { + best = at + } + } + } + return best +} + +// deliveredAlternativeAfter scopes proof of substitute work to one inability. +// A later inability starts a new claim and cannot borrow an earlier fallback. +func deliveredAlternativeAfter(sentence string, after int) bool { + end := len(sentence) + if next := nextInability(sentence, after); next >= 0 { + end = next + } + return deliveredAlternative(sentence[after:end]) +} + +// toolCaveatAt recognizes only the inability occurrence whose own clause says +// which tool was unavailable. A harmless capability note may then be skipped +// without skipping a separate failed action later in the same sentence. +func toolCaveatAt(sentence string, stemAt, stemLen int) bool { + clause := clauseContaining(sentence, stemAt) + if !hasToolGrantContext(clause) { + return false + } + stem := sentence[stemAt : stemAt+stemLen] + localStem := strings.Index(clause, stem) + if localStem < 0 { + return false + } + toolAt := firstIndexOfAny(clause[localStem+stemLen:], toolGrantMarkers) + if toolAt < 0 { + return false + } + between := clause[localStem+stemLen : localStem+stemLen+toolAt] + return !containsClauseBoundary(between) +} + +var completedObjectiveMarkers = []string{ + "task is complete", "task is now complete", "objective is complete", "objective is met", + "assignment is complete", "completed the analysis as requested", "here is what was asked for", +} + +var harmlessGrantLimitedActions = []string{ + "record a plan", "record the plan", "update the plan", "call update_plan", "use update_plan", +} + +// harmlessToolLimitation covers bounded bookkeeping that an explicitly absent +// orchestration tool made impossible. It does not generalize to product work +// such as inspecting a page or running a migration. +func harmlessToolLimitation(sentence string, stemAt, stemLen int) bool { + if !hasToolGrantContext(sentence) { + return false + } + tail := strings.TrimSpace(sentence[stemAt+stemLen:]) + for _, action := range harmlessGrantLimitedActions { + if strings.HasPrefix(tail, action) { + return true + } + } + return containsAny(sentence[stemAt+stemLen:], completedObjectiveMarkers) +} + // objectiveFailureMarkers name the OBJECTIVE rather than a capability. A // sentence carrying one is about whether the job got done, so the tool-grant // exemption above does not apply to it however many tools it mentions. @@ -745,23 +770,43 @@ var unambiguousFailureStates = []string{ // the case the override was added for. What separates them is position: after a // consequence boundary the state is being asserted, inside a "that…" clause it is // the thing being denied. -var consequenceBoundaries = structuralClauseBoundaries +var consequenceBoundaries = []string{ + ", so ", "; so ", " so ", ", but ", "; but ", " but ", + ", therefore", "; therefore", " therefore", ", leaving ", " leaving ", + " because ", " since ", "; ", ": ", " - ", " -- ", " and ", ", ", +} -// reportedConsequence returns the part of a sentence that states the OUTCOME, -// or "" when the sentence never turns to one. Everything before the first -// boundary is what was searched for. -func reportedConsequence(sentence string) string { +// reportedConsequence returns an asserted outcome after one matched inability. +// Separators before the stem are irrelevant, and weak separators inside a +// `that ...` negated proposition remain part of what was not found. Explicit +// result and causal connectors still end that proposition. +func reportedConsequence(sentence string, stemEnd int) string { + if stemEnd < 0 || stemEnd >= len(sentence) { + return "" + } + tail := sentence[stemEnd:] + thatAt := strings.Index(tail, " that ") earliest := -1 width := 0 for _, boundary := range consequenceBoundaries { - if index := strings.Index(sentence, boundary); index >= 0 && (earliest < 0 || index < earliest) { + index := strings.Index(tail, boundary) + if index < 0 { + continue + } + explicit := strings.Contains(boundary, " so ") || strings.Contains(boundary, " but ") || + strings.Contains(boundary, "therefore") || strings.Contains(boundary, "leaving") || + strings.Contains(boundary, "because") || strings.Contains(boundary, "since") + if thatAt >= 0 && index > thatAt && !explicit { + continue + } + if earliest < 0 || index < earliest { earliest, width = index, len(boundary) } } if earliest < 0 { return "" } - return sentence[earliest+width:] + return tail[earliest+width:] } // blockedStateMarkers describe WORK LEFT BLOCKED — unverified, unresolved, @@ -824,7 +869,14 @@ var topicShiftMarkers = []string{ // stem. func countedLabelSentence(sentence string) bool { trimmed := strings.TrimLeft(strings.TrimSpace(sentence), "-*#> \t") - return countedLabelHeading.MatchString(trimmed) + match := countedLabelHeading.FindStringIndex(trimmed) + if match == nil { + return false + } + remainder := strings.TrimSpace(trimmed[match[1]:]) + // A heading may stand alone or introduce a markdown bucket. Prose or another + // admission on the same line is not part of the label and must be inspected. + return remainder == "" || strings.HasPrefix(remainder, "- ") } var countedLabelHeading = regexp.MustCompile(`^unable to [^:()]*\(\s*\d+\s*\)\s*:\s*(?:\*\*)?(?:\s|$)`) @@ -907,19 +959,13 @@ func selfReportedIncompletion(text string) string { // // The exemption exists for a tool that was NOT NEEDED, and what shows it // was not needed is the alternative the sentence goes on to describe. - if hasToolGrantContext(sentence) && - (toolCaveatIsTheStatement(sentence) || deliveredAlternative(sentence)) && - !hasObjectiveFailure(sentence) && - !containsAny(sentence, blockedStateMarkers) { - continue - } // A tool limitation can report blocked work without using a first-person // inability stem: "No edit tool is available, so the change remains // unapplied." The explicit bad state is the admission in that shape. Keep // this narrow to a tool-grant statement plus an unambiguous consequence so // ordinary discussion of an unavailable fixture or platform is not enough. if hasToolGrantContext(sentence) && - containsAny(reportedConsequence(sentence), unambiguousFailureStates) { + containsAny(reportedConsequence(sentence, 0), unambiguousFailureStates) { return selfReportReason("tool limitation left work blocked") } for _, stem := range inabilityStems { @@ -933,6 +979,20 @@ func selfReportedIncompletion(text string) string { } abs := start + rel tail := strings.TrimSpace(sentence[abs+len(stem):]) + scopeEnd := len(sentence) + if next := nextInability(sentence, abs+len(stem)); next >= 0 { + scopeEnd = next + } + scope := sentence[abs:scopeEnd] + // Exempt only this occurrence. A capability clause cannot hide a + // later failed action, and a completed fallback cannot be borrowed + // by a later inability in the same sentence. + if (toolCaveatAt(sentence, abs, len(stem)) && !hasObjectiveFailure(scope)) || + (harmlessToolLimitation(sentence, abs, len(stem)) && !hasObjectiveFailure(scope) && !containsAny(scope, blockedStateMarkers)) || + (hasToolGrantContext(scope) && deliveredAlternativeAfter(sentence, abs+len(stem)) && !hasObjectiveFailure(scope)) { + start = abs + len(stem) + continue + } // The allowance yields to a sentence that also says the work is // blocked: "could not reproduce the crash" is a finding, "could not // reproduce the crash, so the fix is unverified" is an admission, @@ -941,7 +1001,7 @@ func selfReportedIncompletion(text string) string { // The object decides whether finding nothing is a result; an // explicit failure state in the SAME sentence decides that it is // not, whatever the object. - if strong && containsAny(reportedConsequence(sentence), unambiguousFailureStates) { + if strong && containsAny(reportedConsequence(sentence, abs+len(stem)), unambiguousFailureStates) { strong = false } if !hasAnyPrefix(tail, successNegationTails) || (!strong && containsAny(blockedContext, blockedWorkMarkers)) { From c12695f790cc06a2b3c60c38a8dcbabaefb4b90e Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Fri, 28 Aug 2026 22:47:56 +0530 Subject: [PATCH 17/28] fix(agent): scope completion fallback evidence --- internal/agent/completion_policy_test.go | 5 + internal/agent/guardrails.go | 149 ++++++++++++++++------- 2 files changed, 109 insertions(+), 45 deletions(-) diff --git a/internal/agent/completion_policy_test.go b/internal/agent/completion_policy_test.go index 519e358da..984ed4056 100644 --- a/internal/agent/completion_policy_test.go +++ b/internal/agent/completion_policy_test.go @@ -28,6 +28,10 @@ func TestCompletionPolicyClauseLocalAdmissionMatrix(t *testing.T) { "**Unable to verify (1):** I could not complete the audit; the review is unfinished.", "I could not find any issues because I ran out of time before inspecting the code.", "I could not find any issues since I ran out of time before inspecting the code.", + "I could not run the tests because no test tool is available, so I checked the style by hand.", + "I could not run the tests because no test tool is available, so I checked the style by hand, but someone else will need to run the tests.", + "I could not run the tests because no test tool is available, so I checked the style by hand, but the tests remain unverified.", + "**Unable to verify (1):** - I could not complete the audit; the work remains unverified.", } { got := newCompletionPolicy(false).evaluate(text, completionContext{}) if got.Decision != CompletionIncomplete { @@ -40,6 +44,7 @@ func TestCompletionPolicyClauseLocalAdmissionMatrix(t *testing.T) { "I could not record a plan because the update_plan tool isn't available, so I wrote it into this answer instead.", "I tried the automated route first; I could not run the formatter because no formatter tool is available, so I checked it by hand.", "**Unable to verify (1):** - MCP #3 claim was truncated.", + "**Unable to verify (1):** - The source omitted MCP #3's full claim.", "From the source: I could not find any evidence that the issue is unresolved.", "I could not find any evidence that the issue is unresolved and the fix is still unverified.", "I could not find any remaining issues; separately, the documentation is outdated.", diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 503c676c0..e878a6b98 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -4,6 +4,7 @@ import ( "regexp" "strconv" "strings" + "unicode" "github.com/Gitlawb/zero/internal/zeroruntime" ) @@ -185,7 +186,7 @@ var inabilityStems = []string{ // // None name "i" or "we", so no other stem sees them. That traded one false // positive for three false negatives, in the direction this guard exists to - // prevent. countedLabelSentence drops the heading shape instead. + // prevent. countedLabelContent drops only the heading shape instead. "unable to ", "without being able to", } @@ -379,7 +380,7 @@ var clauseBoundaries = append([]string{ "because", "since ", " as ", "due to", "owing to", "given that", }, structuralClauseBoundaries...) -func clauseContaining(sentence string, at int) string { +func clauseBounds(sentence string, at int) (int, int) { start, end := 0, len(sentence) for _, boundary := range structuralClauseBoundaries { if before := strings.LastIndex(sentence[:at], boundary); before >= 0 { @@ -392,6 +393,11 @@ func clauseContaining(sentence string, at int) string { end = at + after } } + return start, end +} + +func clauseContaining(sentence string, at int) string { + start, end := clauseBounds(sentence, at) return strings.TrimSpace(sentence[start:end]) } @@ -419,26 +425,43 @@ var deliveredAlternativeVerbs = []string{ var deliveredAlternativeOutcomes = []string{ "was not needed", "were not needed", "not needed here", "did not need", - "which was unnecessary", "so i read", "so i wrote", "so i checked", - "so i listed", "so i summarised", "so i summarized", "so i used", + "which was unnecessary", +} + +func nextInability(sentence string, after int) int { + best := -1 + for _, stem := range inabilityStems { + if rel := strings.Index(sentence[after:], stem); rel >= 0 { + at := after + rel + if best < 0 || at < best { + best = at + } + } + } + return best } -func deliveredAlternative(sentence string) bool { - // A prior attempt does not erase a later completed fallback. Order matters: - // the relevant clause must itself contain a completion verb and an - // alternative marker. Bare location phrases such as "the error is in this - // answer" never prove that substitute work was delivered. - if containsAny(sentence, deliveredAlternativeOutcomes) { +// deliveredAlternativeAfter scopes proof of substitute work to one inability. +// A later inability starts a new claim and cannot borrow an earlier fallback. +func deliveredAlternativeAfter(sentence string, after int) bool { + end := len(sentence) + if next := nextInability(sentence, after); next >= 0 { + end = next + } + scope := sentence[after:end] + if containsAny(scope, deliveredAlternativeOutcomes) { return true } for _, marker := range deliveredAlternativeMarkers { for start := 0; ; { - rel := strings.Index(sentence[start:], marker) + rel := strings.Index(scope[start:], marker) if rel < 0 { break } at := start + rel - if containsAny(clauseContaining(sentence, at), deliveredAlternativeVerbs) { + fallbackStart, fallbackEnd := clauseBounds(scope, at) + fallback := strings.TrimSpace(scope[fallbackStart:fallbackEnd]) + if containsAny(fallback, deliveredAlternativeVerbs) && alternativeMatchesFailedWork(scope[:fallbackStart], fallback) { return true } start = at + len(marker) @@ -447,27 +470,53 @@ func deliveredAlternative(sentence string) bool { return false } -func nextInability(sentence string, after int) int { - best := -1 - for _, stem := range inabilityStems { - if rel := strings.Index(sentence[after:], stem); rel >= 0 { - at := after + rel - if best < 0 || at < best { - best = at - } +// alternativeMatchesFailedWork keeps substitute-delivery evidence tied to the +// operation it replaces. Display words such as "by hand" prove only that some +// activity happened; they do not make checking style a substitute for running +// tests. The groups are deliberately small because a match grants a completion +// exemption and therefore must fail closed for unfamiliar wording. +func alternativeMatchesFailedWork(failed, fallback string) bool { + recognizedFailedWork := false + for _, group := range [][]string{ + {"plan", "update_plan"}, + {"format", "formatter", "style", "lint", "gofmt"}, + {"test", "tests", "testing", "verify", "verification", "validate", "validation"}, + {"review", "audit", "inspect", "inspection", "analysis", "analyse", "analyze", "read"}, + {"write", "edit", "change", "patch", "modify"}, + {"document", "documentation", "summary", "report", "answer"}, + {"migration", "migrate"}, + {"deploy", "deployment"}, + {"publish", "release"}, + } { + failedInGroup := containsAlternativeTerm(failed, group) + recognizedFailedWork = recognizedFailedWork || failedInGroup + if failedInGroup && containsAlternativeTerm(fallback, group) { + return true } } - return best + // A fallback is also commonly pronominal: "could not run the formatter ... + // checked it by hand" or "could not record a plan ... wrote it into this + // answer". The explicit "it" ties the completed action to a recognized + // failed operation; "checked the style" does not and therefore cannot stand + // in for tests merely because both are checks. + return recognizedFailedWork && containsAny(fallback, []string{ + "wrote it", "written it", "checked it", "read it", "listed it", "provided it", + "completed it", "finished it", "did it", "performed it", "used it", + }) } -// deliveredAlternativeAfter scopes proof of substitute work to one inability. -// A later inability starts a new claim and cannot borrow an earlier fallback. -func deliveredAlternativeAfter(sentence string, after int) bool { - end := len(sentence) - if next := nextInability(sentence, after); next >= 0 { - end = next +func containsAlternativeTerm(text string, terms []string) bool { + words := strings.FieldsFunc(text, func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '_' + }) + for _, word := range words { + for _, term := range terms { + if word == term { + return true + } + } } - return deliveredAlternative(sentence[after:end]) + return false } // toolCaveatAt recognizes only the inability occurrence whose own clause says @@ -854,29 +903,36 @@ var topicShiftMarkers = []string{ "in a different", "on another", "elsewhere in", "in other news", } -// countedLabelSentence reports whether a sentence is a markdown LABEL that -// introduces and counts a bucket of findings, rather than a claim about the -// objective. +// countedLabelContent separates a counted markdown label from any content +// attached to it. A standalone label is not a claim about the objective; a +// same-line bullet still is report content and must be classified normally. // -// "**Unable to verify (1):** - MCP #3 claim was truncated" is a section heading -// in a COMPLETED audit. Read as a sentence it is indistinguishable from an -// admission, which is why the subjectless stem was once deleted to silence it. +// In "**Unable to verify (1):** - MCP #3 claim was truncated", the counted +// prefix is a section heading in a completed audit, while the text after "- " +// is the entry. Treating the entire sentence as either a heading or an +// admission loses one of those two roles. // // NARROW ON PURPOSE: the sentence must BEGIN with the inability phrase, after // markdown emphasis, AND carry a parenthesised count. "Unable to complete the // task; the build never succeeded" begins the same way and has no count, so it // still fires — which is the whole reason this is preferable to dropping the // stem. -func countedLabelSentence(sentence string) bool { +func countedLabelContent(sentence string) (string, bool) { trimmed := strings.TrimLeft(strings.TrimSpace(sentence), "-*#> \t") match := countedLabelHeading.FindStringIndex(trimmed) if match == nil { - return false + return sentence, false } remainder := strings.TrimSpace(trimmed[match[1]:]) - // A heading may stand alone or introduce a markdown bucket. Prose or another - // admission on the same line is not part of the label and must be inspected. - return remainder == "" || strings.HasPrefix(remainder, "- ") + // Only the counted HEADING is exempt. Attached prose or a same-line bullet is + // still ordinary report content and must pass through admission detection. + if remainder == "" { + return "", true + } + if strings.HasPrefix(remainder, "- ") { + return strings.TrimSpace(strings.TrimPrefix(remainder, "- ")), true + } + return sentence, false } var countedLabelHeading = regexp.MustCompile(`^unable to [^:()]*\(\s*\d+\s*\)\s*:\s*(?:\*\*)?(?:\s|$)`) @@ -891,6 +947,12 @@ func normalizeAdmissionText(text string) string { func selfReportedIncompletion(text string) string { sentences := admissionSentences(strings.ToLower(normalizeAdmissionText(stripQuoted(text)))) for index, sentence := range sentences { + if content, countedLabel := countedLabelContent(sentence); countedLabel { + if content == "" { + continue + } + sentence = content + } // THE CONSEQUENCE IS OFTEN THE NEXT SENTENCE. The blocked-work override // only ever saw the sentence the allowance fired in, so the same // admission escaped or was caught purely on its punctuation: @@ -910,10 +972,6 @@ func selfReportedIncompletion(text string) string { if containsAny(sentence, narrativeMarkers) { continue } - // A counted label is a heading, not a claim about the objective. - if countedLabelSentence(sentence) { - continue - } // Guessing and fabrication are high-signal admissions about the output, // independent of which tools were granted. Check them before the tool // caveat exemption so a capability note cannot hide a later admission in @@ -987,9 +1045,10 @@ func selfReportedIncompletion(text string) string { // Exempt only this occurrence. A capability clause cannot hide a // later failed action, and a completed fallback cannot be borrowed // by a later inability in the same sentence. - if (toolCaveatAt(sentence, abs, len(stem)) && !hasObjectiveFailure(scope)) || + if (toolCaveatAt(sentence, abs, len(stem)) && !hasObjectiveFailure(scope) && !containsAny(scope, blockedStateMarkers)) || (harmlessToolLimitation(sentence, abs, len(stem)) && !hasObjectiveFailure(scope) && !containsAny(scope, blockedStateMarkers)) || - (hasToolGrantContext(scope) && deliveredAlternativeAfter(sentence, abs+len(stem)) && !hasObjectiveFailure(scope)) { + (hasToolGrantContext(scope) && deliveredAlternativeAfter(sentence, abs+len(stem)) && !hasObjectiveFailure(scope) && + !containsAny(scope, blockedStateMarkers)) { start = abs + len(stem) continue } From 04627e745bf38a530006a108f36eed9f4b764a08 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sat, 29 Aug 2026 00:02:12 +0530 Subject: [PATCH 18/28] fix(agent): tighten capability completion exemptions --- internal/agent/completion_policy_test.go | 28 ++++++++ internal/agent/guardrails.go | 90 ++++++++++++++++++------ 2 files changed, 98 insertions(+), 20 deletions(-) diff --git a/internal/agent/completion_policy_test.go b/internal/agent/completion_policy_test.go index 984ed4056..0c5806d32 100644 --- a/internal/agent/completion_policy_test.go +++ b/internal/agent/completion_policy_test.go @@ -56,6 +56,34 @@ func TestCompletionPolicyClauseLocalAdmissionMatrix(t *testing.T) { } } +func TestCompletionPolicyToolExemptionPolarityAndObligations(t *testing.T) { + for _, text := range []string{ + "I could not run a test tool available in my toolset.", + "I could not record a plan while update_plan is available.", + "I could not run tests or deploy a release because no tools are available, so I checked the tests by hand.", + "I could not deploy a release or run tests because no tools are available, so I checked the tests by hand.", + "I could not record a plan because update_plan is unavailable. The task remains incomplete.", + "I could not run the formatter because no formatter tool is available, so I checked it by hand. The output remains unverified.", + } { + got := newCompletionPolicy(false).evaluate(text, completionContext{}) + if got.Decision != CompletionIncomplete { + t.Errorf("incomplete capability/fallback report decided %q: %q (%s)", got.Decision, text, got.Reason) + } + } + + for _, text := range []string{ + "The test tool is available in my toolset, and I ran all tests successfully.", + "I could not record a plan because update_plan is unavailable.", + "I could not run the formatter because no formatter tool is available, so I checked it by hand.", + "I could not run the formatter because no formatter tool is available, so I checked it by hand. Separately, documentation remains unverified.", + } { + got := newCompletionPolicy(false).evaluate(text, completionContext{}) + if got.Decision != CompletionComplete { + t.Errorf("completed or unrelated report decided %q: %q (%s)", got.Decision, text, got.Reason) + } + } +} + func TestCompletionPolicyPreservesBoundedPlanStallProtection(t *testing.T) { policy := newCompletionPolicy(false) for attempt := 0; attempt < maxContinueNudges; attempt++ { diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index e878a6b98..c94939969 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -295,8 +295,10 @@ func admissionSentences(lower string) []string { // retells a past exchange (narrativeMarkers) is skipped entirely — an admission // must be the model's own report about the CURRENT objective, not general // language that merely resembles one. -// toolGrantMarkers flag a sentence as reporting WHICH TOOLS this run was given, -// not whether the work was done. +// toolCapabilityMarkers identify statements about tool capability. They do NOT +// establish polarity: positive availability is intentionally included so the +// parser can locate the capability phrase, while hasUnavailableToolContext is +// the only predicate allowed to grant an absence-based exemption. // // A read-only plan task is SUPPOSED to say this. One wrote "I don't have an // update_plan tool available in this specialist context (only read-only @@ -319,26 +321,59 @@ func admissionSentences(lower string) []string { // direction: a false positive costs a re-run, a false negative reports // unfinished work as done. The markers now have to be about what the run WAS // GIVEN, not about a tool being mentioned at all. -var toolGrantMarkers = []string{ +var toolCapabilityMarkers = []string{ "tool available", "tools available", "no such tool", "tool is available", "tools are available", "tool was available", "tools were available", "tool is not available", "tools are not available", "tool isn't available", "tools aren't available", + "tool is unavailable", "tools are unavailable", "tool was unavailable", "tools were unavailable", "read-only tools", "read only tools", "only read-only", "only read only", "tools were provided", "tools were given", "toolset provided", "in this specialist context", "in this context only", "is not in my toolset", "not in my toolset", "not in this toolset", } -func hasToolGrantContext(sentence string) bool { - if !containsAny(sentence, toolGrantMarkers) { +func hasToolCapabilityContext(sentence string) bool { + if !containsAny(sentence, toolCapabilityMarkers) { return false } return containsAny(sentence, []string{ - "tool", "tools", "toolset", "provided", "were given", "specialist context", + "tool", "tools", "toolset", "provided", "were given", "specialist context", "update_plan", }) } +var unavailableToolPattern = regexp.MustCompile(`\bno(?:\s+[[:alnum:]_'-]+){0,6}\s+tools?(?:\s+(?:is|are))?\s+available\b`) + +var explicitUnavailableToolMarkers = []string{ + "no such tool", "tool is not available", "tools are not available", + "tool isn't available", "tools aren't available", + "tool is unavailable", "tools are unavailable", "tool was unavailable", "tools were unavailable", + "is not in my toolset", "not in my toolset", "not in this toolset", + "read-only tools", "read only tools", "only read-only", "only read only", +} + +// hasUnavailableToolContext is the single polarity contract for exemptions. +// A positive statement such as "the test tool is available" must never prove +// that a failed operation was harmless. +func hasUnavailableToolContext(sentence string) bool { + if strings.Contains(sentence, "update_plan") && containsAny(sentence, []string{ + "is unavailable", "was unavailable", "isn't available", "is not available", + }) { + return true + } + if !hasToolCapabilityContext(sentence) { + return false + } + if containsAny(sentence, explicitUnavailableToolMarkers) || unavailableToolPattern.MatchString(sentence) { + return true + } + return false +} + +func possessionDenialStem(stem string) bool { + return containsAny(stem, []string{"do not have", "don't have", "did not have", "didn't have"}) +} + // firstIndexOfAny returns the earliest offset at which any marker occurs, or -1. func firstIndexOfAny(s string, markers []string) int { best := -1 @@ -476,7 +511,8 @@ func deliveredAlternativeAfter(sentence string, after int) bool { // tests. The groups are deliberately small because a match grants a completion // exemption and therefore must fail closed for unfamiliar wording. func alternativeMatchesFailedWork(failed, fallback string) bool { - recognizedFailedWork := false + recognizedGroups := 0 + allGroupsCovered := true for _, group := range [][]string{ {"plan", "update_plan"}, {"format", "formatter", "style", "lint", "gofmt"}, @@ -489,17 +525,29 @@ func alternativeMatchesFailedWork(failed, fallback string) bool { {"publish", "release"}, } { failedInGroup := containsAlternativeTerm(failed, group) - recognizedFailedWork = recognizedFailedWork || failedInGroup - if failedInGroup && containsAlternativeTerm(fallback, group) { - return true + if !failedInGroup { + continue } + recognizedGroups++ + if !containsAlternativeTerm(fallback, group) { + allGroupsCovered = false + } + } + if recognizedGroups == 0 { + return false + } + if allGroupsCovered { + return true } // A fallback is also commonly pronominal: "could not run the formatter ... // checked it by hand" or "could not record a plan ... wrote it into this // answer". The explicit "it" ties the completed action to a recognized // failed operation; "checked the style" does not and therefore cannot stand // in for tests merely because both are checks. - return recognizedFailedWork && containsAny(fallback, []string{ + // A singular pronoun can safely cover one recognized operation, not a list + // of distinct obligations. Every operation in a coordinated failure must + // have substitute evidence before the inability is exempted. + return recognizedGroups == 1 && containsAny(fallback, []string{ "wrote it", "written it", "checked it", "read it", "listed it", "provided it", "completed it", "finished it", "did it", "performed it", "used it", }) @@ -524,7 +572,7 @@ func containsAlternativeTerm(text string, terms []string) bool { // without skipping a separate failed action later in the same sentence. func toolCaveatAt(sentence string, stemAt, stemLen int) bool { clause := clauseContaining(sentence, stemAt) - if !hasToolGrantContext(clause) { + if !hasToolCapabilityContext(clause) { return false } stem := sentence[stemAt : stemAt+stemLen] @@ -532,12 +580,13 @@ func toolCaveatAt(sentence string, stemAt, stemLen int) bool { if localStem < 0 { return false } - toolAt := firstIndexOfAny(clause[localStem+stemLen:], toolGrantMarkers) + toolAt := firstIndexOfAny(clause[localStem+stemLen:], toolCapabilityMarkers) if toolAt < 0 { return false } between := clause[localStem+stemLen : localStem+stemLen+toolAt] - return !containsClauseBoundary(between) + return !containsClauseBoundary(between) && + (hasUnavailableToolContext(clause) || possessionDenialStem(stem)) } var completedObjectiveMarkers = []string{ @@ -553,7 +602,8 @@ var harmlessGrantLimitedActions = []string{ // orchestration tool made impossible. It does not generalize to product work // such as inspecting a page or running a migration. func harmlessToolLimitation(sentence string, stemAt, stemLen int) bool { - if !hasToolGrantContext(sentence) { + stem := sentence[stemAt : stemAt+stemLen] + if !hasUnavailableToolContext(sentence) && !possessionDenialStem(stem) { return false } tail := strings.TrimSpace(sentence[stemAt+stemLen:]) @@ -1022,7 +1072,7 @@ func selfReportedIncompletion(text string) string { // unapplied." The explicit bad state is the admission in that shape. Keep // this narrow to a tool-grant statement plus an unambiguous consequence so // ordinary discussion of an unavailable fixture or platform is not enough. - if hasToolGrantContext(sentence) && + if hasUnavailableToolContext(sentence) && containsAny(reportedConsequence(sentence, 0), unambiguousFailureStates) { return selfReportReason("tool limitation left work blocked") } @@ -1045,10 +1095,10 @@ func selfReportedIncompletion(text string) string { // Exempt only this occurrence. A capability clause cannot hide a // later failed action, and a completed fallback cannot be borrowed // by a later inability in the same sentence. - if (toolCaveatAt(sentence, abs, len(stem)) && !hasObjectiveFailure(scope) && !containsAny(scope, blockedStateMarkers)) || - (harmlessToolLimitation(sentence, abs, len(stem)) && !hasObjectiveFailure(scope) && !containsAny(scope, blockedStateMarkers)) || - (hasToolGrantContext(scope) && deliveredAlternativeAfter(sentence, abs+len(stem)) && !hasObjectiveFailure(scope) && - !containsAny(scope, blockedStateMarkers)) { + if (toolCaveatAt(sentence, abs, len(stem)) && !hasObjectiveFailure(scope) && !containsAny(blockedContext, blockedStateMarkers)) || + (harmlessToolLimitation(sentence, abs, len(stem)) && !hasObjectiveFailure(scope) && !containsAny(blockedContext, blockedStateMarkers)) || + (hasUnavailableToolContext(scope) && deliveredAlternativeAfter(sentence, abs+len(stem)) && !hasObjectiveFailure(scope) && + !containsAny(blockedContext, blockedStateMarkers)) { start = abs + len(stem) continue } From 6a5f750891087ce96457d8f6cafbf42c06fac9b9 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sat, 29 Aug 2026 08:03:06 +0530 Subject: [PATCH 19/28] fix(agent): bind fallbacks to completed operations --- internal/agent/guardrails.go | 156 +++++++++++++++--- .../agent/guardrails_false_admission_test.go | 12 ++ 2 files changed, 149 insertions(+), 19 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index c94939969..c8bee5db3 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -455,6 +455,8 @@ var deliveredAlternativeVerbs = []string{ "i wrote", "i have written", "i've written", "i checked", "i read", "i listed", "i summarised", "i summarized", "i provided", "i completed", "i finished", "i did", "i performed", "i used", "i report", "i reported", + "i ran", "i executed", "i applied", "i migrated", "i tested", "i verified", + "i validated", "i deployed", "i published", "i released", "i proceeded", "i went ahead", "i carried on", } @@ -511,25 +513,36 @@ func deliveredAlternativeAfter(sentence string, after int) bool { // tests. The groups are deliberately small because a match grants a completion // exemption and therefore must fail closed for unfamiliar wording. func alternativeMatchesFailedWork(failed, fallback string) bool { + if containsAny(fallback, []string{ + "i did not", "i didn't", "i have not", "i haven't", "i could not", "i couldn't", + "i failed to", "i was unable to", "i wasn't able to", "i was not able to", + }) { + return false + } recognizedGroups := 0 allGroupsCovered := true - for _, group := range [][]string{ - {"plan", "update_plan"}, - {"format", "formatter", "style", "lint", "gofmt"}, - {"test", "tests", "testing", "verify", "verification", "validate", "validation"}, - {"review", "audit", "inspect", "inspection", "analysis", "analyse", "analyze", "read"}, - {"write", "edit", "change", "patch", "modify"}, - {"document", "documentation", "summary", "report", "answer"}, - {"migration", "migrate"}, - {"deploy", "deployment"}, - {"publish", "release"}, + recognized := "" + for _, group := range []struct { + name string + terms []string + }{ + {"plan", []string{"plan", "update_plan"}}, + {"format", []string{"format", "formatter", "style", "lint", "gofmt"}}, + {"test", []string{"test", "tests", "testing", "verify", "verification", "validate", "validation"}}, + {"review", []string{"review", "audit", "inspect", "inspection", "analysis", "analyse", "analyze", "read"}}, + {"write", []string{"write", "edit", "change", "patch", "modify"}}, + {"document", []string{"document", "documentation", "summary", "report", "answer"}}, + {"migration", []string{"migration", "migrate"}}, + {"deploy", []string{"deploy", "deployment"}}, + {"publish", []string{"publish", "release"}}, } { - failedInGroup := containsAlternativeTerm(failed, group) + failedInGroup := containsAlternativeTerm(failed, group.terms) if !failedInGroup { continue } recognizedGroups++ - if !containsAlternativeTerm(fallback, group) { + recognized = group.name + if !fallbackCompletesObligation(group.name, group.terms, fallback) { allGroupsCovered = false } } @@ -547,10 +560,116 @@ func alternativeMatchesFailedWork(failed, fallback string) bool { // A singular pronoun can safely cover one recognized operation, not a list // of distinct obligations. Every operation in a coordinated failure must // have substitute evidence before the inability is exempted. - return recognizedGroups == 1 && containsAny(fallback, []string{ - "wrote it", "written it", "checked it", "read it", "listed it", "provided it", - "completed it", "finished it", "did it", "performed it", "used it", + return recognizedGroups == 1 && fallbackPronounCompletesObligation(recognized, fallback) +} + +// fallbackCompletesObligation requires evidence for the failed ACTION, not just +// a repeated subject word. A migration plan and test documentation both share +// vocabulary with the failed operation, but neither executes that operation. +// The higher-risk operation groups therefore require an execution/verification +// verb as well as their object. +func fallbackCompletesObligation(name string, terms []string, fallback string) bool { + if containsAny(fallback, []string{ + "i did not", "i didn't", "i have not", "i haven't", "i could not", "i couldn't", + "i failed to", "i was unable to", "i wasn't able to", "i was not able to", + }) { + return false + } + if !containsAlternativeTerm(fallback, terms) { + return false + } + switch name { + case "migration": + if containsAlternativeTerm(fallback, []string{"plan", "report", "documentation", "document"}) { + return false + } + return actionTargetsObligation(fallback, terms, + []string{"migrated"}, []string{"ran", "executed", "applied", "performed", "completed", "did"}) + case "test": + if containsAlternativeTerm(fallback, []string{"plan", "report", "documentation", "document", "style"}) { + return false + } + return actionTargetsObligation(fallback, terms, + []string{"tested", "validated"}, []string{"ran", "executed", "verified", "performed", "completed", "did"}) + case "deploy": + return actionTargetsObligation(fallback, terms, + []string{"deployed"}, []string{"executed", "performed", "completed", "did"}) + case "publish": + return actionTargetsObligation(fallback, terms, + []string{"published", "released"}, []string{"performed", "completed", "did"}) + default: + return true + } +} + +func actionTargetsObligation(text string, objects, specificActions, genericActions []string) bool { + words := strings.FieldsFunc(text, func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '_' }) + for index, word := range words { + if !firstPersonAction(words, index) { + continue + } + if containsWord(specificActions, word) { + return true + } + if !containsWord(genericActions, word) { + continue + } + end := index + 6 + if end > len(words) { + end = len(words) + } + for _, candidate := range words[index+1 : end] { + if containsWord([]string{"wrote", "reported", "documented", "planned", "reviewed", "read", "checked"}, candidate) { + break + } + if candidate == "it" || containsWord(objects, candidate) { + return true + } + } + } + return false +} + +func firstPersonAction(words []string, action int) bool { + start := action - 3 + if start < 0 { + start = 0 + } + for index := action - 1; index >= start; index-- { + switch words[index] { + case "not", "never", "failed", "reported", "wrote", "documented", "planned", "reviewed", "read", "checked": + return false + case "i": + return true + } + } + return false +} + +func containsWord(words []string, want string) bool { + for _, word := range words { + if word == want { + return true + } + } + return false +} + +func fallbackPronounCompletesObligation(name, fallback string) bool { + pronouns := map[string][]string{ + "plan": {"wrote it", "written it", "listed it", "provided it", "completed it", "finished it", "did it"}, + "format": {"checked it", "performed it", "completed it", "finished it", "did it"}, + "test": {"ran it", "executed it", "tested it", "verified it", "validated it", "performed it", "completed it", "did it"}, + "review": {"reviewed it", "checked it", "read it", "inspected it", "analysed it", "analyzed it", "performed it", "completed it", "did it"}, + "write": {"wrote it", "written it", "edited it", "changed it", "patched it", "modified it", "completed it", "did it"}, + "document": {"wrote it", "written it", "documented it", "reported it", "provided it", "completed it", "did it"}, + "migration": {"ran it", "executed it", "applied it", "migrated it", "performed it", "completed it", "did it"}, + "deploy": {"deployed it", "executed it", "performed it", "completed it", "did it"}, + "publish": {"published it", "released it", "performed it", "completed it", "did it"}, + } + return containsAny(fallback, pronouns[name]) } func containsAlternativeTerm(text string, terms []string) bool { @@ -572,10 +691,10 @@ func containsAlternativeTerm(text string, terms []string) bool { // without skipping a separate failed action later in the same sentence. func toolCaveatAt(sentence string, stemAt, stemLen int) bool { clause := clauseContaining(sentence, stemAt) - if !hasToolCapabilityContext(clause) { + stem := sentence[stemAt : stemAt+stemLen] + if !possessionDenialStem(stem) || !hasToolCapabilityContext(clause) { return false } - stem := sentence[stemAt : stemAt+stemLen] localStem := strings.Index(clause, stem) if localStem < 0 { return false @@ -585,8 +704,7 @@ func toolCaveatAt(sentence string, stemAt, stemLen int) bool { return false } between := clause[localStem+stemLen : localStem+stemLen+toolAt] - return !containsClauseBoundary(between) && - (hasUnavailableToolContext(clause) || possessionDenialStem(stem)) + return !containsClauseBoundary(between) } var completedObjectiveMarkers = []string{ diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index a27732b98..ba5c120a4 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -632,6 +632,8 @@ func TestAToolNamedAsAnExcuseDoesNotExemptTheFailure(t *testing.T) { "I could not run the migration because the migration tool is available only on Windows.", "I could not build the image since no docker tool is available.", "I could not publish it due to no release tool being available.", + "I could not run the migration with no migration tool available.", + "I could not run the migration when no migration tool is available.", } { if selfReportedIncompletion(admission) == "" { t.Errorf("a tool named as the reason for a failed action excused it: %q", admission) @@ -677,6 +679,14 @@ func TestCompletionAdmissionClauseAndPolarityMatrix(t *testing.T) { "I could not run the migration because no migration tool is available, although I tried to do it manually.", "The fixture is not available in this checkout; I cannot run the tests.", "I could not run the formatter because no formatter tool is available, but I planned to check it manually.", + "I could not run the migration because no migration tool is available, so I wrote the migration plan manually.", + "I could not run the migration because no migration tool is available, so I reported the migration manually.", + "I could not run the migration because no migration tool is available, so I wrote the migration documentation manually.", + "I could not run the tests because no test tool is available, so I checked the test documentation manually.", + "I could not run the tests because no test tool is available, so I checked the test style manually.", + "I could not run the migration because no migration tool is available, so I did not execute the migration manually.", + "I could not run the tests because no test tool is available, so I did not test it manually.", + "I could not run the migration because no migration tool is available, so I executed diagnostics and wrote the migration plan manually.", } { if selfReportedIncompletion(admission) == "" { t.Errorf("a genuine incomplete outcome was exempted: %q", admission) @@ -690,6 +700,8 @@ func TestCompletionAdmissionClauseAndPolarityMatrix(t *testing.T) { "I could not run the formatter because no formatter tool is available, so I checked it by hand.", "I don't have write tools available, but I was able to complete the task by providing the requested review.", "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided.", + "I could not run the migration because no migration tool is available, so I executed the migration manually instead.", + "I could not run the tests because no test tool is available, so I tested it manually instead.", } { if reason := selfReportedIncompletion(complete); reason != "" { t.Errorf("a completed outcome was reported incomplete: %q -> %s", complete, reason) From aca069307598045bde4d67412c4daf3f1d506b14 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sun, 30 Aug 2026 00:07:58 +0530 Subject: [PATCH 20/28] fix(agent): classify completion claims by obligation --- internal/agent/completion_policy_test.go | 22 ++ internal/agent/guardrails.go | 238 ++++++++++-------- .../agent/guardrails_false_admission_test.go | 76 +++--- 3 files changed, 191 insertions(+), 145 deletions(-) diff --git a/internal/agent/completion_policy_test.go b/internal/agent/completion_policy_test.go index 0c5806d32..b255556d1 100644 --- a/internal/agent/completion_policy_test.go +++ b/internal/agent/completion_policy_test.go @@ -84,6 +84,28 @@ func TestCompletionPolicyToolExemptionPolarityAndObligations(t *testing.T) { } } +func TestCompletionPolicyReviewerSemanticPairs(t *testing.T) { + cases := []struct { + text string + want CompletionDecision + }{ + {"I could not produce any report.", CompletionIncomplete}, + {"I could not produce any crash.", CompletionComplete}, + {"I don't have access to the repository with no read tool available.", CompletionIncomplete}, + {"I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided.", CompletionComplete}, + {"I could not apply the edit because no write tool is available, so I reported the change manually.", CompletionIncomplete}, + {"I could not apply the edit because no write tool is available, so I applied the edit manually instead.", CompletionComplete}, + {"I could not run the full test suite because no test tool is available, so I manually tested only a smoke test.", CompletionIncomplete}, + {"I could not run the full test suite because no test tool is available, so I manually ran the full test suite instead.", CompletionComplete}, + } + for _, tc := range cases { + got := newCompletionPolicy(false).evaluate(tc.text, completionContext{}) + if got.Decision != tc.want { + t.Errorf("evaluate(%q) = %q, want %q (%s)", tc.text, got.Decision, tc.want, got.Reason) + } + } +} + func TestCompletionPolicyPreservesBoundedPlanStallProtection(t *testing.T) { policy := newCompletionPolicy(false) for attempt := 0; attempt < maxContinueNudges; attempt++ { diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index c8bee5db3..662648f9e 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -191,44 +191,6 @@ var inabilityStems = []string{ "without being able to", } -// successNegationTails are negated phrasings that indicate SUCCESS, not an -// admission ("I could not find any remaining issues", "I cannot reproduce the -// bug"). When an inability stem is immediately followed by one of these, it is not -// treated as an admission, so a clean result is not misreported as INCOMPLETE. -var successNegationTails = []string{ - "find any", "found any", "find a ", "see any", "detect any", "identify any", - "reproduce", "spot any", "locate any", - // The OBSERVATION verbs, in their "... any" forms ONLY. strongAbsence is - // consulted after this list has matched, so the observation-family strong - // tails were unreachable without these — but adding the verbs BARE made every - // "could not " a successful negative result, and these are ordinary - // admissions: - // - // "I could not produce the requested report." - // "I could not measure the throughput, so the number is unknown." - // "I could not trigger the migration, so it never ran." - // - // These cases went silent. Looking for something and finding none of it is a - // result; failing to produce a thing you were asked for is not, and the "any" - // is what separates them. Matching the strong tails exactly is also what keeps - // the two lists from drifting apart again. - "trigger any", "produce any", "hit any", "encounter any", - "provoke any", "surface any", "measure any", - // A NEGATIVE SEARCH RESULT IS THE ANSWER, not a failure to produce one. - // - // The list above already encodes this — "I could not find any remaining - // issues" is success — but only for the "any" phrasings. A finder reporting - // "I could NOT find where AllowManifestToolAutoApproval is set to true in - // production code" was marked INCOMPLETE after 53 tool calls and a 19,145 - // character audit, for doing precisely the job it was given: establishing - // that something is not there. - "find where", "find the", "find it", "find that", "find this", - "found where", "found the", - "locate where", "locate the", "locate it", - "determine where", "identify where", "see where", - "reproduce ", "confirm any", "observe any", -} - // narrativeMarkers flag a sentence as RETELLING a past exchange rather than // reporting the outcome of the current objective, so an inability admission in // that sentence is about THEN, not NOW. Grounded in a real false positive: a @@ -451,20 +413,6 @@ var deliveredAlternativeMarkers = []string{ "instead", "by hand", "manually", "directly", "in this answer", "into this answer", } -var deliveredAlternativeVerbs = []string{ - "i wrote", "i have written", "i've written", "i checked", "i read", "i listed", - "i summarised", "i summarized", "i provided", "i completed", "i finished", - "i did", "i performed", "i used", "i report", "i reported", - "i ran", "i executed", "i applied", "i migrated", "i tested", "i verified", - "i validated", "i deployed", "i published", "i released", - "i proceeded", "i went ahead", "i carried on", -} - -var deliveredAlternativeOutcomes = []string{ - "was not needed", "were not needed", "not needed here", "did not need", - "which was unnecessary", -} - func nextInability(sentence string, after int) int { best := -1 for _, stem := range inabilityStems { @@ -486,9 +434,6 @@ func deliveredAlternativeAfter(sentence string, after int) bool { end = next } scope := sentence[after:end] - if containsAny(scope, deliveredAlternativeOutcomes) { - return true - } for _, marker := range deliveredAlternativeMarkers { for start := 0; ; { rel := strings.Index(scope[start:], marker) @@ -498,7 +443,7 @@ func deliveredAlternativeAfter(sentence string, after int) bool { at := start + rel fallbackStart, fallbackEnd := clauseBounds(scope, at) fallback := strings.TrimSpace(scope[fallbackStart:fallbackEnd]) - if containsAny(fallback, deliveredAlternativeVerbs) && alternativeMatchesFailedWork(scope[:fallbackStart], fallback) { + if alternativeMatchesFailedWork(scope[:fallbackStart], fallback) { return true } start = at + len(marker) @@ -527,7 +472,7 @@ func alternativeMatchesFailedWork(failed, fallback string) bool { terms []string }{ {"plan", []string{"plan", "update_plan"}}, - {"format", []string{"format", "formatter", "style", "lint", "gofmt"}}, + {"format", []string{"format", "formatting", "formatter", "style", "lint", "gofmt"}}, {"test", []string{"test", "tests", "testing", "verify", "verification", "validate", "validation"}}, {"review", []string{"review", "audit", "inspect", "inspection", "analysis", "analyse", "analyze", "read"}}, {"write", []string{"write", "edit", "change", "patch", "modify"}}, @@ -549,6 +494,9 @@ func alternativeMatchesFailedWork(failed, fallback string) bool { if recognizedGroups == 0 { return false } + if !fallbackCoversRequiredScope(failed, fallback) { + return false + } if allGroupsCovered { return true } @@ -579,6 +527,30 @@ func fallbackCompletesObligation(name string, terms []string, fallback string) b return false } switch name { + case "plan": + if containsAlternativeTerm(fallback, []string{"report", "documentation", "document"}) { + return false + } + return actionTargetsObligation(fallback, terms, + []string{"planned", "recorded"}, []string{"wrote", "listed", "provided", "completed", "did"}) + case "format": + if containsAlternativeTerm(fallback, []string{"plan", "report", "documentation", "document"}) { + return false + } + return actionTargetsObligation(fallback, terms, + []string{"formatted"}, []string{"ran", "executed", "checked", "verified", "validated", "performed", "completed", "did"}) + case "review": + return actionTargetsObligation(fallback, terms, + []string{"reviewed", "audited", "inspected", "analysed", "analyzed", "read"}, []string{"checked", "performed", "completed", "did"}) + case "write": + if containsAlternativeTerm(fallback, []string{"plan", "report", "documentation", "document", "summary"}) { + return false + } + return actionTargetsObligation(fallback, terms, + []string{"wrote", "edited", "changed", "patched", "modified", "applied"}, []string{"performed", "completed", "did"}) + case "document": + return actionTargetsObligation(fallback, terms, + []string{"documented", "reported", "summarised", "summarized"}, []string{"wrote", "provided", "completed", "did"}) case "migration": if containsAlternativeTerm(fallback, []string{"plan", "report", "documentation", "document"}) { return false @@ -597,9 +569,23 @@ func fallbackCompletesObligation(name string, terms []string, fallback string) b case "publish": return actionTargetsObligation(fallback, terms, []string{"published", "released"}, []string{"performed", "completed", "did"}) - default: - return true } + return false +} + +// fallbackCoversRequiredScope keeps qualifiers that materially widen an +// obligation attached to the operation. A smoke test is not a substitute for a +// full suite, and checking one file is not a substitute for checking all files. +func fallbackCoversRequiredScope(failed, fallback string) bool { + for _, equivalents := range [][]string{ + {"full test suite", "full suite", "entire test suite", "entire suite", "all tests", "complete test suite"}, + {"all files", "every file", "entire repository", "whole repository", "full repository"}, + } { + if containsAny(failed, equivalents) && !containsAny(fallback, equivalents) { + return false + } + } + return true } func actionTargetsObligation(text string, objects, specificActions, genericActions []string) bool { @@ -686,10 +672,10 @@ func containsAlternativeTerm(text string, terms []string) bool { return false } -// toolCaveatAt recognizes only the inability occurrence whose own clause says -// which tool was unavailable. A harmless capability note may then be skipped -// without skipping a separate failed action later in the same sentence. -func toolCaveatAt(sentence string, stemAt, stemLen int) bool { +// capabilityOnlyToolFootnote recognizes a direct statement about the granted +// toolset, not a substantive inability whose explanation happens to mention a +// tool later in the clause. +func capabilityOnlyToolFootnote(sentence string, stemAt, stemLen int) bool { clause := clauseContaining(sentence, stemAt) stem := sentence[stemAt : stemAt+stemLen] if !possessionDenialStem(stem) || !hasToolCapabilityContext(clause) { @@ -704,7 +690,14 @@ func toolCaveatAt(sentence string, stemAt, stemLen int) bool { return false } between := clause[localStem+stemLen : localStem+stemLen+toolAt] - return !containsClauseBoundary(between) + if containsClauseBoundary(between) { + return false + } + return !containsAny(between, []string{ + "access", "repository", "file", "credential", "service", "network", + "inspect", "build", "edit", "change", + "publish", "deploy", "migrate", "verify", "validate", + }) } var completedObjectiveMarkers = []string{ @@ -782,11 +775,10 @@ func hasObjectiveFailure(sentence string) bool { // blockedWorkMarkers are what turns an absence-establishing sentence back into // an admission of failure. // -// MEASURED, NOT ARGUED. The successNegationTails list above exists so a finder -// reporting "I could not find where X is set" is not marked incomplete for doing -// its job. But the stems it added — "find the", "reproduce ", "confirm any", -// "observe any" — also head the most ordinary way of admitting defeat, and the -// allowance fired on the whole sentence regardless of how it ended. Measured +// MEASURED, NOT ARGUED. A finder reporting "I could not find where X is set" is +// not incomplete for doing its job. But search and reproduction wording also +// heads ordinary admissions, and a relationship-free allowance fired on the +// whole sentence regardless of how it ended. Measured // against eleven genuine admissions, TEN passed the detector undetected: // // "I could not reproduce the crash, so the fix is unverified." @@ -814,13 +806,6 @@ var strongAbsenceTails = []string{ "encounter any", "provoke any", "surface any", "measure any", } -// EVERY STRONG TAIL MUST BE REACHABLE. strongAbsence is consulted only after -// hasAnyPrefix(tail, successNegationTails) has already matched, so a strong tail -// whose verb is missing from that list is never asked about. Seven of the eight -// observation verbs added a round earlier were dead on arrival that way — only -// "reproduce" was already a success-negation stem, which is why the corpus that -// was supposed to cover them passed. - // strongAbsenceObjects are the things whose ABSENCE IS THE RESULT: you go // looking for them precisely so you can report there are none, and finding none // is the work succeeding. @@ -842,11 +827,12 @@ var strongAbsenceTails = []string{ // flagging it outright. var strongAbsenceObjects = []string{ "issue", "issues", "problem", "problems", "bug", "bugs", "defect", "defects", - "error", "errors", "failure", "failures", "regression", "regressions", + "error", "errors", "failure", "failures", "regression", "regressions", "crash", "crashes", "evidence", "example", "examples", "occurrence", "occurrences", "instance", "instances", "reference", "references", "match", "matches", "caller", "callers", "usage", "usages", "use", "uses", "case", "cases", "vulnerability", "vulnerabilities", "leak", "leaks", "race", "races", + "slowdown", "slowdowns", "sign", "signs", "trace", "traces", "mention", "mentions", "difference", "differences", "blocker", "blockers", "gap", "gaps", "omission", "omissions", "discrepancy", "discrepancies", "conflict", "conflicts", "violation", "violations", "warning", "warnings", @@ -1112,6 +1098,73 @@ func normalizeAdmissionText(text string) string { ).Replace(text) } +type inabilityClaim struct { + sentence string + blockedContext string + stemAt int + stemLen int + scope string + tail string +} + +func newInabilityClaim(sentence, blockedContext, stem string, stemAt int) inabilityClaim { + stemEnd := stemAt + len(stem) + scopeEnd := len(sentence) + if next := nextInability(sentence, stemEnd); next >= 0 { + scopeEnd = next + } + return inabilityClaim{ + sentence: sentence, + blockedContext: blockedContext, + stemAt: stemAt, + stemLen: len(stem), + scope: sentence[stemAt:scopeEnd], + tail: strings.TrimSpace(sentence[stemEnd:scopeEnd]), + } +} + +var boundedNegativeObservationTails = []string{ + "find where", "find the", "find it", "find that", "find this", + "found where", "found the", + "locate where", "locate the", "locate it", + "determine where", "identify where", "see where", + "reproduce", +} + +// successfulNegativeObservation requires a positive proof that the inability +// wording is actually the result: either a recognized absent object or a +// bounded search/reproduction proposition. A mere verb shape such as +// "produce any" cannot exempt an unrecognized deliverable. +func successfulNegativeObservation(tail string) (matched, strong bool) { + if strongAbsence(tail) { + return true, true + } + return hasAnyPrefix(tail, boundedNegativeObservationTails), false +} + +// exempt reports whether this particular inability is proven harmless. Direct +// failure state has precedence; capability and fallback branches grant an +// exemption only from their bounded clause and obligation evidence. +func (claim inabilityClaim) exempt() bool { + if matched, strong := successfulNegativeObservation(claim.tail); matched { + if strong { + return !containsAny(reportedConsequence(claim.sentence, claim.stemAt+claim.stemLen), unambiguousFailureStates) + } + return !containsAny(claim.blockedContext, blockedWorkMarkers) + } + if hasObjectiveFailure(claim.scope) || containsAny(claim.blockedContext, blockedStateMarkers) { + return false + } + if capabilityOnlyToolFootnote(claim.sentence, claim.stemAt, claim.stemLen) { + return true + } + if harmlessToolLimitation(claim.sentence, claim.stemAt, claim.stemLen) { + return true + } + return hasUnavailableToolContext(claim.scope) && + deliveredAlternativeAfter(claim.sentence, claim.stemAt+claim.stemLen) +} + func selfReportedIncompletion(text string) string { sentences := admissionSentences(strings.ToLower(normalizeAdmissionText(stripQuoted(text)))) for index, sentence := range sentences { @@ -1204,37 +1257,12 @@ func selfReportedIncompletion(text string) string { break } abs := start + rel - tail := strings.TrimSpace(sentence[abs+len(stem):]) - scopeEnd := len(sentence) - if next := nextInability(sentence, abs+len(stem)); next >= 0 { - scopeEnd = next - } - scope := sentence[abs:scopeEnd] - // Exempt only this occurrence. A capability clause cannot hide a - // later failed action, and a completed fallback cannot be borrowed - // by a later inability in the same sentence. - if (toolCaveatAt(sentence, abs, len(stem)) && !hasObjectiveFailure(scope) && !containsAny(blockedContext, blockedStateMarkers)) || - (harmlessToolLimitation(sentence, abs, len(stem)) && !hasObjectiveFailure(scope) && !containsAny(blockedContext, blockedStateMarkers)) || - (hasUnavailableToolContext(scope) && deliveredAlternativeAfter(sentence, abs+len(stem)) && !hasObjectiveFailure(scope) && - !containsAny(blockedContext, blockedStateMarkers)) { + claim := newInabilityClaim(sentence, blockedContext, stem, abs) + if claim.exempt() { start = abs + len(stem) continue } - // The allowance yields to a sentence that also says the work is - // blocked: "could not reproduce the crash" is a finding, "could not - // reproduce the crash, so the fix is unverified" is an admission, - // and the tail prefix alone cannot tell them apart. - strong := strongAbsence(tail) - // The object decides whether finding nothing is a result; an - // explicit failure state in the SAME sentence decides that it is - // not, whatever the object. - if strong && containsAny(reportedConsequence(sentence, abs+len(stem)), unambiguousFailureStates) { - strong = false - } - if !hasAnyPrefix(tail, successNegationTails) || (!strong && containsAny(blockedContext, blockedWorkMarkers)) { - return selfReportReason(strings.TrimSpace(stem) + " …") - } - start = abs + len(stem) + return selfReportReason(strings.TrimSpace(stem) + " …") } } } diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index ba5c120a4..841751e8b 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -1,9 +1,6 @@ package agent -import ( - "strings" - "testing" -) +import "testing" // FOUR COMPLETED TASKS WERE MARKED INCOMPLETE, and the sentences below are // verbatim from those sessions. @@ -498,38 +495,6 @@ func TestAToolCaveatIsRecognisedInItsCopulaForms(t *testing.T) { } } -// EVERY STRONG ABSENCE TAIL MUST BE REACHABLE. -// -// strongAbsence is consulted only after hasAnyPrefix(tail, successNegationTails) -// has already matched, so a strong tail whose verb is missing from that list is -// never asked about. Seven of the eight observation verbs added a round earlier -// were dead that way — only "reproduce" worked, because it was already a -// success-negation stem, and the corpus meant to cover the other seven passed -// on the strength of that one. -// -// Same shape as the unambiguous-state invariant below it: two lists that must -// agree, asserted rather than remembered. -func TestEveryStrongAbsenceTailIsReachable(t *testing.T) { - for _, tail := range strongAbsenceTails { - reachable := false - for _, negation := range successNegationTails { - // ONE DIRECTION, the one the runtime uses. hasAnyPrefix asks - // HasPrefix(tail, negation); the reverse clause this used to carry - // accepted a negation entry LONGER than the strong tail, which can - // never match a real message beginning with that tail — so the - // invariant would pass while the tail stayed dead. No entry is longer - // today, which is exactly why the loose form looked fine. - if strings.HasPrefix(tail, negation) { - reachable = true - break - } - } - if !reachable { - t.Errorf("strongAbsenceTails entry %q has no successNegationTails prefix, so strongAbsence never sees it", tail) - } - } -} - // A CONTRACTED TOOL CAVEAT IS THE SAME CAVEAT. // // The contracted markers shipped as "tool isn\'\'\'t available" — shell quoting @@ -561,10 +526,8 @@ func TestAContractedToolCaveatIsRecognised(t *testing.T) { // "ANY" IS WHAT MAKES A NEGATIVE RESULT A RESULT. // -// The observation verbs were added to successNegationTails BARE, to make the -// observation-family strong tails reachable. That made every "could not " -// a successful negative finding, and these are ordinary admissions that went -// silent: +// Treating every bare observation verb as a successful negative finding made +// these ordinary admissions go silent: // // "I could not produce the requested report." // "I could not measure the throughput, so the number is unknown." @@ -708,3 +671,36 @@ func TestCompletionAdmissionClauseAndPolarityMatrix(t *testing.T) { } } } + +func TestCompletionAdmissionReviewerSemanticPairs(t *testing.T) { + admissions := []string{ + "I could not produce any report.", + "I don't have access to the repository with no read tool available.", + "I don't have access to the file, although only read-only tools are available.", + "I don't have access to the credential with no credential tool available.", + "I don't have access to the service with no network tool available.", + "I could not run the migration because no migration tool is available; I did not need the formatted output.", + "I could not apply the edit because no write tool is available, so I reported the change manually.", + "I could not format the code because no formatter tool is available, so I documented the formatter manually.", + "I could not run the full test suite because no test tool is available, so I manually tested only a smoke test.", + "I could not run the tests and deploy the release because no tools are available, so I manually ran the tests.", + } + for _, text := range admissions { + if reason := selfReportedIncompletion(text); reason == "" { + t.Errorf("incomplete semantic pair was exempted: %q", text) + } + } + + complete := []string{ + "I could not produce any crash.", + "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided.", + "I could not record a plan because update_plan is unavailable, so I wrote the plan into this answer instead.", + "I could not run the formatter because no formatter tool is available, so I checked the formatting manually.", + "I could not run the full test suite because no test tool is available, so I manually ran the full test suite instead.", + } + for _, text := range complete { + if reason := selfReportedIncompletion(text); reason != "" { + t.Errorf("completed semantic pair was rejected: %q -> %s", text, reason) + } + } +} From 55f410e092d5ab7c5d870b2718faf5704085787e Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sun, 30 Aug 2026 22:39:27 +0530 Subject: [PATCH 21/28] fix(agent): coordinate blocked completion outcomes --- internal/agent/guardrails.go | 38 +++++++++++++++++-- .../agent/guardrails_false_admission_test.go | 24 ++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 662648f9e..06cbaccf9 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -754,7 +754,19 @@ var objectiveFailureMarkers = []string{ "do this task", "perform this task", "carry out this task", } +var explicitObjectiveFailureOutcomes = []string{ + "i did not complete the task", "i didn't complete the task", + "i did not complete this task", "i didn't complete this task", + "i did not finish the task", "i didn't finish the task", + "i did not finish this task", "i didn't finish this task", + "we did not complete the task", "we didn't complete the task", + "we did not finish the task", "we didn't finish the task", +} + func hasObjectiveFailure(sentence string) bool { + if containsAny(sentence, explicitObjectiveFailureOutcomes) { + return true + } for _, marker := range objectiveFailureMarkers { for start := 0; ; { rel := strings.Index(sentence[start:], marker) @@ -996,7 +1008,8 @@ func reportedConsequence(sentence string, stemEnd int) string { if index < 0 { continue } - explicit := strings.Contains(boundary, " so ") || strings.Contains(boundary, " but ") || + explicit := strings.HasPrefix(boundary, ";") || + strings.Contains(boundary, " so ") || strings.Contains(boundary, " but ") || strings.Contains(boundary, "therefore") || strings.Contains(boundary, "leaving") || strings.Contains(boundary, "because") || strings.Contains(boundary, "since") if thatAt >= 0 && index > thatAt && !explicit { @@ -1012,6 +1025,21 @@ func reportedConsequence(sentence string, stemEnd int) string { return tail[earliest+width:] } +// hasReportedFailureConsequence keeps the relationship decision made by the +// sentence lookahead while applying the same failure-state test to both forms: +// a consequence after punctuation in the current sentence, or the coordinated +// next sentence. Topic-shifted sentences never enter blockedContext. +func hasReportedFailureConsequence(sentence, blockedContext string, stemEnd int) bool { + if containsAny(reportedConsequence(sentence, stemEnd), unambiguousFailureStates) { + return true + } + if len(blockedContext) <= len(sentence) { + return false + } + next := strings.TrimSpace(blockedContext[len(sentence):]) + return containsAny(next, unambiguousFailureStates) +} + // blockedStateMarkers describe WORK LEFT BLOCKED — unverified, unresolved, // handed off, abandoned for time. Only these break the tool-grant exemption: // they say something about the state the work is in, which a tool caveat cannot @@ -1055,6 +1083,8 @@ var topicShiftMarkers = []string{ "separately", "unrelatedly", "unrelated", "as an aside", "aside from", "out of scope", "outside the scope", "not in scope", "for a different", "in a different", "on another", "elsewhere in", "in other news", + "future cleanup", "belongs to another", "belongs to a different", + "not part of this request", "never part of this request", } // countedLabelContent separates a counted markdown label from any content @@ -1148,11 +1178,11 @@ func successfulNegativeObservation(tail string) (matched, strong bool) { func (claim inabilityClaim) exempt() bool { if matched, strong := successfulNegativeObservation(claim.tail); matched { if strong { - return !containsAny(reportedConsequence(claim.sentence, claim.stemAt+claim.stemLen), unambiguousFailureStates) + return !hasReportedFailureConsequence(claim.sentence, claim.blockedContext, claim.stemAt+claim.stemLen) } return !containsAny(claim.blockedContext, blockedWorkMarkers) } - if hasObjectiveFailure(claim.scope) || containsAny(claim.blockedContext, blockedStateMarkers) { + if hasObjectiveFailure(claim.sentence) || containsAny(claim.blockedContext, blockedStateMarkers) { return false } if capabilityOnlyToolFootnote(claim.sentence, claim.stemAt, claim.stemLen) { @@ -1244,7 +1274,7 @@ func selfReportedIncompletion(text string) string { // this narrow to a tool-grant statement plus an unambiguous consequence so // ordinary discussion of an unavailable fixture or platform is not enough. if hasUnavailableToolContext(sentence) && - containsAny(reportedConsequence(sentence, 0), unambiguousFailureStates) { + hasReportedFailureConsequence(sentence, blockedContext, 0) { return selfReportReason("tool limitation left work blocked") } for _, stem := range inabilityStems { diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 841751e8b..3bf953ea2 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -704,3 +704,27 @@ func TestCompletionAdmissionReviewerSemanticPairs(t *testing.T) { } } } + +func TestCompletionAdmissionCoordinatesToolAndAbsenceConsequences(t *testing.T) { + for _, admission := range []string{ + "I don't have a write tool available, and I did not complete the task.", + "I could not find any bugs. The fix remains unverified.", + "No write tool is available. The change remains unapplied.", + "I could not find any evidence that the fix works; the fix remains unverified.", + } { + if reason := selfReportedIncompletion(admission); reason == "" { + t.Errorf("coordinated failure consequence was exempted: %q", admission) + } + } + + for _, complete := range []string{ + "I don't have a write tool available, but I completed the requested review without edits.", + "I could not find any bugs. Separately, the optional release note remains unverified.", + "No write tool is available. Outside the scope, the example change remains unapplied.", + "I could not find any evidence that the issue is unresolved.", + } { + if reason := selfReportedIncompletion(complete); reason != "" { + t.Errorf("bounded successful result was reported incomplete: %q -> %s", complete, reason) + } + } +} From 64a1444995075d9c71553b2bfb463e891cdd5960 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:29:33 +0530 Subject: [PATCH 22/28] fix(agent): preserve completion obligation scope --- internal/agent/guardrails.go | 121 +++++++++++++++--- .../agent/guardrails_false_admission_test.go | 35 +++++ 2 files changed, 137 insertions(+), 19 deletions(-) diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 06cbaccf9..1f93096cb 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -336,17 +336,6 @@ func possessionDenialStem(stem string) bool { return containsAny(stem, []string{"do not have", "don't have", "did not have", "didn't have"}) } -// firstIndexOfAny returns the earliest offset at which any marker occurs, or -1. -func firstIndexOfAny(s string, markers []string) int { - best := -1 - for _, marker := range markers { - if at := strings.Index(s, marker); at >= 0 && (best < 0 || at < best) { - best = at - } - } - return best -} - // firstStemBefore returns the offset and length of the earliest inability stem // that starts before limit, or -1. func firstStemBefore(s string, limit int) (int, int) { @@ -478,10 +467,18 @@ func alternativeMatchesFailedWork(failed, fallback string) bool { {"write", []string{"write", "edit", "change", "patch", "modify"}}, {"document", []string{"document", "documentation", "summary", "report", "answer"}}, {"migration", []string{"migration", "migrate"}}, - {"deploy", []string{"deploy", "deployment"}}, - {"publish", []string{"publish", "release"}}, + {"deploy", []string{"deploy", "deployment", "deployed"}}, + {"publish", []string{"publish", "publishing", "published", "release", "releasing", "released"}}, } { failedInGroup := containsAlternativeTerm(failed, group.terms) + // "deploy the release" names the deployable object, not a second + // publishing obligation. Requiring both operations made a genuine + // same-target deployment fallback impossible to recognize. + if group.name == "publish" && + !containsAlternativeTerm(failed, []string{"publish", "publishing"}) && + containsAlternativeTerm(failed, []string{"deploy", "deployment"}) { + failedInGroup = false + } if !failedInGroup { continue } @@ -585,9 +582,55 @@ func fallbackCoversRequiredScope(failed, fallback string) bool { return false } } + for _, target := range materialOperationTargets(failed) { + if !containsWord(materialOperationTargets(fallback), target) { + return false + } + } return true } +// materialOperationTargets extracts destination/environment qualifiers from an +// operation clause ("to production", "on Windows", "in us-east-1"). These +// qualifiers are part of the obligation: completing the same verb against a +// different target is not an equivalent fallback. +func materialOperationTargets(text string) []string { + for _, boundary := range []string{" because ", " since ", " due to ", " owing to ", " with no ", " without "} { + if at := strings.Index(text, boundary); at >= 0 { + text = text[:at] + } + } + words := strings.FieldsFunc(text, func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '-' && r != '_' + }) + var targets []string + for index, word := range words { + if !containsWord([]string{"to", "into", "onto", "on", "in", "against"}, word) || index+1 >= len(words) { + continue + } + target := words[index+1] + switch target { + case "the", "a", "an": + if index+2 >= len(words) { + continue + } + target = words[index+2] + } + switch target { + case "complete", "finish", "verify", "validate", "modify", "change", "write", "read", "run", "perform": + continue + case "prod": + target = "production" + case "stage": + target = "staging" + case "dev": + target = "development" + } + targets = append(targets, target) + } + return targets +} + func actionTargetsObligation(text string, objects, specificActions, genericActions []string) bool { words := strings.FieldsFunc(text, func(r rune) bool { return !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '_' @@ -685,19 +728,51 @@ func capabilityOnlyToolFootnote(sentence string, stemAt, stemLen int) bool { if localStem < 0 { return false } - toolAt := firstIndexOfAny(clause[localStem+stemLen:], toolCapabilityMarkers) + afterStem := clause[localStem+stemLen:] + toolAt, toolMarker := firstMarkerOfAny(afterStem, toolCapabilityMarkers) if toolAt < 0 { return false } - between := clause[localStem+stemLen : localStem+stemLen+toolAt] + between := afterStem[:toolAt] if containsClauseBoundary(between) { return false } - return !containsAny(between, []string{ + if containsAny(between, []string{ "access", "repository", "file", "credential", "service", "network", "inspect", "build", "edit", "change", "publish", "deploy", "migrate", "verify", "validate", + }) { + return false + } + return capabilityQualifierOnly(afterStem[toolAt+len(toolMarker):]) +} + +func firstMarkerOfAny(text string, markers []string) (int, string) { + bestAt, bestMarker := -1, "" + for _, marker := range markers { + if at := strings.Index(text, marker); at >= 0 && (bestAt < 0 || at < bestAt) { + bestAt, bestMarker = at, marker + } + } + return bestAt, bestMarker +} + +func capabilityQualifierOnly(text string) bool { + words := strings.FieldsFunc(text, func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '-' }) + allowed := []string{ + "in", "this", "the", "specialist", "context", "only", "read-only", + "read", "exploration", "tool", "tools", "toolset", "were", "was", + "provided", "given", "here", "current", "session", "environment", + "to", "me", "for", "available", + } + for _, word := range words { + if !containsWord(allowed, word) { + return false + } + } + return true } var completedObjectiveMarkers = []string{ @@ -972,6 +1047,12 @@ var unambiguousFailureStates = []string{ "left undone", } +var passiveMissedWorkPattern = regexp.MustCompile(`\b(?:was|were)\s+(?:not|never)\s+(?:applied|built|changed|deployed|edited|inspected|migrated|modified|published|read|reviewed|tested|validated|verified|written)\b`) + +func containsUnambiguousFailureState(text string) bool { + return containsAny(text, unambiguousFailureStates) || passiveMissedWorkPattern.MatchString(text) +} + // consequenceBoundaries separate what was looked for from what followed. // // THE STATE HAS TO BE THE OUTCOME, NOT PART OF WHAT WAS NEGATED. Matching @@ -1030,14 +1111,14 @@ func reportedConsequence(sentence string, stemEnd int) string { // a consequence after punctuation in the current sentence, or the coordinated // next sentence. Topic-shifted sentences never enter blockedContext. func hasReportedFailureConsequence(sentence, blockedContext string, stemEnd int) bool { - if containsAny(reportedConsequence(sentence, stemEnd), unambiguousFailureStates) { + if containsUnambiguousFailureState(reportedConsequence(sentence, stemEnd)) { return true } if len(blockedContext) <= len(sentence) { return false } next := strings.TrimSpace(blockedContext[len(sentence):]) - return containsAny(next, unambiguousFailureStates) + return containsUnambiguousFailureState(next) } // blockedStateMarkers describe WORK LEFT BLOCKED — unverified, unresolved, @@ -1182,7 +1263,9 @@ func (claim inabilityClaim) exempt() bool { } return !containsAny(claim.blockedContext, blockedWorkMarkers) } - if hasObjectiveFailure(claim.sentence) || containsAny(claim.blockedContext, blockedStateMarkers) { + if hasObjectiveFailure(claim.sentence) || + containsAny(claim.blockedContext, blockedStateMarkers) || + containsUnambiguousFailureState(claim.blockedContext) { return false } if capabilityOnlyToolFootnote(claim.sentence, claim.stemAt, claim.stemLen) { diff --git a/internal/agent/guardrails_false_admission_test.go b/internal/agent/guardrails_false_admission_test.go index 3bf953ea2..b278c0ba4 100644 --- a/internal/agent/guardrails_false_admission_test.go +++ b/internal/agent/guardrails_false_admission_test.go @@ -728,3 +728,38 @@ func TestCompletionAdmissionCoordinatesToolAndAbsenceConsequences(t *testing.T) } } } + +func TestCompletionAdmissionPreservesTargetsAndCapabilityOnlyScope(t *testing.T) { + for _, admission := range []string{ + "I could not deploy the release to production because no deployment tool is available, so I deployed it to staging manually instead.", + "I could not publish the package to registry-a because no publishing tool is available, so I published it to registry-b manually instead.", + "I could not run the tests on Windows because no test tool is available, so I ran the tests on Linux manually instead.", + "I don't have write tools available to modify the production configuration.", + "I don't have a browser tool available, so the required UI was never inspected.", + } { + if reason := selfReportedIncompletion(admission); reason == "" { + t.Errorf("materially incomplete result was exempted: %q", admission) + } + } + + for _, complete := range []string{ + "I could not deploy the release to production because no deployment tool is available, so I deployed it to production manually instead.", + "I could not deploy the release because no deployment tool is available, so I deployed it manually instead.", + "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided.", + "I don't have write tools available, but I completed the requested review without edits.", + } { + if reason := selfReportedIncompletion(complete); reason != "" { + t.Errorf("equivalent completion was rejected: %q -> %s", complete, reason) + } + } +} + +func TestFallbackPreservesMaterialOperationTargets(t *testing.T) { + failed := "deploy the release to production because no deployment tool is available" + if alternativeMatchesFailedWork(failed, "i deployed it to staging manually instead") { + t.Fatal("staging deployment satisfied a production obligation") + } + if !alternativeMatchesFailedWork(failed, "i deployed it to production manually instead") { + t.Fatal("same-target manual deployment did not satisfy the production obligation") + } +} From 1bdf520076511a5d29d1b5d6917573bc87b34a44 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Mon, 31 Aug 2026 19:45:06 +0530 Subject: [PATCH 23/28] fix(agent): reject incomplete completion reports --- internal/agent/completion_gate_test.go | 66 +++++++++++++ internal/agent/guardrails.go | 123 +++++++++++++++++-------- internal/agent/guardrails_test.go | 4 +- 3 files changed, 152 insertions(+), 41 deletions(-) diff --git a/internal/agent/completion_gate_test.go b/internal/agent/completion_gate_test.go index fae260ad2..b06af5d3f 100644 --- a/internal/agent/completion_gate_test.go +++ b/internal/agent/completion_gate_test.go @@ -203,3 +203,69 @@ func TestMaxTurnsCutoffIsIncompleteUnderGate(t *testing.T) { t.Fatalf("IncompleteReason = %q, want it to cite max-turns", result.IncompleteReason) } } + +// These cases exercise the public run loop rather than the classifier helpers. +// Each final answer used to pass the headless completion gate even though it +// explicitly admitted that the requested work was not complete. +func TestCompletionGateRejectsCurrentHeadReviewAdmissions(t *testing.T) { + admissions := []string{ + "I could not find the root cause.", + "I could not locate the migration script.", + "I was unable to determine where the regression was introduced, so the change was never applied.", + "I could not locate the file. The change was never applied.", + "I do not have the network access I needed, but the task is complete.", + "I do not have a write tool available in this context and could not complete the task.", + "Unable to complete the task (1):", + "**Unable to verify (1):** - the migration was never run", + "I could not run the migration because no migration tool is available, so I ran it manually instead, but it failed.", + "I could not run the migration because no migration tool is available, so I ran it manually instead. It failed.", + "I could not run the migration because no migration tool is available, so I ran it manually instead and it did not work.", + "I could not deploy to our production cluster because no deployment tool is available, so I deployed to our staging cluster manually instead.", + "I could not deploy to the main environment because no deployment tool is available, so I deployed to the staging environment manually instead.", + "I could not deploy to prod because no deployment tool is available, so I deployed to dev manually instead.", + "I do not have the API key or the tools available in this session.", + } + + for _, admission := range admissions { + t.Run(admission, func(t *testing.T) { + provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{textTurn(admission)}} + result, err := Run(context.Background(), "complete the requested operation", provider, Options{ + Registry: tools.NewRegistry(), + MaxTurns: 2, + RequireCompletionSignal: true, + }) + if err != nil { + t.Fatal(err) + } + if !result.Incomplete { + t.Fatalf("admission passed the production completion gate: %q", admission) + } + }) + } +} + +func TestCompletionGateKeepsCurrentHeadReviewControlsComplete(t *testing.T) { + complete := []string{ + "I could not find where the regression was introduced; the source is the parser boundary.", + "**Unable to verify (1):** - MCP #3 claim was truncated.", + "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided.", + "I could not deploy to our production cluster because no deployment tool is available, so I deployed to our production cluster manually instead.", + } + + for _, answer := range complete { + t.Run(answer, func(t *testing.T) { + provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{textTurn(answer)}} + result, err := Run(context.Background(), "complete the requested operation", provider, Options{ + Registry: tools.NewRegistry(), + MaxTurns: 2, + RequireCompletionSignal: true, + }) + if err != nil { + t.Fatal(err) + } + if result.Incomplete { + t.Fatalf("completed control was rejected: %q (%s)", answer, result.IncompleteReason) + } + }) + } +} diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 1f93096cb..205122038 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -336,22 +336,6 @@ func possessionDenialStem(stem string) bool { return containsAny(stem, []string{"do not have", "don't have", "did not have", "didn't have"}) } -// firstStemBefore returns the offset and length of the earliest inability stem -// that starts before limit, or -1. -func firstStemBefore(s string, limit int) (int, int) { - bestAt, bestLen := -1, 0 - for _, stem := range inabilityStems { - at := strings.Index(s, stem) - if at < 0 || at >= limit { - continue - } - if bestAt < 0 || at < bestAt { - bestAt, bestLen = at, len(stem) - } - } - return bestAt, bestLen -} - // clauseBoundaries end the clause an inability was stated in. Punctuation and // connectives together, because a person separating two statements reaches for // either and the detector should not care which. @@ -608,14 +592,16 @@ func materialOperationTargets(text string) []string { if !containsWord([]string{"to", "into", "onto", "on", "in", "against"}, word) || index+1 >= len(words) { continue } - target := words[index+1] - switch target { - case "the", "a", "an": - if index+2 >= len(words) { - continue - } - target = words[index+2] + targetAt := index + 1 + for targetAt < len(words) && containsWord([]string{ + "the", "a", "an", "our", "my", "your", "their", "its", "this", "that", + }, words[targetAt]) { + targetAt++ } + if targetAt >= len(words) { + continue + } + target := words[targetAt] switch target { case "complete", "finish", "verify", "validate", "modify", "change", "write", "read", "run", "perform": continue @@ -737,16 +723,34 @@ func capabilityOnlyToolFootnote(sentence string, stemAt, stemLen int) bool { if containsClauseBoundary(between) { return false } - if containsAny(between, []string{ - "access", "repository", "file", "credential", "service", "network", - "inspect", "build", "edit", "change", - "publish", "deploy", "migrate", "verify", "validate", - }) { + if !capabilitySubjectOnly(between) { return false } return capabilityQualifierOnly(afterStem[toolAt+len(toolMarker):]) } +// capabilitySubjectOnly is deliberately an allow-list because matching it +// grants a completion exemption. Unknown nouns before the tool marker may name +// the missing resource (an API key, repository, credential, or service), not +// merely qualify the toolset. +func capabilitySubjectOnly(text string) bool { + words := strings.FieldsFunc(text, func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '-' && r != '_' + }) + allowed := []string{ + "a", "an", "any", "no", "the", "this", "these", "those", "current", + "specialist", "orchestration", "update_plan", "read-only", "read", "write", + "edit", "editing", "formatter", "formatting", "test", "testing", "migration", + "release", "deployment", "tool", "tools", "toolset", + } + for _, word := range words { + if !containsWord(allowed, word) { + return false + } + } + return true +} + func firstMarkerOfAny(text string, markers []string) (int, string) { bestAt, bestMarker := -1, "" for _, marker := range markers { @@ -788,8 +792,7 @@ var harmlessGrantLimitedActions = []string{ // orchestration tool made impossible. It does not generalize to product work // such as inspecting a page or running a migration. func harmlessToolLimitation(sentence string, stemAt, stemLen int) bool { - stem := sentence[stemAt : stemAt+stemLen] - if !hasUnavailableToolContext(sentence) && !possessionDenialStem(stem) { + if !hasUnavailableToolContext(sentence) { return false } tail := strings.TrimSpace(sentence[stemAt+stemLen:]) @@ -849,7 +852,7 @@ func hasObjectiveFailure(sentence string) bool { break } at := start + rel - stemAt, stemLen := firstStemBefore(sentence, at) + stemAt, stemLen := lastStemBefore(sentence, at) if stemAt >= 0 && !containsClauseBoundary(sentence[stemAt+stemLen:at]) { return true } @@ -859,6 +862,33 @@ func hasObjectiveFailure(sentence string) bool { return false } +// lastStemBefore returns the nearest inability stem before limit. Coordinated +// clauses can contain an earlier capability footnote and a later objective +// failure ("I do not have ... and could not complete the task"); the nearest +// stem is the one that governs the objective phrase. +func lastStemBefore(s string, limit int) (int, int) { + bestAt, bestLen := -1, 0 + stems := append([]string{}, inabilityStems...) + // Coordinated clauses commonly elide the repeated first-person subject: + // "I do not have ... and could not complete the task." These short forms + // are considered only while locating an explicitly named objective failure. + stems = append(stems, "cannot ", "can't ", "can not ", "could not ", "couldn't ") + for _, stem := range stems { + for start := 0; start < limit; { + rel := strings.Index(s[start:limit], stem) + if rel < 0 { + break + } + at := start + rel + if at > bestAt { + bestAt, bestLen = at, len(stem) + } + start = at + len(stem) + } + } + return bestAt, bestLen +} + // blockedWorkMarkers are what turns an absence-establishing sentence back into // an admission of failure. // @@ -1111,14 +1141,20 @@ func reportedConsequence(sentence string, stemEnd int) string { // a consequence after punctuation in the current sentence, or the coordinated // next sentence. Topic-shifted sentences never enter blockedContext. func hasReportedFailureConsequence(sentence, blockedContext string, stemEnd int) bool { - if containsUnambiguousFailureState(reportedConsequence(sentence, stemEnd)) { + if containsFailureConsequence(reportedConsequence(sentence, stemEnd)) { return true } if len(blockedContext) <= len(sentence) { return false } next := strings.TrimSpace(blockedContext[len(sentence):]) - return containsUnambiguousFailureState(next) + return containsFailureConsequence(next) +} + +var explicitFailureConsequencePattern = regexp.MustCompile(`\b(?:it|that|this|the\s+[[:alnum:]_-]+)?\s*(?:failed|did\s+not\s+work|didn't\s+work)\b`) + +func containsFailureConsequence(text string) bool { + return containsUnambiguousFailureState(text) || explicitFailureConsequencePattern.MatchString(text) } // blockedStateMarkers describe WORK LEFT BLOCKED — unverified, unresolved, @@ -1189,13 +1225,21 @@ func countedLabelContent(sentence string) (string, bool) { return sentence, false } remainder := strings.TrimSpace(trimmed[match[1]:]) + heading := strings.TrimSpace(trimmed[:match[1]]) // Only the counted HEADING is exempt. Attached prose or a same-line bullet is // still ordinary report content and must pass through admission detection. if remainder == "" { + if hasObjectiveFailure(heading) { + return heading, false + } return "", true } if strings.HasPrefix(remainder, "- ") { - return strings.TrimSpace(strings.TrimPrefix(remainder, "- ")), true + content := strings.TrimSpace(strings.TrimPrefix(remainder, "- ")) + if containsUnambiguousFailureState(content) || hasObjectiveFailure(heading) { + return heading + " " + content, false + } + return content, true } return sentence, false } @@ -1235,13 +1279,13 @@ func newInabilityClaim(sentence, blockedContext, stem string, stemAt int) inabil } var boundedNegativeObservationTails = []string{ - "find where", "find the", "find it", "find that", "find this", - "found where", "found the", - "locate where", "locate the", "locate it", + "find where", "found where", "locate where", "determine where", "identify where", "see where", "reproduce", } +var boundedNegativeObservationPattern = regexp.MustCompile(`^find\s+the\s+[^,;:.]+\s+being\b`) + // successfulNegativeObservation requires a positive proof that the inability // wording is actually the result: either a recognized absent object or a // bounded search/reproduction proposition. A mere verb shape such as @@ -1250,7 +1294,7 @@ func successfulNegativeObservation(tail string) (matched, strong bool) { if strongAbsence(tail) { return true, true } - return hasAnyPrefix(tail, boundedNegativeObservationTails), false + return hasAnyPrefix(tail, boundedNegativeObservationTails) || boundedNegativeObservationPattern.MatchString(tail), false } // exempt reports whether this particular inability is proven harmless. Direct @@ -1261,7 +1305,8 @@ func (claim inabilityClaim) exempt() bool { if strong { return !hasReportedFailureConsequence(claim.sentence, claim.blockedContext, claim.stemAt+claim.stemLen) } - return !containsAny(claim.blockedContext, blockedWorkMarkers) + return !containsAny(claim.blockedContext, blockedWorkMarkers) && + !hasReportedFailureConsequence(claim.sentence, claim.blockedContext, claim.stemAt+claim.stemLen) } if hasObjectiveFailure(claim.sentence) || containsAny(claim.blockedContext, blockedStateMarkers) || diff --git a/internal/agent/guardrails_test.go b/internal/agent/guardrails_test.go index ad62cf4fa..d96ef9b14 100644 --- a/internal/agent/guardrails_test.go +++ b/internal/agent/guardrails_test.go @@ -556,8 +556,8 @@ func TestRunInjectsToolFailureHintWithSchema(t *testing.T) { } // An absence-establishing sentence is a finding; the same sentence that also -// says the work is blocked is an admission. The allowance stems ("find the", -// "reproduce ", "confirm any" …) head both, so the tail prefix alone cannot +// says the work is blocked is an admission. Observation allowances such as +// "reproduce" and "confirm any" can head both, so the tail prefix alone cannot // separate them — measured against real phrasings, TEN of eleven genuine // admissions passed the detector before blockedWorkMarkers existed. func TestIncompletionAllowanceYieldsToBlockedWork(t *testing.T) { From 7d98f1e00908eda462843a697a279675f6e73ab1 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Tue, 1 Sep 2026 22:05:38 +0530 Subject: [PATCH 24/28] fix(agent): close completion classifier gaps --- internal/agent/completion_gate_test.go | 6 ++++ internal/agent/guardrails.go | 43 +++++++++++++++++--------- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/internal/agent/completion_gate_test.go b/internal/agent/completion_gate_test.go index b06af5d3f..3af257888 100644 --- a/internal/agent/completion_gate_test.go +++ b/internal/agent/completion_gate_test.go @@ -210,6 +210,7 @@ func TestMaxTurnsCutoffIsIncompleteUnderGate(t *testing.T) { func TestCompletionGateRejectsCurrentHeadReviewAdmissions(t *testing.T) { admissions := []string{ "I could not find the root cause.", + "I could not find where to apply the fix.", "I could not locate the migration script.", "I was unable to determine where the regression was introduced, so the change was never applied.", "I could not locate the file. The change was never applied.", @@ -217,11 +218,15 @@ func TestCompletionGateRejectsCurrentHeadReviewAdmissions(t *testing.T) { "I do not have a write tool available in this context and could not complete the task.", "Unable to complete the task (1):", "**Unable to verify (1):** - the migration was never run", + "Unable to deploy (1): - production deployment failed.", + "**Unable to verify (1):** - the migration did not run", "I could not run the migration because no migration tool is available, so I ran it manually instead, but it failed.", "I could not run the migration because no migration tool is available, so I ran it manually instead. It failed.", "I could not run the migration because no migration tool is available, so I ran it manually instead and it did not work.", "I could not deploy to our production cluster because no deployment tool is available, so I deployed to our staging cluster manually instead.", "I could not deploy to the main environment because no deployment tool is available, so I deployed to the staging environment manually instead.", + "I could not deploy to the primary production environment because no deployment tool is available, so I deployed to the primary staging environment manually instead.", + "I could not publish to an internal production registry because no release tool is available, so I published to an internal staging registry manually instead.", "I could not deploy to prod because no deployment tool is available, so I deployed to dev manually instead.", "I do not have the API key or the tools available in this session.", } @@ -250,6 +255,7 @@ func TestCompletionGateKeepsCurrentHeadReviewControlsComplete(t *testing.T) { "**Unable to verify (1):** - MCP #3 claim was truncated.", "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided.", "I could not deploy to our production cluster because no deployment tool is available, so I deployed to our production cluster manually instead.", + "I could not deploy to the primary production environment because no deployment tool is available, so I deployed to the primary production environment manually instead.", } for _, answer := range complete { diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 205122038..1b102b0ee 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -601,18 +601,30 @@ func materialOperationTargets(text string) []string { if targetAt >= len(words) { continue } - target := words[targetAt] - switch target { + first := words[targetAt] + switch first { case "complete", "finish", "verify", "validate", "modify", "change", "write", "read", "run", "perform": continue - case "prod": - target = "production" - case "stage": - target = "staging" - case "dev": - target = "development" } - targets = append(targets, target) + end := targetAt + for end < len(words) && !containsWord([]string{ + "to", "into", "onto", "on", "in", "against", + "manually", "directly", "instead", "successfully", "by", "using", "with", + }, words[end]) { + end++ + } + targetWords := append([]string{}, words[targetAt:end]...) + for i, target := range targetWords { + switch target { + case "prod": + targetWords[i] = "production" + case "stage": + targetWords[i] = "staging" + case "dev": + targetWords[i] = "development" + } + } + targets = append(targets, strings.Join(targetWords, " ")) } return targets } @@ -1078,9 +1090,11 @@ var unambiguousFailureStates = []string{ } var passiveMissedWorkPattern = regexp.MustCompile(`\b(?:was|were)\s+(?:not|never)\s+(?:applied|built|changed|deployed|edited|inspected|migrated|modified|published|read|reviewed|tested|validated|verified|written)\b`) +var activeMissedWorkPattern = regexp.MustCompile(`\b(?:did\s+not|didn't)\s+(?:apply|build|change|deploy|edit|inspect|migrate|modify|publish|read|review|run|test|validate|verify|write)\b`) func containsUnambiguousFailureState(text string) bool { - return containsAny(text, unambiguousFailureStates) || passiveMissedWorkPattern.MatchString(text) + return containsAny(text, unambiguousFailureStates) || + passiveMissedWorkPattern.MatchString(text) || activeMissedWorkPattern.MatchString(text) } // consequenceBoundaries separate what was looked for from what followed. @@ -1236,7 +1250,7 @@ func countedLabelContent(sentence string) (string, bool) { } if strings.HasPrefix(remainder, "- ") { content := strings.TrimSpace(strings.TrimPrefix(remainder, "- ")) - if containsUnambiguousFailureState(content) || hasObjectiveFailure(heading) { + if containsFailureConsequence(content) || hasObjectiveFailure(heading) { return heading + " " + content, false } return content, true @@ -1279,12 +1293,11 @@ func newInabilityClaim(sentence, blockedContext, stem string, stemAt int) inabil } var boundedNegativeObservationTails = []string{ - "find where", "found where", "locate where", - "determine where", "identify where", "see where", "reproduce", } var boundedNegativeObservationPattern = regexp.MustCompile(`^find\s+the\s+[^,;:.]+\s+being\b`) +var boundedLocationObservationPattern = regexp.MustCompile(`^(?:find|found|locate|determine|identify|see)\s+where\s+(?:.+\s+)?(?:is|are|was|were)\s+(?:set|defined|declared|configured|introduced|registered|used|referenced|called|created|written|stored|assigned|enabled|disabled)\b`) // successfulNegativeObservation requires a positive proof that the inability // wording is actually the result: either a recognized absent object or a @@ -1294,7 +1307,9 @@ func successfulNegativeObservation(tail string) (matched, strong bool) { if strongAbsence(tail) { return true, true } - return hasAnyPrefix(tail, boundedNegativeObservationTails) || boundedNegativeObservationPattern.MatchString(tail), false + return hasAnyPrefix(tail, boundedNegativeObservationTails) || + boundedNegativeObservationPattern.MatchString(tail) || + boundedLocationObservationPattern.MatchString(tail), false } // exempt reports whether this particular inability is proven harmless. Direct From f5b76f51cd86828bc1f23ba9dce4ea6fa0cefb5e Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Wed, 2 Sep 2026 11:28:41 +0530 Subject: [PATCH 25/28] fix(agent): preserve completion obligation semantics --- internal/agent/completion_gate_test.go | 46 +++++ internal/agent/guardrails.go | 251 ++++++++++++++++++++++--- 2 files changed, 271 insertions(+), 26 deletions(-) diff --git a/internal/agent/completion_gate_test.go b/internal/agent/completion_gate_test.go index 3af257888..4d12a3ee1 100644 --- a/internal/agent/completion_gate_test.go +++ b/internal/agent/completion_gate_test.go @@ -275,3 +275,49 @@ func TestCompletionGateKeepsCurrentHeadReviewControlsComplete(t *testing.T) { }) } } + +func TestCompletionGateStructuralReviewerMatrix(t *testing.T) { + cases := []struct { + name string + answer string + incomplete bool + }{ + {name: "capability note only", answer: "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided."}, + {name: "capability note followed by subject-elided failure", answer: "I don't have a write tool available in this context and could not apply the required fix.", incomplete: true}, + {name: "same publish object completed", answer: "I could not publish the package because no publishing tool is available, so I published the package manually instead."}, + {name: "different publish object", answer: "I could not publish the package because no publishing tool is available, so I published the release notes manually instead.", incomplete: true}, + {name: "same destination completed", answer: "I could not publish the package for production because no publishing tool is available, so I published the package for production manually instead."}, + {name: "different destination", answer: "I could not publish the package for production because no publishing tool is available, so I published the package for staging manually instead.", incomplete: true}, + {name: "all coordinated tests completed", answer: "I could not run the unit and integration tests because no test tool is available, so I ran the unit and integration tests manually instead."}, + {name: "coordinated test subset", answer: "I could not run the unit and integration tests because no test tool is available, so I ran the unit tests manually instead.", incomplete: true}, + {name: "every test completed", answer: "I could not run every test because no test tool is available, so I ran every test manually instead."}, + {name: "smoke substituted for every test", answer: "I could not run every test because no test tool is available, so I ran a smoke test manually instead.", incomplete: true}, + {name: "affirmative fallback", answer: "I could not deploy the release because no deployment tool is available, so I deployed it manually instead."}, + {name: "negated fallback", answer: "I could not deploy the release because no deployment tool is available, so I never deployed it manually instead.", incomplete: true}, + {name: "partial fallback", answer: "I could not deploy the release because no deployment tool is available, so I partially deployed it manually instead.", incomplete: true}, + {name: "unsuccessful fallback", answer: "I could not deploy the release because no deployment tool is available, so I unsuccessfully deployed it manually instead.", incomplete: true}, + {name: "attempted fallback", answer: "I could not deploy the release because no deployment tool is available, so I attempted to deploy it manually instead.", incomplete: true}, + {name: "fallback crashed", answer: "I could not run the migration because no migration tool is available, so I ran it manually instead, but it crashed.", incomplete: true}, + {name: "benign counted audit bucket", answer: "**Unable to verify (1):** - MCP #3 claim was truncated."}, + {name: "counted operation rejected", answer: "**Unable to publish (1):** - registry rejected the request.", incomplete: true}, + {name: "exhaustive negative finding", answer: "I could not find any issues after inspecting every changed path."}, + {name: "blocked negative finding", answer: "I could not find any issues due to running out of time.", incomplete: true}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{textTurn(tc.answer)}} + result, err := Run(context.Background(), "complete the requested operation", provider, Options{ + Registry: tools.NewRegistry(), + MaxTurns: 2, + RequireCompletionSignal: true, + }) + if err != nil { + t.Fatal(err) + } + if result.Incomplete != tc.incomplete { + t.Fatalf("Incomplete = %v, want %v for %q (reason: %s)", result.Incomplete, tc.incomplete, tc.answer, result.IncompleteReason) + } + }) + } +} diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 1b102b0ee..026b4de7b 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -414,9 +414,13 @@ func deliveredAlternativeAfter(sentence string, after int) bool { break } at := start + rel - fallbackStart, fallbackEnd := clauseBounds(scope, at) - fallback := strings.TrimSpace(scope[fallbackStart:fallbackEnd]) - if alternativeMatchesFailedWork(scope[:fallbackStart], fallback) { + fallbackStart := deliveredFallbackStart(scope, at) + fallback := strings.TrimSpace(scope[fallbackStart:]) + // Outcome polarity is scoped from the fallback through the remainder + // of this inability, so a later "but it crashed" cannot be cut away by + // the same clause boundary used to isolate the action itself. + if fallbackOutcomeIsAffirmative(scope[fallbackStart:]) && + alternativeMatchesFailedWork(scope[:fallbackStart], fallback) { return true } start = at + len(marker) @@ -425,16 +429,33 @@ func deliveredAlternativeAfter(sentence string, after int) bool { return false } +// deliveredFallbackStart finds the relationship boundary that introduces the +// substitute work. General clauseBounds deliberately treats every "and" as a +// boundary; that is wrong inside one obligation ("unit and integration tests") +// and could split the fallback after one coordinated component. +func deliveredFallbackStart(scope string, markerAt int) int { + start := 0 + for _, boundary := range []string{ + ", so ", "; so ", " so ", ", but ", "; but ", " but ", + "; ", ": ", ", therefore ", "; therefore ", " therefore ", + } { + if before := strings.LastIndex(scope[:markerAt], boundary); before >= 0 { + candidate := before + len(boundary) + if candidate > start { + start = candidate + } + } + } + return start +} + // alternativeMatchesFailedWork keeps substitute-delivery evidence tied to the // operation it replaces. Display words such as "by hand" prove only that some // activity happened; they do not make checking style a substitute for running // tests. The groups are deliberately small because a match grants a completion // exemption and therefore must fail closed for unfamiliar wording. func alternativeMatchesFailedWork(failed, fallback string) bool { - if containsAny(fallback, []string{ - "i did not", "i didn't", "i have not", "i haven't", "i could not", "i couldn't", - "i failed to", "i was unable to", "i wasn't able to", "i was not able to", - }) { + if !fallbackOutcomeIsAffirmative(fallback) { return false } recognizedGroups := 0 @@ -498,10 +519,7 @@ func alternativeMatchesFailedWork(failed, fallback string) bool { // The higher-risk operation groups therefore require an execution/verification // verb as well as their object. func fallbackCompletesObligation(name string, terms []string, fallback string) bool { - if containsAny(fallback, []string{ - "i did not", "i didn't", "i have not", "i haven't", "i could not", "i couldn't", - "i failed to", "i was unable to", "i wasn't able to", "i was not able to", - }) { + if !fallbackOutcomeIsAffirmative(fallback) { return false } if !containsAlternativeTerm(fallback, terms) { @@ -558,22 +576,122 @@ func fallbackCompletesObligation(name string, terms []string, fallback string) b // obligation attached to the operation. A smoke test is not a substitute for a // full suite, and checking one file is not a substitute for checking all files. func fallbackCoversRequiredScope(failed, fallback string) bool { - for _, equivalents := range [][]string{ - {"full test suite", "full suite", "entire test suite", "entire suite", "all tests", "complete test suite"}, - {"all files", "every file", "entire repository", "whole repository", "full repository"}, - } { - if containsAny(failed, equivalents) && !containsAny(fallback, equivalents) { - return false - } + if requiredBreadth(failed) && !requiredBreadth(fallback) { + return false } for _, target := range materialOperationTargets(failed) { if !containsWord(materialOperationTargets(fallback), target) { return false } } + failedComponents := materialObligationComponents(failed) + fallbackComponents := materialObligationComponents(fallback) + // A singular pronoun may carry one already-named object across the fallback + // clause ("deploy the release ... deployed it"). It cannot stand in for a + // coordinated set such as unit and integration tests. + pronounCarriesObject := len(failedComponents) == 1 && containsAlternativeTerm(fallback, []string{"it"}) + for _, component := range failedComponents { + if !pronounCarriesObject && !containsWord(fallbackComponents, component) { + return false + } + } return true } +var nonAffirmativeFallbackPattern = regexp.MustCompile(`\b(?:never|unsuccessfully|partial|partially|attempted|trying|tried|failed|crashed|errored|aborted|rejected)\b`) + +// fallbackOutcomeIsAffirmative is the single result-polarity gate for every +// specific-action and pronoun path. Seeing a past-tense operation word is not +// completion evidence when the same bounded fallback says it was negated, +// partial, attempted-only, unsuccessful, or failed afterwards. +func fallbackOutcomeIsAffirmative(fallback string) bool { + if containsAny(fallback, []string{ + "i did not", "i didn't", "i have not", "i haven't", "i could not", "i couldn't", + "i failed to", "i was unable to", "i wasn't able to", "i was not able to", + }) { + return false + } + return !nonAffirmativeFallbackPattern.MatchString(fallback) && + !containsFailureConsequence(fallback) +} + +func requiredBreadth(text string) bool { + words := obligationWords(text) + for _, word := range words { + switch word { + case "all", "every", "entire", "whole", "full", "complete": + return true + } + } + return false +} + +// materialObligationComponents retains the material nouns/adjectives that +// distinguish obligations inside the same broad operation class. Operation +// words, grammar, breadth, and delivery mechanics are removed; what remains is +// the object/component set that a fallback must preserve (package vs release +// notes, unit+integration vs unit-only, production vs staging). +func materialObligationComponents(text string) []string { + words := obligationWords(text) + ignored := map[string]bool{} + for _, word := range []string{ + "i", "we", "could", "cannot", "can't", "couldn't", "not", "unable", "was", "were", "am", "are", "to", + "the", "a", "an", "our", "my", "your", "their", "this", "that", "it", "its", + "and", "or", "for", "on", "in", "into", "onto", "against", "of", "with", + "because", "since", "due", "owing", "as", "so", "but", "then", + "no", "tool", "tools", "toolset", "available", "unavailable", + "manually", "directly", "instead", "by", "hand", "successfully", + "all", "every", "entire", "whole", "full", "complete", "only", "suite", + "run", "ran", "execute", "executed", "perform", "performed", "check", "checked", "did", "do", "done", + "apply", "applied", "write", "wrote", "written", "edit", "edited", "change", "changed", "patch", "patched", "modify", "modified", + "plan", "planned", "record", "recorded", "format", "formatted", "formatting", "formatter", "style", "lint", "gofmt", + "test", "tests", "testing", "verify", "verified", "verification", "validate", "validated", "validation", + "review", "reviewed", "audit", "audited", "inspect", "inspected", "inspection", "analysis", "analyse", "analyze", "analysed", "analyzed", "read", + "document", "documented", "documentation", "summary", "report", "reported", "answer", + "migration", "migrate", "migrated", "deploy", "deployed", "deployment", + "publish", "published", "publishing", "release", "released", "releasing", + } { + ignored[word] = true + } + components := []string{} + for _, word := range words { + word = normalizeObligationWord(word) + if word == "" || ignored[word] || containsWord(components, word) { + continue + } + components = append(components, word) + } + return components +} + +func obligationWords(text string) []string { + for _, boundary := range []string{" because ", " since ", " due to ", " owing to ", " as no ", ", so ", "; so "} { + if at := strings.Index(text, boundary); at >= 0 { + text = text[:at] + } + } + return strings.FieldsFunc(text, func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsDigit(r) && r != '-' && r != '_' + }) +} + +func normalizeObligationWord(word string) string { + switch word { + case "prod": + return "production" + case "stage": + return "staging" + case "dev": + return "development" + case "configs": + return "config" + } + if len(word) > 4 && strings.HasSuffix(word, "s") && !strings.HasSuffix(word, "ss") { + return strings.TrimSuffix(word, "s") + } + return word +} + // materialOperationTargets extracts destination/environment qualifiers from an // operation clause ("to production", "on Windows", "in us-east-1"). These // qualifiers are part of the obligation: completing the same verb against a @@ -589,7 +707,7 @@ func materialOperationTargets(text string) []string { }) var targets []string for index, word := range words { - if !containsWord([]string{"to", "into", "onto", "on", "in", "against"}, word) || index+1 >= len(words) { + if !containsWord([]string{"to", "into", "onto", "on", "in", "against", "for"}, word) || index+1 >= len(words) { continue } targetAt := index + 1 @@ -608,7 +726,7 @@ func materialOperationTargets(text string) []string { } end := targetAt for end < len(words) && !containsWord([]string{ - "to", "into", "onto", "on", "in", "against", + "to", "into", "onto", "on", "in", "against", "for", "manually", "directly", "instead", "successfully", "by", "using", "with", }, words[end]) { end++ @@ -741,6 +859,33 @@ func capabilityOnlyToolFootnote(sentence string, stemAt, stemLen int) bool { return capabilityQualifierOnly(afterStem[toolAt+len(toolMarker):]) } +var subjectElidedInabilityStems = []string{ + "cannot ", "can't ", "can not ", "could not ", "couldn't ", + "am unable to", "was unable to", "wasn't able to", "was not able to", +} + +// hasUnexemptedSubjectElidedInability continues classification after a narrow +// capability clause. The original first-person subject commonly governs a +// coordinated short form ("I don't have ... and could not apply ..."). Each +// remainder is classified as its own synthetic first-person claim so a genuine +// negative observation can still earn its own exemption. +func hasUnexemptedSubjectElidedInability(sentence string, after int) bool { + for _, connector := range []string{" and ", " but ", " so ", " then ", " yet "} { + for _, stem := range subjectElidedInabilityStems { + needle := connector + stem + if rel := strings.Index(sentence[after:], needle); rel >= 0 { + remainder := strings.TrimSpace(sentence[after+rel+len(connector):]) + synthetic := "i " + remainder + claim := newInabilityClaim(synthetic, synthetic, "i "+stem, 0) + if !claim.exempt() { + return true + } + } + } + } + return false +} + // capabilitySubjectOnly is deliberately an allow-list because matching it // grants a completion exemption. Unknown nouns before the tool marker may name // the missing resource (an API key, repository, credential, or service), not @@ -1035,7 +1180,7 @@ var blockedWorkMarkers = []string{ // Windows path" is a finding. They are kept because they do carry a genuine // handoff-because-blocked ("someone else will need to pick this up"), and the // strongAbsenceTails exemption above is what stops them flipping a finding. - "ran out of", "run out of", "out of time", "in the time available", + "ran out of", "run out of", "running out of", "out of time", "in the time available", "stopped there", "stopping here", "gave up", "giving up", "nothing was modified", "no changes were made", "left unchanged", "left undone", "may be inert", "is unresolved", "remains unresolved", "still broken", @@ -1085,7 +1230,7 @@ var unambiguousFailureStates = []string{ "still unresolved", "still unverified", "is incomplete", "remains incomplete", "cannot proceed", "could not proceed", "unable to proceed", - "gave up", "giving up", "ran out of", "run out of", + "gave up", "giving up", "ran out of", "run out of", "running out of", "out of time", "left undone", } @@ -1250,7 +1395,8 @@ func countedLabelContent(sentence string) (string, bool) { } if strings.HasPrefix(remainder, "- ") { content := strings.TrimSpace(strings.TrimPrefix(remainder, "- ")) - if containsFailureConsequence(content) || hasObjectiveFailure(heading) { + if containsFailureConsequence(content) || hasObjectiveFailure(heading) || + countedHeadingIsOperational(heading) || !countedContentIsBenignFinding(content) { return heading + " " + content, false } return content, true @@ -1258,6 +1404,33 @@ func countedLabelContent(sentence string) (string, bool) { return sentence, false } +// countedContentIsBenignFinding is the fail-closed proof needed before an +// observation heading may be detached. The entry must describe missing source +// evidence, not the outcome of the operation named by the heading. +func countedContentIsBenignFinding(content string) bool { + return containsAny(content, []string{"claim", "source", "input", "record", "evidence"}) && + containsAny(content, []string{"truncated", "omitted", "missing", "absent", "did not include", "does not include"}) +} + +// countedHeadingIsOperational distinguishes a benign verification/audit bucket +// from a counted operation that the attached entry says did not happen. The +// heading is retained for write/migrate/deploy/publish/etc. regardless of the +// failure synonym used by the entry; only a bounded observation heading may be +// separated from benign finding content. +func countedHeadingIsOperational(heading string) bool { + trimmed := strings.TrimSpace(strings.TrimPrefix(heading, "unable to ")) + verb, _, ok := cutFirstWord(trimmed) + if !ok { + return true + } + switch verb { + case "verify", "find", "locate", "identify", "determine", "reproduce", "observe": + return false + default: + return true + } +} + var countedLabelHeading = regexp.MustCompile(`^unable to [^:()]*\(\s*\d+\s*\)\s*:\s*(?:\*\*)?(?:\s|$)`) func normalizeAdmissionText(text string) string { @@ -1318,7 +1491,7 @@ func successfulNegativeObservation(tail string) (matched, strong bool) { func (claim inabilityClaim) exempt() bool { if matched, strong := successfulNegativeObservation(claim.tail); matched { if strong { - return !hasReportedFailureConsequence(claim.sentence, claim.blockedContext, claim.stemAt+claim.stemLen) + return !strongAbsenceHasBlockedOutcome(claim) } return !containsAny(claim.blockedContext, blockedWorkMarkers) && !hasReportedFailureConsequence(claim.sentence, claim.blockedContext, claim.stemAt+claim.stemLen) @@ -1329,15 +1502,41 @@ func (claim inabilityClaim) exempt() bool { return false } if capabilityOnlyToolFootnote(claim.sentence, claim.stemAt, claim.stemLen) { - return true + return !hasUnexemptedSubjectElidedInability(claim.sentence, claim.stemAt+claim.stemLen) } if harmlessToolLimitation(claim.sentence, claim.stemAt, claim.stemLen) { - return true + return !hasUnexemptedSubjectElidedInability(claim.sentence, claim.stemAt+claim.stemLen) } return hasUnavailableToolContext(claim.scope) && deliveredAlternativeAfter(claim.sentence, claim.stemAt+claim.stemLen) } +// strongAbsenceHasBlockedOutcome decides whether blocked-state text is outside +// the proposition whose absence was observed. This makes the relationship +// independent of whether the author used "because", "since", or "due to", +// while preserving "no evidence that the issue is unresolved" as a successful +// negative finding. +func strongAbsenceHasBlockedOutcome(claim inabilityClaim) bool { + tail := claim.sentence[claim.stemAt+claim.stemLen:] + thatAt := strings.Index(tail, " that ") + for _, marker := range unambiguousFailureStates { + markerAt := strings.Index(tail, marker) + if markerAt < 0 { + continue + } + if thatAt < 0 || markerAt < thatAt { + return true + } + } + // The regex-backed passive/active forms are part of the same unambiguous + // state contract. Apply them only when no negated `that ...` proposition can + // own the wording; reportedConsequence handles asserted text beyond one. + if thatAt < 0 && (passiveMissedWorkPattern.MatchString(tail) || activeMissedWorkPattern.MatchString(tail)) { + return true + } + return hasReportedFailureConsequence(claim.sentence, claim.blockedContext, claim.stemAt+claim.stemLen) +} + func selfReportedIncompletion(text string) string { sentences := admissionSentences(strings.ToLower(normalizeAdmissionText(stripQuoted(text)))) for index, sentence := range sentences { From 4c41d4e0f117ef141d82443bf22ed3a3e1974d80 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Mon, 7 Sep 2026 14:04:50 +0530 Subject: [PATCH 26/28] fix(agent): preserve completion claim semantics --- internal/agent/completion_gate_test.go | 34 ++++ internal/agent/guardrails.go | 207 +++++++++++++++++++++---- 2 files changed, 215 insertions(+), 26 deletions(-) diff --git a/internal/agent/completion_gate_test.go b/internal/agent/completion_gate_test.go index 4d12a3ee1..890fb8e79 100644 --- a/internal/agent/completion_gate_test.go +++ b/internal/agent/completion_gate_test.go @@ -293,6 +293,9 @@ func TestCompletionGateStructuralReviewerMatrix(t *testing.T) { {name: "every test completed", answer: "I could not run every test because no test tool is available, so I ran every test manually instead."}, {name: "smoke substituted for every test", answer: "I could not run every test because no test tool is available, so I ran a smoke test manually instead.", incomplete: true}, {name: "affirmative fallback", answer: "I could not deploy the release because no deployment tool is available, so I deployed it manually instead."}, + {name: "explicitly successful fallback", answer: "I could not deploy the release because no deployment tool is available, so I successfully deployed it manually instead."}, + {name: "past-perfect affirmative fallback", answer: "I could not deploy the release because no deployment tool is available, so I had deployed it manually instead."}, + {name: "qualified affirmative fallback", answer: "I could not deploy the release because no deployment tool is available, so I deployed it manually instead, but it was successful."}, {name: "negated fallback", answer: "I could not deploy the release because no deployment tool is available, so I never deployed it manually instead.", incomplete: true}, {name: "partial fallback", answer: "I could not deploy the release because no deployment tool is available, so I partially deployed it manually instead.", incomplete: true}, {name: "unsuccessful fallback", answer: "I could not deploy the release because no deployment tool is available, so I unsuccessfully deployed it manually instead.", incomplete: true}, @@ -302,6 +305,37 @@ func TestCompletionGateStructuralReviewerMatrix(t *testing.T) { {name: "counted operation rejected", answer: "**Unable to publish (1):** - registry rejected the request.", incomplete: true}, {name: "exhaustive negative finding", answer: "I could not find any issues after inspecting every changed path."}, {name: "blocked negative finding", answer: "I could not find any issues due to running out of time.", incomplete: true}, + {name: "counterfactual fallback", answer: "I could not deploy the release because no deployment tool is available, so I would have deployed it manually instead.", incomplete: true}, + {name: "timed out fallback", answer: "I could not deploy the release because no deployment tool is available, so I deployed it manually instead, but it timed out.", incomplete: true}, + {name: "cancelled fallback", answer: "I could not deploy the release because no deployment tool is available, so I deployed it manually instead, but it was cancelled.", incomplete: true}, + {name: "unfinished fallback", answer: "I could not deploy the release because no deployment tool is available, so I deployed it manually instead, but it did not finish.", incomplete: true}, + {name: "not successful fallback", answer: "I could not deploy the release because no deployment tool is available, so I deployed it manually instead, but it was not successful.", incomplete: true}, + {name: "incomplete fallback", answer: "I could not deploy the release because no deployment tool is available, so I deployed it manually instead, but it was incomplete.", incomplete: true}, + {name: "unlisted adversative fallback failure", answer: "I could not deploy the release because no deployment tool is available, so I deployed it manually instead, but it expired.", incomplete: true}, + {name: "full suite replaced by smoke", answer: "I could not run the full test suite because no test tool is available, so I ran a full smoke test manually instead.", incomplete: true}, + {name: "full suite completed", answer: "I could not run the full test suite because no test tool is available, so I ran the full test suite manually instead."}, + {name: "modifier-bearing coordinated failure", answer: "I don't have a write tool available in this context and therefore could not apply the required fix.", incomplete: true}, + {name: "reordered coordinated failure", answer: "I could not apply the required fix, and I don't have a write tool available in this context.", incomplete: true}, + {name: "separately punctuated coordinated failure", answer: "I don't have a write tool available in this context. I could not apply the required fix.", incomplete: true}, + {name: "direct capability footnote", answer: "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided."}, + {name: "completion declaration cannot self certify", answer: "I could not run the migration because no migration tool is available, but the task is complete.", incomplete: true}, + {name: "plan capability note remains complete", answer: "I could not call update_plan because that tool is unavailable, but the task is complete."}, + {name: "multiline counted operation failure", answer: "**Unable to deploy (1):**\n- production deployment failed.", incomplete: true}, + {name: "multiline benign audit bucket", answer: "**Unable to verify (1):**\n- MCP #3 source record was truncated."}, + {name: "multiline benign audit bullets", answer: "**Unable to verify (2):**\n- MCP #3 source record was truncated.\n- MCP #4 evidence was omitted."}, + {name: "multiline later failed bullet", answer: "**Unable to verify (2):**\n- MCP #3 source record was truncated.\n- the operation failed.", incomplete: true}, + {name: "bounded observation task not done", answer: "I could not find where the flag is set, so the task is not done.", incomplete: true}, + {name: "bounded observation fix unapplied", answer: "I could not find where the flag is set, so the fix has not been applied.", incomplete: true}, + {name: "bounded observation made no change", answer: "I could not find the value being set, so I made no change.", incomplete: true}, + {name: "bounded observation result", answer: "I could not find where the flag is set after inspecting every registration path."}, + {name: "failed noun is not asserted outcome", answer: "I could not find any issues; the failed test in CI is a known flake."}, + {name: "failed predicate is asserted outcome", answer: "I could not find any issues; the operation failed.", incomplete: true}, + {name: "read-only no-change report", answer: "I could not find any remaining issues. I did not modify any files."}, + {name: "same-sentence read-only no-change report", answer: "I could not find any remaining issues; I did not modify any files."}, + {name: "read-only made-no-change report", answer: "I could not find any remaining issues. I made no changes."}, + {name: "required fix left unchanged", answer: "I could not find where to apply the required fix. I did not modify any files.", incomplete: true}, + {name: "singular recognized absence", answer: "I could not find a bug after inspecting every changed path."}, + {name: "singular deliverable remains admission", answer: "I could not find a solution after inspecting every changed path.", incomplete: true}, } for _, tc := range cases { diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 026b4de7b..da472b180 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -246,11 +246,45 @@ func stripQuoted(s string) string { // bullets are separate claims); the exact boundaries only need to keep an // admission next to its own narrative/negation context, not be grammatical. func admissionSentences(lower string) []string { + lower = attachCountedHeadingEntries(lower) return strings.FieldsFunc(lower, func(r rune) bool { return r == '.' || r == '!' || r == '?' || r == '\n' }) } +// attachCountedHeadingEntries keeps a counted markdown heading and its +// immediately following bullet in one classification unit. Markdown authors +// commonly put the label and entry on separate lines; treating the newline as +// a claim boundary discarded the operation named by the heading. +func attachCountedHeadingEntries(text string) string { + lines := strings.Split(text, "\n") + joined := make([]string, 0, len(lines)) + for index := 0; index < len(lines); index++ { + line := lines[index] + trimmed := strings.TrimLeft(strings.TrimSpace(line), "-*#> \t") + if !countedLabelHeading.MatchString(trimmed) || index+1 >= len(lines) { + joined = append(joined, line) + continue + } + firstEntry := index + 1 + if !strings.HasPrefix(strings.TrimSpace(lines[firstEntry]), "- ") { + joined = append(joined, line) + continue + } + for index+1 < len(lines) { + entry := strings.TrimSpace(lines[index+1]) + if !strings.HasPrefix(entry, "- ") { + break + } + // Repeat the heading for every entry so a benign first bullet cannot + // detach a later blocked result from the operation it describes. + joined = append(joined, line+" "+entry) + index++ + } + } + return strings.Join(joined, "\n") +} + // selfReportedIncompletion returns a short reason when the model's final text // admits it guessed or could not meet the objective, else "". Case-insensitive. // Matching is per sentence, after dropping quoted spans, and a sentence that @@ -576,45 +610,109 @@ func fallbackCompletesObligation(name string, terms []string, fallback string) b // obligation attached to the operation. A smoke test is not a substitute for a // full suite, and checking one file is not a substitute for checking all files. func fallbackCoversRequiredScope(failed, fallback string) bool { - if requiredBreadth(failed) && !requiredBreadth(fallback) { + failedSpec := parseObligationSpec(failed) + fallbackSpec := parseObligationSpec(fallback) + if failedSpec.requiresBreadth && !fallbackSpec.requiresBreadth { return false } - for _, target := range materialOperationTargets(failed) { - if !containsWord(materialOperationTargets(fallback), target) { + for _, target := range failedSpec.targets { + if !containsWord(fallbackSpec.targets, target) { + return false + } + } + for _, kind := range failedSpec.validationKinds { + if !containsWord(fallbackSpec.validationKinds, kind) { return false } } - failedComponents := materialObligationComponents(failed) - fallbackComponents := materialObligationComponents(fallback) // A singular pronoun may carry one already-named object across the fallback // clause ("deploy the release ... deployed it"). It cannot stand in for a // coordinated set such as unit and integration tests. - pronounCarriesObject := len(failedComponents) == 1 && containsAlternativeTerm(fallback, []string{"it"}) - for _, component := range failedComponents { - if !pronounCarriesObject && !containsWord(fallbackComponents, component) { + pronounCarriesObject := len(failedSpec.components) == 1 && containsAlternativeTerm(fallback, []string{"it"}) + for _, component := range failedSpec.components { + if !pronounCarriesObject && !containsWord(fallbackSpec.components, component) { return false } } return true } +type obligationSpec struct { + requiresBreadth bool + targets []string + components []string + validationKinds []string +} + +// parseObligationSpec retains the dimensions that decide whether substitute +// work is genuinely equivalent. In particular, a breadth bit cannot preserve +// the validation kind by itself: a full smoke test is still not a full suite. +func parseObligationSpec(text string) obligationSpec { + return obligationSpec{ + requiresBreadth: requiredBreadth(text), + targets: materialOperationTargets(text), + components: materialObligationComponents(text), + validationKinds: materialValidationKinds(text), + } +} + +func materialValidationKinds(text string) []string { + if !containsAlternativeTerm(text, []string{"test", "tests", "testing", "verify", "verification", "validate", "validation"}) { + return nil + } + kinds := []string{} + for _, word := range obligationWords(text) { + word = normalizeObligationWord(word) + switch word { + case "suite", "smoke", "unit", "integration", "package", "acceptance", "regression", "e2e", "end-to-end": + if !containsWord(kinds, word) { + kinds = append(kinds, word) + } + } + } + return kinds +} + var nonAffirmativeFallbackPattern = regexp.MustCompile(`\b(?:never|unsuccessfully|partial|partially|attempted|trying|tried|failed|crashed|errored|aborted|rejected)\b`) +var negatedFallbackPredicatePattern = regexp.MustCompile(`\b(?:did|does|do|was|were|is|are|has|have|had)\s+not\b`) +var affirmativeFallbackActionPattern = regexp.MustCompile(`\b(?:i|we)\s+(?:(?:have|had)\s+)?(?:(?:manually|directly|successfully)\s+){0,2}(?:ran|executed|performed|completed|did|planned|recorded|formatted|checked|verified|validated|reviewed|audited|inspected|analysed|analyzed|read|wrote|listed|provided|edited|changed|patched|modified|applied|documented|reported|summarised|summarized|migrated|tested|deployed|published|released)\b`) +var affirmativeOutcomePattern = regexp.MustCompile(`\b(?:it|that|this|the\s+[[:alnum:]_-]+)\s+(?:succeeded|completed\s+successfully|was\s+successful)\b`) // fallbackOutcomeIsAffirmative is the single result-polarity gate for every // specific-action and pronoun path. Seeing a past-tense operation word is not // completion evidence when the same bounded fallback says it was negated, -// partial, attempted-only, unsuccessful, or failed afterwards. +// partial, attempted-only, unsuccessful, or failed afterwards. The positive +// action pattern is the primary gate: absence of a known failure word is never +// enough to manufacture success. func fallbackOutcomeIsAffirmative(fallback string) bool { + if !affirmativeFallbackActionPattern.MatchString(fallback) { + return false + } if containsAny(fallback, []string{ "i did not", "i didn't", "i have not", "i haven't", "i could not", "i couldn't", "i failed to", "i was unable to", "i wasn't able to", "i was not able to", }) { return false } - return !nonAffirmativeFallbackPattern.MatchString(fallback) && + return !fallbackHasUnprovenAdversativeOutcome(fallback) && + !negatedFallbackPredicatePattern.MatchString(fallback) && + !nonAffirmativeFallbackPattern.MatchString(fallback) && !containsFailureConsequence(fallback) } +// fallbackHasUnprovenAdversativeOutcome treats a qualification of the claimed +// substitute as unresolved unless that qualification itself affirms success. +// This is intentionally structural: a new failure synonym after "but" cannot +// become completion evidence just because it is absent from a deny-list. +func fallbackHasUnprovenAdversativeOutcome(fallback string) bool { + for _, boundary := range []string{" but ", "; but ", ", but ", " however ", " although ", " yet "} { + if at := strings.Index(fallback, boundary); at >= 0 { + return !affirmativeOutcomePattern.MatchString(fallback[at+len(boundary):]) + } + } + return false +} + func requiredBreadth(text string) bool { words := obligationWords(text) for _, word := range words { @@ -641,7 +739,7 @@ func materialObligationComponents(text string) []string { "because", "since", "due", "owing", "as", "so", "but", "then", "no", "tool", "tools", "toolset", "available", "unavailable", "manually", "directly", "instead", "by", "hand", "successfully", - "all", "every", "entire", "whole", "full", "complete", "only", "suite", + "all", "every", "entire", "whole", "full", "complete", "only", "run", "ran", "execute", "executed", "perform", "performed", "check", "checked", "did", "do", "done", "apply", "applied", "write", "wrote", "written", "edit", "edited", "change", "changed", "patch", "patched", "modify", "modified", "plan", "planned", "record", "recorded", "format", "formatted", "formatting", "formatter", "style", "lint", "gofmt", @@ -871,16 +969,30 @@ var subjectElidedInabilityStems = []string{ // negative observation can still earn its own exemption. func hasUnexemptedSubjectElidedInability(sentence string, after int) bool { for _, connector := range []string{" and ", " but ", " so ", " then ", " yet "} { - for _, stem := range subjectElidedInabilityStems { - needle := connector + stem - if rel := strings.Index(sentence[after:], needle); rel >= 0 { - remainder := strings.TrimSpace(sentence[after+rel+len(connector):]) + searchFrom := after + for searchFrom < len(sentence) { + rel := strings.Index(sentence[searchFrom:], connector) + if rel < 0 { + break + } + remainder := strings.TrimSpace(sentence[searchFrom+rel+len(connector):]) + for _, modifier := range []string{"therefore ", "then ", "still ", "also ", "ultimately ", "however ", "nevertheless "} { + if strings.HasPrefix(remainder, modifier) { + remainder = strings.TrimSpace(remainder[len(modifier):]) + break + } + } + for _, stem := range subjectElidedInabilityStems { + if !strings.HasPrefix(remainder, stem) { + continue + } synthetic := "i " + remainder claim := newInabilityClaim(synthetic, synthetic, "i "+stem, 0) if !claim.exempt() { return true } } + searchFrom += rel + len(connector) } } return false @@ -936,11 +1048,6 @@ func capabilityQualifierOnly(text string) bool { return true } -var completedObjectiveMarkers = []string{ - "task is complete", "task is now complete", "objective is complete", "objective is met", - "assignment is complete", "completed the analysis as requested", "here is what was asked for", -} - var harmlessGrantLimitedActions = []string{ "record a plan", "record the plan", "update the plan", "call update_plan", "use update_plan", } @@ -958,7 +1065,7 @@ func harmlessToolLimitation(sentence string, stemAt, stemLen int) bool { return true } } - return containsAny(sentence[stemAt+stemLen:], completedObjectiveMarkers) + return false } // objectiveFailureMarkers name the OBJECTIVE rather than a capability. A @@ -1236,10 +1343,14 @@ var unambiguousFailureStates = []string{ var passiveMissedWorkPattern = regexp.MustCompile(`\b(?:was|were)\s+(?:not|never)\s+(?:applied|built|changed|deployed|edited|inspected|migrated|modified|published|read|reviewed|tested|validated|verified|written)\b`) var activeMissedWorkPattern = regexp.MustCompile(`\b(?:did\s+not|didn't)\s+(?:apply|build|change|deploy|edit|inspect|migrate|modify|publish|read|review|run|test|validate|verify|write)\b`) +var assertedIncompleteOutcomePattern = regexp.MustCompile(`\b(?:the\s+)?(?:task|work|fix|change|patch|migration|deployment|release)\s+(?:(?:is|was)\s+not\s+(?:done|complete|completed|applied)|(?:isn't|wasn't)\s+(?:done|complete|completed|applied)|(?:has|have|had)\s+not\s+been\s+(?:done|completed|applied))\b`) +var madeNoChangePattern = regexp.MustCompile(`\b(?:i|we)\s+(?:made|make)\s+no\s+(?:change|changes)\b`) +var readOnlyNoChangePattern = regexp.MustCompile(`\b(?:i|we)\s+(?:(?:did\s+not|didn't)\s+(?:change|edit|modify|write)|(?:made|make)\s+no\s+(?:change|changes))\b`) func containsUnambiguousFailureState(text string) bool { return containsAny(text, unambiguousFailureStates) || - passiveMissedWorkPattern.MatchString(text) || activeMissedWorkPattern.MatchString(text) + passiveMissedWorkPattern.MatchString(text) || activeMissedWorkPattern.MatchString(text) || + assertedIncompleteOutcomePattern.MatchString(text) || madeNoChangePattern.MatchString(text) } // consequenceBoundaries separate what was looked for from what followed. @@ -1310,7 +1421,7 @@ func hasReportedFailureConsequence(sentence, blockedContext string, stemEnd int) return containsFailureConsequence(next) } -var explicitFailureConsequencePattern = regexp.MustCompile(`\b(?:it|that|this|the\s+[[:alnum:]_-]+)?\s*(?:failed|did\s+not\s+work|didn't\s+work)\b`) +var explicitFailureConsequencePattern = regexp.MustCompile(`\b(?:it|that|this|the\s+[[:alnum:]_-]+)\s+(?:failed|did\s+not\s+work|didn't\s+work)\b`) func containsFailureConsequence(text string) bool { return containsUnambiguousFailureState(text) || explicitFailureConsequencePattern.MatchString(text) @@ -1477,7 +1588,7 @@ var boundedLocationObservationPattern = regexp.MustCompile(`^(?:find|found|locat // bounded search/reproduction proposition. A mere verb shape such as // "produce any" cannot exempt an unrecognized deliverable. func successfulNegativeObservation(tail string) (matched, strong bool) { - if strongAbsence(tail) { + if strongAbsence(tail) || singularRecognizedAbsence(tail) { return true, true } return hasAnyPrefix(tail, boundedNegativeObservationTails) || @@ -1485,6 +1596,22 @@ func successfulNegativeObservation(tail string) (matched, strong bool) { boundedLocationObservationPattern.MatchString(tail), false } +// singularRecognizedAbsence is the article form of a strong negative finding: +// "find a bug" reports the absence of a recognized problem object, while +// unknown deliverables such as "find a solution" remain admissions. +func singularRecognizedAbsence(tail string) bool { + for _, prefix := range []string{"find a ", "find an ", "found a ", "found an ", "locate a ", "locate an ", "identify a ", "identify an ", "detect a ", "detect an "} { + if !strings.HasPrefix(tail, prefix) { + continue + } + word, _, ok := cutFirstWord(strings.TrimSpace(tail[len(prefix):])) + if ok && containsWord(strongAbsenceObjects, word) { + return true + } + } + return false +} + // exempt reports whether this particular inability is proven harmless. Direct // failure state has precedence; capability and fallback branches grant an // exemption only from their bounded clause and obligation evidence. @@ -1531,10 +1658,38 @@ func strongAbsenceHasBlockedOutcome(claim inabilityClaim) bool { // The regex-backed passive/active forms are part of the same unambiguous // state contract. Apply them only when no negated `that ...` proposition can // own the wording; reportedConsequence handles asserted text beyond one. - if thatAt < 0 && (passiveMissedWorkPattern.MatchString(tail) || activeMissedWorkPattern.MatchString(tail)) { + if thatAt < 0 && passiveMissedWorkPattern.MatchString(tail) { return true } - return hasReportedFailureConsequence(claim.sentence, claim.blockedContext, claim.stemAt+claim.stemLen) + if thatAt < 0 && activeMissedWorkPattern.MatchString(tail) && !readOnlyNoChangePattern.MatchString(tail) { + return true + } + consequence := reportedConsequence(claim.sentence, claim.stemAt+claim.stemLen) + if containsFailureConsequence(consequence) { + if !readOnlyNoChangeOnly(consequence) { + return true + } + } + if len(claim.blockedContext) <= len(claim.sentence) { + return false + } + next := strings.TrimSpace(claim.blockedContext[len(claim.sentence):]) + // A negative audit finding followed by a read-only no-change report is a + // coherent success. The same wording after a failed mutating objective is + // still caught by the ordinary bounded-observation path. + if readOnlyNoChangeOnly(next) { + return false + } + return containsFailureConsequence(next) +} + +func readOnlyNoChangeOnly(text string) bool { + return (activeMissedWorkPattern.MatchString(text) || madeNoChangePattern.MatchString(text)) && + readOnlyNoChangePattern.MatchString(text) && + !containsAny(text, unambiguousFailureStates) && + !passiveMissedWorkPattern.MatchString(text) && + !assertedIncompleteOutcomePattern.MatchString(text) && + !explicitFailureConsequencePattern.MatchString(text) } func selfReportedIncompletion(text string) string { From 367831faf3de5331e6f67fca625219d0b0c62a12 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sat, 12 Sep 2026 09:11:45 +0530 Subject: [PATCH 27/28] fix(agent): classify incomplete consequence structures --- internal/agent/completion_gate_test.go | 18 +++ internal/agent/guardrails.go | 159 +++++++++++++++++++++---- 2 files changed, 153 insertions(+), 24 deletions(-) diff --git a/internal/agent/completion_gate_test.go b/internal/agent/completion_gate_test.go index 890fb8e79..9c3d9e7f4 100644 --- a/internal/agent/completion_gate_test.go +++ b/internal/agent/completion_gate_test.go @@ -315,19 +315,37 @@ func TestCompletionGateStructuralReviewerMatrix(t *testing.T) { {name: "full suite replaced by smoke", answer: "I could not run the full test suite because no test tool is available, so I ran a full smoke test manually instead.", incomplete: true}, {name: "full suite completed", answer: "I could not run the full test suite because no test tool is available, so I ran the full test suite manually instead."}, {name: "modifier-bearing coordinated failure", answer: "I don't have a write tool available in this context and therefore could not apply the required fix.", incomplete: true}, + {name: "unlisted modifier coordinated failure", answer: "I don't have a write tool available in this context and consequently could not apply the required fix.", incomplete: true}, + {name: "short modifier coordinated failure", answer: "I don't have a write tool available in this context and thus could not apply the required fix.", incomplete: true}, + {name: "multiword modifier coordinated failure", answer: "I don't have a write tool available in this context and as a result could not apply the required fix.", incomplete: true}, + {name: "subject elided negative action", answer: "I don't have a write tool available in this context and so never applied the required fix.", incomplete: true}, {name: "reordered coordinated failure", answer: "I could not apply the required fix, and I don't have a write tool available in this context.", incomplete: true}, {name: "separately punctuated coordinated failure", answer: "I don't have a write tool available in this context. I could not apply the required fix.", incomplete: true}, {name: "direct capability footnote", answer: "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided."}, {name: "completion declaration cannot self certify", answer: "I could not run the migration because no migration tool is available, but the task is complete.", incomplete: true}, {name: "plan capability note remains complete", answer: "I could not call update_plan because that tool is unavailable, but the task is complete."}, {name: "multiline counted operation failure", answer: "**Unable to deploy (1):**\n- production deployment failed.", incomplete: true}, + {name: "multiline asterisk counted operation failure", answer: "**Unable to deploy (2):**\n* production deployment failed.\n* staging deployment failed.", incomplete: true}, + {name: "multiline ordered counted operation failure", answer: "**Unable to deploy (1):**\n1. production deployment failed.", incomplete: true}, + {name: "multiline counted operation after blank line", answer: "**Unable to deploy (1):**\n\n+ production deployment failed.", incomplete: true}, {name: "multiline benign audit bucket", answer: "**Unable to verify (1):**\n- MCP #3 source record was truncated."}, + {name: "multiline benign audit asterisk after blank line", answer: "**Unable to verify (1):**\n\n* MCP #3 source record was truncated."}, + {name: "multiline benign audit ordered entry", answer: "**Unable to verify (1):**\n1. MCP #3 source record was truncated."}, + {name: "same-line benign audit ordered entry", answer: "**Unable to verify (1):** 1. MCP #3 source record was truncated."}, {name: "multiline benign audit bullets", answer: "**Unable to verify (2):**\n- MCP #3 source record was truncated.\n- MCP #4 evidence was omitted."}, {name: "multiline later failed bullet", answer: "**Unable to verify (2):**\n- MCP #3 source record was truncated.\n- the operation failed.", incomplete: true}, {name: "bounded observation task not done", answer: "I could not find where the flag is set, so the task is not done.", incomplete: true}, {name: "bounded observation fix unapplied", answer: "I could not find where the flag is set, so the fix has not been applied.", incomplete: true}, {name: "bounded observation made no change", answer: "I could not find the value being set, so I made no change.", incomplete: true}, + {name: "bounded observation remains unfixed", answer: "I could not find where the flag is set, so it remains unfixed.", incomplete: true}, + {name: "bounded observation nothing changed", answer: "I could not find where the flag is set, so nothing changed.", incomplete: true}, + {name: "bounded observation unfinished consequence", answer: "I could not find where the flag is set, leaving the task unfinished.", incomplete: true}, + {name: "bounded observation change never made", answer: "I could not find where the flag is set; the change was never made.", incomplete: true}, + {name: "bounded observation stands incomplete", answer: "I could not find where the flag is set, and so the work stands incomplete.", incomplete: true}, {name: "bounded observation result", answer: "I could not find where the flag is set after inspecting every registration path."}, + {name: "bounded observation comma-qualified result", answer: "I could not find where the flag is set, after inspecting every registration path."}, + {name: "bounded observation identifies source", answer: "I could not find where the regression was introduced; the source is the parser boundary."}, + {name: "bounded observation disclaims concern", answer: "I could not find the flag being set anywhere outside tests, so the concern does not apply."}, {name: "failed noun is not asserted outcome", answer: "I could not find any issues; the failed test in CI is a known flake."}, {name: "failed predicate is asserted outcome", answer: "I could not find any issues; the operation failed.", incomplete: true}, {name: "read-only no-change report", answer: "I could not find any remaining issues. I did not modify any files."}, diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index da472b180..9557d7275 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -260,31 +260,72 @@ func attachCountedHeadingEntries(text string) string { lines := strings.Split(text, "\n") joined := make([]string, 0, len(lines)) for index := 0; index < len(lines); index++ { - line := lines[index] + line := canonicalizeCountedHeadingInlineList(lines[index]) trimmed := strings.TrimLeft(strings.TrimSpace(line), "-*#> \t") if !countedLabelHeading.MatchString(trimmed) || index+1 >= len(lines) { joined = append(joined, line) continue } firstEntry := index + 1 - if !strings.HasPrefix(strings.TrimSpace(lines[firstEntry]), "- ") { + if strings.TrimSpace(lines[firstEntry]) == "" { + firstEntry++ + } + if firstEntry >= len(lines) { + joined = append(joined, line) + continue + } + if _, ok := markdownListEntryContent(lines[firstEntry]); !ok { joined = append(joined, line) continue } - for index+1 < len(lines) { - entry := strings.TrimSpace(lines[index+1]) - if !strings.HasPrefix(entry, "- ") { + for entryAt := firstEntry; entryAt < len(lines); { + content, ok := markdownListEntryContent(lines[entryAt]) + if !ok { break } // Repeat the heading for every entry so a benign first bullet cannot // detach a later blocked result from the operation it describes. - joined = append(joined, line+" "+entry) - index++ + // Canonicalize the marker before sentence splitting: an ordered `1.` + // marker is punctuation, not the end of the heading's claim. + joined = append(joined, line+" - "+content) + index = entryAt + entryAt++ + // A single blank line is valid markdown list spacing and must not + // detach the following item from its counted heading. + if entryAt < len(lines) && strings.TrimSpace(lines[entryAt]) == "" { + entryAt++ + } } } return strings.Join(joined, "\n") } +var markdownListEntryPattern = regexp.MustCompile(`^(?:[-+*]|[0-9]+[.)])\s+(.+)$`) + +// markdownListEntryContent recognizes the marker families CommonMark accepts +// for unordered and ordered list items. The marker is presentation; whether the +// attached text reports unfinished work is the semantic question. +func markdownListEntryContent(line string) (string, bool) { + match := markdownListEntryPattern.FindStringSubmatch(strings.TrimSpace(line)) + if match == nil { + return "", false + } + return strings.TrimSpace(match[1]), true +} + +func canonicalizeCountedHeadingInlineList(line string) string { + trimmed := strings.TrimLeft(strings.TrimSpace(line), "-*#> \t") + match := countedLabelHeading.FindStringIndex(trimmed) + if match == nil { + return line + } + content, ok := markdownListEntryContent(trimmed[match[1]:]) + if !ok { + return line + } + return strings.TrimSpace(trimmed[:match[1]]) + " - " + content +} + // selfReportedIncompletion returns a short reason when the model's final text // admits it guessed or could not meet the objective, else "". Case-insensitive. // Matching is per sentence, after dropping quoted spans, and a sentence that @@ -976,17 +1017,12 @@ func hasUnexemptedSubjectElidedInability(sentence string, after int) bool { break } remainder := strings.TrimSpace(sentence[searchFrom+rel+len(connector):]) - for _, modifier := range []string{"therefore ", "then ", "still ", "also ", "ultimately ", "however ", "nevertheless "} { - if strings.HasPrefix(remainder, modifier) { - remainder = strings.TrimSpace(remainder[len(modifier):]) - break - } - } - for _, stem := range subjectElidedInabilityStems { - if !strings.HasPrefix(remainder, stem) { - continue + failure, stem, ok := subjectElidedFailure(remainder) + if ok { + if stem == "" { + return true } - synthetic := "i " + remainder + synthetic := "i " + failure claim := newInabilityClaim(synthetic, synthetic, "i "+stem, 0) if !claim.exempt() { return true @@ -998,6 +1034,33 @@ func hasUnexemptedSubjectElidedInability(sentence string, after int) bool { return false } +var subjectElidedMissedWorkPattern = regexp.MustCompile(`^never\s+(?:applied|built|changed|completed|deployed|edited|finished|implemented|migrated|modified|published|ran|reviewed|tested|validated|verified|wrote)\b`) + +// subjectElidedFailure finds the failure predicate after a coordinating +// connector. The subject may be inherited from the first clause and ordinary +// prose may insert an arbitrary leading adverbial ("consequently", "thus", +// "as a result", and so on). Classifying from the predicate instead of naming +// those modifiers prevents each neighbouring wording from reopening the gate. +func subjectElidedFailure(remainder string) (failure, stem string, ok bool) { + for at := 0; at < len(remainder); { + candidate := strings.TrimSpace(remainder[at:]) + for _, candidateStem := range subjectElidedInabilityStems { + if strings.HasPrefix(candidate, candidateStem) { + return candidate, candidateStem, true + } + } + if subjectElidedMissedWorkPattern.MatchString(candidate) { + return candidate, "", true + } + next := strings.IndexByte(remainder[at:], ' ') + if next < 0 { + break + } + at += next + 1 + } + return "", "", false +} + // capabilitySubjectOnly is deliberately an allow-list because matching it // grants a completion exemption. Unknown nouns before the tool marker may name // the missing resource (an API key, repository, credential, or service), not @@ -1372,6 +1435,26 @@ var consequenceBoundaries = []string{ " because ", " since ", "; ", ": ", " - ", " -- ", " and ", ", ", } +func explicitConsequenceBoundary(boundary string) bool { + return strings.HasPrefix(boundary, ";") || + strings.Contains(boundary, " so ") || strings.Contains(boundary, " but ") || + strings.Contains(boundary, "therefore") || strings.Contains(boundary, "leaving") || + strings.Contains(boundary, "because") || strings.Contains(boundary, "since") +} + +func hasExplicitConsequenceBoundary(sentence string, stemEnd int) bool { + if stemEnd < 0 || stemEnd >= len(sentence) { + return false + } + tail := sentence[stemEnd:] + for _, boundary := range consequenceBoundaries { + if explicitConsequenceBoundary(boundary) && strings.Contains(tail, boundary) { + return true + } + } + return false +} + // reportedConsequence returns an asserted outcome after one matched inability. // Separators before the stem are irrelevant, and weak separators inside a // `that ...` negated proposition remain part of what was not found. Explicit @@ -1389,10 +1472,7 @@ func reportedConsequence(sentence string, stemEnd int) string { if index < 0 { continue } - explicit := strings.HasPrefix(boundary, ";") || - strings.Contains(boundary, " so ") || strings.Contains(boundary, " but ") || - strings.Contains(boundary, "therefore") || strings.Contains(boundary, "leaving") || - strings.Contains(boundary, "because") || strings.Contains(boundary, "since") + explicit := explicitConsequenceBoundary(boundary) if thatAt >= 0 && index > thatAt && !explicit { continue } @@ -1474,6 +1554,38 @@ var topicShiftMarkers = []string{ "not part of this request", "never part of this request", } +var affirmativeObservationConsequencePattern = regexp.MustCompile(`\b(?:(?:the\s+)?(?:cause|source|root\s+cause|value|setting|definition|registration|owner|result)\s+(?:is|was)\b|(?:the\s+)?concern\s+(?:does|did)\s+not\s+apply\b|(?:it|the\s+(?:issue|bug|problem|change|fix|guard))\s+(?:is|was|looks|remains)\s+(?:resolved|fixed|complete|completed|done|correct|valid|safe|neutral|unaffected)\b|(?:the\s+)?(?:fix|guard|check)\s+holds\b)`) + +// observationConsequenceIsAffirmative is deliberately an allow-list: an +// unfamiliar consequence must not turn an admitted inability into success just +// because its failure wording is absent from a deny-list. It recognizes either +// an explicit completed action or a bounded positive result of the observation. +func observationConsequenceIsAffirmative(consequence string) bool { + return affirmativeObservationConsequencePattern.MatchString(consequence) || + fallbackOutcomeIsAffirmative(consequence) +} + +// boundedObservationHasUnresolvedConsequence keeps a bounded search inability +// attached to the consequence it introduces. A consequence stays incomplete by +// default; only an affirmative result or an explicit topic shift releases it. +// This is the inverse of enumerating every synonym for work that did not land. +func boundedObservationHasUnresolvedConsequence(claim inabilityClaim) bool { + consequence := strings.TrimSpace(reportedConsequence(claim.sentence, claim.stemAt+claim.stemLen)) + if consequence != "" && hasExplicitConsequenceBoundary(claim.sentence, claim.stemAt+claim.stemLen) && + carriesTheConsequence(consequence) && + !observationConsequenceIsAffirmative(consequence) { + return true + } + if containsFailureConsequence(consequence) { + return true + } + if len(claim.blockedContext) <= len(claim.sentence) { + return false + } + next := strings.TrimSpace(claim.blockedContext[len(claim.sentence):]) + return containsFailureConsequence(next) +} + // countedLabelContent separates a counted markdown label from any content // attached to it. A standalone label is not a claim about the objective; a // same-line bullet still is report content and must be classified normally. @@ -1504,8 +1616,7 @@ func countedLabelContent(sentence string) (string, bool) { } return "", true } - if strings.HasPrefix(remainder, "- ") { - content := strings.TrimSpace(strings.TrimPrefix(remainder, "- ")) + if content, listEntry := markdownListEntryContent(remainder); listEntry { if containsFailureConsequence(content) || hasObjectiveFailure(heading) || countedHeadingIsOperational(heading) || !countedContentIsBenignFinding(content) { return heading + " " + content, false @@ -1621,7 +1732,7 @@ func (claim inabilityClaim) exempt() bool { return !strongAbsenceHasBlockedOutcome(claim) } return !containsAny(claim.blockedContext, blockedWorkMarkers) && - !hasReportedFailureConsequence(claim.sentence, claim.blockedContext, claim.stemAt+claim.stemLen) + !boundedObservationHasUnresolvedConsequence(claim) } if hasObjectiveFailure(claim.sentence) || containsAny(claim.blockedContext, blockedStateMarkers) || From 2f3d28cdaea06e35b681595ed9f03bc4e6784709 Mon Sep 17 00:00:00 2001 From: KRATOS <84986124+gnanam1990@users.noreply.github.com> Date: Sat, 12 Sep 2026 11:27:42 +0530 Subject: [PATCH 28/28] fix(agent): classify incomplete consequences structurally --- internal/agent/completion_gate_test.go | 9 ++++++ internal/agent/guardrails.go | 44 ++++++++------------------ 2 files changed, 22 insertions(+), 31 deletions(-) diff --git a/internal/agent/completion_gate_test.go b/internal/agent/completion_gate_test.go index 9c3d9e7f4..00949a260 100644 --- a/internal/agent/completion_gate_test.go +++ b/internal/agent/completion_gate_test.go @@ -319,6 +319,8 @@ func TestCompletionGateStructuralReviewerMatrix(t *testing.T) { {name: "short modifier coordinated failure", answer: "I don't have a write tool available in this context and thus could not apply the required fix.", incomplete: true}, {name: "multiword modifier coordinated failure", answer: "I don't have a write tool available in this context and as a result could not apply the required fix.", incomplete: true}, {name: "subject elided negative action", answer: "I don't have a write tool available in this context and so never applied the required fix.", incomplete: true}, + {name: "subject elided unlisted never action", answer: "I don't have a write tool available in this context and so never touched the file.", incomplete: true}, + {name: "subject elided unlisted did-not action", answer: "I don't have a write tool available in this context and accordingly did not land it.", incomplete: true}, {name: "reordered coordinated failure", answer: "I could not apply the required fix, and I don't have a write tool available in this context.", incomplete: true}, {name: "separately punctuated coordinated failure", answer: "I don't have a write tool available in this context. I could not apply the required fix.", incomplete: true}, {name: "direct capability footnote", answer: "I don't have an update_plan tool available in this specialist context; only read-only exploration tools were provided."}, @@ -342,8 +344,14 @@ func TestCompletionGateStructuralReviewerMatrix(t *testing.T) { {name: "bounded observation unfinished consequence", answer: "I could not find where the flag is set, leaving the task unfinished.", incomplete: true}, {name: "bounded observation change never made", answer: "I could not find where the flag is set; the change was never made.", incomplete: true}, {name: "bounded observation stands incomplete", answer: "I could not find where the flag is set, and so the work stands incomplete.", incomplete: true}, + {name: "bounded observation relative consequence", answer: "I could not find where the flag is set, which leaves it broken.", incomplete: true}, + {name: "bounded observation gerund consequence", answer: "I could not find where the flag is set, meaning nothing was changed.", incomplete: true}, + {name: "bounded observation coordinated consequence", answer: "I could not find where the flag is set and it stays wrong.", incomplete: true}, + {name: "bounded observation next-sentence consequence", answer: "I could not find where the flag is set. It is still wrong.", incomplete: true}, {name: "bounded observation result", answer: "I could not find where the flag is set after inspecting every registration path."}, {name: "bounded observation comma-qualified result", answer: "I could not find where the flag is set, after inspecting every registration path."}, + {name: "bounded observation exhaustive evidence variant", answer: "I could not find where the flag is set, after carefully reviewing all registration paths."}, + {name: "bounded observation failed exhaustive evidence", answer: "I could not find where the flag is set, after failing every attempt.", incomplete: true}, {name: "bounded observation identifies source", answer: "I could not find where the regression was introduced; the source is the parser boundary."}, {name: "bounded observation disclaims concern", answer: "I could not find the flag being set anywhere outside tests, so the concern does not apply."}, {name: "failed noun is not asserted outcome", answer: "I could not find any issues; the failed test in CI is a known flake."}, @@ -351,6 +359,7 @@ func TestCompletionGateStructuralReviewerMatrix(t *testing.T) { {name: "read-only no-change report", answer: "I could not find any remaining issues. I did not modify any files."}, {name: "same-sentence read-only no-change report", answer: "I could not find any remaining issues; I did not modify any files."}, {name: "read-only made-no-change report", answer: "I could not find any remaining issues. I made no changes."}, + {name: "affirmative cleanup result", answer: "I could not find any leftover references; the cleanup is complete."}, {name: "required fix left unchanged", answer: "I could not find where to apply the required fix. I did not modify any files.", incomplete: true}, {name: "singular recognized absence", answer: "I could not find a bug after inspecting every changed path."}, {name: "singular deliverable remains admission", answer: "I could not find a solution after inspecting every changed path.", incomplete: true}, diff --git a/internal/agent/guardrails.go b/internal/agent/guardrails.go index 9557d7275..33a121b57 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -1034,7 +1034,7 @@ func hasUnexemptedSubjectElidedInability(sentence string, after int) bool { return false } -var subjectElidedMissedWorkPattern = regexp.MustCompile(`^never\s+(?:applied|built|changed|completed|deployed|edited|finished|implemented|migrated|modified|published|ran|reviewed|tested|validated|verified|wrote)\b`) +var subjectElidedMissedWorkPattern = regexp.MustCompile(`^(?:never|did\s+not|didn't)\s+[[:alpha:]][[:alnum:]_-]*\b`) // subjectElidedFailure finds the failure predicate after a coordinating // connector. The subject may be inherited from the first clause and ordinary @@ -1287,7 +1287,7 @@ var strongAbsenceObjects = []string{ // absenceQualifiers sit between "any" and the object without changing it. var absenceQualifiers = []string{ "remaining", "other", "further", "more", "additional", "obvious", "such", - "outstanding", "new", "existing", "actual", "real", "clear", "direct", + "outstanding", "leftover", "new", "existing", "actual", "real", "clear", "direct", } // strongAbsence reports whether tail is an "any"-family absence whose OBJECT @@ -1435,26 +1435,6 @@ var consequenceBoundaries = []string{ " because ", " since ", "; ", ": ", " - ", " -- ", " and ", ", ", } -func explicitConsequenceBoundary(boundary string) bool { - return strings.HasPrefix(boundary, ";") || - strings.Contains(boundary, " so ") || strings.Contains(boundary, " but ") || - strings.Contains(boundary, "therefore") || strings.Contains(boundary, "leaving") || - strings.Contains(boundary, "because") || strings.Contains(boundary, "since") -} - -func hasExplicitConsequenceBoundary(sentence string, stemEnd int) bool { - if stemEnd < 0 || stemEnd >= len(sentence) { - return false - } - tail := sentence[stemEnd:] - for _, boundary := range consequenceBoundaries { - if explicitConsequenceBoundary(boundary) && strings.Contains(tail, boundary) { - return true - } - } - return false -} - // reportedConsequence returns an asserted outcome after one matched inability. // Separators before the stem are irrelevant, and weak separators inside a // `that ...` negated proposition remain part of what was not found. Explicit @@ -1472,7 +1452,10 @@ func reportedConsequence(sentence string, stemEnd int) string { if index < 0 { continue } - explicit := explicitConsequenceBoundary(boundary) + explicit := strings.HasPrefix(boundary, ";") || + strings.Contains(boundary, " so ") || strings.Contains(boundary, " but ") || + strings.Contains(boundary, "therefore") || strings.Contains(boundary, "leaving") || + strings.Contains(boundary, "because") || strings.Contains(boundary, "since") if thatAt >= 0 && index > thatAt && !explicit { continue } @@ -1555,6 +1538,7 @@ var topicShiftMarkers = []string{ } var affirmativeObservationConsequencePattern = regexp.MustCompile(`\b(?:(?:the\s+)?(?:cause|source|root\s+cause|value|setting|definition|registration|owner|result)\s+(?:is|was)\b|(?:the\s+)?concern\s+(?:does|did)\s+not\s+apply\b|(?:it|the\s+(?:issue|bug|problem|change|fix|guard))\s+(?:is|was|looks|remains)\s+(?:resolved|fixed|complete|completed|done|correct|valid|safe|neutral|unaffected)\b|(?:the\s+)?(?:fix|guard|check)\s+holds\b)`) +var exhaustiveObservationEvidencePattern = regexp.MustCompile(`^after\s+(?:(?:[[:alpha:]][[:alnum:]_-]*ly)\s+)*(?:auditing|checking|examining|inspecting|reading|reviewing|searching|tracing|validating|verifying)\s+(?:every|all)\b`) // observationConsequenceIsAffirmative is deliberately an allow-list: an // unfamiliar consequence must not turn an admitted inability into success just @@ -1562,6 +1546,7 @@ var affirmativeObservationConsequencePattern = regexp.MustCompile(`\b(?:(?:the\s // an explicit completed action or a bounded positive result of the observation. func observationConsequenceIsAffirmative(consequence string) bool { return affirmativeObservationConsequencePattern.MatchString(consequence) || + exhaustiveObservationEvidencePattern.MatchString(consequence) || fallbackOutcomeIsAffirmative(consequence) } @@ -1571,19 +1556,16 @@ func observationConsequenceIsAffirmative(consequence string) bool { // This is the inverse of enumerating every synonym for work that did not land. func boundedObservationHasUnresolvedConsequence(claim inabilityClaim) bool { consequence := strings.TrimSpace(reportedConsequence(claim.sentence, claim.stemAt+claim.stemLen)) - if consequence != "" && hasExplicitConsequenceBoundary(claim.sentence, claim.stemAt+claim.stemLen) && - carriesTheConsequence(consequence) && - !observationConsequenceIsAffirmative(consequence) { - return true - } - if containsFailureConsequence(consequence) { - return true + if consequence != "" { + return carriesTheConsequence(consequence) && + !observationConsequenceIsAffirmative(consequence) } if len(claim.blockedContext) <= len(claim.sentence) { return false } next := strings.TrimSpace(claim.blockedContext[len(claim.sentence):]) - return containsFailureConsequence(next) + return carriesTheConsequence(next) && + !observationConsequenceIsAffirmative(next) } // countedLabelContent separates a counted markdown label from any content