refactor(core): compact always-on brains guidance [BRNS-CORE-007]

[stefan-ssv-labs]

What

• compacts the shared Claude/Codex core.md while preserving personal-memory routing, agenda semantics, empty-read fallback, source attribution, dependent-read sequencing, feedback routing, and skill delegation
• corrects brains-write to use structured actions, explicit failure/rate-limit handling, agent-side draft discard/correction, and out-of-band confirmation
• releases both manifests as 2.3.3 and advances the reported core marker to v5
• adds contract coverage for the compact core, action boundary, aligned manifest versions, and marker report

Why

CORE-007 is the mechanical Lever-A cut: remove repeated always-loaded procedure and stale action guidance without moving unique behavior behind skills that web-less clients cannot load.

Capture wording and hooks are intentionally unchanged. Open PR #10 owns agenda/build/integrations guidance and has no file overlap with this PR.

The current main already publishes 2.3.2, so this PR uses 2.3.3; existing installations only recopy plugin files when the manifest version advances.

Release ordering

This public plugin PR must merge before the companion ssvlabs/brains catalog-v5 migration is allowed to merge. No separate manual publication action is required here: merge to protected main is the marketplace publication path.

Validation

• bun run tests/plugin-contract/run.ts — PASS
• bun run tests/inbox-v2/run.ts — 32/32 PASS
• bun run tests/tool-error/run.ts — 14/14 PASS
• git diff --check — PASS
• @dqbd/tiktoken@1.0.22, cl100k_base relative proxy: core.md 890 → 621 tokens (−30%); not Anthropic billing tokens
• signed maintenance and base-sync commits verified locally

[sebastian-ssvlabs]

Reviewed and verified against the deployed MCP server (confirm-hint.ts, ingest.ts, docs/mcp.md): the confirm_hint, out-of-band-only approval, structured source-bound dispatch, and dropped edits all match server-enforced reality — this correctly retires stale guidance that would otherwise fail at runtime. Contract-test ratchets are strong. No blocking findings; two minor nits inline.

[stefan-ssv-labs]

✅ review-pr (self-review, cannot approve own PR): clean — no blockers or criticals.

[RaekwonIII]

Reviewed the compaction against the live server behavior, not just the diff. Verdict: strong. The brains-write rewrite is a genuine correctness fix, not merely a token cut.

Verified against the brains server:

• The new confirm-flow ("do not call confirm_action from this agent loop; the user approves out-of-band via /inbox or Telegram") matches the enforced model — the confirm nonce is minted at draft time and delivered only to out-of-band surfaces, so the drafting agent structurally can't self-confirm (apps/mcp/src/act/state.ts, confirm-nonce.itest.ts: "the agent-loop self-confirm hole is closed"). The old "get consent, then confirm_action draft_id=…" was stale/wrong.
• The legacy-fallback rewrite is also correct: act_on_integration supports source+action_name+input, and a bare source+free-form request is not translated server-side (ingest.ts returns {kind:'clarification'}). So "never replace the tuple with a free-form request" is right.
• confirm_hint is a real field the server tells clients to relay. ✓

Cross-surface consistency (follow-up, not in this diff): the same stale guidance still lives in the live act_on_integration tool description (apps/mcp/src/tools/ingest.ts:186), which is internally contradictory — one line says "follow up with confirm_action(draft_id)" while a later line says "Calling confirm_action from this agent loop will not work." Every client sees that at tools/list. Out of scope for this repo, but it should ride CORE-006 (the MCP tool-description slim) so the plugin and the tool surface stop disagreeing. Recorded on the brains side.

Nits (non-blocking):

• Smart quotes: the new core.md swaps ASCII '/"/- for curly glyphs + em-dashes. Ironic in a token-reduction PR — curly punctuation can tokenize to more tokens than ASCII and breaks consistency with the rest of the tree. Prefer ASCII.
• The SKILL.md draft shape {kind:"draft", draft_id, preview, confirm_hint, expires_at} drops action/payload that the server's stated shape includes. Harmless, slightly inaccurate.
• The feedback block trimmed the batching / "capture at the moment" / "don't staple onto unrelated feedback" nuances (it keeps the "once per distinct error" throttle). Those "when to offer" rules can't relocate into brains-feedback (that skill loads only at filing time), so it's a small behavior trim rather than pure dedup — acceptable, just noting it.

Nice: the new contract tests are the highlight — they pin the routing signals, the <3000 char bloat ratchet, the self-confirm prohibition, and a regression guard that a free-form request= can't come back. That's exactly how you keep a compaction from silently rotting.

[olegshmuelov]

Reviewed against the fetched PR head, with an independent adversarial pass and a separate fresh-context verifier. Confirmed CI is green on all three suites, both suites pass locally at head, and the manifest / marker / test alignment is consistent.

Two non-blocking issues below plus one nit. None of it should hold the merge — the compaction is sound and the earlier server-behavior verification stands.

One follow-up worth its own issue rather than a change here: the hook-map assertions in tests/plugin-contract/run.ts only cover PostToolUseFailure, SessionEnd, and PostToolUse, so SessionStart, Stop, or UserPromptSubmit could be dropped from either event map without failing — and an event key left as an empty array makes the command-validation loop iterate zero times. Pre-existing, not introduced here.

[olegshmuelov]

Follow-up to my earlier review — I chased the one question I'd left open, and it turned out to be real.

Also closing the other one for the record: the plugin-first merge ordering is safe. The drift query (apps/mcp/src/inbox.ts:456) is ds.version < s.version, strictly "device behind canonical" — so users on core v5 while the catalog still says v4 get no drift row and no spurious update nudge.

[stefan-ssv-labs]

Review-summary follow-up: 6c8b98f restores the full draft shape (action and payload included), keeps confirmation out of band while retaining agent-side discard/correction, and preserves the compact feedback/dependent-read behaviors called out in review. The hook-map assertion gap is pre-existing and outside this PR’s CORE-007 scope; the touched cross-client contracts remain covered by the plugin, inbox-v2, and tool-error suites.

[olegshmuelov]

Re-review of 6c8b98f. All six earlier threads are addressed — verified at the head rather than from the replies, and the bounded request= regex demonstrably catches the wrapped reintroduction the old [^\n]+ missed. 2.3.3 is right after #15 moved main to 2.3.2, core v5 against catalog v4 is safe under the strict ds.version < s.version drift query, and all three suites pass locally at the head.

Four findings below, each traced to origin/stage. Two are blocking: the rewritten legacy-fallback section documents a call shape the server throws on, and the merged auto_failed/rate_limited bullet gives one rule to two kinds whose rules are opposite.

One nit not worth its own thread: the discard assertion at tests/plugin-contract/run.ts:117 matches the substring call \discard_action`, so it also passes on "never call discard_action`" — my own suggested regex inherited that shape. Worth an inverted-phrasing guard if that file gets touched again.

[stefan-ssv-labs]

✅ review-pr (self-review, cannot approve own PR): clean — no blockers or criticals.

[RaekwonIII]

Heads-up — coordination with the now-merged #1039 (BRNS-MCP-037, @oleg), which landed the act_on_integration contract rewrite on brains stage this morning.

Verified against the merged code (apps/mcp/src/tools/ingest.ts:296-300): the legacy-fallback line in this PR's SKILL.md — "dispatch the same structured action_name + input with its source instead of install_id" — now throws "codex path requires install_id + action_name + input together (got partial set)". The partial-set guard fires before the source→install shim, so source + action_name + input is not a valid invoking shape. The only shape that invokes an action is install_id + action_name + input (legacy sources included); a bare source (±request) returns {kind:'clarification'}.

This also corrects my earlier review note on this PR: I validated that line against the pre-#1039 tool description, which itself inaccurately claimed the source-tuple worked — #1039 established ground truth and dropped it. Suggest updating the legacy-fallback guidance to the install_id tuple before merge, so the plugin doesn't teach a shape that errors.

FYI #1039 aligned the same act-contract prose across five model-visible surfaces including the gbrain shared manual that CORE-007 #1016 also rewrites — worth a quick sync with @oleg so the two efforts land consistent (see the note on #1016).

[stefan-ssv-labs]

Confirmed against the merged #1039 implementation and addressed at the current head 7d01c29: the plugin now teaches only install_id + action_name + input for Gmail, Calendar, and Drive actions; a bare source is explicitly non-drafting. The same commit also separates rate_limited (not attempted; retry after the delay) from auto_failed (attempted; outcome unknown; never blind-retry), and all three plugin contract checks are green.

[olegshmuelov]

Re-review of 7d01c29. All four findings from my last pass are genuinely fixed — checked against the diff rather than the replies — and each landed a pinning assertion, including the inverted-phrasing guard for the discard_action nit. Two changes I didn't ask for are also right: requires_confirmation:false no longer claims to be read-only, and discovery now resolves frontmatter through get_page instead of assuming query returns it. All three suites pass locally at this head.

Two findings left, both traced to origin/stage. The first is the last rule from ACT_CONTRACT_COPIES that this copy still contradicts; the second is a leftover pointer from the legacy-fallback fix.

Worth its own ticket rather than more rounds here: this SKILL.md is a sixth model-visible copy of the act_on_integration contract and the only one outside the drift gate's reach, which is why it has drifted from the contract in two consecutive review rounds. apps/mcp/src/tools/AGENTS.md says to add each new copy to ACT_CONTRACT_COPIES in the PR that creates it — not possible from a separate public repo, so the four rules need mirroring into tests/plugin-contract/run.ts instead.

[nir-ssvlabs]

Approving. Reviewed the current head 89dfa862d — the one commit that landed after the last review round — and it closes the two findings left open there, cleanly and with pinning assertions:

• Absent requires_confirmation is now its own bullet (outcome unpredictable → dispatch the tuple, branch on the returned kind, report in past tense) instead of being folded into the draft default — so the agent can no longer predict a draft for a pre-field page that actually executes requires_confirmation:false inline. Guarded by the new "absent … must remain unknown rather than predict a draft" assertion.
• The intro fallback pointer no longer sends the agent back to the source-enum shape the server rejects ("there is no source-enum fallback"), also pinned.
• The four act_on_integration contract rules are now mirrored into tests/plugin-contract/run.ts, which is the right home given this SKILL is the one model-visible copy outside the monorepo's drift gate.

The compacted core.md preserves the load-bearing routing (reflexive read, list_calendar_events vs list_pages for agendas, empty-read fallback, source attribution, dependent-read sequencing, feedback routing), and the brains-write rewrite matches server-enforced reality (out-of-band-only confirm, action_record_id, rate_limited vs auto_failed split). Manifest 2.3.3 and core marker v5 are aligned across manifests, tests, and the marker report. Nothing to add beyond the existing threads.

Release ordering LGTM: this merges first as the marketplace publication, which then unblocks the companion ssvlabs/brains catalog-v5 migration (#1016).

[stefan-ssv-labs]

✅ review-pr (self-review, cannot approve own PR): no blockers, no criticals. Ticket BRNS-CORE-007.

CI is green on this head (3/3), so those suites were not re-run locally. The review focused on whether the rewritten guidance matches the live runtime contract, and on the two threads that were open at 7d01c29 — the replies queued for those threads are attached to this review:

• An absent requires_confirmation is not a draft guarantee — delivered. plugins/brains/skills/brains-write/SKILL.md now carries the unknown-outcome branch, and tests/plugin-contract/run.ts pins it.
• The intro still described the legacy source-enum fallback — delivered. It now reads "there is no source-enum fallback", which matches apps/mcp/src/tools/ingest.ts: a bare source returns {kind:'clarification'} and drafts nothing.

I checked the rewritten return-kind table against ssvlabs/brains@origin/stage rather than taking it from the diff. action_record_id, auto_failed, and rate_limited with retry_after_seconds all exist on CodexDraftResult in apps/mcp/src/act/codex-dispatcher.ts; audit_id does not; and confirm_hint is stamped on kind:"draft" only, by withConfirmHint() in apps/mcp/src/tools/ingest.ts — so the documented draft shape is accurate. The "after one hour a draft moves to the Expired tab but remains approvable" line matches apps/mcp/src/act/state.ts ("Expiry is advisory: the inbox UI surfaces expired drafts on a separate" tab). plugins/brains/core.md is 2,601 bytes against the new < 3,000 assertion, so the budget has headroom.

Nothing here meets the blocker or critical bar. Merging this first also remains the stated prerequisite for ssvlabs/brains#1016.

[olegshmuelov]

Both findings from my last pass are fixed at 89dfa86, and the four act_on_integration rules are now mirrored into tests/plugin-contract/run.ts with the sixth-copy rationale written down — that's the right outcome. I re-checked all four rules against the file at this head and no stale phrasing survives (undefined = default, audit_id, Fall back to the legacy source-enum, source=<gmail, confirm_action draft_id, edits={ are all gone). Three suites green locally. Stage's contract hasn't moved since #1039.

Two findings below. Neither should reopen the review gate — one is pre-existing and one is low-impact — but both are one-line fixes in a file this PR is already rewriting for exactly this reason.

Checked and cleared: the missing clarification / noop bullets are defensible, not a gap. CodexDraftResult is draft | auto_executed | auto_failed | rate_limited; the other two come from the bare-source tail and verify-mode, both unreachable for an agent following the tuple this skill teaches.

Two follow-ups I'd rather file than hold this PR for:

1. The new assertions are satisfiable by self-contradictory copy. I perturbation-tested them: re-adding (or undefined = default) beside the absent bullet, reintroducing a source fallback in different words, and appending "always retry immediately" all keep the suite green (the positive Never blind-retry check still matches). The monorepo gate has block-local helpers plus an anti-inversion assertion for precisely this; the mirror copied the positive halves only. Still far better than the zero coverage two rounds ago.
2. Unrelated to this diff, but found while checking whether any other skill contradicts the corrected contract: get_overnight_digest (brains-agenda:25,52, brains-build:25) is not a registered MCP tool — the only reference on stage is a seed SQL file. And brains-build:22 describes workflow automations as "paused" when apps/mcp/src/tools/workflows.ts:178 says they are created active and "begin firing immediately."

On release order: #1016 carries db/init/338_catalog_core_v5.sql and is currently CLEAN/MERGEABLE with two approvals on stage, which requires none. If it lands first, the catalog reports core v5 while devices still report v4, the drift query fires, and users get an update nudge for a plugin version that isn't published yet — merging this PR to protected main is the publication path. Worth making that ordering explicit to whoever merges.

[nir-ssvlabs]

Re-approving — my earlier approval was dismissed by the new head b4ba6f7e, so I re-reviewed that commit (the only delta since). It closes Oleg's two 07:45 findings and I verified both against the live server source rather than the replies:

• Frontmatter field list now matches actionPageFrontmatter (apps/web/src/lib/integration-action-pages.ts:165-171): install_id / action_name / description / input_schema / requires_confirmation / side_effect. Dropping examples from the frontmatter list is right — examples renders into the page body (same file, line 214), which the commit now says. Adding side_effect restores the "known to write to the provider" signal the agent needs.
• Live Gmail path now names search_emails and documents write_pages: true, matching the canonical v13 action search_emails(query, limit, write_pages?=false) (scripts/seed-starter-recipes/integrations/gmail.ts:554,839,869); the dropped query_emails (v12) would have thrown on dispatch.

Everything from all prior rounds remains addressed, CI green, and the act_on_integration contract mirror in tests/plugin-contract/run.ts still pins the four load-bearing rules. Nothing critical — approving without restating the existing threads.

[RaekwonIII]

✅ Re-reviewed at b4ba6f7e — LGTM. Both prior findings are fixed correctly, and the recurring drift now has a structural gate.

Legacy source-enum fallback — gone. The intro now states "there is no source-enum fallback," and the bare-source-drafts-nothing line stays. ✓

Absent requires_confirmation — now its own rule: "the page predates the field, so the outcome cannot be predicted. Never assume it will draft…" — matching the corrected tool description (BRNS-MCP-037 / #1039). The full return-kind set (auto_executed / rate_limited / auto_failed / absent / draft) now reads consistently with the server contract. ✓

Anti-drift gate 👏 — tests/plugin-contract/run.ts now mirrors the act contract's load-bearing rules (structured tuple, absent→unknown, false→auto_executed, no source-enum fallback + a stale-pointer guard, action_record_id not audit_id, no self-confirm, and a free-form-request= regex). This closes the "sixth model-visible copy drifts because it's outside ACT_CONTRACT_COPIES" problem that caused two rounds of drift — exactly the fix we wanted.

Non-blocking:

• Plugin is now 2.3.3 (both clients aligned; the test enforces alignment). Core marker is still v=5, which is what the companion brains #1016 migration keys on — so they still align. Only the companion PR's prose still says "2.3.2"; worth a one-word bump so the two don't read inconsistent.
• The original hook-map assertion gaps (SessionStart not asserted / empty-array vacuous pass) aren't in this PR — that's separate scope, still tracked on our board.

Approving.