diff --git a/docs/adr/ADR-046-event-sourced-proposals.md b/docs/adr/ADR-046-event-sourced-proposals.md index 826d3579..e442af12 100644 --- a/docs/adr/ADR-046-event-sourced-proposals.md +++ b/docs/adr/ADR-046-event-sourced-proposals.md @@ -247,7 +247,7 @@ maintains as a fold over the four proposal events: ```sql CREATE TABLE proposals_open ( - proposal_id BLOB PRIMARY KEY, + proposal_id TEXT PRIMARY KEY, namespace TEXT NOT NULL, proposer TEXT NOT NULL, title TEXT NOT NULL, @@ -255,7 +255,7 @@ CREATE TABLE proposals_open ( created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL, expiry INTEGER, - last_decision TEXT, -- JSON of the most recent ProposalReviewedPayload.decision + last_decision TEXT, -- bare decision string from the most recent ProposalReviewedPayload review_count INTEGER NOT NULL DEFAULT 0, approve_count INTEGER NOT NULL DEFAULT 0, reject_count INTEGER NOT NULL DEFAULT 0 @@ -287,8 +287,8 @@ the proposal is not permanently stuck. `'applying'` is never written to the event log — it is a purely transient projection state. Hard-state (status != 'open' | 'changes_requested') rows are retained for -audit; cleanup is operator-driven (`kkernel call kg proposal_cleanup ---older-than 90d`). +audit. A `proposal_cleanup` operator command is deferred; future work must +define the CLI surface, retention policy, and safe-delete semantics. **Review history retrieval (Fix 7):** The projection stores only aggregates (`review_count`, `approve_count`, `reject_count`). Individual `ProposalReviewed` @@ -375,68 +375,30 @@ feedback. The check fires only when `decision=approve`; rejecting one's own proposal is allowed (treated as withdrawal-via-reject). When `ProposalPolicy::allow_self_approve = true`, the check is skipped entirely. -This is configurable in pack manifest (`kg.proposals`): +The shipped v1 default is **one approve from any non-self actor, no recorded +reject**. The inline self-approval guard in the `review` handler is the only +shipped policy enforcement point. -```toml -[packs.kg.proposals] -approval_threshold = 1 # number of approve votes required -allow_self_approve = true # OSS default: single-developer deployments work out-of-box -require_listed_reviewer = false # if true, only the listed reviewers can approve -``` - -**Defaults for OSS happy-path (Fix 10):** - -- `approval_threshold = 1` -- `allow_self_approve = true` — single-developer deployments work out-of-box - without needing a second actor - -Multi-actor deployments override with -`ProposalPolicy { approval_threshold: 2, allow_self_approve: false, ... }`. -The OSS default optimizes for single-developer ergonomics; review-gated -deployments are explicit opt-in. +Configurable approval thresholds, pack manifest TOML configuration +(`[packs.kg.proposals]`), `ProposalPolicy` struct instantiation, and +`require_listed_reviewer` are deferred. Multi-actor deployments requiring +configurable thresholds or reviewer lists must await a future ADR amendment +before those controls are available. -More sophisticated policies (M-of-N, weighted reviewers, mandatory operator -approval for high-blast-radius changesets) are operator-config additions, not -v1 ADR scope. +### ProposalPolicy: pack-owned, gate-enforced (deferred) -### ProposalPolicy: pack-owned, gate-enforced - -`ProposalPolicy` is proposal-pack configuration (not ADR-018-owned). ADR-046 -defines the policy struct and registers a `PackGatePolicy` implementation with -the authorization gate (ADR-018). The gate invokes the registered policy for -proposal verbs. - -```rust -pub struct ProposalPolicy { - pub allow_self_approve: bool, - pub approval_threshold: u32, - pub require_listed_reviewer: bool, -} - -pub struct ProposalGatePolicy { - policy: ProposalPolicy, -} - -impl PackGatePolicy for ProposalGatePolicy { - fn evaluate(&self, input: &GateRequest, ctx: &GateContext) -> GateDecision { - if input.verb == VerbName::new("review") - && !self.policy.allow_self_approve - && proposer_of(&input, ctx) == input.actor.id - { - return GateDecision::Deny { - reason: "self-approve forbidden by ProposalPolicy".to_string(), - }; - } - GateDecision::Allow { obligations: vec![] } - } -} -``` +`ProposalPolicy`, `ProposalGatePolicy`, and `PackGatePolicy` are deferred. +The shipped v1 enforcement is the inline self-approval guard in `handle_review`: +the handler reads `proposals_open.proposer` and rejects with +`RuntimeError::SelfApprovalForbidden { proposal_id, actor_id }` when +`decision=approve` and `actor.id == proposer`. This check fires before any +event is emitted, giving immediate feedback. -The `ProposalGatePolicy` is registered with the gate via -`VerbRegistryBuilder::with_pack_policy` during KG pack initialization. The gate -remains the authoritative trust boundary; the handler also defensively checks -`proposals_open.proposer == actor.id` to give callers immediate feedback before -emitting events (defense-in-depth, not sole enforcement). +The full configurable policy struct, gate registration, and +`VerbRegistryBuilder::with_pack_policy` wiring are future work. When shipped, +`ProposalGatePolicy` will register with the ADR-018 authorization gate as the +authoritative trust boundary; the handler's inline check will remain as a +defense-in-depth layer but not the sole enforcement point. ### 7. Brain integration @@ -465,10 +427,12 @@ verbs and the apply worker each have policy hooks: who need restrictions add a rego rule. - `review`: policy decides whether `actor` can review proposals in `namespace`. Default: any actor. Operators may restrict to specific roles. -- `propose-apply` worker: policy decides whether the worker's `system:propose-apply` - actor can write the target namespace's records. Default: yes for the - pack-owned namespace, denied for foreign namespaces. This is the security - boundary that prevents cross-namespace proposal injection. +- `propose-apply` worker: the worker emits `ActorRef { kind: "system", id: "propose-apply" }` + as its actor identity. The authorization gate evaluates this identity against + the active policy; with the OSS default (AllowAllGate) it is permitted + transparently. A dedicated `system:propose-apply` policy rule is future work; + production deployments requiring explicit cross-namespace injection prevention + should add a rego rule for this actor class when configuring ADR-018. ### 9. Failure modes @@ -478,20 +442,20 @@ verbs and the apply worker each have policy hooks: | Apply fails (validation, network, etc.) | `ProposalApplied { Failed }` emitted; status is reverted from `'applying'` back to `'approved'` (best-effort CAS) so the proposal is not permanently stuck. Apply retry is deferred to a follow-up ADR. v1 behavior: failed applies return to `'approved'`; operators may issue a new `propose` (with `parent_id` referencing the failed proposal) to retry. Direct re-emission of `apply` events is not supported in v1. | | Apply policy denied | Same as Apply fails with `error = "denied by policy"`. Operator adjusts policy and issues a new `propose` (with `parent_id`) to retry; direct `apply` re-emission is not supported in v1. | | Reviewer reverses Approve to Reject | Each review is its own event; the worker uses the latest decision per reviewer. If a previously-approved proposal hits Reject before Apply fires, status moves to 'rejected'. | -| Two reviewers race (both Approve simultaneously) | Each emits its own `ProposalReviewed` event; the apply worker is single-threaded per process — it sees them in event order and fires apply on the first one that crosses threshold. Idempotency on the worker side: it checks `proposals_open.status` before applying; if already applied, no-op. | +| Two reviewers race (both Approve simultaneously) | Each `review(approve)` call invokes the apply worker synchronously after `reviewed_and_emit`. The `reviewed_and_emit` CAS serializes concurrent reviews at the projection layer; the apply worker’s `approved → applying` CAS ensures only one invocation executes the changeset. The worker checks `proposals_open.status` before applying; if already `applied` or `applying`, it returns without re-executing. | | Proposal expires | A background sweep (TBD: cron-style, not v1) emits `ProposalWithdrawn` with `by = "system:expiry"` on proposals past their `expiry` timestamp. | | Stale-target conflict (Fix 6) | An `UpdateEntity` or `MergeEntities` proposal targets a specific entity ID. Between propose-time and apply-time the entity may be independently modified. v1 default: **last-writer-wins** — the proposal applies its patch unconditionally. Optional: proposals may include `expected_version: Option` in the payload (if entity versioning is introduced via ADR-014 amendment). The apply worker would then check `current_version == expected_version` and emit `ProposalApplied { Failed { error: "stale: target was modified since proposal", current_version, expected_version } }` on mismatch. v1 does NOT introduce entity versioning; this knob is gated on a future ADR-014 amendment. | ### 10. CLI / MCP surface summary -| Surface | Action | How | -| -------------------------------------------------------------- | --------------------------------------------------- | -------------------------------------- | -| MCP `propose(...)` | Create a proposal | Verb | -| MCP `review(proposal_id, decision, comment?)` | Cast a review | Verb | -| MCP `withdraw(proposal_id, rationale?)` | Withdraw a proposal (proposer-only) | Verb | -| MCP `list(kind=proposal, status="open")` | Browse open proposals | Lists from `proposals_open` projection | -| MCP `get(id=)` | Fetch a single proposal's `ProposalCreated` payload | Resolves to the event payload | -| CLI `kkernel call kg proposal_cleanup --older-than ` | Archive resolved proposals | Operator housekeeping | +| Surface | Action | How | +| -------------------------------------------------------------- | --------------------------------------------------------- | -------------------------------------- | +| MCP `propose(...)` | Create a proposal | Verb | +| MCP `review(proposal_id, decision, comment?)` | Cast a review | Verb | +| MCP `withdraw(proposal_id, rationale?)` | Withdraw a proposal (proposer-only) | Verb | +| MCP `list(kind=proposal, status="open")` | Browse open proposals | Lists from `proposals_open` projection | +| MCP `get(id=)` | Fetch a single proposal's `ProposalCreated` payload | Resolves to the event payload | +| CLI `kkernel call kg proposal_cleanup --older-than ` | Archive resolved proposals (deferred — not shipped in v1) | Future operator housekeeping | `list(kind=proposal)` dispatches to a new `kg.list_proposals` handler under the kg pack — it queries `proposals_open` directly, supports the standard @@ -622,24 +586,22 @@ supersede, compound). Future arms add to the enum via additive semver bumps. ### Crate placement -- Verb handlers: `khive-pack-kg::proposals` -- Apply worker: `khive-pack-kg::proposals::apply_worker` -- Projection table + projection worker: `khive-pack-kg::proposals::projection_worker` +- Verb handlers: `crates/khive-pack-kg/src/handlers.rs` +- Apply worker: `crates/khive-pack-kg/src/apply_worker.rs` +- Projection table + projection worker: `crates/khive-pack-kg/src/projection_worker.rs` - Payload types: `khive-types::events::proposal_payloads` ### Migration -The current latest migration in `khive-db` is V4 (`dedupe_graph_edge_triples`). -ADR-043 (Embedding Model Migration) takes V5. This ADR's migration is **V6**. -**Ordering rule:** ADR-043 migration (V5) MUST be applied before this ADR's -migration (V6) — the version sequence must remain contiguous and ADR-043 was -specified first. `proposals_open` has no schema dependency on ADR-043's tables. +The `proposals_open` projection table was created in migration V15 in +`crates/khive-db/src/migrations.rs`. The `applying` transient status and +its CAS invariants were added in V18. -A new `VersionedMigration` in `crates/khive-db/src/migrations.rs`: +A `VersionedMigration` in `crates/khive-db/src/migrations.rs`: ```rust VersionedMigration { - version: 6, + version: 15, name: "proposals_open", up: PROPOSALS_OPEN_DDL, } @@ -674,25 +636,25 @@ form (ADR-017 §pack handler trait shape; `VerbDef/VERBS` is deprecated): pub const HANDLERS: &[HandlerDef] = &[ // ... existing handlers ... HandlerDef { - name: "propose", - visibility: Visibility::Verb, - speech_act: SpeechAct::Commissive, - handler: Handler::Proposals(ProposalsHandler::Propose), - presentation_policy: VerbPresentationPolicy::Standard, + name: "propose", + description: "Create a proposal for a KG change.", + visibility: Visibility::Verb, + category: Category::Proposals, + params: &PROPOSE_PARAMS, }, HandlerDef { - name: "review", - visibility: Visibility::Verb, - speech_act: SpeechAct::Declaration, - handler: Handler::Proposals(ProposalsHandler::Review), - presentation_policy: VerbPresentationPolicy::Standard, + name: "review", + description: "Approve, reject, comment, or request changes on a proposal.", + visibility: Visibility::Verb, + category: Category::Proposals, + params: &REVIEW_PARAMS, }, HandlerDef { - name: "withdraw", - visibility: Visibility::Verb, - speech_act: SpeechAct::Commissive, - handler: Handler::Proposals(ProposalsHandler::Withdraw), - presentation_policy: VerbPresentationPolicy::Standard, + name: "withdraw", + description: "Rescind a proposal (proposer only).", + visibility: Visibility::Verb, + category: Category::Proposals, + params: &WITHDRAW_PARAMS, }, ]; ``` @@ -743,11 +705,12 @@ for v1, the JSON path is sufficient. Lookup wire shape: - `get(id=)` resolves to the specific event record by event UUID. -- For aggregate-ID lookup (all events for a proposal), use the events query - surface with `EventFilter { kinds: vec![EventKind::ProposalCreated], ... }` - and a payload predicate on `proposal_id`. Do NOT overload bare `get(id=...)` - to silently resolve event UUIDs OR aggregate IDs — that creates ambiguous - collision policy. +- `get(id=)` resolves raw proposal IDs and short prefixes via + `proposals_open` and returns the `ProposalCreated` event payload from the + event log. +- For full review history, use the events query surface with + `EventFilter { kinds: vec![EventKind::ProposalReviewed], ... }` and a + payload predicate on `proposal_id`. ## References diff --git a/docs/adr/ADR-048-knowledge-section-profiles.md b/docs/adr/ADR-048-knowledge-section-profiles.md index 91919962..07be18d9 100644 --- a/docs/adr/ADR-048-knowledge-section-profiles.md +++ b/docs/adr/ADR-048-knowledge-section-profiles.md @@ -22,6 +22,36 @@ compose/suggest, hooks, lint, export, and observability phases. | Resource entity dual-write | deferred | `knowledge.upsert_atoms` and `knowledge.upsert_domains` write corpus tables; domain mirror is into `knowledge_atoms` for FTS, not graph `entities`. | | `knowledge.lint`, `knowledge.lint_config`, `knowledge.export` | deferred | These verbs are not registered in the shipped knowledge pack. | +## Governance for Shipped Knowledge Sections and Profiles + +ADR-048 treats knowledge sections as shipped corpus storage and lifecycle state, while +profile persistence remains owned by the brain pack. The knowledge pack does not ship a +`knowledge_profiles` table or knowledge-local profile verbs. Shipped profile persistence +is the V20 brain profile snapshot/event-log model, and section posterior learning is +driven through brain profile state and feedback events. + +`knowledge_sections` is the authoritative table for atom sections. Section edits are +keyed by `(atom_id, section_type)`: `knowledge.edit` upserts only the specified sections, +preserves sibling sections, clears stale section embeddings, and downgrades edited verified +sections to reviewed. `knowledge.import` supports `atlas_md` file/directory import and, +with the default section chunk strategy, parses section headings into section rows. + +Section lifecycle governance is explicit. `knowledge.challenge` marks an eligible section +as disputed and increments the atom dispute counter. `knowledge.adjudicate` requires a +disputed section; accept marks the section verified, reject returns it to reviewed, and the +atom dispute counter is decremented. + +`knowledge.suggest` is a base domain-discovery verb. It accepts query/role/limit, searches +domains, may fuse ANN results when the index is warm, reranks with embeddings, and returns +domain IDs, names, and scores. It does not implicitly resolve a brain profile or apply +profile-weighted section scoring in the shipped implementation. + +`knowledge.compose` is a base explicit-composition verb. It requires explicit domain IDs +and/or atom IDs plus query context, resolves those records, reranks atom text, and returns +markdown with atom/domain metadata. It does not emit implicit feedback, does not call +`brain.resolve`, and does not perform section-manifest weighting in the shipped +implementation. + ## Context Knowledge atoms in the corpus tier ([ADR-047](ADR-047-knowledge-pack.md)) store content as @@ -84,13 +114,15 @@ CREATE TABLE IF NOT EXISTS knowledge_sections ( embedding BLOB, created_at INTEGER NOT NULL, updated_at INTEGER NOT NULL, + status TEXT NOT NULL DEFAULT 'draft', FOREIGN KEY (atom_id) REFERENCES knowledge_atoms(id), UNIQUE(atom_id, section_type) ); ``` -V21 also creates indexes on `atom_id`, `(namespace, section_type)`, and `(namespace, atom_id)`, +V21 creates indexes on `atom_id`, `(namespace, section_type)`, and `(namespace, atom_id)`, plus an external-content FTS5 table `fts_sections` with insert/delete/update triggers. +V22 adds the `status` column and `idx_knowledge_sections_status`. Section_type is a closed enum matching the atlas schema v1: `overview`, `core_model`, `boundary_conditions`, `formalism`, `operational_guidance`, `examples`, `failure_modes`, @@ -1045,6 +1077,41 @@ Every phase ships with benchmarks that gate merge: --- +## Shipped Schema Reference + +### `knowledge_sections` (V21 + V22) + +V21 creates `knowledge_sections`; V22 adds the `status` lifecycle column. The shipped +columns, constraints, and indexes are: + +- `id TEXT PRIMARY KEY` +- `atom_id TEXT NOT NULL` +- `namespace TEXT NOT NULL` +- `section_type TEXT NOT NULL` +- `heading TEXT NOT NULL DEFAULT ''` +- `content TEXT NOT NULL DEFAULT ''` +- `tokens INTEGER NOT NULL DEFAULT 0` +- `sort_order INTEGER NOT NULL DEFAULT 0` +- `embedding BLOB` (nullable) +- `created_at INTEGER NOT NULL` +- `updated_at INTEGER NOT NULL` +- `status TEXT NOT NULL DEFAULT 'draft'` (added by V22) +- `FOREIGN KEY (atom_id) REFERENCES knowledge_atoms(id)` +- `UNIQUE(atom_id, section_type)` + +Indexes: `idx_knowledge_sections_atom`, `idx_knowledge_sections_ns_type`, +`idx_knowledge_sections_ns_atom`, `idx_knowledge_sections_status`. + +Full-text search: `fts_sections`, an external-content FTS5 table over `heading` and +`content`, with `id`, `namespace`, `atom_id`, and `section_type` as unindexed metadata. +FTS insert, delete, and update triggers maintain the FTS table on section changes. + +### Profile Persistence (brain-owned, not knowledge-local) + +ADR-048 does not ship a `knowledge_profiles` table. Profile persistence is brain-owned; +the authoritative shipped tables are `brain_profile_snapshots` and `brain_event_log` +(see V20 DDL note in the Amendment section below and ADR-032). + ## Amendment: Research-Informed Design Corrections (2026-05-27) **Authors**: Ocean, lambda:khive @@ -1121,9 +1188,26 @@ order produces different posteriors. This breaks event-sourced snapshot recovery ```markdown V20 brain persistence DDL in this ADR is superseded by ADR-032 and ADR-015 V20. The -authoritative shipped tables are `brain_profile_snapshots(profile_id, namespace, -snapshot_json, updated_at)` and `brain_event_log(id, profile_id, namespace, event_kind, -payload, created_at)`, plus `idx_brain_events_profile`. +authoritative shipped tables are defined in the brain pack. There is no shipped +`knowledge_profiles` table; profile persistence is brain-owned. + +`brain_profile_snapshots`: + +- `profile_id TEXT NOT NULL` +- `namespace TEXT NOT NULL DEFAULT 'default'` +- `snapshot_json TEXT NOT NULL` +- `updated_at INTEGER NOT NULL` +- `PRIMARY KEY (profile_id, namespace)` + +`brain_event_log`: + +- `id INTEGER PRIMARY KEY AUTOINCREMENT` +- `profile_id TEXT NOT NULL` +- `namespace TEXT NOT NULL DEFAULT 'default'` +- `event_kind TEXT NOT NULL` +- `payload TEXT NOT NULL` +- `created_at INTEGER NOT NULL` +- `idx_brain_events_profile` on `(profile_id, namespace, created_at)` ``` ### Correction 3: Filtered ANN — StitchedVamana, not ACORN