feat(autonudge): optional per-loop banner for the visible nudge row

[rnoack1]

Optional per-loop banner on auto-nudge

Problem / Motivation

A nudge loop's message serves two consumers with opposite needs, and today they
get the same string.

_fire_dashboard_nudge composes one body and uses it twice:

msg    = await compose_nudge_body(loop.message, loop.stop_sentinel_path, loop.slot_key)
tagged = f"[auto-nudge cycle {loop.cycle_count + 1}]\n{msg}"
slot.append("nudge", tagged, "msg msg-nudge", meta={...})   # the VISIBLE row
_run_chat(self.dashboard_state, slot, tagged, ...)          # the PROMPT

The model needs the whole instruction re-delivered every cycle — that is the
guarantee the nudge exists to provide. A person reading the transcript needs only
"a nudge happened".

Why it matters

Measured on one long-running loop's session file, dashboard_chat-1896-1787695225.jsonl:

Rows	What	Chars	Share of session
44	nudge	348,295	51.8%
228	assistant	169,791	25.3%
879	tool	150,506	22.4%

Each nudge row was 7,915–7,916 chars — the same payload, 44 times. The largest
single contributor to the file was not the agent's work but the repeated
re-statement of its instructions.

What changed

One optional field. When set, it replaces the message body in the appended row
only. The prompt is untouched.

banner = (loop.banner or "").strip()
if banner:
    visible = (
        f"[auto-nudge cycle {loop.cycle_count + 1}]\n"
        f"{render_nudge_message(banner, loop.stop_sentinel_path)}"
    )
else:
    visible = tagged
...
slot.append("nudge", visible, "msg msg-nudge", meta={...})
_run_chat(self.dashboard_state, slot, tagged, ...)   # UNCHANGED

Threaded through NudgeLoop, AutoNudgeService.add/update, both
autonudge_authz chokepoints, and POST/PATCH /api/autonudge. Capped at 500
chars (MAX_BANNER_CHARS), two orders of magnitude under the 8000-char message
limit — a generous ceiling here would reintroduce the very bloat the field
removes, so the cap is part of the feature rather than a safety afterthought.

The [auto-nudge cycle N] prefix is kept on both branches: it is the counter,
and the one part of the row a reader wants.

A banner deliberately skips compose_nudge_body, which prefixes the session's
work-ledger snapshot. That prefix is right for the model (each cycle starts from
durable state) and self-defeating for a display line. render_nudge_message
still applies, so {{STOP_FILE}} resolves in a banner as it does in a message.

PATCH accepts it too, which is what lets a running loop be quieted without
re-registering: re-arming resets cycle_count and the wall-clock budget anchor,
so a loop found to be noisy mid-run could not otherwise be fixed without
discarding its accounting.

The MCP arming surface also gains banner

monitor_start and monitor_update declare an optional banner
(mcp_tools/control.py, bounded in validation.py, applied in
dashboard/session_directive_apply.py). Without it the only writer was a
hand-crafted REST call, so the field would have shipped with no reachable setter
on the surface that armed the loop this PR's own 51.8% measurement came from.

This is the PR's only LLM-facing surface, and it carries a trust asymmetry worth
naming rather than disclaiming: the agent can author the short row shown in place
of its own recurring instruction. It is acceptable because the same
autonudge_authz chokepoints, SEL audit, and GET /api/autonudge visibility
apply to it as to the REST path — the banner is bounded and scrubbed by the same
code either way.

The two sibling loader redactions are DEFERRED to their own PR

An earlier revision of this PR also redacted the loader warnings in cron.py and
apps/builtins/ops_mission_control/backend/store.py. Both have been removed from this
diff. Design and First Principles independently asked for that split -- they share a fix
SHAPE with the autonudge loader, not this feature's fate, and bundling them made a revert
of the banner also revert fleet-wide hardening.

What that leaves here: redact_store_value STAYS, because it is not a rider. It has seven
call sites in autonudge.py and is load-bearing for this feature's own loader and sentinel
warnings. Design's suggestion listed it with the riders; that is measurably wrong at source,
so it is kept rather than deleted. It also moves back INTO autonudge.py: it was relocated
to platform/context.py in an earlier revision precisely because cron.py imported it, and
with that importer gone the helper belongs beside its only remaining consumer. cron.py,
store.py and platform/context.py are therefore no longer in the changed-file list.

The follow-up PR carries those two loaders together with the five remaining %r siblings
named below, so that sweep lands as one complete fix instead of three of eight.

_load hardening, which reaches every loop field and not just banner

Loading is where a hand-edited autonudge.json becomes objects, so the banner
arms live there — and two of the four change behaviour for stores that carry no
banner at all:

• Repair arms coerce a non-str or over-cap banner to "" rather than
  rejecting the row, so one bad field cannot cost a whole loop.
• The load warnings share one spelling. All four render their value as
  redact(repr(...)) — the repr escapes a control character so a store-supplied
  newline cannot split one warning into two records, and the redact strips a
  credential. An earlier draft of this PR carried a hand-rolled _scrub_for_log
  helper for that job; it was deleted in favour of redact_store_value, so there
  is one definition rather than two. There is no _scrub_for_log in this diff, and a
  test pins its absence.
• The addressing fields are REFUSED, not scrubbed. _serialize exempts id
  and slot_key because the client addresses the row by them, so _load is the
  trust boundary that has to guarantee they are safe to serve. A row whose
  addressing field is not a str, is not isprintable(), or is credential-shaped
  is skipped with a warning naming the field. The isprintable() arm is what stops
  log injection: redact only rewrites credential- and URL-shaped text, so a
  newline rode through it, and the id reaches ~15 bare %s log calls where one
  newline forges a second record. Refusing at this one boundary was chosen over
  escaping at each sink, which are many and each new one would have to remember.
• A declined row is dropped by the next write, exactly like the malformed-entry arm.
  This took four attempts and the history matters, because _serialize_state rewrites the
  store WHOLESALE from memory:
  1. Hold the row and write it back — built with a per-slot eviction, a rollback of that
     eviction, and resurrection handling. Two reviewers called that machinery an over-build.
  2. Drop the row — GPT 5.6 blocked it as data loss: a row merely absent from _loops is
     deleted by the next add/update/fire.
  3. Fail the whole load — safe for the file, but it disarmed every OTHER loop and refused
     all new arming until a human repaired the file and restarted. Design flagged that cliff.
  4. Hold without the machinery — this then needed a retirement on slot close, a rollback
     for that retirement, and a rollback in _add_locked. The last one could not be
     completed: an aborted close rolls back through
     chat_handlers._restore_slot_nudge_loop(exc.loop, …) where exc.loop is
     get_by_slot(...), which searches _loops and is therefore None for a row that never
     armed — so the retirement had already reached disk with no token to undo it. That gap
     sits in a caller autonudge.py does not own.
     Shipped: QUARANTINE THE ROW, arm its siblings. The addressing-guard arm appends the
     offending row to _quarantined and continues, so _load_refused is NOT set and every
     healthy loop still arms. The row is re-emitted verbatim under a new persisted
     quarantined key, so the entry the warning asks an operator to fix survives a wholesale
     write instead of being deleted — which is what "the next write drops the row", the earlier
     contract, did. Refusing the WHOLE store was the previous shipped answer and was worse in
     the common case: one bad row disarmed every unrelated loop. That whole-store arm remains,
     but ONLY for a host whose credential policy will not compose at all, where no row can be
     vetted. Recovery is repairing the row: _load revalidates quarantined on every load, so
     a repaired row re-arms without a hand edit and without a restart.
     test_a_refused_row_does_not_strand_the_loops_loaded_before_it pins that the siblings DO
     arm and the declined row is preserved; test_the_two_skip_arms_diverge_deliberately pins
     the split that remains — a malformed row is dropped and loading continues past it, an
     unusable addressing field is quarantined.
     Back-compat for the held-aside rows, the same scrutiny banner gets below: they
     live in a SEPARATE autonudge.quarantine.json sidecar rather than in the store payload,
     so an untouched store keeps its existing wire shape byte for byte and an older build
     reads it unchanged. The sidecar is written ADDITIVELY before the main store lands and
     compacted only once it has, so a failed replacement cannot leave a repaired row in
     neither file; an unreadable sidecar refuses every write in that process and is moved
     aside under a .corrupt-<ts> name, so recovery is a restart, not a hand repair.
     The same mechanism covers the case it was built for — a host that cannot compose its
     credential policy, so NO row can be vetted. There too an empty _loops means "could not
     vet" rather than "empty", and every write raises AutoNudgeStoreUnvetted so the store is
     not overwritten with nothing.
• The malformed-entry warning is a fix to a pre-existing leak, not new
  hardening. Base autonudge.py:579 logged
  "AutoNudge: skipping malformed loop entry: %r" of the entire row —
  message and any credential inside it — into the log ring and /api/logs,
  before any banner existed. It now logs the scrubbed id plus the field names
  present. That is a security fix and an operator-visible diagnostics change for
  all loops, riding under a display feature.
• A non-dict row ([null], [42], ["oops"]) is skipped rather than
  raising AttributeError out of _load, restoring the tolerance the old %r
  handler had.

Redaction, corrected from an earlier draft of this section: message is scrubbed
on the way in too. authorize_autonudge_write runs both redactors over it
(autonudge_authz.py:226-227), exactly as it does for banner, so the gap was
never input-side. It was the producers that BYPASS that authorizer — a
hand-edited autonudge.json, an internal svc.add — whose message reached the
persisted, broadcast transcript row unscanned, and only on the dashboard path:
_fire_slack_nudge already scrubbed its own replay row.

That gap is closed in code, not deferred. The no-banner branch now scrubs the
visible ROW at the sink (gateway.py:5721-5723) and deliberately leaves tagged
— the PROMPT — untouched, because rewriting the instruction the model receives
would corrupt the one guarantee a nudge exists to provide.

So the row is no longer unconditionally byte-identical, and that is the
intended trade. Redaction is idempotent and the authorizer already scrubbed
anything written through REST or MCP, so on every supported path the sink scrub
is a no-op and the row is unchanged; it alters a row only when that row's
message carries a credential-shaped span AND reached the store bypassing the
authorizer. Two independent arms pin both halves:
test_the_no_banner_row_is_redacted_like_the_slack_sibling (secret scrubbed from
the row, prompt intact, row != prompt) and
test_a_clean_no_banner_row_stays_byte_identical (a clean message still passes
through unchanged).

Rejected alternatives

• Move the payload to /context. That is the invisible channel, but
  drain_pending_context is one-shot and clearing, so it offers no per-cycle
  guarantee. The nudge's whole value is unconditional delivery every cycle;
  moving the payload would need a new per-cycle poster and forfeit exactly the
  property the nudge exists to provide.
• Shorten the loop's message. Deletes real instruction to quiet a display,
  and forfeits guaranteed delivery of whatever was removed.
• Always append a short row. Changes behaviour for every loop and removes the
  legitimate case where seeing the full nudge text is what you want.

Hence: opt-in, per loop.

Backwards compatibility is the acceptance bar

No store-version bump. _load filters unknown keys:

loop = NudgeLoop(**{k: raw[k] for k in raw if k in NudgeLoop.__dataclass_fields__})

so persistence is already tolerant in both directions:

• Old autonudge.json (no banner) on new code → field takes its default.
• New autonudge.json (with banner) on old code → key filtered, no
  TypeError. A downgrade degrades to today's verbose display rather than
  crashing.

_STORE_VERSION stays at 1. Bumping it would signal a breaking change that is
not happening. The field is appended last in the dataclass for the same
reason — and because nothing constructs NudgeLoop positionally, verified by
grep across src/ and test/.

The default path is unchanged for a clean message.
test_no_banner_appends_the_pre_change_string pins the appended row against a
literal rather than against the prompt, so it would still fail if both sides
changed together. It stays green because the sink scrub is a no-op on a message
with nothing credential-shaped in it — the row differs only in the
credential-bearing case, which
test_the_no_banner_row_is_redacted_like_the_slack_sibling pins.

Deliberate decision: the WS broadcast does not carry banner

_observer in _init_autonudge builds an explicit dict rather than using
asdict, so a new field is not picked up unless added by hand. It is not added,
for three reasons:

1. That dict is a curated set of fields the sidebar and goal popover render.
   There is no banner control in the UI, so the field would be dead payload on
   every added / updated / fired / removed / expired event.
2. The banner's effect is already fully observable through the surface it exists
   for — the transcript row — which reaches the browser via the normal chat
   stream path.
3. GET /api/autonudge and the POST/PATCH responses go through _serialize,
   which starts from asdict and so gains the key automatically. Any consumer
   that wants the value has a read path today.

Adding it later is additive and non-breaking; the reverse is not true. If a UI
control lands, that PR is the right place to widen the broadcast.

The broadcast does, however, gain a scrub. _observer now renders the loop's
message through the same scrub_loop_text helper the REST serializer uses — one
definition, two callers — so the autonudge_state payload cannot serve a
credential-bearing message verbatim to every connected browser. The helper is
type-dispatched, so the nine declared numeric and boolean fields pass through
untouched and clients can still do arithmetic on them.

Corrected from an earlier draft of this section, which is the claim both the design
and first-principles lanes flagged: _serialize is no longer plain asdict. It
is asdict plus a DENYLIST scrub — every field is scrubbed through
scrub_loop_text unless it is one of the two addressing fields, and the nested
monitor mapping is routed through the structure-preserving _redact_monitor_value
walker instead, so its shape survives while every nested string is still redacted.
A denylist was chosen over an allowlist because an allowlist silently misses the
next free-text field added to NudgeLoop — which is exactly how banner came to
need a scrub of its own. Non-string values are not skipped either: an
agent-written message: ["AKIA..."] used to be emitted verbatim. The REST payload
still grows the one banner key — additive, and no consumer does exhaustive key
validation. svc.add/svc.update gain a defaulted kwarg, so the other callers
(dashboard/server.py's ctx.nudge bridge, session_directive_apply,
spec_builder's handoff) are unaffected.

Beyond the original design sketch

Three things the sketch did not call for, each found by writing the tests:

1. The banner is redacted for credentials and exfiltration URLs, on both the
   arm and update paths, exactly as message is. It is caller-supplied,
   persisted to the loop store, and broadcast to every connected browser as a
   transcript row on each fire — the same exposure with a shorter string. Being
   display-only is what makes this easy to forget. The update path needs it
   independently or it is a trivial bypass of the arm-time guard.
2. The fire path strips independently of the authorizer. A whitespace-only
   banner is truthy, so the row would render blank — worse than the verbose row
   it replaced, because it hides the cycle body and puts nothing in its place.
   This is not a duplicated guard: the authorizer normalizes REST/workflow input,
   while this covers a loop that reached the store another way (hand-edited
   autonudge.json, an internal svc.add). Caught by
   test_whitespace_only_banner_is_treated_as_absent, which failed on the first
   run.
3. Banner validation sits at the cap site, not beside the message redaction
   at the top of authorize_and_add_nudge, so a rejection routes through _deny
   and lands in the SEL audit like every other refusal on that path. banner is
   also added to the update path's invoked audit fields list.

Scope

Dashboard transcript row only. Channel-bound loops (slack: / discord: /
webex:) deliver the nudge as the turn's own input and have no separate display
surface to shorten, so a non-blank banner on one is refused with a 400 at both
authz chokepoints via banner_unsupported_for (pinned by
TestChannelBoundLoopsRefuseABanner). The reason is mechanical: _fire routes a
channel key to _fire_slack_nudge / _fire_discord_nudge / _fire_webex_nudge,
none of which reads loop.banner; both read sites sit inside
_fire_dashboard_nudge. Storing a setting the runtime can never honour — and
returning 200 for it — left the caller no way to notice but the row not changing.
A blank banner is still accepted there, because banner="" is the default every
channel-bound caller already passes.

monitor_start and monitor_update do gain an optional banner — see the
MCP surface note under "What changed". No change to /context,
drain_pending_context, /note, or any loop's message content. No UI change —
the field is opt-in via the API and MCP.

What applies to EVERY loop, banner or not

The banner field itself is opt-in, but several changes here are not scoped to loops
that use it. Stated plainly because it bears on how this PR can be reverted:

• Every REST and WS read is now credential-scrubbed. _serialize and the
  autonudge_state broadcast run every string field through scrub_loop_text, so a
  no-banner loop's message and stopped_reason are no longer byte-identical on the
  wire. Numeric fields are exempt BY FIELD NAME, so client arithmetic is unchanged.
• One declined row is quarantined, not fatal to the store. A persisted id or slot_key
  that is credential-shaped, non-printable or non-str is held aside under quarantined while
  its SIBLINGS arm normally, and the file is left untouched. _load_refused is set only by the
  whole-store arm, for a host whose credential policy will not compose. This applies to loops
  that never set banner. Recovery is repairing the store and
  restarting the process, and while it holds, arming through the API fails too. That is the
  deliberate trade: the alternative dropped the row the operator was told to fix, or left healthy
  loops armed against a store that refuses every write. A malformed row is the one case still
  dropped, with loading continuing past it.
• Both authorizers gain 503 outcomes. An unusable credential policy now denies
  with an audited 503 instead of raising, on the arm and update paths alike,
  including for requests that carry no banner.
• The MCP monitor_* schemas changed shape to carry the optional field, which
  every caller of those tools sees.

This was previously recorded as an open question. It is now DECIDED in the direction
both review lanes asked for: the cron.py and ops_mission_control loader fixes have
been removed from this PR and will ride their own. What remains under the banner is the
autonudge surface itself -- its loader, its serializer, its broadcast and its
authorizers -- which is genuinely entangled with the feature and cannot be split the
same way.

The redacted projection is now visible in the UI

GET /api/autonudge serves a credential-SCRUBBED projection of message, and
AutoNudgePopover seeds its textarea from it. Two hazards followed, both invisible to the
person typing, and both are now surfaced rather than merely guarded server-side:

• The mask is marked. When the projection sets message_redacted, the popover renders a
  notice on the textarea saying credentials are shown masked and that saving an edit stores
  the masked text. Previously a user whose goal legitimately contained credential-shaped
  text saw [REDACTED: ...] in their own words with no explanation.
• The echo-drop is no longer a silent success. A PATCH that re-submits that exact masked
  text is dropped by the server's echo guard, which answers 200 with message_ignored: true
  -- a singular boolean, because message is the only field this path can ever drop. The
  popover reads it, says in prose that the goal text was left unchanged while the other
  settings were saved, and stays open instead of closing on a save that did not fully happen.
  The notice interpolates nothing: rendering the raw wire key gave an ambiguous English noun
  phrase and an untranslated Latin token in every other locale.
• The overwrite now needs an explicit act. Editing the textarea on a redacted loop and
  pressing Save no longer writes: the button becomes "Replace goal with masked text", and only
  that second press sends the PATCH. The overwrite is irreversible and the server cannot return
  the original, so passive 11px copy was the only guard on a destructive default. A user who
  changed only interval or cycles is NOT gated, and message_redacted_notice now names the
  safe path ("Leave the text unchanged to keep the original").

This is also the answer to First Principles' Subtraction, which asked for
message_redacted and the ignored-field report to be deleted on the stated ground of ZERO
consumers. Wiring the popover gives both fields a real consumer, so that premise no longer
holds. Deleting them instead would not have cured the UX hazard: with message_redacted
gone the popover would still seed from the scrubbed message, and the silent overwrite would
become invisible rather than fixed.

Covered by website/src/test/AutoNudgeRedactedProjection.test.tsx, including a negative
control that fails if the notice renders unconditionally.

Tests

test/test_autonudge_banner.py, 85 tests. Two carry the change:

• Default byte-identity — the acceptance bar. Also covers a ledger-snapshot
  body (the branch that must still route through compose_nudge_body),
  whitespace-only input, and the dataclass default.
• test_row_shows_the_banner_and_the_prompt_keeps_the_message — asserts on
  the argument handed to _run_chat, not only on slot.append. A test checking
  only the row would pass just as well if the prompt had also been shortened,
  which is precisely the defect this must not introduce. Four assertions, because
  three can pass while the change is still wrong.

Plus: the ledger snapshot stays in the prompt and leaves the row; the cycle prefix
survives the banner branch; {{STOP_FILE}} renders; the row shrinks below 100
chars while the prompt stays above 6000; both persistence directions including a
downgrade simulation against a pre-field dataclass (with a negative control
proving the key filter, not the dataclass, does the work); the cap at and one over
the boundary on both POST and PATCH; non-string rejection; None vs ""
semantics differing correctly between arm ("" ) and update (leave alone); and
redaction on both paths.

14 break-arm controls run, 14 confirmed detectable — one per load-bearing
assertion, including the prompt-shortened defect (fails on
assert loop.message in prompt), an off-by-one cap (> → >=), a dropped
type check, dropped redaction on each path independently, and a dropped
handler pass-through.

Verification

Full suite, same venv, changes stashed and unstashed to isolate the diff as the
only variable:

• baseline upstream/main @ c7f5ba788: 68,848 passed / 57 failed / 35 errors
• with this change: 68,869 passed / 58 failed / 35 errors

Set-diffed by test ID. The single remaining delta is
test_pr_watchers.py::TestRegistrySurface::test_get_log_shape_and_incremental_since,
which fails 3 of 5 runs on the pristine baseline in isolation — a pre-existing
flake in the auto_improvement PR-watcher suite, whose "nudge loop" is an unrelated
concept. The other pre-existing failures are git commit and openssl genpkey
subprocess errors on this host, identical on both sides.

Two hand-rolled test doubles needed updating rather than working around: the
FakeNudgeSvc.add signature in test_workflows_nudge_wiring.py and the _Loop
stand-in in test_unattended_slot_guardrails.py. Both are explicitly test
doubles and production only ever receives a real NudgeLoop, so a defensive
getattr(loop, "banner", "") in the fire path would have been fitting the code
to the test — and would mask a genuine type error later.

Why no screenshot: this revision DOES change what the popover renders -- that earlier
"no pixels" claim is superseded and wrong. Two notices are added above and below the goal
textarea: an amber line reading "Credentials in this goal are shown masked. Saving an edit
stores the masked text, replacing the original instruction. Leave the text unchanged to keep
the original." when the served projection sets message_redacted, and an amber line reading
"Your goal text was left unchanged; the other settings were saved." after a PATCH returns
message_ignored. A third change is the Save button itself, which becomes "Replace goal with
masked text" for one press when an edit would overwrite a redacted goal. All reuse the existing
text-warning text-[11px] mb-1 treatment already used elsewhere in this popover, so no new
styling, spacing rule or layout is introduced -- the delta is two conditional text rows.

No captured image is attached because Chromium cannot launch in this build environment
(browserType.launch: Target page, context or browser has been closed, reproduced with and
without --no-sandbox --disable-dev-shm-usage --disable-gpu), and fabricating or hand-drawing
a mock would be worse than saying so. The rendered result is pinned instead by
website/src/test/AutoNudgeRedactedProjection.test.tsx, which asserts both notices by
data-testid and includes a negative control that fails if the redaction notice renders
unconditionally. A maintainer who wants a real capture should ask for one.

The reason for the change is a review finding from two lanes: the GET that populates the
popover returns a SCRUBBED projection of message, so echoing it back unconditionally
overwrote the stored message with its own redaction. The server had been detecting that
echo and silently dropping the field; the dirty check fixes the cause, and the remaining
server guard now logs when it fires instead of being silent.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[dwu96]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 401cba6fbfa8e028eb1fd7988d4847c6603101e5 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

The banner itself is sound and well-bounded; the concern is the fleet-wide store/egress machinery riding under it, which dominates the PR and its revert story.

Watch

• Revert entanglement is only partly cured by the earlier split. The _serialize denylist scrub, WS scrub, message_redacted/message_ignored surface, echo-projection guard, and the popover confirm flow are all message-specific — none depends on banner ("several changes here are not scoped to loops that use it," per the description). Reverting the display feature also reverts every-loop wire behavior and new PATCH semantics; these could still ship as their own PR.
• The quarantine sidecar applies its preservation principle to one corruption class only. A row with an unusable addressing field gets a second durable store, two-phase additive-write/compact ordering, .corrupt-<ts> move-aside, and process-wide write refusal — while the sibling malformed-row arm still lets the next wholesale write delete an operator's row ("a malformed row is the one case still dropped"). Both classes lose the same data; the divergence is pinned as deliberate but justified by review-round history, not by differing harm. Future maintainers own two recovery contracts plus new restart-required failure states in a display feature's subsystem.

Suggestions

• If row preservation is the invariant, route the malformed-entry arm through the same quarantine sidecar in the follow-up sweep — reusing the machinery is cheaper than maintaining the documented divergence.

[DESIGN-REVIEWED] 401cba6

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 401cba6fbfa8e028eb1fd7988d4847c6603101e5 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered: the full patch, the intent file, cron's sibling loader contract (cron.py:4083 — a bad entry is warned and "dropped from the store on the next write"), and the dashboard's NudgeCard (the row already collapses to a one-line chip; the body renders only on expand). Here is the review.

First-Principles-Verdict: CONCERNS

The banner earns its place, but a ~200-line quarantine-sidecar subsystem rides in to save rows this codebase's own sibling contract says to drop.

What this change ships

Intent: show one short operator-authored line per nudge cycle instead of re-storing the multi-KB instruction — ADDITION.

1. Optional per-loop banner replaces the transcript row's body; the prompt is untouched — justified (measured 51.8%)
2. PATCH accepts banner, so a running loop is quieted without resetting budgets — justified
3. MCP monitor_start/monitor_update gain banner; the agent can author its own row — justified, asymmetry declared
4. Non-blank banner on a channel-bound loop refused with 400 — justified (dead config otherwise)
5. REST and WS now serve a credential-scrubbed message plus a message_redacted flag — rides along, boundary-derived
6. PATCH echoing the scrubbed projection is dropped; response carries message_ignored; popover warns — rides along
7. Popover confirm-overwrite gate before saving an edit to a masked goal (13 locales) — rides along, third defense layer
8. Loader blanks a non-string/oversized banner; all load warnings share one scrubbed spelling — justified
9. Rows with unsafe id/slot_key never arm; held in a new autonudge.quarantine.json sidecar with .corrupt-<ts> move-aside — oversized
10. Uncomposable host credential policy: 503 on writes, arms nothing, persists raise — justified fail-closed

Watch

• The sidecar rebuilds the cliff the description says Design flagged for option 3: an unreadable sidecar now arms nothing and refuses every write until restart — a fleet-wide failure mode that exists only because the sidecar does.
• The description's own split rationale ("a revert of the banner also reverts fleet-wide hardening") is applied to two loader redactions but not to items 5–7, a strictly larger separable hardening stack about message, a field this feature never touches.
• The banner is an opt-in patch on a universal cost: every bannerless loop still stores ~8KB/cycle, and the MCP schema now coaches the model to set one "whenever message is long" — the cause (the row duplicates the full body per cycle for expansion) is untouched, and an expanded banner card no longer shows what was actually sent.

Subtractions

• Delete the quarantine machinery (_QUARANTINE_FILE, the seven _*quarantine* methods, _quarantine_row_key, the spec's sidecar paragraph): warn-and-skip already suffices — cron.py:4083 documents drop-on-next-write for exactly this store class, and this same diff keeps that fate for its malformed-row arm.
• Defer items 5–7 (the _serialize/WS message scrub, echo guard, message_redacted/message_ignored, confirm UI + 13 locale files) to the hardening follow-up this PR already created for the two loader redactions.

[FIRST-PRINCIPLES-REVIEWED] 401cba6

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 401cba6fbfa8e028eb1fd7988d4847c6603101e5 via the fork AI-review pipeline; updated in place on each push.

Review details

The candidate dies under falsification:

• The shipped client never echoes the projection. AutoNudgePopover.save sends patch.message only when message !== (loop.message ?? '') — a genuine edit. The unchanged over-cap projection is never resubmitted, so the pure-echo 400 path is not reachable from any shipped client, and the echo-drop guard is a pure backstop.
• The triggering input is unconfirmed. Reaching the harm requires a stored message near 8000 chars carrying enough credential-shaped tokens that the scrubbed projection crosses 8000. The candidate's own reasoning could not establish that this occurs in practice — condition (a) resolves to "could," which the instructions say to drop on.
• The outcome is not a blocking class. In every branch the stored message is preserved (echo → message=None drop; over-cap → 400 writes nothing), so there is no data loss/corruption. The residual case — a user editing an already-over-cap displayed goal without shortening it — is the pre-existing 8000 cap correctly applying to edited text, a UX annoyance at most.

No new grounded defect surfaced in the changed lines.

No findings.

[OPUS-REVIEWED] 401cba6

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed 401cba6fbfa8e028eb1fd7988d4847c6603101e5 via the fork AI-review pipeline; updated in place on each push.

BLOCKING -- src/kiro_crew/autonudge.py:968 -- Failed replacement duplicates a quarantined row
self._quarantined.append(deepcopy(raw))
Main-store replace fails after sidecar write -> next load reads the unsafe row from both files -> subsequent repair persists duplicate quarantine records.
Anchor: residual/crash-data-loss-corruption
Fix: Deduplicate exact rows with _quarantine_row_key before appending.

[BLOCK-MERGE] 401cba6
[GPT-REVIEWED] 401cba6

[bolichen97]

Full-diff overlap audit after rereading updated head 77bbecb2ebe7f0c20498048a5d613e506a3b7bc6: #5999 and #5184 change the same AutoNudge wire contract—NudgeLoop/service persistence, authz, REST serializers, directive and MCP schemas, and dashboard/Slack/Discord-orchestrator delivery sinks. #5184 introduces structured monitor create/update/stop/probe state; #5999 adds a separate visible banner while retaining the loop message as the actual prompt. They are complementary features, not duplicates, but cannot safely merge as divergent public/persistence schemas.

Please integrate the structured-monitor stack first (#5184 → #5185 → #5186 → #5305), then rebase this banner work and decide once whether banner is a legacy-loop field or part of public MonitorState. Preserve this updated diff's security invariants across create/update/restore/observer paths: canonical redact_via_context/scrub_loop_text, fail-closed composition, redacted projection without overwriting the stored raw message, channel-bound authorization, and refused-row persistence/eviction rollback. The same field and scrub contract must be used by REST, directive, MCP and every delivery sink.

[github-actions]

UX Review (Fable 5, fork) — 🟡 CONCERNS

UX-level review of 401cba6fbfa8e028eb1fd7988d4847c6603101e5 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

I have everything I need. The transcript append site confirms the nudge row's meta gains no banner indicator, and the frontend confirm flow, strings, and locales are all in view. Composing the review.

UX-Verdict: CONCERNS

A banner row is pixel-identical to a real nudge row, so the transcript silently stops showing what the agent was actually told.

Watch

• Unmarked banner rows. slot.append("nudge", visible, ...) keeps meta={"nudge": {"cycle", "loop_id"}} with no banner flag, and the banner is agent-settable via monitor_update — so a user auditing "why did my agent do X" reads a short row they've learned means "the injected message" while the real ~8KB prompt differs. Every cycle of every banner loop, misleading on the product's main audit surface; and rows persisted today without a meta flag can never be retro-distinguished. Fix: add banner: true to the nudge meta now (client indicator can follow).
• Raw 503 copy in the popover. _deny("credential policy unavailable — message cannot be compared", 503) renders verbatim in the error line via data.error — internal jargon with no next step for a user pressing Save. Rare (broken composed policy), but the user's only exit is guessing. Fix: user-vocabulary text ("Couldn't verify this text for secrets — try again or check the host's security settings").

Suggestions

• message_redacted_notice says "replaces the original instruction" while confirm_overwrite_question and both buttons say "goal" — use "goal" in the notice; also replace "a flagged URL" (internal redaction vocabulary) with "suspicious links".

[UX-REVIEWED] 401cba6

[bolichen97]

Reviewed the full diff as part of a maintainer sweep of readiness: passed PRs. All 60+ checks pass and the branch is not API-stale (render_nudge_message, compose_nudge_body and redact_via_context all still exist on main; 1 ahead / 17 behind, MERGEABLE). The banner feature core is sound and genuinely default-off (banner: str = "", prompt untouched, test_no_banner_appends_the_pre_change_string). Three things still keep me from approving.

1. Blocking correctness bug in this PR's own new code.

src/kiro_crew/dashboard/handlers/autonudge.py:284 builds:

ignored_fields_for_update({"message": None if echoed else body.get("message")})

When the client omits message, body.get("message") returns None, so the helper's key-present-and-None test fires and the 200 falsely reports ignored_fields: ["message"]. That omission is not an edge case: it is the shipped popover's own normal path, and it is every {"active": false} stop. This is deterministic, not intermittent. Opus 4.8 flagged it as advisory.

The new test file does not exercise it: the test calls the helper with svc.update.await_args.kwargs rather than the handler's own construction, so the handler's argument shape is never asserted.

2. New silent-overwrite path (data loss).

_serialize now scrubs message for every loop, but AutoNudgePopover.tsx:56/144 still seeds the textarea from loop.message and never reads the new message_redacted flag. Editing a loop whose message contains credential-shaped text therefore persists the redaction over the real instruction. GET used to return the raw message, so this is a regression the PR introduces rather than pre-existing behavior. This is the UX Review CONCERNS finding.

3. Scope, which the description itself leaves open.

6218 additions for an opt-in display field, carrying unrelated cron.py and ops_mission_control log-redaction fixes, a fleet-wide REST+WS scrub that breaks byte-identity for all loops, _load row-refusal plus whole-store AutoNudgeStoreUnvetted semantics, and an MCP schema change. The body records that two reviewers judged it too broad and leaves "should these ride here or in a follow-up" unanswered. Design and First Principles are both at CONCERNS, the latter noting that message_redacted and ignored_fields ship with zero consumers.

There is also a sequencing point: the #5184 to #5186 monitor stack is still open and rewrites the same AutoNudge wire and persistence contract, so landing this first means resolving that overlap twice.

I would approve a version of this that is the banner alone, with items 1 and 2 fixed. Auto-merge is not armed here; nothing lands while this is open.

[bolichen97]

Closing — superseded by your own re-cut, #7777

Verified relationship: functional overlap

Both are OPEN, both are exactly ONE commit off the SAME merge-base d402acd (no stacking), and origin/main carries no banner at all: no banner field, no MAX_BANNER_CHARS, and _serialize is still plain asdict + monitor redaction. Same author rnoack1, byte-identical title, and 7777's branch is literally feat/autonudge-banner-split-a off 5999's feat/autonudge-banner -- it is the author's own re-cut. On the CURRENT heads, seven files land byte-identical post-change blobs (constants.py dee030df, chat_handlers.py a61dc380, session_directive_apply.py 0be3aae7, validation.py f251413e, test_slot_close_nudge_race.py 08f4f934, test_unattended_slot_guardrails.py 32cb5d3f, test_workflows_nudge_wiring.py d3f9e079); banner: str = "" arrives with a byte-identical 22-line comment block and is threaded through the same six call sites; normalize_banner and banner_unsupported_for are the same two helpers with the same signatures and the same error strings; and both ADD test/test_autonudge_banner.py, where 60 of 7777's 74 top-level names are also in 5999's copy. Only one can land: 5999's TestTheBannerStaysOptIn::test_no_shipped_producer_sets_a_banner asserts "banner=" not in inspect.getsource(chat_runner) while 7777 writes banner=normalize_banner(_objective, absent_ok=True, truncate=True)[0] into chat_runner and asserts that literal is present; both also register autonudge.py in security_posture.py with contradictory rationale text. 7777 survives because it is exactly what maintainer bolichen97 said he would approve on 2026-08-30 ("I would approve a version of this that is the banner alone"), it delivers the /goal producer without which the measured bloat is only conditionally removed, and its own blocking findings are closed on its current head (redaction moved to the write path incl. /goal, plus a _load scrub of banner AND message). 5999 cannot land as written: a live GPT 5.6 BLOCKING data-corruption finding on the quarantine sidecar, the maintainer's blocking ignored_fields correctness bug, a maintainer-identified silent-overwrite data-loss regression, and Design + First Principles both asking for the rider bundle to be split out. Close 5999 only AFTER its ~5,700 lines of non-banner residue are re-cut as the split-b PR its own body promises; no such PR exists among the 280 open ones today.

Carry this over first

This closure is about redundancy, and these items are the exception: they are not on main and not in the surviving PR, so they need a home before the topic is finished. Please don't let them go with the branch.

Re-cut as the promised split-b BEFORE closing 5999; none of it exists anywhere else. (1) src/kiro_crew/dashboard/handlers/autonudge.py::_serialize -- the denylist egress scrub (_UNSCRUBBED_FIELDS = ADDRESSING_FIELDS, per-field scrub_loop_text, monitor routed to _redact_monitor_value) plus the out["message_redacted"] wire flag; main still serves plain asdict. (2) src/kiro_crew/autonudge.py::scrub_loop_text and ADDRESSING_FIELDS -- the shared type-dispatched per-value redactor both the REST and WS surfaces call, so they cannot disagree on what is credential-shaped. (3) src/kiro_crew/slack/gateway.py::GatewayOrchestrator._observer -- the autonudge_state WS broadcast scrub (safe_message = scrub_loop_text(loop.message, field="message") + message_redacted), and the no-banner fire arm visible = redact_via_context(tagged). (4) src/kiro_crew/autonudge_authz.py::message_is_echoed_projection and ::_scrub_policy_unavailable -- the PATCH echo guard and the audited 503 for a host whose credential policy will not compose; plus handlers/autonudge.py's message_ignored: true response key. (5) website/src/components/AutoNudgePopover.tsx -- the editsRedactedGoal overwrite-confirm gate ("Replace goal with masked text") and the two amber notices, their 5 i18n keys across all 13 locales, and website/src/test/AutoNudgeRedactedProjection.test.tsx (409 lines). (6) src/kiro_crew/autonudge.py quarantine sidecar -- _QUARANTINE_FILE / _quarantined / _load_refused / AutoNudgeStoreUnvetted / _read_quarantine_sidecar, holding aside a row whose id/slot_key is credential-shaped, non-printable or non-str while its siblings arm; carries the live GPT 5.6 BLOCKING finding at autonudge.py:795 (an unreadable sidecar still arms loops) which must be fixed in the re-cut, and the maintainer wanted it in the deferred hardening PR regardless. (7) src/kiro_crew/security_posture.py -- the "Auto-nudge loop inventory" egress row and the autonudge_state broadcast sentence; reconcile rather than copy, since 7777 registers autonudge.py with different text. (8) src/kiro_crew/autonudge.py::_load -- the malformed-entry warning fix (scrubbed id + field names in place of %r of the whole row, a pre-existing leak on main) and the non-dict-row tolerance. Nothing needs harvesting from 7777 into 5999.

Current state

Neither side has merged. the issue/PR reference check lists both as open, and origin/main (1a765b8) contains no part of either: git grep MAX_BANNER_CHARS/normalize_banner/banner_unsupported_for origin/main -- src/kiro_crew returns nothing, NudgeLoop has no banner field, and handlers/autonudge.py::_serialize is still payload = asdict(loop) plus the monitor-only redaction. So no landed work covers either side and the ruling rests entirely on the two live heads (both 1 ahead / 25 behind the same merge-base d402acd).

---

_{From a repository-wide duplicate/overlap audit of every pull request open against main (2026-09-02, 330 PRs, one reviewer per PR). Each PR was read as its full merge-base diff plus its description and every comment and review, then compared against each candidate PR's own diff and against origin/main at 1a765b88ceb7. Findings that implied a closure were re-adjudicated independently, including an adversarial pass whose only job was to refute them; the reasoning above is what survived. If it is wrong, reopening costs nothing — please say so, and treat the reasoning rather than the outcome as the thing to correct.}