Files
stack/docs/reports/code-review/756-code-review.md
T
veronica f0d2dd9920 docs(W4): stamp kind and status front matter on 104 live documents
Applies the document contract from
docs/plans/2026-08-20_stack-docs-flatten-and-alignment.md section 3, partially:
`kind` and `status` only. `parent` is deliberately held until the flatten in
section 4 lands, so that 127 documents do not have to be re-pointed by hand
when docs/fleet/NORTH_STAR.yaml moves to docs/NORTH_STAR.yaml.

Scope, measured on origin/next at 63069149:

  127 live docs   = all *.md under docs/ minus docs/archive/ minus docs/_old_structure/
  104 stamped     here
   19 held        operator judgement (plan section 9), worklist in the same PR
    3 held        the SUPERSEDED TASKS.md stamps, which cite the moving path
    1 untouched   docs/fleet/FLEET-DOCTRINE.md, already stamped in W1

Kinds applied: 54 guide, 34 record, 9 spec, 6 tracking, 1 projection.
Every row carries a confidence and a one-line rationale in the worklist.

Two collisions with the existing state, both flagged rather than resolved:

1. docs/README.md:150-160 already documents a front-matter convention
   (title/type/audience/status/source_of_truth) with its own allowed values.
   It is applied to 4 of 127 files. Its `status` vocabulary is
   current|draft|deprecated|historical; the new contract's is active|superseded-by.
   The key collides. This commit lets the new contract win and rewrites
   `status: current` to `status: active` on those 4 files, keeping their other
   legacy keys untouched. No code reads any of them: `git grep source_of_truth`
   outside docs/ returns nothing. docs/README.md still prescribes the old
   convention and is an operator row, so it is not edited here.

2. Two of the plan's 20 operator rows are YAML files, not markdown
   (docs/fleet/examples/roster-v2.yaml, docs/openapi-tess.yaml), and the
   contract's front-matter form has no defined meaning for a .yaml document.
   That gap also applies to docs/fleet/NORTH_STAR.yaml, the source of truth
   itself. Raised in the worklist.

A third row from the plan, docs/fleet/north-star.md, no longer exists: W1
renamed it to docs/fleet/FLEET-DOCTRINE.md.

Verification: 104/104 parse with the expected kind and status in front matter;
the check was shown to reject a wrong kind before it was trusted. The diff
removes 4 lines total, all of them `status: current`.
2026-08-20 19:30:25 -05:00

2.5 KiB

kind, status
kind status
record active

Independent Code Review — #756 Official Discord Channel Plugin

Verdict: APPROVE

Scope reviewed

Complete current uncommitted #756 delta: multi-binding trusted agent selection, privileged ingress-route validation, attachment validation/persistence/resume, egress route lifecycle and idempotency, concurrent gateway stream state, Discord lifecycle/thread/rate behavior, compatibility ingress, and tests.

Review result

No blocking or change-request finding remains.

  • Trusted multi-agent routing: each binding requires a provisioned agentConfigId; the gateway resolves that config server-side, verifies its logical-agent name, and never accepts a Discord-controlled agent selection or applies generic routing to Discord ingress.
  • Auth and route integrity: allowlist, pairing, role, and canonical logical-agent/channel-or-thread conversation-route validation occur before gateway processing. Privileged approval/stop paths use the same binding and route validation.
  • Attachments: ingress rejects malformed, over-bounded, credential-bearing, fragment-bearing, or query-bearing URLs. Valid attachment metadata, including sizeBytes, persists and is reconstructed into resume history as explicitly untrusted context.
  • Discord reliability: the adapter supports degraded Socket.IO reconnect, parent/thread routing semantics, bounded pre-side-effect ingress rates, and terminal response-route cleanup. Egress validates route/message alignment, sends deterministic nonces, distinguishes permanent from transient errors, and uses bounded retries.
  • Concurrent state: per-client/conversation keys isolate listener, redaction, tool, and stream state for simultaneous threads sharing a Discord socket; disconnect cleanup covers all associated conversations.
  • Harness neutrality: contracts retain logical-agent/channel data only; no harness/provider identity leaks into adapter routes or message boundaries.

Verification performed

  • git diff --check — passed.
  • pnpm --filter @mosaicstack/discord-plugin typecheck — passed.
  • pnpm --filter @mosaicstack/discord-plugin lint — passed.
  • pnpm --filter @mosaicstack/discord-plugin test — passed: 44 tests; coverage 92.18% statements/lines, 86.55% branches, 100% functions (all ≥85% threshold).
  • pnpm --filter @mosaicstack/gateway typecheck — passed.
  • cd apps/gateway && pnpm exec vitest run src/plugin/discord-ingress.security.spec.ts src/chat/chat.gateway-redaction.spec.ts src/__tests__/integration/tess-cross-surface.integration.test.ts — passed: 32 tests.