Review round 2 found three defects where audit observation changed
endpoint behavior. Audit capture reads in both handlers now degrade
best-effort: a failed Old-capture or New re-read logs a warning and
skips the entry instead of aborting the request or rolling back the
completed upsert, restoring byte-identical behavior with main under
read failure. The write path stays the failure surface: the upserts
and a new advisory lock (LockIDChatSettingsWrites) that serializes
change-detection with the write still fail the request on error. The
lock also closes a race where two concurrent identical PUTs each
captured a stale Old and both emitted a change entry; the second
transaction now sees the first's committed state and suppresses its
duplicate. Handler tests pin all of it with mutation-proven regression
cases; a test-only DiffOverride on InitRequest (wired through the API,
nil in production) proves the fallback include-default flip reaches
the audit diff with the re-read value.

Round 2 review ruled the DiffOverride seam a production-exported audit
bypass: RequestParams.DiffOverride and the API setter were exported in
normal builds and could replace or fabricate audit diffs for any
audited resource. The seam is removed entirely from production code
(RequestParams field, the InitRequest commit branch, the API field and
setter), and the assertion support moves into the test double:
MockAuditor gains a construction-time NewMockWithDiffFn whose supplied
function computes the entry's diff, so the fallback-flip test pins the
captured Old/New pair without any production-code hook. The flip test
still dies under the stale-New mutation (re-proven).

Wrapping the previously non-transactional plan-mode upsert in InTx
changed the error response body: main's detail was the raw write
error, the wrapped version's was "execute transaction: <error>". The
handler now records the callback's write error and responds from it
exactly as main did, keeping the InTx wrapper only for lock, begin,
commit and rollback failures so those stay distinguishable. The
existing rollback regression test pins the raw detail, mutation-proven
by restoring the wrapper.

The plan-mode handler built its audit New from request-derived text,
contradicting the rule the system-prompt endpoint already follows: the
write may normalize the value, so request-derived text can misreport
the change. It now re-reads the stored value after the upsert inside
the same transaction and uses that read for both New and the change
comparison, with the same best-effort degradation on a failed re-read.
A normalizing test store proves the audited New is the stored value,
mutation-proven by restoring the request-derived assignment. Also
renumber the enum migration off the 000561 collision main took.

Round 3 review found the audit machinery itself could change what a
member sees: the advisory lock and the plan-mode transaction exist
only for auditing, and their failures replaced main's successful
response. The write path is now authoritative on both endpoints: on
any audit-machinery failure (lock, begin, commit, rollback) the
handler runs main's idempotent write path directly and derives the
response from it, while write failures keep the exact response the
endpoint produced before the audit wiring existed (transaction error
for the system prompt, which was always transactional; the raw write
error for plan mode, which was not). The full InTx error is logged
before the response detail is chosen so rollback failures cannot
vanish. Accepted consequence: when the lock cannot be taken, two
concurrent identical writes can both record, which is audit
degradation, not a behavior change.

Operator decision D5 lands in the same change. The resource type is
renamed chat_instruction_settings (Go type ChatInstructionSettings,
friendly string, frontend filter label) because it carries the
plan-mode instructions too. Each endpoint now has a fixed resource ID
and a human-readable target ("System prompt", "Plan mode
instructions"), so history-by-setting works; identity is assigned
before authorization, so denied and failed PUTs record the attempt
with the real status and an empty diff, and capture-degraded writes
record instead of going silent, distinguishing "nothing changed" from
"something changed and capture degraded". No-op suppression moves off
the nil-ID skip to InitRequestWithCancel, which also removes the trap
where setting Old.ID would have emitted a row claiming every field was
cleared. Migration renumbered to 000562 (main took 000561).

The fallback rule was wrong on the post-commit position: when the
audited transaction's machinery fails after Old was captured and the
write succeeded, the fallback direct write lands the same value and
the client sees 204, so the request is a success in every respect and
gets the ordinary entry (Old to the stored New, real diff, 204). An
empty diff would under-report a change that happened, and a failed
entry would contradict the success the client saw. When the machinery
fails before any baseline exists (lock or begin), no truthful diff is
possible, so the attempt row keeps an empty diff and the real status,
matching S2's documented limit with S1's D5 identity. A
commit-failure test proves the ordinary entry, mutation-proven by
dropping the post-fallback New population (the entry's New went empty
and the test went red); a fallback-then-ordinary test proves the
degraded write does not desync the next baseline.

Two fixes from review round 2. A failed post-write New read left the
audit New at its identity-only zero payload while the request
succeeded, so the production enterprise differ recorded Old-to-empty
and the row claimed the setting was cleared when it was not. Every
degraded capture branch on both endpoints now sets New = Old before
returning, so a failed audit read yields an unknown value and an empty
diff, never a fabricated deletion; the rule is stated in the code
comment and pinned with the production enterprise differ (the
empty-diff mock is exactly why it escaped earlier rounds),
mutation-proven by restoring the zero New.

The include-default audited state now also carries whether the
override row EXISTS, not only its effective value: writing explicit
false over a legacy absent row does not move the effective boolean but
inserts the persistent row and changes future behavior, so the
presence field (include_default_system_prompt_set) is captured, stored
in the audited struct, enumerated in the audit table, and included in
the no-op comparison. The nil-to-explicit transition audits even when
the effective value does not move, and explicit-false to
explicit-false stays silent, both pinned with the production differ.

Final structure for the audit write path, settled against a real
PostgreSQL 13 instance. The best-effort premise collapses for these
queries: siteconfig reads are single non-null text scans with no
data-dependent conversion, and the reviewer could not make one fail
while leaving the write able to succeed, so there is no reachable
degraded path to defend. Audit reads (Old and New) are now fatal like
usersecrets.go: the error surfaces through the endpoint's existing
error path exactly as main's does. The whole fallback structure is
deleted: no retried write, no direct-write path, no writeErr
classification, no post-fallback re-read, no degraded rows, and the
sentinel store kit and recording-sink degradation assertions that only
existed to prove degradation go with it. One transaction per request,
shaped like main's.

The global write lock is replaced by a per-setting advisory lock taken
first inside the transaction. Lock IDs derive from the exact
site_configs key with GenLockID (FNV-1a 64) instead of the sequential
LockID* block, so writers of different settings never contend and the
IDs cannot collide with the sequential block or another subsystem's
GenLockID output (the key strings are unique to these settings).
LockIDChatSettingsWrites is deleted; nothing else used it. FOR UPDATE
is not added: the per-key lock subsumes it and covers the absent-row
case a row lock cannot. Mutation-proved by removing the plan-mode
lock: two concurrent identical PUTs each captured a stale Old and both
emitted, and removing it turned the suppression test red.

Kept unchanged: the rename to chat_instruction_settings, the fixed
per-setting identity and targets, denied-attempt rows with empty
diffs, the no-fabricated-deletion guarantee (now unreachable rather
than defended against), include-default presence auditing, and no-op
suppression through InitRequestWithCancel taken inside the
transaction. Accepted costs documented in the PR body: concurrent
first writes to a not-yet-existing setting row can each record an
empty Old and two identical first writes can each emit; and the added
lock introduces one theoretical failure position, a lock wait
exceeding the request deadline, whose only holders are sibling
requests holding it for a single upsert.

GenLockID derives the per-setting advisory lock IDs from a string, so
collision-freedom is a property to verify, not argue. The test
computes both generated IDs, asserts they are pairwise distinct, and
asserts each is distinct from every sequential LockID* constant,
listed explicitly because iota constants cannot be enumerated
programmatically. A future constant that collides now fails in review
rather than in a production deadlock.

The previous premise, that no read failure can leave the write able to
succeed, was falsified on real PostgreSQL: a database.Store wrapper can
error before any SQL reaches Postgres, and query-local context
cancellation fails the individual query without marking the
server-side transaction failed (InTx begins it with
context.Background). The post-write New read was the reachable case:
on main the write lands and the client just never sees the response,
while the audited handler rolled it back, silently discarding a change
an administrator made. That is worse than any audit imprecision.

The post-write read is deleted rather than protected. New is derived
from what was written: the upserts store $1 verbatim, so the stored
value is the request value, and the effective include-default flag
follows GetChatSystemPromptConfig's rule from the written toggle row
and the written prompt. A parity test compares the derivation against
the getter's result on the happy path for every combination of written
prompt and include-default input, so drift is caught. The Old capture
moves back inside the per-setting lock but stays best effort: it runs
before the write, so abandoning the transaction on its error costs
nothing, and the handler runs the direct write path with no entry and
a warning. The no-op decision is staged into a local and applied only
after the transaction commits, so a commit failure cannot suppress an
attempt row. The per-setting lock stays, now the only audit-added
statement in the transaction, with a service-owned 5s bound on the
wait so a waiter cannot hang past the client deadline. Plan-mode's
audit-added transaction machinery (lock, begin, commit, rollback, and
the Old read) runs the direct write instead, matching main's
non-transactional behavior, while the system-prompt endpoint, already
transactional on main, surfaces those errors exactly as main's. The
full InTx error is logged before the response detail is chosen, so a
rollback failure layered on a write failure is never absent from
response, audit row and logs simultaneously. The concurrency tests are
strengthened to the barrier shape: with the lock, two concurrent
identical PUTs produce one entry; with the lock removed and both
transactions parked at the Old capture, both read the same stale Old
and both emit, the deterministic mutation proof.

The without-lock concurrency subtest parked goroutines before the
Old-capture read. The goroutine that released the barrier could write
and commit before a slowly waking waiter executed its read, which then
saw the committed value under read committed, detected no change, and
suppressed its audit entry, failing the expected-duplicate assertion.
Read before parking so every waiter deterministically holds the same
stale value.

…overage

- Fall back to the unaudited write path when the chat system prompt
  advisory lock times out, instead of failing the update with 500. The
  lock exists only for audit change detection, so contention must not
  block a write that succeeded before the audit wiring existed.
- Add Storybook interaction coverage that opens the audit resource-type
  filter and selects the Chat Instruction Settings option, verifying the
  generated resource type and its friendly label together.

Main gained 000571_pool_aware_chat_acquisition_index and
000572_chat_files_token_crypto_key_feature while this PR aged, so the
branch migration moves past both to keep versions unique and ordered.

…aseline

A lock-wait timeout in putChatPlanModeInstructions fell back to the
direct write but never canceled the deferred audit, emitting a
successful 204 entry with an empty diff despite the instructions
changing. Gate the suppression on the captured baseline instead of the
Old-read error so every fallback without a baseline emits no entry,
matching the system-prompt handler. A commit failure keeps its baseline
and still emits the attempt row with its real diff.