Fix A1 of plans/2026-07-11-nomos-agent-code-review.md. isAssent and
isTypedConfirmation used a space-padded word-boundary check for negation
words but a bare strings.Contains for assent/confirm words — confirmed live
via test probes: isAssent("...maybe yesterday's logs...") returned true
("yes" matched inside "yesterday"), and isTypedConfirmation("I haven't
confirmed anything yet") returned true ("confirm" matched inside "confirmed",
and "haven't" wasn't in negationWords — only "don't"/"do not" were).
isTypedConfirmation is the sole gate for DESTRUCTIVE actions, so the second
case meant a message merely stating something hadn't been confirmed could
read as an explicit confirmation.
- Replaced the ad-hoc space-padding/prefix-check negation logic with proper
tokenization (wordTokenRe) + containsPhrase, matching WHOLE tokens/phrases
only — never a mid-word substring. Handles curly apostrophes too (a
pre-existing gap: the old straight-quote-only check would have missed
"don't" typed with a smart quote).
- Added contracted negatives (haven't, hasn't, isn't, wasn't, aren't, can't,
cannot, won't, wouldn't, shouldn't, didn't, doesn't) to negationWords.
Deliberately did NOT add a bare "not" — too broad, would false-negative
ordinary assent like "go ahead, this is not risky".
- Added regression tests for both confirmed cases plus a couple of adjacent
ones (eyesight/isn't, can't confirm) so a future change can't silently
reintroduce either bug.
All existing assent/confirmation tests pass unchanged — this is a pure
robustness fix, not a behavior change for any previously-correct case.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
137 lines
4.6 KiB
Go
137 lines
4.6 KiB
Go
package main
|
|
|
|
import (
|
|
"encoding/json"
|
|
"testing"
|
|
)
|
|
|
|
func TestIsAssent_Positive(t *testing.T) {
|
|
cases := []string{
|
|
"go ahead", "Go ahead.", "yes", "Yes!", "yeah", "yep", "do it",
|
|
"proceed", "approve", "ship it", "sounds good", "lgtm", "please do",
|
|
"ok go ahead and run it",
|
|
}
|
|
for _, c := range cases {
|
|
if !isAssent(c) {
|
|
t.Errorf("isAssent(%q) = false, want true", c)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestIsAssent_Negative(t *testing.T) {
|
|
cases := []string{
|
|
"no", "no, don't", "wait", "hold on", "not yet", "cancel that",
|
|
"nevermind", "what's the plan for tomorrow?", "how many CPUs does strong have?",
|
|
"maybe later", "",
|
|
}
|
|
for _, c := range cases {
|
|
if isAssent(c) {
|
|
t.Errorf("isAssent(%q) = true, want false", c)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestIsAssent_NegationBeatsAssentWord(t *testing.T) {
|
|
// Contains "yes" as a substring pattern risk word but is clearly not
|
|
// assent — negation must win.
|
|
cases := []string{
|
|
"no, don't do it yet",
|
|
"wait, not yet please",
|
|
}
|
|
for _, c := range cases {
|
|
if isAssent(c) {
|
|
t.Errorf("isAssent(%q) = true, want false (negation should block)", c)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestIsAssent_WholeWordBoundary regression-tests a real false positive found
|
|
// live: the old substring check matched "yes" inside "yesterday" (and would
|
|
// equally match "confirm" inside "confirmed"/"unconfirmed" for
|
|
// isTypedConfirmation below) because only negation used a word-boundary
|
|
// check — assent/confirm words used a bare strings.Contains. Confirmed via a
|
|
// throwaway probe before being fixed; kept here permanently so a future
|
|
// change can't silently reintroduce it.
|
|
func TestIsAssent_WholeWordBoundary(t *testing.T) {
|
|
cases := []string{
|
|
"not sure, maybe yesterday's logs show something useful",
|
|
"my eyesight isn't great, what does that say",
|
|
}
|
|
for _, c := range cases {
|
|
if isAssent(c) {
|
|
t.Errorf("isAssent(%q) = true, want false (word-boundary: 'yes' must not match inside 'yesterday'/'eyesight')", c)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestIsTypedConfirmation_ContractedNegation regression-tests the other real
|
|
// false positive: isTypedConfirmation gates DESTRUCTIVE actions, and
|
|
// "confirm" matching inside "confirmed" combined with contracted negatives
|
|
// ("haven't") not being in negationWords meant a message that explicitly
|
|
// says the operator has NOT confirmed something could read as confirming it.
|
|
func TestIsTypedConfirmation_ContractedNegation(t *testing.T) {
|
|
cases := []string{
|
|
"I haven't confirmed anything yet, let me think",
|
|
"that isn't confirmed on my end",
|
|
"we can't confirm that until tomorrow",
|
|
}
|
|
for _, c := range cases {
|
|
if isTypedConfirmation(c) {
|
|
t.Errorf("isTypedConfirmation(%q) = true, want false (contracted negation should block)", c)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestIsTypedConfirmation(t *testing.T) {
|
|
positive := []string{
|
|
"I confirm destroy 135 in strong",
|
|
"confirm",
|
|
"Confirmed.",
|
|
"yes I confirm",
|
|
}
|
|
for _, c := range positive {
|
|
if !isTypedConfirmation(c) {
|
|
t.Errorf("isTypedConfirmation(%q) = false, want true", c)
|
|
}
|
|
}
|
|
negative := []string{
|
|
"yes", "go ahead", "do it", "proceed", "lgtm", // loose assent must NOT satisfy this
|
|
"no, don't confirm yet", "wait", "",
|
|
}
|
|
for _, c := range negative {
|
|
if isTypedConfirmation(c) {
|
|
t.Errorf("isTypedConfirmation(%q) = true, want false (only explicit confirm should pass)", c)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestExtractPendingApprovals(t *testing.T) {
|
|
mkCall := func(text string) persistedCall {
|
|
b, _ := json.Marshal(text)
|
|
return persistedCall{id: "x", name: "run", result: json.RawMessage(b)}
|
|
}
|
|
calls := []persistedCall{
|
|
mkCall("run on host:strong requires approval (risk: config_mutation) — execution 019f4930-e22b-7c47-8c6e-715dcd59df19 queued. Present the command..."),
|
|
mkCall("some unrelated read-only result, no approval here"),
|
|
mkCall("run on lxc:caddy requires approval (risk: destructive) — execution 019f4931-aaaa-7c47-8c6e-715dcd59df20 queued. This is classified DESTRUCTIVE — flag that clearly."),
|
|
}
|
|
got := extractPendingApprovals(calls)
|
|
if len(got) != 2 {
|
|
t.Fatalf("expected 2 pending approvals, got %d: %+v", len(got), got)
|
|
}
|
|
if got[0].execID != "019f4930-e22b-7c47-8c6e-715dcd59df19" || got[0].destructive {
|
|
t.Errorf("first approval wrong: %+v", got[0])
|
|
}
|
|
if got[1].execID != "019f4931-aaaa-7c47-8c6e-715dcd59df20" || !got[1].destructive {
|
|
t.Errorf("second approval should be flagged destructive: %+v", got[1])
|
|
}
|
|
}
|
|
|
|
func TestExtractPendingApprovals_NoneWhenNoneQueued(t *testing.T) {
|
|
b, _ := json.Marshal("fleet is healthy, nothing to report")
|
|
calls := []persistedCall{{id: "x", result: json.RawMessage(b)}}
|
|
if got := extractPendingApprovals(calls); len(got) != 0 {
|
|
t.Errorf("expected no pending approvals, got %+v", got)
|
|
}
|
|
}
|