diff --git a/Plugin/unraid/include/ai_repair.php b/Plugin/unraid/include/ai_repair.php index d95d0d5..48616ff 100644 --- a/Plugin/unraid/include/ai_repair.php +++ b/Plugin/unraid/include/ai_repair.php @@ -308,11 +308,13 @@ function vv_ai_finding_write(array $f): array { $rec['closed_at'] = $old['closed_at'] ?? null; } // An acknowledgement holds only while the thing acknowledged is still true. Compare - // the live value against what it read when the ack was given: unchanged means stay - // quiet, changed means the note is stale and the finding comes back by itself. + // what it is pinned to now against what it read when the ack was given: unchanged + // means stay quiet, changed means the note is stale and the finding comes back by + // itself. $rec carries this sighting's evidence, so a fault that has changed shape + // fails this comparison even when no conf key is involved. if (($old['state'] ?? '') === 'acknowledged') { $ackedAt = (string)($old['ack_value'] ?? ''); - if ($ackedAt === (string)(vv_conf_vars()[$confKey] ?? '')) { + if ($ackedAt === vv_ai_finding_ack_pin($rec)) { $rec['state'] = 'acknowledged'; $rec['ack_value'] = $ackedAt; $rec['closed_at'] = $old['closed_at'] ?? null; @@ -365,17 +367,32 @@ function vv_ai_finding_dismiss(string $id, string $note = ''): bool { return vv_ai_finding_set_state($id, 'dismissed', $note); } +// What an acknowledgement is pinned to — the thing that has to stay the same for the ack to keep +// meaning what it meant. +// +// For a conf-bound finding that is the key's value, which is what the operator was looking at +// when they said "I know". A finding with no key had nothing to pin to and so compared '' with +// '' — every ack on an arr health item was silently permanent, which is dismiss wearing ack's +// label. Those pin to the shape of the fault instead: "indexers unavailable: NzbNoob" and +// "indexers unavailable: NzbNoob, Miatrix" are one finding getting worse, and an ack given for +// the first has not been given for the second. +function vv_ai_finding_ack_pin(array $f): string { + $key = (string)($f['conf_key'] ?? ''); + if ($key !== '') return (string)(vv_conf_vars()[$key] ?? ''); + return 'ev:' . substr(sha1((string)($f['evidence'] ?? '')), 0, 16); +} + // "I know about this — leave it, and tell me if it changes." // -// Stamps the key's current value onto the record. Every later sighting compares against that -// stamp, so the acknowledgement covers this state and not the key forever. Acking that critical +// Stamps what it is pinned to onto the record. Every later sighting compares against that stamp, +// so the acknowledgement covers this state and not the finding forever. Acking that critical // rsync is off says nothing about critical rsync being on. function vv_ai_finding_ack(string $id, string $note = ''): bool { $r = vv_ai_finding_get($id); if ($r === null) return false; $r['state'] = 'acknowledged'; - $r['ack_value'] = (string)(vv_conf_vars()[$r['conf_key'] ?? ''] ?? ''); + $r['ack_value'] = vv_ai_finding_ack_pin($r); $r['closed_at'] = time(); if ($note !== '') $r['note'] = mb_substr(vv_ai_redact($note), 0, 1000); @@ -384,12 +401,23 @@ function vv_ai_finding_ack(string $id, string $note = ''): bool { } // What the operator can do about a finding, and what each choice means. Returned rather than -// hardcoded in the UI so the chat and the page cannot offer different options for the same row. +// hardcoded in the UI so the chat and the page cannot offer different options for the same row, +// and enforced in vv_ai_finding_apply_action() so neither can act on one it was not offered. // // Fix appears for anything with a proposed value, toggle or not — the prohibition is on the -// sweep choosing, never on the operator choosing. Everything carries ack and cancel, because -// "I know" and "not now" are always valid answers to being told something. +// sweep choosing, never on the operator choosing. Open rows also carry ack, dismiss and cancel, +// because "I know", "this is never a problem" and "not now" are all valid answers to being told +// something, and they are three different answers. function vv_ai_finding_actions(array $f): array { + // A closed finding has one question left, and it is not the original one: was closing it + // right? Offering fix or ack on a row that is already dismissed is offering to decide + // something that has been decided. Reopen is here because dismiss is otherwise permanent — + // the write path keeps a dismissed finding dismissed however many times the fault recurs, + // so a mis-click would need someone editing JSON on disk to undo. + if (!in_array((string)($f['state'] ?? 'open'), ['open', 'needs_operator'], true)) { + return ['reopen' => 'Put it back on the list — either closing it was wrong, or it is back']; + } + $actions = []; if (($f['proposed'] ?? null) !== null) { @@ -398,8 +426,11 @@ function vv_ai_finding_actions(array $f): array { : 'Write the proven value to ' . $f['conf_key']; } - $actions['ack'] = 'Known and intended. Stays quiet until ' . ($f['conf_key'] ?? 'it') . ' changes'; - $actions['cancel'] = 'Leave it alone for now'; + $actions['ack'] = ($f['conf_key'] ?? '') !== '' + ? 'Known and intended. Stays quiet until ' . $f['conf_key'] . ' changes' + : 'Known and intended. Stays quiet until the fault itself changes'; + $actions['dismiss'] = 'Not a problem, ever. Stays closed even when it is seen again'; + $actions['cancel'] = 'Leave it alone for now'; return $actions; } @@ -597,13 +628,32 @@ const VV_AI_ACTION_PATTERNS = [ ], // Not now — no state written, it comes back next sweep. 'cancel' => [ - '/\b(not now|later|leave it (alone|for now)|skip( it)?|cancel|ignore for now)\b/u', + // "leave it" is matched bare, not only as "leave it alone" / "leave it for now". The + // ambiguity this function is built to refuse — "leave it, I know" reading as both cancel + // and ack — did not actually arise with the longer forms, so that sentence resolved to + // ack and silenced the finding until the fault changed. The looser pattern is what makes + // the two readings collide and sends it back to be restated. + '/\b(not now|later|leave it\b|skip( it)?|cancel|ignore for now)\b/u', '/\b(no|nope|nah)\b[\s,.!]*$/u', '/\b(don\'?t|do not) (fix|touch|change|write|apply)\b/u', ], + // Never a problem. Deliberately narrow: this is the one answer that cannot expire on its + // own, so it is only read from a sentence that says so outright. Anything vaguer than these + // is meant to land on ack, which comes back by itself when the fault changes. + 'dismiss' => [ + '/\bdismiss\b/u', + '/\b(this|that|it)(?:\'s| is) not (a |an )?(problem|bug|issue|real)\b/u', + '/\bnever (a problem|an issue|report this)\b/u', + ], + // Undo a close. + 'reopen' => [ + '/\breopen\b/u', + '/\bun-?dismiss\b/u', + ], ]; -// Returns 'fix' | 'ack' | 'cancel', or null when the reply does not clearly mean one of them. +// Returns one of the keys in VV_AI_ACTION_PATTERNS, or null when the reply does not clearly mean +// exactly one of them. // // Only call this when a finding is actually pending. A bare "yes" means fix in answer to "shall // I fix it" and means nothing at all on its own, and the difference is context this function @@ -634,10 +684,27 @@ function vv_ai_finding_apply_action(string $id, string $action, string $note = ' $f = vv_ai_finding_get($id); if ($f === null) return ['ok' => false, 'error' => 'no such finding']; + // Only what this finding actually offers, in the state it is actually in. The page renders + // its buttons from the same function, but a stale tab holds buttons the store has moved past + // — a row acked in one window is still showing Fix in another — and the endpoint is reachable + // without either. Checking here is what makes vv_ai_finding_actions() the authority rather + // than a suggestion. + if (!isset(vv_ai_finding_actions($f)[$action])) { + return ['ok' => false, 'error' => 'not offered for this finding: ' . $action]; + } + switch ($action) { case 'ack': return ['ok' => vv_ai_finding_ack($id, $note), 'action' => 'ack']; + case 'dismiss': + return ['ok' => vv_ai_finding_dismiss($id, $note), 'action' => 'dismiss']; + + // Back to open, never straight back to needs_operator: whether it still cannot be + // repaired here is the next sweep's finding to make, not a state to restore. + case 'reopen': + return ['ok' => vv_ai_finding_set_state($id, 'open', $note), 'action' => 'reopen']; + case 'cancel': // Deliberately writes nothing at all. "Not now" is not a state, it is the absence of // one — recording it would make the finding look decided when it is still open.