diff --git a/internal/agent/completion_gate_test.go b/internal/agent/completion_gate_test.go index fae260ad2..00949a260 100644 --- a/internal/agent/completion_gate_test.go +++ b/internal/agent/completion_gate_test.go @@ -203,3 +203,182 @@ 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 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.", + "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", + "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.", + } + + 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.", + "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 { + 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) + } + }) + } +} + +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: "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}, + {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}, + {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: "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: "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."}, + {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 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."}, + {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: "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}, + } + + 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/completion_policy_test.go b/internal/agent/completion_policy_test.go index 220961684..b255556d1 100644 --- a/internal/agent/completion_policy_test.go +++ b/internal/agent/completion_policy_test.go @@ -19,6 +19,93 @@ 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.", + "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 { + 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.", + "**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.", + } { + got := newCompletionPolicy(false).evaluate(text, completionContext{}) + if got.Decision != CompletionComplete { + t.Errorf("complete report decided %q: %q (%s)", got.Decision, text, got.Reason) + } + } +} + +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 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 ccd9d26e9..33a121b57 100644 --- a/internal/agent/guardrails.go +++ b/internal/agent/guardrails.go @@ -1,8 +1,10 @@ package agent import ( + "regexp" "strconv" "strings" + "unicode" "github.com/Gitlawb/zero/internal/zeroruntime" ) @@ -168,19 +170,27 @@ 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", + "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. countedLabelContent drops only the heading shape instead. + "unable to ", "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", -} - // 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 @@ -236,27 +246,1627 @@ 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 := 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.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 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. + // 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 // 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. +// 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 +// 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 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 hasToolCapabilityContext(sentence string) bool { + if !containsAny(sentence, toolCapabilityMarkers) { + return false + } + return containsAny(sentence, []string{ + "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"}) +} + +// 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 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 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 { + 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 start, end +} + +func clauseContaining(sentence string, at int) string { + start, end := clauseBounds(sentence, at) + return strings.TrimSpace(sentence[start:end]) +} + +func containsClauseBoundary(between string) bool { + return containsAny(between, clauseBoundaries) +} + +// 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", "in this answer", "into this answer", +} + +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 + } + scope := sentence[after:end] + for _, marker := range deliveredAlternativeMarkers { + for start := 0; ; { + rel := strings.Index(scope[start:], marker) + if rel < 0 { + break + } + at := start + rel + 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) + } + } + 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 !fallbackOutcomeIsAffirmative(fallback) { + return false + } + recognizedGroups := 0 + allGroupsCovered := true + recognized := "" + for _, group := range []struct { + name string + terms []string + }{ + {"plan", []string{"plan", "update_plan"}}, + {"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"}}, + {"document", []string{"document", "documentation", "summary", "report", "answer"}}, + {"migration", []string{"migration", "migrate"}}, + {"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 + } + recognizedGroups++ + recognized = group.name + if !fallbackCompletesObligation(group.name, group.terms, fallback) { + allGroupsCovered = false + } + } + if recognizedGroups == 0 { + return false + } + if !fallbackCoversRequiredScope(failed, fallback) { + 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. + // 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 && 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 !fallbackOutcomeIsAffirmative(fallback) { + return false + } + if !containsAlternativeTerm(fallback, terms) { + 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 + } + 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"}) + } + 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 { + failedSpec := parseObligationSpec(failed) + fallbackSpec := parseObligationSpec(fallback) + if failedSpec.requiresBreadth && !fallbackSpec.requiresBreadth { + return false + } + for _, target := range failedSpec.targets { + if !containsWord(fallbackSpec.targets, target) { + return false + } + } + for _, kind := range failedSpec.validationKinds { + if !containsWord(fallbackSpec.validationKinds, kind) { + return false + } + } + // 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(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. 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 !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 { + 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", + "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 +// 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", "for"}, word) || index+1 >= len(words) { + continue + } + 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 + } + first := words[targetAt] + switch first { + case "complete", "finish", "verify", "validate", "modify", "change", "write", "read", "run", "perform": + continue + } + end := targetAt + for end < len(words) && !containsWord([]string{ + "to", "into", "onto", "on", "in", "against", "for", + "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 +} + +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 { + 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 false +} + +// 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) { + return false + } + localStem := strings.Index(clause, stem) + if localStem < 0 { + return false + } + afterStem := clause[localStem+stemLen:] + toolAt, toolMarker := firstMarkerOfAny(afterStem, toolCapabilityMarkers) + if toolAt < 0 { + return false + } + between := afterStem[:toolAt] + if containsClauseBoundary(between) { + return false + } + if !capabilitySubjectOnly(between) { + return false + } + 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 "} { + searchFrom := after + for searchFrom < len(sentence) { + rel := strings.Index(sentence[searchFrom:], connector) + if rel < 0 { + break + } + remainder := strings.TrimSpace(sentence[searchFrom+rel+len(connector):]) + failure, stem, ok := subjectElidedFailure(remainder) + if ok { + if stem == "" { + return true + } + synthetic := "i " + failure + claim := newInabilityClaim(synthetic, synthetic, "i "+stem, 0) + if !claim.exempt() { + return true + } + } + searchFrom += rel + len(connector) + } + } + return false +} + +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 +// 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 +// 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 { + 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 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 !hasUnavailableToolContext(sentence) { + return false + } + tail := strings.TrimSpace(sentence[stemAt+stemLen:]) + for _, action := range harmlessGrantLimitedActions { + if strings.HasPrefix(tail, action) { + return true + } + } + 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. +// 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 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", + "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", +} + +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) + if rel < 0 { + break + } + at := start + rel + stemAt, stemLen := lastStemBefore(sentence, at) + if stemAt >= 0 && !containsClauseBoundary(sentence[stemAt+stemLen:at]) { + return true + } + start = at + len(marker) + } + } + 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. +// +// 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." +// "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. +// 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", + // 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 +// 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", "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", + "call", "calls", "site", "sites", "callsite", "callsites", "consumer", "consumers", + "dependency", "dependencies", "user", "users", "path", "paths", +} + +// absenceQualifiers sit between "any" and the object without changing it. +var absenceQualifiers = []string{ + "remaining", "other", "further", "more", "additional", "obvious", "such", + "outstanding", "leftover", "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", + "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 + // 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", "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", + "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", "still blocked", + "cannot proceed", "could not proceed", "can not proceed", "unable to proceed", + "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 +// 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"} + +// 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", + "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", + "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", "running out of", "out of time", + "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`) +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) || + assertedIncompleteOutcomePattern.MatchString(text) || madeNoChangePattern.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 +// "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 ", "; so ", " so ", ", but ", "; but ", " but ", + ", therefore", "; therefore", " therefore", ", leaving ", " leaving ", + " because ", " since ", "; ", ": ", " - ", " -- ", " and ", ", ", +} + +// 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 { + index := strings.Index(tail, boundary) + 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") + if thatAt >= 0 && index > thatAt && !explicit { + continue + } + if earliest < 0 || index < earliest { + earliest, width = index, len(boundary) + } + } + if earliest < 0 { + return "" + } + 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 containsFailureConsequence(reportedConsequence(sentence, stemEnd)) { + return true + } + if len(blockedContext) <= len(sentence) { + return false + } + next := strings.TrimSpace(blockedContext[len(sentence):]) + 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, +// 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 +}() + +// 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", + "future cleanup", "belongs to another", "belongs to a different", + "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)`) +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 +// 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) || + exhaustiveObservationEvidencePattern.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 != "" { + return carriesTheConsequence(consequence) && + !observationConsequenceIsAffirmative(consequence) + } + if len(claim.blockedContext) <= len(claim.sentence) { + return false + } + next := strings.TrimSpace(claim.blockedContext[len(claim.sentence):]) + return carriesTheConsequence(next) && + !observationConsequenceIsAffirmative(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. +// +// 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 countedLabelContent(sentence string) (string, bool) { + trimmed := strings.TrimLeft(strings.TrimSpace(sentence), "-*#> \t") + match := countedLabelHeading.FindStringIndex(trimmed) + if match == nil { + 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 content, listEntry := markdownListEntryContent(remainder); listEntry { + if containsFailureConsequence(content) || hasObjectiveFailure(heading) || + countedHeadingIsOperational(heading) || !countedContentIsBenignFinding(content) { + return heading + " " + content, false + } + return content, true + } + 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 { + return strings.NewReplacer( + "\u2019", "'", "\u2018", "'", "\u02bc", "'", + "\u2014", " - ", "\u2013", " - ", + ).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{ + "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 +// 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) || singularRecognizedAbsence(tail) { + return true, true + } + return hasAnyPrefix(tail, boundedNegativeObservationTails) || + boundedNegativeObservationPattern.MatchString(tail) || + 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. +func (claim inabilityClaim) exempt() bool { + if matched, strong := successfulNegativeObservation(claim.tail); matched { + if strong { + return !strongAbsenceHasBlockedOutcome(claim) + } + return !containsAny(claim.blockedContext, blockedWorkMarkers) && + !boundedObservationHasUnresolvedConsequence(claim) + } + if hasObjectiveFailure(claim.sentence) || + containsAny(claim.blockedContext, blockedStateMarkers) || + containsUnambiguousFailureState(claim.blockedContext) { + return false + } + if capabilityOnlyToolFootnote(claim.sentence, claim.stemAt, claim.stemLen) { + return !hasUnexemptedSubjectElidedInability(claim.sentence, claim.stemAt+claim.stemLen) + } + if harmlessToolLimitation(claim.sentence, claim.stemAt, claim.stemLen) { + 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) { + return true + } + 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 { - for _, sentence := range admissionSentences(strings.ToLower(stripQuoted(text))) { + 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: + // + // "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 } + // 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. + // + // 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". + // 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. + // 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. + // 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 hasUnavailableToolContext(sentence) && + hasReportedFailureConsequence(sentence, blockedContext, 0) { + 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 @@ -267,11 +1877,12 @@ func selfReportedIncompletion(text string) string { break } abs := start + rel - tail := strings.TrimSpace(sentence[abs+len(stem):]) - if !hasAnyPrefix(tail, successNegationTails) { - return selfReportReason(strings.TrimSpace(stem) + " …") + claim := newInabilityClaim(sentence, blockedContext, stem, abs) + if claim.exempt() { + start = abs + len(stem) + continue } - 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 new file mode 100644 index 000000000..b278c0ba4 --- /dev/null +++ b/internal/agent/guardrails_false_admission_test.go @@ -0,0 +1,765 @@ +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) + } + } +} + +// 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) + } + } +} + +// 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) + } + } +} + +// 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) + } + } +} + +// 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) + } + } +} + +// 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) + } + } +} + +// 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 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.", + "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) + } + } +} + +// 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) + } + } +} + +// "ANY" IS WHAT MAKES A NEGATIVE RESULT A RESULT. +// +// 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." +// +// 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) + } + } +} + +// 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) { + // 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.", + "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) + } + } + + // 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) + } + } +} + +// 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.", + "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) + } + } + + 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.", + "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) + } + } +} + +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) + } + } +} + +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) + } + } +} + +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") + } +} diff --git a/internal/agent/guardrails_test.go b/internal/agent/guardrails_test.go index 5ae974eb4..d96ef9b14 100644 --- a/internal/agent/guardrails_test.go +++ b/internal/agent/guardrails_test.go @@ -554,3 +554,118 @@ 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. 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) { + // 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.", + } { + 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.", + // 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) + } + } + + // 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 +// "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") + } +}