Skip to content

sync: update Matrix pilot with nearai/main 2026-06-20 - #24

Merged
github-actions[bot] merged 3 commits into
native-matrix-channel-pilotfrom
sync/native-matrix-pilot
Jun 20, 2026
Merged

github-actions[bot] merged 3 commits into
native-matrix-channel-pilotfrom
sync/native-matrix-pilot

Conversation

@personal-upstream-sync

Copy link
Copy Markdown

Automated sync from nearai/ironclaw. The upstream-main branch is an exact fast-forward mirror of nearai/main; this PR merges it into native-matrix-channel-pilot for CI. When checks pass, the Matrix pilot branch is fast-forwarded so upstream ancestry is preserved.

serrrfirat and others added 3 commits June 20, 2026 23:48
…d behavior (nearai#5105)

Three guard tests failed on main because they asserted pre-change behavior
that was intentionally updated, not because the guards regressed.

provider_tool_call_validation_rejects_sensitive_metadata (loop_support) and
provider_reference_validation_rejects_sensitive_arguments_and_text (threads)
both delegate to ironclaw_safety::provider_validation. nearai#5001 deliberately
dropped the crude bare-word substring markers (traceback / password /
stack trace) on model reasoning/metadata text, leaving the entropy-based
LeakDetector as the sole guard there (locked in by the safety crate's own
updated tests). The secret-token half of each test already passed; only the
bare-word assertion failed. Switch those assertions to a real secret-like
token so they verify the actual guard: secrets leaked into reasoning text
are rejected.

google_callback_state_rejects_unapproved_requested_scopes (auth): nearai#4326
intentionally added the Drive/Docs/Sheets/Slides scopes to the approved
GSuite set (is_allowed_google_scope) to support ported GSuite capabilities,
so .../auth/drive is no longer unapproved. Use .../auth/gmail.insert -- a
real sensitive scope still outside the approved set -- so the test keeps
guarding the real boundary.

These crates were outside the CI closure, so the stale tests went
unnoticed. Test-only change; no production behavior change.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…nightly deep CI (nearai#4829)

The reborn-integration workflow only triggered on the long-dead
reborn-integration branch and duplicated reborn-tests.yml job-for-job
(same crate-family matrix, same root parity partitions), so it never
ran for main-targeted work. Delete it and drop its entry from the
shared-path classifier.

Nightly Deep CI previously covered only the legacy test.yml suite,
leaving the Reborn binary with no scheduled safety net. Wire in
reborn-tests.yml and reborn-e2e.yml via workflow_call (both already
support it with a ref input) and fold their results into the existing
alert-issue job.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added size: L Changed-line size classification scope: ci risk: medium Risk classification contributor: regular Contributor history classification and removed size: L Changed-line size classification labels Jun 20, 2026
@github-actions
github-actions Bot merged commit 4e60f54 into native-matrix-channel-pilot Jun 20, 2026
16 checks passed
theredspoon pushed a commit that referenced this pull request Jun 21, 2026
…i#2840)

* feat(safety): projection-exempt lint for gateway event sources

Phase 1 of the gateway state-convergence epic (nearai#2792): add check #9 to
`scripts/pre-commit-safety.sh` that flags newly-added
`sse.broadcast(` / `sse.broadcast_for_user(` calls without a
`// projection-exempt: <reason>` annotation on the same line.

The invariant is documented in the new `.claude/rules/gateway-events.md`:

- Every `AppEvent` must project from a typed source log (engine
  `EventKind`, sandbox `JobEvent`, or a channel-lifecycle log).
- A short transport-only allowlist (`Heartbeat`, `StreamChunk`) covers
  the ephemeral variants with no state backing them.
- Direct emits are the root cause of the state-drift class — UI stream
  and replayable source end up with different stories. Four recent
  incidents (nearai#2654, nearai#2534, nearai#2731, nearai#2079) share this shape.

The lint is diff-based, so pre-existing unannotated call sites aren't
broken. Baseline annotation of the ~20 existing emit sites is the next
PR under Phase 1 — this one establishes the gate.

Suppressions require a named category (`bridge dispatcher`,
`channel-lifecycle`, `sandbox JobEvent`, `transport-only, heartbeat`,
or `migrate in #NNNN`). An unnamed `legacy` reason is rejected by
review, not by the lint itself.

Tested locally:
- Fires on unannotated `sse.broadcast(...)` in a new file.
- Suppressed by `// projection-exempt: transport-only, heartbeat`.
- Does not match `Channel::broadcast` (different trait).
- Does not match calls inside `#[cfg(test)] mod tests` blocks (via
  the shared `strip_test_mod_lines` filter).

Refs: nearai#2792, nearai#2654

* refactor(safety): address review feedback on projection-exempt check

Four review comments from Copilot and Gemini on nearai#2840:

1. **Match rustfmt's method-chain wrapping.** The original regex only
   caught same-line `sse.broadcast(...)`. Long calls like
   `state\n    .sse\n    .broadcast_for_user(...)` — produced by
   rustfmt and already in-tree at
   `src/channels/web/features/extensions/mod.rs:645` — would bypass the
   check. New matcher adds a dangling-method alternation that catches
   `.broadcast_for_user(` at line start. Only the `_for_user` suffix
   (SseManager-unique) is matched in dangling form; bare
   `.broadcast(` can be `Channel::broadcast` trait, which is
   intentionally out of scope.

2. **Enforce the documented annotation format.** The check previously
   accepted any `// projection-exempt:` comment, including bare
   `// projection-exempt: legacy` that the rule doc explicitly forbids.
   Negative filter now requires `<category>, <detail>` — presence of a
   comma separating the category from the detail.

3. **Point at the real path in the warning.** Replace
   `bridge::thread_event_to_app_events` with `thread_event_to_app_events`
   in `src/bridge/router.rs` — the actual file location.

4. **Update suppression hint** to show the `<category>, <detail>`
   format rather than the generic `<reason>`.

Verified against a 6-case fixture (same-line fire + suppress,
dangling-chain fire + suppress, unnamed-category fire,
`Channel::broadcast` silent).

Refs: nearai#2792, nearai#2840 review

* fix(safety): match header exclusion against grep -n prefixed output

After `grep -nE '^\+'`, every line is prefixed with `N:`, so the
`^\+\+\+` anchor for filtering diff header lines (`+++ b/file.rs`)
never fires. The positive patterns already exclude header lines by
shape, so today this is harmless — but the dead branch masks future
defense-in-depth failures if the template is reused with a less
specific positive match.

Replace `^\+\+\+` with `:\+\+\+ ` in DISPATCH, CREDNAME, and PROJECTION
checks so the exclusion works against the `grep -n` output shape.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* test(safety): regression for grep-n-prefixed header exclusion

Covers PROJECTION / DISPATCH / CREDNAME pipelines:
- diff header lines (`+++ b/path`) are filtered after `grep -n`
- real broadcast/state/CredentialName lines are still flagged
- `// projection-exempt: <category>, <detail>` exempts
- bare `// projection-exempt: legacy` (no comma) is not exempt

Locks in that `:\+\+\+ ` (matches the `grep -n` prefixed shape)
behaves as intended, where the prior `^\+\+\+` anchor silently
never fired.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* chore(deny): ignore RUSTSEC-2026-0104 (rustls-webpki CRL panic)

Same transitive pin as 0049/0098/0099 — rustls-webpki 0.102.8 is
held by libsql 0.6.0 → rustls 0.22 → hyper-rustls 0.25. The
advisory explicitly notes that applications not parsing CRLs are
unaffected; we do not parse CRLs.

[skip-regression-check] — deny.toml-only config change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(safety): portable grep boundary + broadened broadcast_for_user match

Two PROJECTION bypass paths flagged in review:

1. `\b` is a GNU-grep extension (works in grep 3.x, not portable to BSD
   grep on macOS dev envs) — replace with `(^|[^[:alnum:]_])sse\.` so
   the check fires uniformly across `grep -E` implementations.

2. `broadcast_for_user(...)` on a non-`sse` receiver (e.g.
   `manager.broadcast_for_user(...)`) previously slipped through. The
   method is defined only on `SseManager`
   (`src/channels/web/platform/sse.rs:144`), so matching
   `\.broadcast_for_user\(` on any receiver is safe and makes the
   enforcement match the documented rule.

Regression tests extended: chained-receiver, non-`sse` receiver, bare
`sse.broadcast(`, and a portable-boundary negative case (identifier
ending in `sse`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(gateway-events): align matcher description with broadened check

Update the enforcement section to describe the two current PROJECTION
matcher shapes after the review follow-up in the preceding commit:

1. Any-receiver `.broadcast_for_user(...)` — catches the non-`sse`
   receiver bypass and rustfmt wraps alike.
2. `<word-boundary>sse.broadcast(...)` with a portable boundary
   (`(^|[^[:alnum:]_])`), which is needed because `grep -E`'s `\b`
   is a GNU extension and not available on BSD grep.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(safety): tighten CREDNAME + projection-exempt lints, sync header

Three follow-ups from the review:

1. CREDNAME portability — `\bCredentialName\b` used GNU-grep `\b`,
   which BSD grep does not recognise. Replace with the same
   `(^|[^[:alnum:]_])…([^[:alnum:]_]|$)` boundary used for
   PROJECTION and matches cleanly across GNU and BSD `grep -E`.

2. Empty-detail suppression bypass — `// projection-exempt: [^,]+,`
   accepted `// projection-exempt: foo,` (empty detail) as exempt
   even though `.claude/rules/gateway-events.md` requires a
   non-empty detail. Tighten to `[^,]+,[[:space:]]*[^[:space:]]`
   so a comma without a trailing token still fires the check.

3. Header suppression hint (`#24`) said
   `// projection-exempt: <reason>` — update to
   `<category>, <detail>` to match what the check actually accepts
   so contributors don't copy an unsupported format.

Regression tests extended: `PROJECTION: empty detail after comma
still flagged`, `PROJECTION: comma + whitespace-only detail still
flagged`, `CREDNAME: CredentialNameExt (different type) is not
flagged`. All 16 cases pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
theredspoon pushed a commit that referenced this pull request Jun 21, 2026
* docs(reborn): WU-B subagent durability sub-spec

Sub-spec for WU-B per docs/plans/2026-06-06-subagent-compaction-impl.md.
Blocks WU-C. Doc-only.

Covers 4 in-memory stores (gate resolution, goal, tombstone, capability
result) + 2 new tables (settlement event log, idempotency ledger).
Decides typed-repo vs ScopedFilesystem per _contract-freeze-index.md §2.
Introduces CapabilityResultStore + SubagentRestartReconciler traits.
Specifies libSQL + PostgreSQL schemas, first-writer-wins semantics,
scope propagation, migration/rollback under subagent.background_enabled
toggle, and the dual-backend parity test (nearai#4431 follow-on).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B straightforward review fixes

Applies 18 straightforward findings from multi-agent code review on PR
nearai#4582. Design-level items still pending discussion.

Schema:
- F2: PostgreSQL ledger run_id/child_run_id UUID → TEXT (§8.3 convention)
- F3: align libSQL/PostgreSQL undelivered_terminal partial index
- F5: add result_ref column to subagent_gate_settlement_log both backends
- F6: capability_results uses explicit PRIMARY KEY (result_ref)
- F8: scope predicates mandatory on UPDATE/DELETE templates in §1.6
- F9: define post-result-write flag update path (separate transaction)
- F18: 8 MiB CHECK constraint on capability_results.payload (MUST)

Contracts:
- F1: CapabilityResultStore trait scope &ResourceScope → &TurnScope
- F7: drop CapabilityRunId alias; use TurnRunId directly
- F10: specify sanitized_reason source + sanitization transform
- F11: specify delivery_node validation (length, allowlist, source)
- F12: §6.3 restated in binary INSERT-OR-IGNORE ledger semantics
- F13: resolve tombstone trait scope-param decision in spec

Doc consistency:
- F4: remove delivered_at IS NULL filter (column doesn't exist)

Test plan:
- F14: name positive production-readiness tests (goal, tombstone, capres)
- F15: tombstone first-writer-wins distinguishing test
- F16: agent_id cross-leakage parity test
- F17: reconciler crash-between-ledger-insert-and-gate-write test

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B two-phase ledger + orphan handling (D1+D9)

Resolves two reconciler bugs surfaced by multi-agent review on PR nearai#4582:

D1 — Crash-between-ledger-insert-and-gate-write strands parent silently.
  Idempotency ledger goes two-phase. `delivered_at TIMESTAMPTZ NULL`:
  - INSERT OR IGNORE leaves `delivered_at = NULL` (pencil receipt;
    claim, mid-flight).
  - After successful gate-store write, UPDATE seals row with
    `delivered_at = NOW()` (pen receipt; final).
  - Pencil rows surviving a crash become `retryable` on next boot,
    not silently `skipped_idempotent` as before.
  Matches the existing `IdempotencyLedger::begin_or_replay` precedent in
  `crates/ironclaw_product_workflow/src/ledger.rs`. Both gate-store and
  seal UPDATE are idempotent at the row level so duplicate delivery
  cannot occur and missed delivery cannot occur.

D9 — Orphan settlement-log rows produced perpetual `failed` count.
  Reconciler now checks `gate_store.gate_exists(scope, gate_ref)` first.
  If the gate is gone (parent cancelled, gate row deleted): write
  `SubagentResultTombstone { disposition: DiscardedParentGone }`, seal
  the ledger row, count as `skipped_orphan`. One pass per orphan; future
  passes skip via sealed ledger row. Settlement log stays append-only.

ReplayReport gains `retryable: u32` and `skipped_orphan: u32` so each
counter has one meaning. `failed > 0` is now operator-actionable only —
no more phantom alerts.

Spec changes:
- §5.2 ReplayReport struct extended.
- §5.3 algorithm rewritten: gate-exists check, then tombstone check,
  then pencil-claim, then deliver, then seal. Pencil read on
  insert-skip distinguishes sealed (skipped_idempotent) from
  pencil (retryable).
- §5.4 + §5.5 ledger DDL: `delivered_at` becomes nullable. INSERT
  examples split into pencil + seal.
- §5.8 test plan: orphan-gate test case added; existing test names
  updated.
- §5.9 risks: stale-children GC bullet rewritten; capability-result-
  missing conclusion sentence updated.
- §6.3 re-flip narrative updated to use sealed/pencil vocabulary.
- "Decisions ratified up front" table gains rows 11–13.
- Closing checklist gains 3 WU-C action items.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B reconciler perf + cold-start shape (D4+D5 comprehensive)

Resolves cold-start / scaling concerns surfaced by multi-agent review on
PR nearai#4582. Comprehensive: D4 + D5 + 5 long-term concerns folded in.

D4 — Reconciler replay batch-phased (no more N+1).
  §5.3 algorithm rewritten:
    Phase 0  bound input via LEFT JOIN against ledger (only pending
             pencil-or-missing rows enter the algorithm; replay's scan
             size stays proportional to outstanding work, not historical
             log size).
    Phase 1  batched preflight: one query for gates_exist_batch, one for
             read_tombstones_batch.
    Phase 2  multi-row ledger writes:
               2a — orphan + tombstoned cleanup (one upsert-sealed batch)
               2b — pencil claim (one INSERT OR IGNORE batch)
    Phase 3  parallel capability loads via `join_all` (capped at
             replay_pool size).
    Phase 4  per-row deliver + seal (sequential per row, each row hits a
             different parent's mailbox).
  Phases 0–3 are O(1) DB calls regardless of N. Net cost dominated by
  Phase 4's per-row delivery, ~5–30 ms per row depending on backend
  latency. 10–50× speedup over the previous N+1 form.

D5 — Background replay + per-scope admission gate.
  §5.6 composition wire-up rewritten:
    - Replay dispatched via `tokio::spawn` from boot; foreground traffic
      accepts immediately (<100 ms cold start regardless of backlog).
    - Per-scope `ReplayState { completed_at, last_report }` tracks
      completion. Background-mode `SpawnSubagentPort` consults the gate
      before admitting; rejects with `SubagentSpawnError::ReplayInProgress`
      until per-scope replay completes. Foreground / blocking subagent
      calls NEVER consult this gate.
    - Dedicated `replay_pool` (default 4 DB connections, configurable via
      `RebornEventStoreConfig.replay_pool_size`) — replay never starves
      foreground writes during recovery storms.
    - Eager active-scope enumeration at boot via runs-table query.
      Bounded by active-runs count, not historical user count. Lazy
      per-scope replay deferred as future optimization.

Long-term concerns folded in:
  - HA replicas: spec is HA-safe (correctness via Phase 2b INSERT OR
    IGNORE + single-winner seal UPDATE), HA-redundant (each replica
    runs replay independently — N× DB load at boot). Active-active
    leader election deferred to cross-cutting follow-up. Documented in
    §5.6 + §5.9.
  - Settlement log growth: Phase 0 LEFT JOIN bounds input — replay's
    scan size is independent of historical log size. Archival /
    materialized-view summarization deferred as ops follow-up.
  - Replay pool sizing: default 4 fine for typical fan-outs; tuning
    via P95 metric. Spec does not mandate auto-tuning.

§5.7 NEW — Observability contract:
  - `RebornEventKind::SubagentReplayCompleted` event per scope.
  - 5 required metrics: replay_duration_seconds (histogram),
    replay_pending_rows (gauge), replay_outcomes_total{outcome=…}
    (counter), pencil_age_seconds (gauge), replay_in_progress (gauge).
    All labeled by (tenant_id, agent_id).
  - 3 required alerts: `failed > 0`, `pencil_age_seconds > 60`,
    `replay_duration_seconds{P95} > 30`.
  - OpenTelemetry spans: one per scope (`reborn.subagent.replay`) +
    child spans per phase.
  - WU-F WebUI surfaces `replay_in_progress` per-scope; background-spawn
    rejection during replay shown to user as "starting up, retrying in
    N seconds" affordance.
  Prerequisite for WU-G E2E + WU-F integration.

Decisions table gains rows 14–19. Closing checklist gains 7 WU-C action
items.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B hot-path perf + multi-tenant scaling (D6+D8+E.A+A.A+A.B)

Resolves hot-path overhead + multi-tenant scaling concerns for the
spawn / capability-write / replay paths.

D6-A — Durable per-scope capacity counter.
  Replace per-spawn SELECT COUNT(*) with a sidecar
  `subagent_gate_capacity_counter` table — one transactional UPDATE per
  spawn (no extra round-trip). Race-safe via SELECT FOR UPDATE (PG) /
  BEGIN IMMEDIATE (libSQL). Symmetric increment on INSERT, decrement on
  delivery / delete via GREATEST(undelivered - N, 0) safety net.

E.A — Sharded counter for hot scopes (CAPACITY_COUNTER_BUCKETS = 16).
  Per-scope counter row becomes a write hotspot when one mega-tenant
  runs 10k+ concurrent background subagents under the same scope.
  Shard into K=16 rows per (tenant_id, user_id, agent_id) keyed by
  `bucket SMALLINT/INTEGER NOT NULL`. Spawn picks bucket via
  `hash(child_run_id) % K`. Cap check is `SUM(undelivered)` across all
  K buckets — index-only at K=16.
  `subagent_gate_awaited_children.counter_bucket` stores bucket-of-record
  for symmetric decrement on cleanup. Per-scope spawn throughput lifts
  from ~100/sec (single-row lock contention) to ~1600/sec on PostgreSQL.
  Drift bound: ≤ K-1 rows over cap under maximum concurrency.

D8-A — CapabilityResultStore trait takes Vec<u8>, not serde_json::Value.
  Executor: `let bytes = serde_json::to_vec(&output)?;` ONCE. `byte_len`
  is `bytes.len() as u64` — derived for free. `bytes` is MOVED into the
  store, not cloned. Store INSERTs bytes directly into BLOB (libSQL) /
  JSONB (PostgreSQL) without re-serializing. `read()` returns Vec<u8>;
  caller deserializes lazily via `serde_json::from_slice` only when a
  Value is needed (prompt assembly, compaction).
  Eliminates 2× full-tree serialization + 1 Value clone per capability
  call. ~50% CPU reduction on capability-write hot path at production
  scale. Trait shape reflects what crosses the boundary (bytes, not a
  tree). Composes with future streaming variants (BoxStream<Bytes>).

A.A — Reconciler replay jitter for fleet rollouts.
  `RebornEventStoreConfig.reconciler_replay_jitter_ms: u64` (default
  5000). Each replica sleeps a uniform-random 0..jitter ms before
  launching its background replay task. Spreads the deploy-time
  reconciler stampede over a wider window — at 50-replica rollout, peak
  DB reconciler conn count drops from N×replay_pool to ~jitter-spread
  fraction. Foreground traffic NEVER pays the jitter cost. Set to 0
  for single-node deployments.

A.B — HA per-scope leader election (new §5.10, future, NOT WU-C scope).
  Documented direction: Postgres `pg_try_advisory_xact_lock` per scope.
  Replicas that lose election skip replay for that scope; still consume
  settlement events via gate-store mailbox as normal. Total fleet
  reconciler work drops from O(N × scopes) to O(scopes). Promotion
  trigger documented (P95 replay duration > 30s + sustained
  replay_in_progress aggregate > 60s). Lock is transaction-scope so
  auto-releases on leader crash — composes cleanly with D1's two-phase
  ledger. libSQL fallback: noop election (every replica is leader);
  libSQL deployments are typically single-node so redundancy is moot.

Decisions table gains rows 20–24 (D6-A, E.A, D8-A, A.A, A.B).
Closing checklist gains 4 WU-C action items + 1 follow-up note.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B round-2 review fixes (R1-R17)

Round-2 multi-agent review surfaced 5 High-severity regressions
introduced by prior fix commits, plus medium consistency issues.
All resolved here.

SQL TEMPLATES (§1.6) — fix regressions in transaction shapes:
  R3 — Conditional agent_id predicate. Replace blanket
       `(agent_id = ? OR agent_id IS NULL)` (which lets agent-scoped
       callers reach system-level rows) with placeholder
       `<agent_predicate>` bound conditionally per caller's scope:
       `agent_id = ?` when Some, `agent_id IS NULL` when None.
       New §1.6 preamble paragraph documents the rule.
  R4 — Capacity counter SUM + UPDATE use the conditional predicate too.
       Bare `agent_id = ?` with NULL parameter evaluated to UNKNOWN,
       silently bypassing the 4096 cap for non-agent runs.
  R5 — Delivery-claim DELETE on deliverable_queue gains `child_run_id`.
       Previously wiped ALL queue rows for a gate when only one child
       was delivered — stranded N-1 siblings.
  R10 — Delivery-claim UPDATE SET also flips `delivery_claimed = 1`
        (prose was inconsistent with SQL).
  R13 — Delete-path DELETEs on deliverable_queue + child_index gain
        `user_id` predicate. awaited_children DELETE uses the
        conditional agent_predicate.
  R15 — Settlement log dedup decision resolved (was deferred). Ledger
        UNIQUE + gate-store idempotency + Phase 0 LEFT JOIN make
        duplicate log rows benign; no MIN(id) needed.
  R16 — parent_run_context_json gains sensitivity audit requirement
        (closing-checklist gate): WU-C MUST verify LoopRunContext is
        credential-free or strip sensitive fields at write site.

ALGORITHM PSEUDOCODE (§5.3) — fix wrong column names + bounded fan-out:
  R1 — Phase 0 LEFT JOIN uses `s.parent_run_id` (column actually exists;
       schema does NOT have `s.run_id`).
  R2 — Phase 0 filter uses `s.terminal_kind` (column actually exists;
       schema does NOT have `s.event_kind`).
  R11 — Phase 3 capability loads use `buffer_unordered(replay_pool_size)`
        not `join_all`. Unbounded fan-out at 10k pending rows would
        starve foreground writes on the 4-conn replay pool.
  R12 — Phase 2a tombstone writes use `write_tombstones_batch` (single
        round-trip), not a per-row `for` loop. Trait gains batch method.

TRAIT + VARIANT CONSISTENCY:
  R8 — InMemoryCapabilityResultStore type is `Mutex<HashMap<String,
       Vec<u8>>>` (was self-contradicted in §4.5 — D8-A regression).
  R9 — SubagentResultDisposition variants documented: today's
       `DiscardedByParentCancel` + WU-C addition `DiscardedParentGone`
       (used by §5.3 Phase 2a orphan cleanup). §3.7 risks bullet
       updated. Forward-compat with WU-D variants (`Delivered`,
       `SettledByBackground`).
  R17 — `scope_from_run_context` helper defined in §4.8 (was undefined).
        Maps LoopRunContext → TurnScope; documents user_id resolution
        via `explicit_owner_user_id()` + SYSTEM_RESERVED_ID sentinel.

DOC HYGIENE:
  R6 — Tombstoned-result test assertion fixed: `skipped_orphan == 1,
       failed == 0` (matches §5.3 algorithm; was `failed == 1`).
  R7 — Double-replay test assertion fixed: `skipped_idempotent == 1`
       (was `skipped == 1` — field does not exist on ReplayReport).
  R14 — Duplicate `### 5.8` heading resolved. Test plan now §5.9, Risks
        §5.10, HA leader election §5.11.
  Pen→pencil terminology consistency in §5.4 + §5.5 SQL comments.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B Copilot + Codex review fixes (CF1-CF5)

Round-2 Copilot + Codex automated reviewers caught issues skill
reviewers missed. All resolved here.

CF1 — subagent_gate_child_index + subagent_gate_deliverable_queue
       gain `user_id TEXT NOT NULL` + `agent_id TEXT` columns per
       Decision #5 (also fixes a latent SQL syntax error: prior R13
       added user_id to DELETE predicates without the columns
       existing). New `idx_sgci_scope` / `idx_sgdq_scope` indexes on
       `(tenant_id, user_id, agent_id, child_run_id)` replace the
       prior `tenant_child` indexes. Both libSQL + PostgreSQL.

CF2 — capability_results.created_at gains
       `DEFAULT (datetime('now'))` in libSQL (PostgreSQL already had
       `DEFAULT NOW()`). Needed because §4.6/§4.7 rely on this column
       for `idx_capability_results_run` ordering and `list_by_run`
       ORDER BY — silent inserter mistakes would break replay
       ordering.

CF3 — CapabilityResultStore::write now takes
       `invocation_id: InvocationId`. UNIQUE INDEX
       `(tenant_id, user_id, run_id, capability_id, invocation_id)`
       enforces true first-writer-wins idempotency. Previous design
       minted a fresh UUID per call, so `INSERT OR IGNORE` could
       never collide — idempotency claim was misleading. Now a
       retry-after-transient-error returns the same `result_ref`.
       Trait + in-memory impl note + §4.8 wire-up updated; both
       backend schemas gain the column + unique index.

CF4 — §6.2 rollback step rewritten. Goal store stays on
       FilesystemSubagentGoalStore (durable) when the toggle flips
       OFF; the toggle gates only background-mode spawn admission,
       NOT backend selection. Prior wording about
       "re-selects InMemoryBoundedSubagentGoalStore" contradicted
       §2.1.

CF5 — Decision #5 reworded. Scope columns are always PRESENT on
       every durable table and reached via a scoped index
       (`idx_*_scope`). PKs remain shape-appropriate per table
       (e.g. `(gate_ref, child_run_id)`, `(result_ref)`) — scope
       need not LEAD every PK. Matches actual schema guidance and
       removes the false-positive interpretation that all PKs must
       be scope-prefixed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B resolve 2 leftover review items

Two open threads from round-2 review now decided + applied.

1. InMemoryCapabilityResultStore gains bounded eviction.
   `INMEMORY_CAPABILITY_RESULT_STORE_MAX_ENTRIES = 1024` +
   `INMEMORY_CAPABILITY_RESULT_STORE_MAX_BYTES = 4 MiB` (FIFO by
   insertion order). Prevents local-dev / CI OOM on long sessions
   that accumulate megabyte-scale payloads. Production-readiness
   check still gates the impl to LocalDevTest mode regardless.

2. `gate_resolution_scoped_query_excludes_rows_from_other_agents`
   promoted from WU-G to WU-C. This is a security gate (cross-tenant
   / cross-agent leakage class via missing agent_id predicate), not
   an E2E gate. Shipping the gate-resolution backend in WU-C without
   this guard would mean releasing the durable code with no test
   that catches a missing agent_id WHERE clause — unacceptable per
   §1.7 + `_contract-freeze-index.md` §8.

Closing checklist gains two WU-C action items.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B round-3 review fixes (R3-1..R3-15)

Round-3 multi-agent review at head 707e2cd found 12 straightforward
fixes. All applied here. 3 design-level items deferred to discussion.

SCOPE PREDICATE GAPS (security):
R3-1  §1.6 three DELETE statements gain conditional <agent_predicate>:
      delivery-claim path + delete-path queue + delete-path child_index.
      Without these, agent-scoped callers can delete other agents'
      auxiliary rows under the same (tenant_id, user_id).
R3-2  Phase 0 LEFT JOIN gains <agent_predicate_on_s> — agent-scoped
      replay must not surface settlement-log rows belonging to other
      agents. Performance shape paragraph documents the rule.
R3-13 subagent_idempotency_ledger UNIQUE constraint extended to include
      (tenant_id, user_id, agent_id, ...). Without scope cols in the
      UNIQUE, a cross-tenant collision (UUID or migration artifact)
      would be silently ON CONFLICT DO NOTHING'd.
R3-14 capability_results UNIQUE idempotency index gains agent_id. Two
      agents producing the same (tenant, user, run_id, capability_id,
      invocation_id) would otherwise have their second write silently
      dropped. PostgreSQL uses COALESCE(agent_id, '__non_agent__') for
      uniqueness across NULL-agent rows.

SPEC-VS-TRAIT DRIFT:
R3-9  buffer_unordered propagation: Decision #14 + D4 prose + closing
      checklist all now say buffer_unordered(replay_pool_size). Prior
      contradiction: §5.3 pseudocode MUSTed buffer_unordered but other
      surfaces still said join_all. WU-C reading checklist literally
      would reintroduce the pool-starvation regression.
R3-10 Phase 3 pseudocode + §5.10 risks: .load() → .read() to match the
      §4.3 trait method name. Plus §5.10 capability_result_store.load
      reference updated.
R3-15 scope_from_run_context helper deleted — LoopRunContext.scope IS
      already TurnScope. Replaced with &write.run_context.scope direct
      borrow. §4.8 "Scope source" paragraph documents the canonical
      pattern.

CHECKLIST + TEST NAMES:
R3-4  Credential audit promoted to MERGE-BLOCKING checklist item (top
      of list). WU-C MUST complete LoopRunContext audit + add compile-
      time lint OR verify write-site stripping before merging the
      durable gate-resolution backend.
R3-5  Six reconciler test scenarios in §5.9 get canonical function
      names under tests::reconciler_integration::*. WU-C now has exact
      targets for redelivery, idempotency, tombstoned, missing-result,
      crash-between-insert-and-deliver, and orphan-gate paths.
R3-6  §4.9 + §7.3 name the payload-size-cap test:
      capability_result_store_write_rejects_payload_exceeding_8_mib_with_capacity_exceeded.
      Asserts typed CapacityExceeded error, not raw Backend/Io error.
R3-7  §5.6 names the admission-gate test:
      background_spawn_rejected_with_replay_in_progress_while_reconciler_is_running.
      Foreground / blocking subagent paths must succeed throughout.

CROSS-REF FIXES:
R3-8  Decision #24 + closing checklist HA leader election refs:
      §5.10 → §5.11 (R14 renumber missed these).

DEFERRED FOR DISCUSSION:
- R3-3  Phase 4 per-row seal vs seal_batch trait method
- R3-11 Formal batch trait signatures subsection placement
- R3-12 skipped_orphan counter — split vs combined for tombstoned

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B round-3 design decisions (R3-3 + R3-11 + R3-12)

Three coupled design-level changes from round-3 review now ratified
and applied.

R3-3 — Phase 4 seal_batch.
  Spec §5.3 Phase 4 previously issued per-row `idempotency_ledger.seal`
  on each successful delivery. At 100 children through a 4-conn
  replay_pool that is 25 sequential rounds (~125-750 ms) on the seal
  step alone.

  Fix: Phase 4 now collects sealed-row keys into a `sealed_keys` vec
  during the per-row loop, then issues ONE `seal_batch(scope, Vec<
  LedgerKey>)` call at the end. Single multi-row UPDATE. Idempotent
  per-row via the `delivered_at IS NULL` guard.

  Single-row `seal` retained for orphan / tombstone paths in Phase 2a
  (which already batch via `upsert_sealed_batch`) and for any future
  operator-driven manual interventions on stuck rows.

R3-11 — Formal batch method signatures in §5.2.1.
  §5.3 algorithm calls 8 batch methods. Only `write_tombstones_batch`
  had a rough signature; the rest were implicit. WU-C would need to
  reverse-engineer 7 method signatures from pseudocode call sites.

  Fix: new §5.2.1 "Batch method signatures (reconciler-facing)"
  subsection lists all 8 method signatures with full async-trait
  syntax plus `LedgerKey` + `LedgerRow` struct definitions. Single-
  row variants documented alongside batch variants for completeness.
  WU-C now reads §5.2.1 literally as the trait surface contract.

R3-12 — Split skipped_orphan counter.
  Old: `skipped_orphan = orphan_rows.len() + tombstoned_rows.len()`.
  Two semantically distinct cases conflated:
    - orphan       = gate row gone (parent cancel + cleanup)
    - tombstoned   = gate live but child pre-tombstoned (parent
                     cancelled the specific child)

  Different operational signals; merging them prevented operators
  from distinguishing gate-cleanup spikes (high `skipped_orphan`
  alone) from parent-cancel spikes (high `skipped_tombstoned`).

  Fix: `ReplayReport` gains `skipped_tombstoned: u32`. Phase 2a
  increments each counter independently. §5.7 metric label list
  extended; §5.9 `reconciler_skips_tombstoned_child` test asserts
  `skipped_tombstoned == 1, skipped_orphan == 0`. Decisions table
  row 13 updated to six counters.

Closing checklist gains 3 new WU-C items (seal_batch impl,
batch-method trait surface, ReplayReport split).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B round-4 review fixes (R4-1 critical + 12 more)

R4-1 CRITICAL — Phase 3 buffer_unordered → buffered.
  buffer_unordered emits futures in completion order, not input order.
  Phase 4's to_attempt.zip(load_results) pairs row identity with
  payload positionally → SILENT cross-child payload delivery on every
  replay with >1 pending row. gate_store.record_background_settlement
  called with row_A.parent_run_id + row_A.child_run_id + payload_B.

  Fix: .buffered(replay_pool_size) — same concurrency bound, preserves
  input order. Decision #14, D4 prose, closing checklist all updated.

Schema + SQL invariants:
R4-2  Seal UPDATEs (§5.4 libSQL + §5.5 PostgreSQL) add user_id +
      <agent_predicate> — were missing despite §1.6 mandate.
R4-5  §1.6 INSERT pseudocode for child_index + deliverable_queue add
      user_id + agent_id columns (CF1 added schema cols but not
      pseudocode → would NOT NULL violation on verbatim execution).
f-sec-1  PostgreSQL ledger UNIQUE uses COALESCE(agent_id,
         '__non_agent__') — NULL agent_id rows were silently
         double-INSERTable.
f-sec-2  §1.7 prose no longer describes forbidden (agent_id = ? OR
         agent_id IS NULL) pattern; references §1.6 conditional
         convention instead.
f-bug-3  Phase 0 LEFT JOIN matches scope cols on ledger side too —
         cross-tenant UUID collision could otherwise suppress this
         tenant's replay.
f-bug-5  Phase 2a maps Vec<SettlementLogRow> → Vec<LedgerKey> before
         upsert_sealed_batch — type mismatch fixed.
f-perf-1 + partial-index DDL — idx_subagent_idempotency_ledger_pending
         (partial on delivered_at IS NULL) added to §5.4 + §5.5 so
         Phase 0 scan stays bounded by outstanding work.

Trait surface + tests:
R4-4  reconciler_replays_undelivered_settled_child step 6 fixed:
      `skipped == 0` → 3 real counter fields.
R4-6  SubagentIdempotencyLedger trait drops redundant `scope:
      &TurnScope` arg from 6 methods — LedgerKey embeds scope (single
      source of truth).
R4-7  reconciler_counts_failed_on_missing_capability_result gains
      pencil-receipt-survives assertion (delivered_at IS NULL).
R4-8  delivery_node_invalid_substituted_to_unknown test named in §5.5
      + §5.9 (4 cases: oversized, control chars, disallowed chars,
      empty).
R4-9  §5.6 active-scope enumeration gains max_active_scopes_at_boot
      (default 1000) cap + 5s timeout + overflow → lazy fallback +
      operator metrics.
f-maint-1  §5.4/§5.5 SQL path comments reference inline Rust
           constants in migrations.rs per §8.5 (not separate .sql
           files).

R4-3 (MERGE-BLOCKING marker on credential audit): verified already
applied via R3-4 (line 1928).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* docs(reborn): WU-B round-5 review fixes (R5-1..R5-10)

- R5-1: PG capability_results.payload BYTEA not JSONB (byte-exact
  round-trip contract; JSONB normalization breaks parity + byte_len)
- R5-2: capability-write idempotency conflict target = invocation
  unique index, insert-then-select read-back (was result_ref, which
  never conflicts on retry)
- R5-3: lazy per-scope replay ships in WU-C as admission-gate trigger
  (capped scopes were rejected with ReplayInProgress forever)
- R5-4: flat tombstone ScopedPath (no thread segment — settlement log
  carries no thread_id); read_tombstone also gains scope param
- R5-5: Phase 3 = exists_batch existence check, no payload loads;
  delivery via new redeliver_settled_child (record_background_settlement
  was undefined and payload-shaped)
- R5-6: libSQL MAX() not GREATEST in counter decrements
- R5-7: tombstoned rows resolve live gate row (capacity-leak fix);
  new resolve_undeliverable_batch
- R5-8: ReplayState keyed by (tenant_id, user_id, agent_id)
- R5-9: SubagentReplayCompleted contract-freeze callout
- R5-10: CapacityExceeded maps to CapabilityOutcome::Failed, never
  aborts the loop
- minors: 11-method count, pseudocode scope-arg drift, six-counter
  comment, PG ON CONFLICT expression target, MountView user-isolation
  verification, stale §5.10 throughput bullet
- plan: drop stale duplicate WU-C 'Files modified' block (contradicted
  the corrected block above it)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(reborn): WU-B add §9 parent-initiated child cancel + inspect

Audit of existing host plumbing (request_cancel, RunCancellationHandle,
children_of, event projection) shows the only gap is model-visible
action surface. Ratifies decisions 33-35: two thin WU-D actions
(subagent_cancel, subagent_status) over existing machinery; parent-
requested cancel delivers a Cancelled settlement (never tombstones —
DiscardedByParentCancel stays reserved for the parent-run-cancel
cascade); status is metadata-only so settle-time delivery remains the
sole sanitization choke point. Child-pushed progress notes deferred
pending WU-G evidence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: regular Contributor history classification risk: medium Risk classification scope: ci

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant