Creatidy/creatidy-kernel
5
0
Fork 0
Code Issues 9 Pull requests 2 Projects Releases Packages Wiki Activity Actions

[K2B] Journal external operations and durable artifacts #17

Merged
adrian.tkacz merged 1 commit from feat/13-operation-journal into develop 2026-09-25 21:47:35 +00:00
Conversation 2 Commits 1 Files changed 4 +917 -11
kilo commented 2026-09-25 20:54:42 +00:00
Member
Copy link

Refs #13.

Implemented K2B local-only journal/outbox, versioned Operation requests and distinct dedupe identities; atomic K1 admission and pending intent; durable fenced attempts, transport/domain observations and reconciliation; content-addressed artifacts with verified backup/restore and v1->v2 migration. Synthetic B/D/E regressions cover crash/receipt boundaries. No live provider/network, K3/K4, or automatic merge.

Reviewed feature HEAD a7035f31c36c9a19d7c3a24c96abbced6ac80116: make check (152 passed), make package-check, make audit (no known vulnerabilities), and git diff --check passed; make check and make package-check reran after review. Forgejo PR CI #1944 succeeded.

Fresh independent full review #600 covers all issue criteria and concludes APPROVE, no blocking findings, at the reviewed commit. Forgejo recorded it as COMMENT, not platform approval, because the bot account authored the PR. The owner merged this PR into develop as 059482d045bd8e772005c9fbc1a9b89ce64ce26f; issue #13 is closed. Delivery ledger records integration and remediation 0/10. This is not promotion to main.

Refs #13. Implemented K2B local-only journal/outbox, versioned Operation requests and distinct dedupe identities; atomic K1 admission and pending intent; durable fenced attempts, transport/domain observations and reconciliation; content-addressed artifacts with verified backup/restore and v1->v2 migration. Synthetic B/D/E regressions cover crash/receipt boundaries. No live provider/network, K3/K4, or automatic merge. Reviewed feature HEAD `a7035f31c36c9a19d7c3a24c96abbced6ac80116`: `make check` (152 passed), `make package-check`, `make audit` (no known vulnerabilities), and `git diff --check` passed; `make check` and `make package-check` reran after review. Forgejo PR [CI #1944](https://forgejo.creatidy.com/Creatidy/creatidy-kernel/actions/runs/30) succeeded. Fresh independent full [review #600](https://forgejo.creatidy.com/Creatidy/creatidy-kernel/pulls/17#issuecomment-11673) covers all issue criteria and concludes **APPROVE**, no blocking findings, at the reviewed commit. Forgejo recorded it as `COMMENT`, not platform approval, because the bot account authored the PR. The owner merged this PR into `develop` as `059482d045bd8e772005c9fbc1a9b89ce64ce26f`; issue #13 is closed. [Delivery ledger](https://forgejo.creatidy.com/Creatidy/creatidy-kernel/issues/13#issuecomment-11654) records integration and remediation `0/10`. This is not promotion to `main`.
feat: journal external operations and durable artifacts
All checks were successful
ci / check (pull_request) Successful in 18s
Details
a7035f31c3
kilo reviewed 2026-09-25 21:29:41 +00:00
kilo left a comment
Copy link

PR Review: [K2B] Journal external operations and durable artifacts (#17)

Review-state note: the reviewer's verdict is APPROVE (no blocking findings). Forgejo refused a formal APPROVED review state because the reviewer MCP identity (kilo) matches the PR author, and the platform forbids approving one's own pull request (approve your own pull is not allowed). This COMMENT-state review is the verbatim record of the independent review; it is not a Forgejo-recorded approval, and merge authority remains with the owner.

Repository: Creatidy/creatidy-kernel
Branch: feat/13-operation-journal -> develop
Frozen head: a7035f31c36c9a19d7c3a24c96abbced6ac80116 (base 2fa51e9)
Linked issue: #13
Reviewer: fresh read-only session ses_f258ca123ffeg2RLzMN8vVC9B4, distinct from the implementation session; running model zai-coding-plan/GLM-5.3 (max) matches the routed selection zai/glm-5.3/max. Local HEAD verified identical to the frozen head; no branch edits, pushes, or merges performed.

Summary

Adds the K2B external-operation journal to the SQLite adapter: versioned/canonical Operation requests with distinct operation/effect identities, atomic K1 admission + INTENT/outbox (admit_with_intent) plus a standalone intent(), fenced and leased delivery attempts with transport/observation/reconciliation facts in append-only tables, retry gated on authoritative absence or proven idempotency, and content-addressed artifacts flushed to disk before an immutable DB reference. Schema v1->v2 migration, artifact-aware backup manifests, and verified restore. 10 new synthetic tests; no live provider, network, K3/K4, or merge automation.

Intent / Acceptance Evidence

#13 acceptance criterion Evidence at frozen head Status
Stable identity, versioned/canonical digest; command-admission, logical-effect, inbound-observation dedupe distinct and independently tested operations.operation_id PK + effect_key UNIQUE; {"version":1,"request":...} canonical JSON + sha256 digest; K2A command_admissions vs _insert_intent effect binding (tests B, test_command_and_effect_outbox_are_one_transaction) vs observation_id PK with identity-bound evidence (test E) MET
INTENT + outbox commit before fake effect; no external I/O inside a DB transaction claim() commits attempt/lease/fence before deliver_fake invokes the callable; test D asserts in_transaction is False inside the fake; test_outbox_failure_rolls_back_command_admission proves atomic rollback MET
Attempts, domain ack/rejection, running/waiting/terminal/unknown distinct durable facts; observations deduplicated Four append-only tables (delivery_attempts, operation_transports, operation_observations, operation_reconciliations) with no-update/no-delete triggers; observation replay idempotent, rebinding conflicts (test E) MET
Leases/fences prevent concurrent ownership; stale callback/fence rejected without rewriting history claim() lease + retry-proof gates; observe()/record_transport() require the current fence and reject reconciled/late publication (tests D, E) MET
Crash uncertainty preserved; reconcile before retry; never advances from intent or delivery alone Post-crash status stays dispatched; retry requires retry_proof in {absent, idempotent}; _validate_operation_records re-derives acceptance from durable receipts and rejects forgery (tests E, test_restart_rejects_forged_acceptance_and_missing_outbox) MET
Artifacts content-addressed, finalized before immutable reference; reopen/backup/restore preserve verified references finalize_artifact: fsynced temp + atomic rename before DB insert; immutability conflict on divergence; startup/backup/restore re-hash blobs; tamper raises CorruptHistory (test 5) MET
Named B/D/E synthetic regressions test_b_intent_is_not_dispatch_or_review_acceptance (no accepted-review wait, Program stays DRAFT, waiting-without-acceptance rejected), test_d_transport_success_is_not_domain_acceptance (transport success persisted as rejection), test_e_missing_rejected_accepted_and_uncertain_receipts (distinct receipts, durable acceptance reference, reconcile-and-redispatch same operation) MET
Synthetic-only tests, no network/Forgejo/Runtime/Codex/M5-B/Prefect/cloud Local tmp SQLite + fake callables only; imports limited to adapters/core/stdlib MET

Architecture / Information Flow

Dependency direction preserved: changes confined to adapters/sqlite_store.py plus tests; stdlib-only additions (shutil, Callable). The K2B surface lives at adapter level, consistent with ADR 0003 (durability in the store) and with the K2A precedent where the ProgramStore protocol covers only create/load/admit; consumers arrive with K3 (#6). Implementation tracks ADR 0003 point-for-point: intent+outbox before dispatch, fence/lease before I/O, uncertainty retention, found-effect reconciliation, authoritative-absence-only retry, lease expiry as lost coordination rather than remote failure, artifacts flushed and atomically finalized before reference. Fail-closed posture throughout (OperationConflict, CorruptHistory).

Blocking Issues

None found.

Non-blocking Suggestions

  1. _verify_backup_manifest validates artifact digests by length only. A non-hex or path-traversing name is currently neutralized structurally (a digest string containing / can never equal a sha256 hexdigest), but an explicit hex-charset check would encode that invariant instead of relying on it.
  2. __init__ detects a backup bundle (kernel.sqlite3 + manifest.json), verifies it, then opens the bundle database in place for read/write; later writes silently diverge from the verified manifest digest. Consider refusing in-place writes or documenting the guard as open-time integrity verification.
  3. _validate_operation_records re-hashes every artifact blob on each open (O(total artifact bytes)). Correctness-first is appropriate now; worth remembering when artifact volumes grow.
  4. artifact() verifies the blob, then reopens it to read (small TOCTOU window); negligible under the single-controller model.

Questions for the Author

None blocking.

Security / Privacy Notes

No secrets, no network, no provider integrations. Authority and corruption boundaries fail closed: forged acceptance rejected at startup, bundle tampering detected on open and restore, stale fences and late callbacks rejected without history rewrites.

Test / Documentation Gaps

  • test_restart_rejects_forged_acceptance_and_missing_outbox exercises the forged-acceptance branch; the missing-outbox branch of _validate_operation_records has no direct regression.
  • README status line update is accurate for the new boundary ("live external effects").

Follow-Up Candidates

None new; K3 consumption of this surface is already issue #6. Record suggestions 1-3 for owner triage if desired.

Overall Recommendation

APPROVE (recorded here as review text; see review-state note above)

Validation cross-checked: CI #1944 (pull_request, a7035f3) success on the exact frozen head; PR-reported make check (ruff format/check, basedpyright), make package-check, and make audit outcomes are consistent with the inspected code; this reviewer did not execute branch code. Issue #13 acceptance criteria are met at this head; the issue stays open and merge decisions remain with the owner.

## PR Review: [K2B] Journal external operations and durable artifacts (#17) > **Review-state note:** the reviewer's verdict is **APPROVE** (no blocking findings). Forgejo refused a formal `APPROVED` review state because the reviewer MCP identity (`kilo`) matches the PR author, and the platform forbids approving one's own pull request (`approve your own pull is not allowed`). This COMMENT-state review is the verbatim record of the independent review; it is **not** a Forgejo-recorded approval, and merge authority remains with the owner. **Repository:** Creatidy/creatidy-kernel **Branch:** `feat/13-operation-journal` -> `develop` **Frozen head:** `a7035f31c36c9a19d7c3a24c96abbced6ac80116` (base `2fa51e9`) **Linked issue:** #13 **Reviewer:** fresh read-only session `ses_f258ca123ffeg2RLzMN8vVC9B4`, distinct from the implementation session; running model `zai-coding-plan/GLM-5.3` (max) matches the routed selection `zai/glm-5.3/max`. Local HEAD verified identical to the frozen head; no branch edits, pushes, or merges performed. ### Summary Adds the K2B external-operation journal to the SQLite adapter: versioned/canonical Operation requests with distinct operation/effect identities, atomic K1 admission + INTENT/outbox (`admit_with_intent`) plus a standalone `intent()`, fenced and leased delivery attempts with transport/observation/reconciliation facts in append-only tables, retry gated on authoritative absence or proven idempotency, and content-addressed artifacts flushed to disk before an immutable DB reference. Schema v1->v2 migration, artifact-aware backup manifests, and verified restore. 10 new synthetic tests; no live provider, network, K3/K4, or merge automation. ### Intent / Acceptance Evidence | #13 acceptance criterion | Evidence at frozen head | Status | |---|---|---| | Stable identity, versioned/canonical digest; command-admission, logical-effect, inbound-observation dedupe distinct and independently tested | `operations.operation_id` PK + `effect_key` UNIQUE; `{"version":1,"request":...}` canonical JSON + sha256 digest; K2A `command_admissions` vs `_insert_intent` effect binding (tests B, `test_command_and_effect_outbox_are_one_transaction`) vs `observation_id` PK with identity-bound evidence (test E) | MET | | INTENT + outbox commit before fake effect; no external I/O inside a DB transaction | `claim()` commits attempt/lease/fence before `deliver_fake` invokes the callable; test D asserts `in_transaction is False` inside the fake; `test_outbox_failure_rolls_back_command_admission` proves atomic rollback | MET | | Attempts, domain ack/rejection, running/waiting/terminal/unknown distinct durable facts; observations deduplicated | Four append-only tables (`delivery_attempts`, `operation_transports`, `operation_observations`, `operation_reconciliations`) with no-update/no-delete triggers; observation replay idempotent, rebinding conflicts (test E) | MET | | Leases/fences prevent concurrent ownership; stale callback/fence rejected without rewriting history | `claim()` lease + retry-proof gates; `observe()`/`record_transport()` require the current fence and reject reconciled/late publication (tests D, E) | MET | | Crash uncertainty preserved; reconcile before retry; never advances from intent or delivery alone | Post-crash status stays `dispatched`; retry requires `retry_proof` in {absent, idempotent}; `_validate_operation_records` re-derives acceptance from durable receipts and rejects forgery (tests E, `test_restart_rejects_forged_acceptance_and_missing_outbox`) | MET | | Artifacts content-addressed, finalized before immutable reference; reopen/backup/restore preserve verified references | `finalize_artifact`: fsynced temp + atomic rename before DB insert; immutability conflict on divergence; startup/backup/restore re-hash blobs; tamper raises `CorruptHistory` (test 5) | MET | | Named B/D/E synthetic regressions | `test_b_intent_is_not_dispatch_or_review_acceptance` (no accepted-review wait, Program stays DRAFT, waiting-without-acceptance rejected), `test_d_transport_success_is_not_domain_acceptance` (transport success persisted as rejection), `test_e_missing_rejected_accepted_and_uncertain_receipts` (distinct receipts, durable acceptance reference, reconcile-and-redispatch same operation) | MET | | Synthetic-only tests, no network/Forgejo/Runtime/Codex/M5-B/Prefect/cloud | Local tmp SQLite + fake callables only; imports limited to adapters/core/stdlib | MET | ### Architecture / Information Flow Dependency direction preserved: changes confined to `adapters/sqlite_store.py` plus tests; stdlib-only additions (`shutil`, `Callable`). The K2B surface lives at adapter level, consistent with ADR 0003 (durability in the store) and with the K2A precedent where the `ProgramStore` protocol covers only `create`/`load`/`admit`; consumers arrive with K3 (#6). Implementation tracks ADR 0003 point-for-point: intent+outbox before dispatch, fence/lease before I/O, uncertainty retention, found-effect reconciliation, authoritative-absence-only retry, lease expiry as lost coordination rather than remote failure, artifacts flushed and atomically finalized before reference. Fail-closed posture throughout (`OperationConflict`, `CorruptHistory`). ### Blocking Issues None found. ### Non-blocking Suggestions 1. `_verify_backup_manifest` validates artifact digests by length only. A non-hex or path-traversing name is currently neutralized structurally (a digest string containing `/` can never equal a `sha256` hexdigest), but an explicit hex-charset check would encode that invariant instead of relying on it. 2. `__init__` detects a backup bundle (`kernel.sqlite3` + `manifest.json`), verifies it, then opens the bundle database in place for read/write; later writes silently diverge from the verified manifest digest. Consider refusing in-place writes or documenting the guard as open-time integrity verification. 3. `_validate_operation_records` re-hashes every artifact blob on each open (O(total artifact bytes)). Correctness-first is appropriate now; worth remembering when artifact volumes grow. 4. `artifact()` verifies the blob, then reopens it to read (small TOCTOU window); negligible under the single-controller model. ### Questions for the Author None blocking. ### Security / Privacy Notes No secrets, no network, no provider integrations. Authority and corruption boundaries fail closed: forged acceptance rejected at startup, bundle tampering detected on open and restore, stale fences and late callbacks rejected without history rewrites. ### Test / Documentation Gaps - `test_restart_rejects_forged_acceptance_and_missing_outbox` exercises the forged-acceptance branch; the missing-outbox branch of `_validate_operation_records` has no direct regression. - README status line update is accurate for the new boundary ("live external effects"). ### Follow-Up Candidates None new; K3 consumption of this surface is already issue #6. Record suggestions 1-3 for owner triage if desired. ### Overall Recommendation **APPROVE** (recorded here as review text; see review-state note above) Validation cross-checked: CI #1944 (pull_request, `a7035f3`) success on the exact frozen head; PR-reported `make check` (ruff format/check, basedpyright), `make package-check`, and `make audit` outcomes are consistent with the inspected code; this reviewer did not execute branch code. Issue #13 acceptance criteria are met at this head; the issue stays open and merge decisions remain with the owner.
adrian.tkacz deleted branch feat/13-operation-journal 2026-09-25 21:47:35 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
adrian.tkacz
Labels
Clear labels   
bug
Something is not working

  
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
help wanted
Need some help

  
invalid
Something is wrong

  
question
More information is needed

  
wontfix
This won't be fixed

No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Creatidy/creatidy-kernel!17
No description provided.