This commit is contained in:
Jason Woltje
@@ -7,6 +7,8 @@ Use the canonical guide, API contract, source, and tests to determine current be
|
||||
## Documentation evidence
|
||||
|
||||
- [Documentation catalog and truth audit](documentation/2026-08-10-docs-catalog-audit.md) — migration inventory and static truth assessment.
|
||||
- [Fleet documentation IA closure](documentation/758-fleet-config-ia-closure.md) — active Fleet documentation acceptance evidence; the executable book remains at `docs/fleet/`.
|
||||
- [Fleet documentation deferrals](deferred/758-fleet-config-deferrals.md) — explicit holds and deferred live-action boundaries.
|
||||
- [Issue #756 documentation checklist](documentation/756-discord-plugin-checklist.md) — historical completion checklist for the official Discord plugin workstream.
|
||||
- [Framework consistency audit — 2026-02-17](documentation/AUDIT-2026-02-17-framework-consistency.md) — historical framework consistency and remediation snapshot.
|
||||
- [Compaction-refresh #830 checklist](compaction-refresh/830-documentation-checklist.md) — historical incomplete-at-snapshot documentation checklist.
|
||||
@@ -32,8 +34,7 @@ These reports preserve the review sequence for issue #751. Their GO verdicts app
|
||||
- [Initial independent NO-GO](native-kanban-sot/canon-initial-review-no-go.md) — original blocking review.
|
||||
- [Independent re-review GO](native-kanban-sot/canon-final-rereview-go.md) — follow-up closure review.
|
||||
- [Ultron final GO](native-kanban-sot/ultron-final-go.md) — historical final-gate record.
|
||||
|
||||
The KBN-101 security review remains migration-coupled to its requirements and held database workstream.
|
||||
- [KBN-101 security/architecture review](native-kanban-sot/kbn-101-contract-security-review-82ce325.md) — active review evidence coupled to the held KBN-101 authority surface.
|
||||
|
||||
## Retention rules
|
||||
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
# FCM-M5-001 Fleet Documentation Deferrals and Holds
|
||||
|
||||
**Issue:** #758 · **Branch:** `docs/758-fleet-config-operator-docs`
|
||||
|
||||
These are accepted existing DAG boundaries, not omissions silently claimed as delivered.
|
||||
|
||||
## FCM-M3-002 hold
|
||||
|
||||
- Boot/reboot preservation for roster members persisted stopped or disabled.
|
||||
- Current installation may enable all agent units, while the launcher projection does not yet carry
|
||||
`lifecycle.enabled` or `desired_state`; documentation therefore does not claim lifecycle-safe reboot.
|
||||
- Heartbeat/liveness integration into roster-v2 `status`, `doctor`, and `verify`; current observations
|
||||
cover systemd active state, tmux sessions, holder ownership, and unmanaged sessions only.
|
||||
|
||||
## FCM-M4-002 hold
|
||||
|
||||
- Executable v1-to-v2 cutover, reversible canary, and rollback.
|
||||
- Stale-projection/orphan migration classification and current-host managed/unmanaged fixture coverage.
|
||||
- Any live migration, lifecycle, systemd/tmux/session, or rollback action.
|
||||
|
||||
M5 docs describe prerequisites and the preview boundary only. A ready preview is not migration or rollback evidence.
|
||||
|
||||
## Explicit validate-operation gap
|
||||
|
||||
- `FCM-REQ-03` requires a documented programmatic `mosaic fleet validate` operation.
|
||||
- The current CLI does not expose that operation. Existing mutation/reconcile validation and the
|
||||
documentation example test are not a replacement for the missing command.
|
||||
- FCM-M5-001 documents this implementation gap without inventing syntax, JSON, exit behavior, or an
|
||||
owning implementation card. Parent #758 must remain open until the requirement is implemented and
|
||||
evidenced or the PRD/DAG is explicitly revised through the authoritative process.
|
||||
|
||||
## FCM-M5-002 hold
|
||||
|
||||
- Deterministic source-versus-installed asset revision detection and safe refresh implementation.
|
||||
- Rolling local canary, independent validator certificate, final release evidence, merge-gate approval, and parent #758 closure.
|
||||
|
||||
`operations/upgrade-assets.md` is therefore a fail-closed hold, not an invented procedure.
|
||||
|
||||
## Compatibility interpretation
|
||||
|
||||
The M0 cross-cutting row requiring every retained/migrated artifact to validate through the executable contract is satisfied by each artifact's declared executable disposition, not by forcing versioned v1 fixtures through the v2 parser:
|
||||
|
||||
- retained examples are explicit `version: 1` fixtures validated by the production v1 parser;
|
||||
- canonical profiles validate through the shared baseline plus `roles.local` resolver;
|
||||
- the service preset validates through its production service-policy reader;
|
||||
- migration candidates validate through the production v2 compiler and shared semantic resolver.
|
||||
|
||||
The executable disposition inventory rejects undeclared additions/removals and prevents silent legacy drift.
|
||||
|
||||
## Repository-wide documentation structure
|
||||
|
||||
The accepted #758 IA is the domain book under `docs/fleet/`. Creating global `USER-GUIDE`, `ADMIN-GUIDE`, or `DEVELOPER-GUIDE` books and cleaning unrelated pre-existing `docs/` root files are outside this bounded card. The repository sitemap links the fleet book. No HTTP/API/auth contract changed, so OpenAPI and endpoint-index updates are not applicable.
|
||||
|
||||
Canonical documentation remains in-repository; no external publishing or generated publishing output is in scope. Parent issue #758 stays open through M5.
|
||||
@@ -0,0 +1,44 @@
|
||||
# FCM-M5-001 Fleet Documentation IA Closure Evidence
|
||||
|
||||
**Issue:** #758 · **Task:** FCM-M5-001
|
||||
|
||||
## Artifact map
|
||||
|
||||
- Fleet entry point and desired/observed decision tree: `docs/fleet/README.md`.
|
||||
- Concepts: `docs/fleet/concepts/` covers authority/projections, identity separation, role authority/leases, and the generated launch chain.
|
||||
- Operator workflows: `docs/fleet/how-to/` covers CRUD, lifecycle, interaction and validator instances, and role overrides.
|
||||
- Operations: `docs/fleet/operations/` covers reconciliation/recovery, quarantine, systemd/tmux troubleshooting, backup/restore boundaries, and upgrade-asset holds.
|
||||
- References: executable schema, complete field/default/constraint reference, CLI/JSON/exit behavior, lifecycle/status/drift, role authority, and generated environment boundary under `docs/fleet/reference/`.
|
||||
- Migration: preview field map, lifecycle preservation, backup/recovery prerequisites, aliases, and executable artifact dispositions under `docs/fleet/migration/`.
|
||||
- Navigation: `docs/SITEMAP.md` and the fleet entry point.
|
||||
|
||||
## Acceptance mapping
|
||||
|
||||
| Checklist area | Evidence |
|
||||
| ------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| Roster authority and fail-closed legacy handling | Root PRD FCM-REQ-01/05/08; desired/observed and quarantine pages. |
|
||||
| Classes and authority | Root PRD FCM-REQ-07; role authority concept/reference; configurable interaction/validator how-tos. |
|
||||
| Lifecycle | Root PRD FCM-REQ-04; lifecycle transition table and operator lifecycle how-to. |
|
||||
| Local-only generated launch boundary | Root PRD FCM-REQ-05/09; generated launch concept/reference. |
|
||||
| Complete DAG and artifact inventory | `docs/TASKS.md`; M0 inventory; executable disposition tests. |
|
||||
| IA pages | Every path named by the M0 checklist exists and is linked from `docs/fleet/README.md`. |
|
||||
| Examples | `docs/fleet/examples/roster-v2.yaml` validates through production v2 compiler/shared resolver; shipped artifact dispositions validate through declared production readers. |
|
||||
| Links | Deterministic local Markdown link test covers the entire fleet book and sitemap, including local heading-fragment resolution. |
|
||||
| Sensitive/example safety | Validator scans backtick- and tilde-fenced fleet-book examples plus the canonical roster for sensitive-looking keys, common credential formats (including Anthropic, OpenAI project, and Stripe restricted keys), path-qualified privileged commands, package-manager/root commands, arbitrary command override, and hardcoded Tess/Ultron identities; findings report only file/block and violation kind, never matched values. |
|
||||
| Holds | `docs/reports/deferred/758-fleet-config-deferrals.md` records M3-002, M4-002, M5-002, compatibility, and repository-structure boundaries. |
|
||||
|
||||
## Documentation completion checklist
|
||||
|
||||
- [x] Root PRD exists and remains the #758 requirements authority.
|
||||
- [ ] The accepted project-specific fleet book is indexed, but it is not complete against `FCM-REQ-03`: the required explicit programmatic `mosaic fleet validate` operation is not implemented. The CLI reference and deferral report record this gap without inventing behavior.
|
||||
- [x] Sitemap links the fleet entry point and operator-critical pages.
|
||||
- [x] No HTTP/API/auth contract changed; OpenAPI/endpoint rows are not applicable.
|
||||
- [x] Working evidence remains under `docs/scratchpads/`; closure and deferral evidence remains under `docs/reports/`.
|
||||
- [x] Canonical source remains in-repository; no external publishing action is in scope.
|
||||
- [ ] Independent exact-head documentation review, PR CI, and FCM-M5-002 release certificate remain post-PR gates and are not claimed here.
|
||||
|
||||
## Live-action boundary
|
||||
|
||||
No migration, canary, rollback, deployment, systemd/tmux/session operation, generated projection, or product mutation was performed. `roster.yaml` remains the sole writable desired-state authority. `mos-comms` remains temporary. Parent issue #758 remains open.
|
||||
|
||||
Validation command results and exact commit/tree evidence are recorded in the task scratchpad and PR body after execution.
|
||||
@@ -0,0 +1,109 @@
|
||||
# KBN-101 contract independent security/architecture review
|
||||
|
||||
**Verdict: REQUEST CHANGES**
|
||||
|
||||
## Review identity and scope
|
||||
|
||||
- **Exact reviewed head:** `da742ca2da4a2ff466916c818fe275c4f7ffd384` (`docs(#771): record role-split review evidence`)
|
||||
- **Required comparison:** `origin/main...da742ca2da4a2ff466916c818fe275c4f7ffd384`
|
||||
- **Range:** `82ce3252df38a687c50485f8d048b53ca8db5989` is an ancestor of the reviewed head; the final head adds the scratchpad evidence commit and was reviewed.
|
||||
- **Changed docs:** `docs/PRD.md`, `docs/SITEMAP.md`, `docs/native-kanban-sot/{INDEX.md,KBN-101-DB-ROLE-SPLIT.md,SHARED-CONTRACT.md,TASKS.md}`, and `docs/scratchpads/771-kbn101-db-role-split.md` (300 additions / 25 deletions).
|
||||
- **Reviewed inputs:** issue #771; current DB/Gateway/storage/config/wizard/installer/compose/Portainer/CI sources; all current migration/DDL references; KBN-010, rc.4/rc.5 shared contract, requirements/canon, KBN-100 #769 branch context, and the final scratchpad.
|
||||
- **Repository/provider state:** not modified. The pre-existing `.mosaic/orchestrator/*` dirt was not touched.
|
||||
|
||||
The role graph itself is sound in principle: a NOLOGIN platform database owner, separate NOLOGIN schema owner, NOINHERIT migrator which explicitly `SET ROLE`s, and runtime membership only in a capability role with `SET FALSE` does not create circular privilege or application-created login roles. The split of foundation certification before KBN-100 and immutable-operation certification after KBN-100 is also correctly ordered.
|
||||
|
||||
## Findings
|
||||
|
||||
### HIGH — DDL/migration control plane is not closed at every current entrypoint
|
||||
|
||||
The contract requires an explicit, locked migration phase and forbids Gateway/runtime DDL (`KBN-101-DB-ROLE-SPLIT.md:34-39`), but its KBN-101-02 result merely says migration-capable commands use the migration DTO (`:109`). It does not prohibit or route every existing bypass through that one command.
|
||||
|
||||
Current bypasses include:
|
||||
|
||||
- `runMigrations()` falls back from an argument to `DATABASE_URL` and a hard-coded URL (`packages/db/src/migrate.ts:24-35`), while `drizzle.config.ts` likewise uses `DATABASE_URL` plus a default (`packages/db/drizzle.config.ts:3-9`).
|
||||
- Package scripts expose direct `drizzle-kit migrate` **and** `drizzle-kit push` (`packages/db/package.json:23-26`); `db:push` bypasses the planned journal/fingerprint/lock entirely.
|
||||
- `mosaic storage migrate --run` shells out to the direct `db:migrate` script (`packages/storage/src/cli.ts:413-452`).
|
||||
- The federated integration test can create types, tables, and indexes directly against `DATABASE_URL` and intentionally operates without a Drizzle ledger (`packages/db/src/federation.integration.test.ts:28-30,46-134`).
|
||||
|
||||
**Failure mode:** a runtime or CI environment with only `DATABASE_URL`, or an operator invoking an existing command, can apply unverified DDL outside the lock, `SET ROLE` preflight, exact-ledger gate, and deployment sequencing. This breaks the requested fail-closed split even if Gateway startup is repaired.
|
||||
|
||||
**Required remediation:** amend KBN-101-02/03/06 to enumerate these entrypoints and make the dedicated migrator runner the only PostgreSQL DDL path. Production-like `db:push` must be removed/blocked; `db:migrate`, `storage migrate --run`, and migration tests must invoke the same migration runner with `DATABASE_MIGRATION_URL`, lock, identity preflight, and ledger verification. Tests needing schema must consume a pre-migrated disposable database, or be explicitly run only by that migration phase. Add negative tests showing each command refuses `DATABASE_URL`-only execution and cannot reach DDL.
|
||||
|
||||
### HIGH — TLS requirement has no deployable server/bootstrap contract
|
||||
|
||||
The contract correctly requires a mounted CA and hostname-verified TLS (`KBN-101-DB-ROLE-SPLIT.md:25,28,93-95`). However KBN-101-05 promises only a “migration phase and secret binding boundary” (`:112`), not PostgreSQL server TLS, certificate issuance/SANs, CA distribution, startup ordering, or the fresh/existing-database bootstrap trust path.
|
||||
|
||||
Current standalone and federated compose expose plain PostgreSQL with no server TLS configuration or CA mount (`docker-compose.yml:2-14`; `docker-compose.federated.yml:27-44`). The Portainer test stack passes a single plaintext in-network URL and uses the same database login for Gateway and database bootstrap (`deploy/portainer/federated-test.stack.yml:51-60,110-117`).
|
||||
|
||||
**Failure mode:** enforcing the mandatory CA makes current local standalone/federated topologies unable to start; relaxing it to make bootstrap work silently violates K101-REQ-03. A first database cannot be safely migrated until the server certificate, its SAN for the actual service/DNS name, and trusted CA are provisioned, but this lifecycle is not owned or tested.
|
||||
|
||||
**Required remediation:** add a concrete KBN-101-00/05 TLS bootstrap sub-contract: issuer/CA owner; server key/cert and SAN inputs; secure storage/mount permissions; `postgresql.conf`/container TLS enablement; migration and runtime CA mounts; hostname used by each compose/Swarm service; readiness only after TLS authentication; CA overlap rotation; and an existing-database transition. Require a disposable standalone and federated/Swarm test to prove verified TLS succeeds and missing CA, wrong CA, wrong SAN, and `sslmode` downgrade fail before readiness. Do not merge KBN-101-05 with an implicit plaintext exception.
|
||||
|
||||
### HIGH — exact ledger fingerprint and historical 0009 repair are underspecified for existing databases
|
||||
|
||||
The contract requires an “ordered complete set” and rejection of out-of-order rows (`KBN-101-DB-ROLE-SPLIT.md:36-38`), but does not define the canonical serialized tuple, ledger ordering source, or safe upgrade rule for a historical ledger. The current ledger stores only `id`, `hash`, and `created_at` (`packages/db/src/migrate.ts:70-82,105-107`). Its journal is demonstrably non-monotonic: `0008` has `when=1776822435828`, followed by `0009` at `1745280000000` (`packages/db/drizzle/meta/_journal.json:62-79`); the existing PostgreSQL runner documents that this causes skipping (`packages/db/src/migrate.ts:29-35`).
|
||||
|
||||
**Failure mode:** an implementation can either reject a legitimate historical database after correcting 0009, or accept a reordered/duplicated ledger because no precise comparison rule exists. A count/hash-set implementation would fail to detect the condition that this contract explicitly calls unsafe; physical `id` order is not an adequate substitute after historical repair.
|
||||
|
||||
**Required remediation:** freeze a versioned manifest algorithm before implementation: canonical record fields (at least journal index/tag, corrected logical order, migration content hash, and an explicit migration-manifest version), canonical byte serialization, SHA-256 input, and exact observed-ledger mapping. State whether physical ledger insertion order is normative; if not, compare hash-to-manifest tuples rather than timestamps. Add an idempotent migrator-only 0009 existing-database remediation/reconciliation procedure with backup/rollback evidence. Require clean, pre-0009, 0009-skipped, 0009-applied-late, duplicate, unknown, missing, corrupt-pair, and stale-replica cases. No manual ledger insertion is an acceptable production recovery path.
|
||||
|
||||
### MEDIUM — advisory-lock namespace is collision-prone and lacks a fixed identifier contract
|
||||
|
||||
The specified lock is `pg_try_advisory_lock(hashtext('mosaic-schema-migration-v1'))` (`KBN-101-DB-ROLE-SPLIT.md:34`). `hashtext` produces a 32-bit key. Session ownership/crash behavior is otherwise correctly stated (one session, same-session release, connection-close release), but an unrelated database user can accidentally collide or deliberately hold the key and force `DATABASE_MIGRATION_LOCKED`.
|
||||
|
||||
**Failure mode:** avoidable migration denial of service in a shared PostgreSQL database. The current repository already uses separate `hashtext` advisory-lock names for migrate-tier, demonstrating the need for a documented namespace rather than a collision-prone implicit one.
|
||||
|
||||
**Required remediation:** freeze a two-int advisory-lock namespace (fixed documented class/object values) or a documented 64-bit `hashtextextended` key with fixed seed; keep acquisition, migration, verification, and release on the single `max:1` migrator session. Add tests for concurrent migration, connection loss/crash release, readiness while the lock holder is active, and an unrelated lock-key non-interference case.
|
||||
|
||||
### MEDIUM — identifier safety and `search_path` verification need executable constraints
|
||||
|
||||
The contract rightly requires `pg_catalog, <mosaic_application_schema>` and rejects writable paths (`KBN-101-DB-ROLE-SPLIT.md:54,70-77`), but uses dynamic placeholders for database/schema and does not state how migration/bootstrap SQL will avoid identifier interpolation. Existing code has raw-SQL facilities (`packages/storage/src/migrate-tier.ts` uses `.unsafe`), so this is not merely theoretical.
|
||||
|
||||
**Failure mode:** a future operator-configured database/schema value that reaches bootstrap or `SET search_path` through raw string construction can inject DDL, or a pooled connection can retain a mutable search path.
|
||||
|
||||
**Required remediation:** require fixed allowlisted identifiers or server-side identifier quoting (`format('%I', ...)`) only; never interpolate URL/config values into SQL. Set and verify the trusted path per connection/session before any query (`SET LOCAL` inside transactions where applicable), forbid `public`/`$user` additions, and add injection-shaped identifier and pooled-connection reset negatives. Include this in KBN-101-00/01 tests.
|
||||
|
||||
## Acceptance and threat traceability
|
||||
|
||||
| Requirement / threat | Review result | Evidence or blocking finding |
|
||||
| ----------------------------------------------------- | ------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| K101-REQ-01 / AC-K101-01 split runtime/migration URLs | Partial | Role/DTO boundary is coherent; HIGH DDL-path finding requires all current commands to be closed. |
|
||||
| K101-REQ-02 / AC-K101-02 explicit migration/readiness | Blocked | HIGH ledger definition and HIGH DDL-bypass findings. |
|
||||
| K101-REQ-03 / AC-K101-03 least privilege, TLS, grants | Partial | Role model, default privileges, ledger read-only, TEMP/function checks are well specified (`KBN-101...:47-56,70-79`); HIGH TLS bootstrap and MEDIUM identifier constraints remain. |
|
||||
| K101-REQ-04 / AC-K101-04 immutable relations | Correctly deferred | KBN-101-09 after KBN-100 is the correct serial gate (`KBN-101...:58-66,115-118`); no synthetic-only certification claim found. |
|
||||
| K101-REQ-05 / AC-K101-05 N-1, secrets, rollback | Partial | No owner-runtime exception and rollback keeps migration URL out of Gateway (`:83-95`); deployable TLS and full command inventory are missing. |
|
||||
| K101-REQ-06 / AC-K101-07 KBN gates and DAG | Structurally sound | DAG is acyclic: 00→01/{03}; 02→06; 00/01/03→05; 00/04/05/06→07→08→KBN-100→09→KBN-105. KBN-100’s current branch contains docs-only baseline tracking, not schema implementation. |
|
||||
| T: runtime DDL / migration fallback | Blocked | HIGH finding 1. Current Gateway/storage, CLI, direct Drizzle scripts, and integration DDL require explicit closure. |
|
||||
| T: race/crash/readiness | Partial | Same-session nonblocking lock and replica-unready rules are present (`:34-38`); lock namespace remediation required. |
|
||||
| T: immutable evidence rewrite | Correctly staged | Explicit INSERT/SELECT-only matrix and RESTRICT retention are retained; proof is properly after table creation. |
|
||||
| T: secret leakage / TLS downgrade | Partial | Redaction and distinct Vault paths are specified (`:93-97`), but no server TLS/bootstrap implementation contract exists. |
|
||||
|
||||
## Unresolved assumptions
|
||||
|
||||
1. `standalone` and `federated` are the complete PostgreSQL production-like set (K101-A1).
|
||||
2. Each eligible deployment can execute a dedicated migration Job/one-shot phase (K101-A2).
|
||||
3. Vault path names are targets, not verified existing paths; deployment ownership remains to be established.
|
||||
4. PostgreSQL 17 is available for the selected membership and advisory-lock implementation.
|
||||
5. The required server-side TLS issuer/certificate lifecycle and Swarm/compose secret transport have not been decided; this is blocking, not a permissible implicit plaintext bootstrap.
|
||||
6. Historical databases containing the 0009 journal/ledger anomaly have no frozen reconciliation procedure.
|
||||
|
||||
## Independent test and consistency evidence
|
||||
|
||||
Read-only checks run in this review:
|
||||
|
||||
| Check | Result |
|
||||
| --------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| `git diff --check origin/main...da742ca2...` | PASS |
|
||||
| `pnpm exec prettier --check` on all seven changed docs | PASS |
|
||||
| `pnpm exec tsc --noEmit -p docs/native-kanban-sot/tsconfig.json` | PASS |
|
||||
| `docker compose -f docker-compose.yml config --quiet` (isolated test ports) | PASS |
|
||||
| `docker compose -f docker-compose.federated.yml --profile federated config --quiet` (isolated test ports) | PASS |
|
||||
| Static journal inspection | FAILS the required monotonic ordering premise: 0008 → 0009 `when` decreases; current runner documents skipping behavior. |
|
||||
| Static DDL-entrypoint inventory | Found direct Drizzle scripts, storage CLI shell-out, runtime extension/migration calls, fleet backlog migration, tier probe extension creation, and a direct-DLL federated integration test. |
|
||||
|
||||
No live database, Vault, CI, deployment, issue, PR, or repository mutation was performed. The pass results validate documentation syntax/contract compilation and compose syntax only; they do **not** certify the proposed security behavior.
|
||||
|
||||
## Conclusion
|
||||
|
||||
Do not merge this frozen contract as implementation-ready until the HIGH findings are corrected and independently re-reviewed. The central role ownership/default-privilege design, immutable-table staging, and KBN-100/KBN-105 serial gating should be retained; they are not the reason for this REQUEST CHANGES verdict.
|
||||
Reference in New Issue
Block a user