[K3] Prove synthetic execution, workspace and authority boundaries #18

Merged

adrian.tkacz merged 3 commits from issue-6-synthetic-execution into develop 2026-09-26 10:09:33 +00:00

Conversation 5 Commits 3 Files changed 11 +1134 -3

kilo commented 2026-09-25 22:31:25 +00:00

Member

Copy link

Implements #6. Keep issue open until owner integration into develop.

• Add replaceable Runtime, Workspace and authority ports with immutable Attempt/Operation-bound values and deterministic fakes.
• Exercise exact-key recovery, lost replies, duplicate/terminal replay, fresh waiting activity, cancel races, unknown identity/recovery, path/network/mount/expiry/revocation, stale publication, and uncertain partial workspace launch.
• Integrate the fake boundary with K2B SQLite intent/fence/observation and K1 Attempt lifecycle; candidate submission does not satisfy a WorkUnit.
• Synthetic trusted-development checks are not an OS sandbox, live broker, durable authority journal or live worker/provider. Broker consumption is atomic only within its in-memory intent ledger, not with SQLite; real adapters must provide durable intent-bound grant consumption.

Validation on HEAD 97ac9661709310d701adb790e0afcafc9d33b21d: uv run --locked pytest tests/test_fake_authority.py tests/test_fake_execution.py tests/test_k3_integration.py (12 passed); make check (164 passed, formatting, lint, basedpyright, boundaries and secrets); make package-check (wheel/sdist/isolated install). make audit passed before the code-only remediation; dependency and tooling files unchanged. See issue #6 delivery ledger for review/correction provenance.

Implements #6. Keep issue open until owner integration into `develop`. - Add replaceable Runtime, Workspace and authority ports with immutable Attempt/Operation-bound values and deterministic fakes. - Exercise exact-key recovery, lost replies, duplicate/terminal replay, fresh waiting activity, cancel races, unknown identity/recovery, path/network/mount/expiry/revocation, stale publication, and uncertain partial workspace launch. - Integrate the fake boundary with K2B SQLite intent/fence/observation and K1 Attempt lifecycle; candidate submission does not satisfy a WorkUnit. - Synthetic trusted-development checks are not an OS sandbox, live broker, durable authority journal or live worker/provider. Broker consumption is atomic only within its in-memory intent ledger, not with SQLite; real adapters must provide durable intent-bound grant consumption. Validation on HEAD `97ac9661709310d701adb790e0afcafc9d33b21d`: `uv run --locked pytest tests/test_fake_authority.py tests/test_fake_execution.py tests/test_k3_integration.py` (12 passed); `make check` (164 passed, formatting, lint, basedpyright, boundaries and secrets); `make package-check` (wheel/sdist/isolated install). `make audit` passed before the code-only remediation; dependency and tooling files unchanged. See issue #6 delivery ledger for review/correction provenance.

kilo added 1 commit 2026-09-25 22:31:25 +00:00

feat: prove synthetic execution and authority boundaries 3cf0b229a1

kilo referenced this pull request 2026-09-25 22:31:41 +00:00

[K3] Synthetic execution, workspace, and authority enforcement #6

kilo reviewed 2026-09-25 22:38:51 +00:00

kilo left a comment

Copy link

PR Review: [K3] Prove synthetic execution, workspace and authority boundaries (#18)

Repository: Creatidy/creatidy-kernel
Branch: issue-6-synthetic-execution -> develop
Frozen head: 3cf0b229a1 (base 059482d045; head confirmed unchanged before and after review)
Linked issue: #6 (from PR body "Implements #6")
Reviewer context: fresh read-only session; local checkout verified identical to the frozen head; no implementation-session context used. Note: an APPROVED-state review was attempted first; Forgejo rejected it because the PR author and the review identity are the same kilo account ("approve your own pull is not allowed"). The verdict below stands on its evidence; the formal approval state must come from a different account or the owner.

Summary

Adds core execution/authority values, ports/execution.py (Runtime, Workspace) and ports/authority.py (AuthorityBroker) protocols, deterministic FakeRuntime/FakeWorkspace/FakeAuthorityBroker adapters, three test modules (unit + K2B/K1 integration), and honest README/threat-model updates. 11 files, +1088/−3. Dependency direction is clean: core imports only core; ports import core; adapters import ports+core; lint-imports contract kept. No vendor imports, no new dependencies.

Intent / Acceptance Evidence

#6 acceptance criterion	Evidence at frozen head
Fake Runtime executes an immutable Attempt via candidate publication; control-plane mutations stay authority-checked	ExecutionRequest.__post_init__ binds attempt/workspace/context/allocation/agent-definition references (src/creatidy_kernel/core/execution.py:166); FakeRuntime.publish validates attempt_id/spec_digest/workspace binding and marks terminal; FakeAuthorityBroker.issue/revoke are owner-only; integration test ends with owner FinishAttempt and WorkUnit READY (candidate does not satisfy) — tests/test_k3_integration.py:39
Unsupported recovery/identity is unknown/unavailable; duplicate starts and terminal replays safe	FakeRuntime(recoverable=False)/lose()/wrong-handle reconcile → Presence.UNKNOWN; identity_supported=False → resolved/observed None; duplicate start returns same handle, changed inputs → ExecutionConflict; candidate replay identical/no-op, changed → conflict — tests/test_fake_execution.py:83
Quiet healthy activity stays healthy/waiting; recovery targets the exact execution, not global age	per-observation fresh_until (waiting at now=299 with fresh_until=300); reconcile requires exact OperationKey equality and optional exact handle — tests/test_fake_execution.py:103
Worker grants/acceptance/gate changes, control-state/credential access, workspace escape, stale-fence publication rejected	AuthorityGrant restricts operations ⊆ WORKER_OPERATIONS; workspace access denies control-plane ops and path/symlink/network/mount escapes; stale scope.fence publish → conflict; store.observe with fence−1 → OperationConflict — tests/test_fake_authority.py, tests/test_fake_execution.py:111, tests/test_k3_integration.py:136
Deterministic synthetic tests preserve replaceable ports and no vendor imports	protocols typed as Runtime/Workspace in tests; make check lint-imports "Adapters depend inward… KEPT"; stdlib+pytest only
Custom sandbox explicitly excluded; supported trust mode honest	TrustMode.ISOLATED → UnsupportedExecution in FakeWorkspace.materialize; docstrings "never a hostile-code sandbox"; README + threat-model updated to state no OS isolation/credential broker/live worker

All six criteria are met with observable evidence.

Architecture / Information Flow

Conforms to adapters -> ports -> core and ADR 0004/0005 vocabulary: start receives immutable Attempt + explicit workspace/context/allocation/capability references + Operation identity and fence; observe/cancel/candidate/reconcile distinguish found/absent/unknown; requested/resolved/observed identity and agent-definition version recorded separately; single-use grants are consumed atomically with the Operation intent and exact replay does not re-consume or authorize different content (ADR 0005). Integration uses K2B admit_with_intent/claim/record_transport/observe signatures exactly as they exist; domain acceptance stays separate from transport status. Stale-fence publication rejected on both runtime scope and journal fence.

Blocking Issues

None found.

Non-blocking Suggestions

1. check_scope (src/creatidy_kernel/core/authority.py:86) compares the resolved target (grant.root / intent.path).resolve() against unresolved grant.paths. If the grant root or an allowed path contains a symlinked component (macOS /tmp → /private/tmp, symlinked TMPDIR), in-scope paths are spuriously denied and the symlink-escape test could fail in the other direction. Resolve the allowed paths once (or compare resolved-to-resolved) so the check is environment-independent.
2. FakeWorkspace.access (src/creatidy_kernel/adapters/fake_execution.py:70) enforces a hardcoded control-plane denylist while core defines WORKER_OPERATIONS as the positive vocabulary. Checking operation in WORKER_OPERATIONS would use one vocabulary and also reject operation names no grant can confer.
3. Minor consistency: FakeRuntime.candidate/progress raise KeyError on unknown handles where observe raises ExecutionConflict, and core/execution.py mixes ValueError (from _text) with ExecutionConflict in validation paths.

Questions for the Author

1. ADR 0004 states workspace creation uses explicit repository/base revision, overlays, toolchain digest, filesystem/network policy and lifetime. WorkspaceSpec carries no lifetime field and cleanup is gated only on worker_settled. Is lifetime deliberately deferred to the runtime/reconcile path? If so, a one-line note in the port docstring would keep code and ADR aligned.
2. docs/architecture/contracts.md lists workspace conformance case "partial launch" and authority case "overbroad provider credential"; neither is simulated here. Confirm these are intentionally left to the real adapters rather than forgotten.

Security / Privacy Notes

Honest boundary labeling throughout: trusted-development fakes are documented as policy simulation, ISOLATED fails closed, control-plane operations never reachable through worker scope, grants cannot confer control authority, authentication is explicitly trusted-channel (ports/authority.py docstring). No secrets, no network access, no live external effects; tools/check_secrets.py clean. Informational: core/authority.py performs live filesystem checks (root.is_dir(), Path.resolve()), which is fine for a synthetic enforcement simulation but makes that module's behavior filesystem-state-dependent (see suggestion 1).

Test / Documentation Gaps

• The two conformance cases named in question 2 are unexercised (acceptable if confirmed as real-adapter scope).
• WorkspaceSpec lifetime omission (question 1).

Follow-Up Candidates

None durable enough to warrant an issue now; the unexercised conformance cases belong to the future real workspace/authority adapters and are already pinned in contracts.md.

Validation

Reviewer-run against the frozen head: focused tests uv run --locked pytest tests/test_fake_authority.py tests/test_fake_execution.py tests/test_k3_integration.py → 10 passed; make check → uv lock check, ruff format/check, basedpyright, lint-imports contract kept, 162 tests passed, secrets scan clean, git diff --check clean; make package-check → wheel/sdist rebuild and isolated install passed. make audit not re-run by reviewer: this PR changes no dependency or tooling files (uv.lock/pyproject untouched), matching the validation rules' relevance condition; the PR reports it passed.

Overall Recommendation

APPROVE — for the frozen head 3cf0b229a1 only. Integration and issue closure remain owner-controlled.

## PR Review: [K3] Prove synthetic execution, workspace and authority boundaries (#18) **Repository:** Creatidy/creatidy-kernel **Branch:** issue-6-synthetic-execution -> develop **Frozen head:** 3cf0b229a10b8254892a0d36d24013ab7d372458 (base 059482d045bd8e772005c9fbc1a9b89ce64ce26f; head confirmed unchanged before and after review) **Linked issue:** #6 (from PR body "Implements #6") **Reviewer context:** fresh read-only session; local checkout verified identical to the frozen head; no implementation-session context used. Note: an `APPROVED`-state review was attempted first; Forgejo rejected it because the PR author and the review identity are the same `kilo` account ("approve your own pull is not allowed"). The verdict below stands on its evidence; the formal approval state must come from a different account or the owner. ### Summary Adds core execution/authority values, `ports/execution.py` (`Runtime`, `Workspace`) and `ports/authority.py` (`AuthorityBroker`) protocols, deterministic `FakeRuntime`/`FakeWorkspace`/`FakeAuthorityBroker` adapters, three test modules (unit + K2B/K1 integration), and honest README/threat-model updates. 11 files, +1088/−3. Dependency direction is clean: core imports only core; ports import core; adapters import ports+core; `lint-imports` contract kept. No vendor imports, no new dependencies. ### Intent / Acceptance Evidence | #6 acceptance criterion | Evidence at frozen head | | --- | --- | | Fake Runtime executes an immutable Attempt via candidate publication; control-plane mutations stay authority-checked | `ExecutionRequest.__post_init__` binds attempt/workspace/context/allocation/agent-definition references (src/creatidy_kernel/core/execution.py:166); `FakeRuntime.publish` validates attempt_id/spec_digest/workspace binding and marks terminal; `FakeAuthorityBroker.issue/revoke` are owner-only; integration test ends with owner `FinishAttempt` and WorkUnit `READY` (candidate does not satisfy) — tests/test_k3_integration.py:39 | | Unsupported recovery/identity is unknown/unavailable; duplicate starts and terminal replays safe | `FakeRuntime(recoverable=False)`/`lose()`/wrong-handle reconcile → `Presence.UNKNOWN`; `identity_supported=False` → resolved/observed `None`; duplicate `start` returns same handle, changed inputs → `ExecutionConflict`; candidate replay identical/no-op, changed → conflict — tests/test_fake_execution.py:83 | | Quiet healthy activity stays healthy/waiting; recovery targets the exact execution, not global age | per-observation `fresh_until` (waiting at now=299 with fresh_until=300); `reconcile` requires exact `OperationKey` equality and optional exact handle — tests/test_fake_execution.py:103 | | Worker grants/acceptance/gate changes, control-state/credential access, workspace escape, stale-fence publication rejected | `AuthorityGrant` restricts operations ⊆ `WORKER_OPERATIONS`; workspace `access` denies control-plane ops and path/symlink/network/mount escapes; stale `scope.fence` publish → conflict; `store.observe` with fence−1 → `OperationConflict` — tests/test_fake_authority.py, tests/test_fake_execution.py:111, tests/test_k3_integration.py:136 | | Deterministic synthetic tests preserve replaceable ports and no vendor imports | protocols typed as `Runtime`/`Workspace` in tests; `make check` lint-imports "Adapters depend inward… KEPT"; stdlib+pytest only | | Custom sandbox explicitly excluded; supported trust mode honest | `TrustMode.ISOLATED` → `UnsupportedExecution` in `FakeWorkspace.materialize`; docstrings "never a hostile-code sandbox"; README + threat-model updated to state no OS isolation/credential broker/live worker | All six criteria are met with observable evidence. ### Architecture / Information Flow Conforms to `adapters -> ports -> core` and ADR 0004/0005 vocabulary: start receives immutable Attempt + explicit workspace/context/allocation/capability references + Operation identity and fence; observe/cancel/candidate/reconcile distinguish found/absent/unknown; requested/resolved/observed identity and agent-definition version recorded separately; single-use grants are consumed atomically with the Operation intent and exact replay does not re-consume or authorize different content (ADR 0005). Integration uses K2B `admit_with_intent`/`claim`/`record_transport`/`observe` signatures exactly as they exist; domain acceptance stays separate from transport status. Stale-fence publication rejected on both runtime scope and journal fence. ### Blocking Issues None found. ### Non-blocking Suggestions 1. `check_scope` (src/creatidy_kernel/core/authority.py:86) compares the resolved target `(grant.root / intent.path).resolve()` against unresolved `grant.paths`. If the grant root or an allowed path contains a symlinked component (macOS `/tmp` → `/private/tmp`, symlinked `TMPDIR`), in-scope paths are spuriously denied and the symlink-escape test could fail in the other direction. Resolve the allowed paths once (or compare resolved-to-resolved) so the check is environment-independent. 2. `FakeWorkspace.access` (src/creatidy_kernel/adapters/fake_execution.py:70) enforces a hardcoded control-plane denylist while core defines `WORKER_OPERATIONS` as the positive vocabulary. Checking `operation in WORKER_OPERATIONS` would use one vocabulary and also reject operation names no grant can confer. 3. Minor consistency: `FakeRuntime.candidate`/`progress` raise `KeyError` on unknown handles where `observe` raises `ExecutionConflict`, and core/execution.py mixes `ValueError` (from `_text`) with `ExecutionConflict` in validation paths. ### Questions for the Author 1. ADR 0004 states workspace creation uses explicit repository/base revision, overlays, toolchain digest, filesystem/network policy **and lifetime**. `WorkspaceSpec` carries no lifetime field and cleanup is gated only on `worker_settled`. Is lifetime deliberately deferred to the runtime/reconcile path? If so, a one-line note in the port docstring would keep code and ADR aligned. 2. `docs/architecture/contracts.md` lists workspace conformance case "partial launch" and authority case "overbroad provider credential"; neither is simulated here. Confirm these are intentionally left to the real adapters rather than forgotten. ### Security / Privacy Notes Honest boundary labeling throughout: trusted-development fakes are documented as policy simulation, `ISOLATED` fails closed, control-plane operations never reachable through worker scope, grants cannot confer control authority, authentication is explicitly trusted-channel (ports/authority.py docstring). No secrets, no network access, no live external effects; `tools/check_secrets.py` clean. Informational: core/authority.py performs live filesystem checks (`root.is_dir()`, `Path.resolve()`), which is fine for a synthetic enforcement simulation but makes that module's behavior filesystem-state-dependent (see suggestion 1). ### Test / Documentation Gaps - The two conformance cases named in question 2 are unexercised (acceptable if confirmed as real-adapter scope). - `WorkspaceSpec` lifetime omission (question 1). ### Follow-Up Candidates None durable enough to warrant an issue now; the unexercised conformance cases belong to the future real workspace/authority adapters and are already pinned in contracts.md. ### Validation Reviewer-run against the frozen head: focused tests `uv run --locked pytest tests/test_fake_authority.py tests/test_fake_execution.py tests/test_k3_integration.py` → 10 passed; `make check` → uv lock check, ruff format/check, basedpyright, lint-imports contract kept, 162 tests passed, secrets scan clean, `git diff --check` clean; `make package-check` → wheel/sdist rebuild and isolated install passed. `make audit` not re-run by reviewer: this PR changes no dependency or tooling files (uv.lock/pyproject untouched), matching the validation rules' relevance condition; the PR reports it passed. ### Overall Recommendation APPROVE — for the frozen head 3cf0b229a10b8254892a0d36d24013ab7d372458 only. Integration and issue closure remain owner-controlled.

kilo commented 2026-09-26 00:07:42 +00:00

Author

Member

Copy link

Architecture / model-correction assessment (not a full approval review)

Frozen head: 3cf0b229a1 (base 059482d045), verified unchanged in this session.
Question assessed: review #602's claim that single-use grants are "consumed atomically with the Operation intent", versus the separate in-memory FakeAuthorityBroker ledger and tests calling store.admit_with_intent before broker.check — violation of issue #6 or ADR 0005?

What the frozen head actually does

• FakeAuthorityBroker.check (src/creatidy_kernel/adapters/fake_authority.py:41) performs replay lookup, effect-key exclusivity, single-use consumption, intent registration and effect binding inside one lock. Consumption is atomic within the broker's in-memory ledger, and the ADR semantics hold there: exact replay returns the recorded intent without re-consuming (adapters/fake_authority.py:47, tests/test_fake_authority.py:57), changed input under the same operation identity is denied (:49, tests/test_fake_authority.py:59-66), a second operation cannot use a consumed grant (:54, tests/test_fake_authority.py:67-68), and replay still rechecks revocation/expiry (:46, tests/test_fake_authority.py:100).
• That ledger is in-memory and separate from the K2B journal. store.admit_with_intent (src/creatidy_kernel/adapters/sqlite_store.py:349) commits the K1 decision plus operation/outbox row in one SQLite transaction; the integration test caller-composes the two (admit_with_intent at tests/test_k3_integration.py:65, broker.check at :98). There is no cross-store transaction, and the code states this honestly: adapters/fake_authority.py:2 ("In-memory synthetic admission; not the durable Operation journal"), the PR body ("not ... durable authority journal"), tests/test_k3_integration.py:2 ("not a second journal").

Verdict on the question

• Review #602's sentence is accurate when "the Operation intent" means the core OperationIntent value the broker consumes, and an overstatement only if read as "atomically with the SQLite intent admission". The defect is in the review's wording, not in the PR.
• No violation of issue #6: none of its six acceptance criteria requires durable grant consumption or broker↔journal transactional coupling; the scope is producer-shaped fakes and synthetic enforcement, and the live broker is explicitly out of scope.
• No violation of ADR 0005: the atomic-consumption decision (ADR 0005 lines 14-16) targets the real trusted-broker mechanism; the synthetic slice preserves its semantics (effect/input digest bound, consume-once with intent, replay-safe, no different content) without claiming durability, and the ADR itself states the enforcement controls are not yet implemented. The real-adapter obligation is already pinned: reuse.md ("Kernel must bind/consume single-use authority with Operation intent") and contracts.md ("consume exact single-use grants with intent"). No resurrection of consumed authority exists in the composition — the journal independently rejects stale/regressing observations (tests/test_k3_integration.py:144).

Classification

BOUNDED_CORRECTION — finite in-scope changes, no owner architecture/scope decision required:

1. This comment corrects the record on review #602's atomicity phrasing (reviewer-wording defect; per AGENTS.md and ADR 0005's HumanGate definition, not a Human Gate).
2. Optional, docs-only: one line in the AuthorityBroker.check port docstring stating that durable consumption-atomicity with the Operation journal is an obligation of the real broker adapter, not of synthetic fakes.
3. WorkspaceSpec (src/creatidy_kernel/core/execution.py:71) carries no lifetime field while ADR 0004 requires workspace creation to use "...filesystem/network policy and lifetime": add the field to the synthetic value or record the deferral in the port docstring (review #602 question 1).
4. "Partial launch" (contracts.md Workspace conformance case) is unexercised: add one deterministic fake case or state the deferral to the real workspace adapter (review #602 question 2).

Explicitly not part of this correction: a durable grant journal or a broker↔journal transaction. That is future real-broker adapter scope already decided by the accepted documents; pulling it into issue #6 would be scope expansion and only then an owner decision.

Integration and merge remain owner-controlled; this assessment is not an approval.

## Architecture / model-correction assessment (not a full approval review) **Frozen head:** 3cf0b229a10b8254892a0d36d24013ab7d372458 (base 059482d045bd8e772005c9fbc1a9b89ce64ce26f), verified unchanged in this session. **Question assessed:** review #602's claim that single-use grants are "consumed atomically with the Operation intent", versus the separate in-memory `FakeAuthorityBroker` ledger and tests calling `store.admit_with_intent` before `broker.check` — violation of issue #6 or ADR 0005? ### What the frozen head actually does - `FakeAuthorityBroker.check` (src/creatidy_kernel/adapters/fake_authority.py:41) performs replay lookup, effect-key exclusivity, single-use consumption, intent registration and effect binding **inside one lock**. Consumption is atomic *within the broker's in-memory ledger*, and the ADR semantics hold there: exact replay returns the recorded intent without re-consuming (adapters/fake_authority.py:47, tests/test_fake_authority.py:57), changed input under the same operation identity is denied (:49, tests/test_fake_authority.py:59-66), a second operation cannot use a consumed grant (:54, tests/test_fake_authority.py:67-68), and replay still rechecks revocation/expiry (:46, tests/test_fake_authority.py:100). - That ledger is in-memory and separate from the K2B journal. `store.admit_with_intent` (src/creatidy_kernel/adapters/sqlite_store.py:349) commits the K1 decision plus operation/outbox row in one SQLite transaction; the integration test caller-composes the two (admit_with_intent at tests/test_k3_integration.py:65, broker.check at :98). There is **no cross-store transaction**, and the code states this honestly: adapters/fake_authority.py:2 ("In-memory synthetic admission; not the durable Operation journal"), the PR body ("not ... durable authority journal"), tests/test_k3_integration.py:2 ("not a second journal"). ### Verdict on the question - Review #602's sentence is accurate when "the Operation intent" means the core `OperationIntent` value the broker consumes, and an overstatement only if read as "atomically with the SQLite intent admission". The defect is in the review's wording, not in the PR. - **No violation of issue #6:** none of its six acceptance criteria requires durable grant consumption or broker↔journal transactional coupling; the scope is producer-shaped fakes and synthetic enforcement, and the live broker is explicitly out of scope. - **No violation of ADR 0005:** the atomic-consumption decision (ADR 0005 lines 14-16) targets the real trusted-broker mechanism; the synthetic slice preserves its semantics (effect/input digest bound, consume-once with intent, replay-safe, no different content) without claiming durability, and the ADR itself states the enforcement controls are not yet implemented. The real-adapter obligation is already pinned: reuse.md ("Kernel must bind/consume single-use authority with Operation intent") and contracts.md ("consume exact single-use grants with intent"). No resurrection of consumed authority exists in the composition — the journal independently rejects stale/regressing observations (tests/test_k3_integration.py:144). ### Classification **BOUNDED_CORRECTION** — finite in-scope changes, no owner architecture/scope decision required: 1. This comment corrects the record on review #602's atomicity phrasing (reviewer-wording defect; per AGENTS.md and ADR 0005's HumanGate definition, not a Human Gate). 2. Optional, docs-only: one line in the `AuthorityBroker.check` port docstring stating that durable consumption-atomicity with the Operation journal is an obligation of the real broker adapter, not of synthetic fakes. 3. `WorkspaceSpec` (src/creatidy_kernel/core/execution.py:71) carries no lifetime field while ADR 0004 requires workspace creation to use "...filesystem/network policy and lifetime": add the field to the synthetic value or record the deferral in the port docstring (review #602 question 1). 4. "Partial launch" (contracts.md Workspace conformance case) is unexercised: add one deterministic fake case or state the deferral to the real workspace adapter (review #602 question 2). Explicitly **not** part of this correction: a durable grant journal or a broker↔journal transaction. That is future real-broker adapter scope already decided by the accepted documents; pulling it into issue #6 would be scope expansion and only then an owner decision. Integration and merge remain owner-controlled; this assessment is not an approval.

adrian.tkacz added 1 commit 2026-09-26 00:10:04 +00:00

fix: bound synthetic workspace recovery and path scope 97ac966170

kilo reviewed 2026-09-26 00:15:05 +00:00

kilo left a comment

Copy link

PR Review: [K3] Prove synthetic execution, workspace and authority boundaries (#18)

Repository: Creatidy/creatidy-kernel
Branch: issue-6-synthetic-execution -> develop
Frozen head: 97ac966170 (base 059482d045; local checkout verified identical, clean tree, head unchanged before and after review)
Linked issue: #6 (from PR body "Implements #6")
Reviewer context: fresh read-only session; no implementation-session context or verdict relied on. Review #602 covered head 3cf0b229 and is stale for this head; this review independently covers the remediation commit 97ac966 ("fix: bound synthetic workspace recovery and path scope", +47/−1) and re-verifies the whole diff. Forgejo rejects an APPROVED-state review from the same kilo account as the PR author, so this is posted as COMMENT; the verdict stands on its evidence and formal approval remains with a different account or the owner.

Summary

Adds core execution/authority values, ports/execution.py (Runtime, Workspace) and ports/authority.py (AuthorityBroker) protocols, deterministic FakeRuntime/FakeWorkspace/FakeAuthorityBroker adapters, three test modules, and honest README/threat-model updates. 11 files, +1115/−3; no dependency or tooling files touched. Dependency direction kept (adapters -> ports -> core, lint-imports KEPT); stdlib+pytest only.

Remediation verification (97ac966 vs 3cf0b22, against comment 11699 and review #602)

• Correction item 2 (durable-consumption obligation): documented in the port docstring src/creatidy_kernel/ports/authority.py:11.
• Correction item 3 / review question 1 (ADR 0004 workspace lifetime): deferral documented in src/creatidy_kernel/ports/execution.py:31 ("Real adapters enforce workspace lifetime; the fake models only worker-scope expiry"). Field deliberately not added; code and ADR stay aligned via the docstring.
• Correction item 4 / review question 2 (partial launch): exercised — fail_next_materialize (src/creatidy_kernel/adapters/fake_execution.py:37), lost-key semantics in materialize, and tests/test_fake_execution.py:161 proving unknown-not-safe-to-replace.
• Review #602 suggestion 1 (symlink/env-dependent scope comparison): fixed — check_scope now resolves root, target, and each allowed path, and requires allowed paths to resolve inside the root (src/creatidy_kernel/core/authority.py:88-96), with a new regression test proving a symlinked allowed path cannot widen the grant (tests/test_fake_authority.py:141). This also closes an escape the old comparison allowed (an allowed path that itself resolves outside the grant root).
• Review #602 suggestion 2 (positive WORKER_OPERATIONS vocabulary in workspace access): not adopted; see non-blocking item 1.
• Review #602 suggestion 3 (exception consistency): partially adopted (observe now raises ExecutionConflict, src/creatidy_kernel/adapters/fake_execution.py:171); see non-blocking item 2.

Intent / Acceptance Evidence

#6 acceptance criterion	Evidence at frozen head
Fake Runtime executes an immutable Attempt via candidate publication; control-plane mutations stay authority-checked	ExecutionRequest.__post_init__ binds attempt/workspace/context/allocation/agent-definition (src/creatidy_kernel/core/execution.py:188); FakeRuntime.publish validates scope attempt/workspace/fence/expiry/operation and binds candidate attempt/spec/artifacts (src/creatidy_kernel/adapters/fake_execution.py:190); issue/revoke owner-only (src/creatidy_kernel/adapters/fake_authority.py:22,33); candidate does not satisfy the WorkUnit — owner FinishAttempt required, then READY (tests/test_k3_integration.py:141)
Unsupported recovery/identity is unknown/unavailable; duplicate starts and terminal replays safe	identity_supported=False blanks resolved/observed (src/creatidy_kernel/adapters/fake_execution.py:143); reconcile requires exact OperationKey equality and optional exact handle, lost keys → UNKNOWN (src/creatidy_kernel/adapters/fake_execution.py:226); duplicate start returns same handle, changed inputs conflict; identical terminal replay no-op, changed candidate conflict (tests/test_fake_execution.py:38,57)
Quiet healthy activity stays healthy/waiting; recovery targets the exact execution, not global age	per-observation fresh_until (WAITING at now=299, fresh_until=300, tests/test_fake_execution.py:98); reconcile keys on the exact effect key — a quiet false child stall cannot redirect recovery (tests/test_fake_execution.py:103)
Worker grants/acceptance/gate changes, control-state/credential access, workspace escape, stale-fence publication rejected	grant operations ⊆ WORKER_OPERATIONS (src/creatidy_kernel/core/authority.py:51); access denies control-plane operations and path/symlink/network/mount escapes (src/creatidy_kernel/adapters/fake_execution.py:67-105); stale scope.fence publish conflicts; journal rejects fence−1 (OperationConflict, tests/test_k3_integration.py:136); replay of a single-use grant rechecks revocation/expiry even when exact (src/creatidy_kernel/adapters/fake_authority.py:46)
Deterministic synthetic tests preserve replaceable ports and no vendor imports	tests bind ports as Runtime/Workspace (tests/test_fake_execution.py:83); reviewer-run lint-imports contract KEPT; stdlib+pytest only; uv.lock/pyproject unchanged
Custom sandbox explicitly excluded; supported trust mode honest	ISOLATED fails closed with UnsupportedExecution (src/creatidy_kernel/adapters/fake_execution.py:38); docstrings and README/threat-model state trusted-development is not an OS/container boundary

All six criteria are met with observable evidence.

Architecture / Information Flow

Conforms to adapters -> ports -> core and ADR 0004/0005 vocabulary. Start receives an immutable Attempt plus explicit workspace/context/allocation/capability references, Operation identity and fence; observe/cancel/candidate/reconcile distinguish FOUND/ABSENT/UNKNOWN; requested/resolved/observed identity recorded separately; single-use grants consumed inside one broker lock with exact replay returning the recorded intent without re-consuming, and no resurrection of consumed authority in the composition (the SQLite journal independently rejects stale fences). Per correction comment 11699: broker atomicity is in-memory only — correctly disclosed in code, PR body, and the port docstring; durable intent-bound consumption remains pinned to the real adapter. Integration uses the actual K2B signatures (admit_with_intent/claim/record_transport/observe verified against src/creatidy_kernel/adapters/sqlite_store.py:349,521,564,585).

Blocking Issues

None found.

Non-blocking Suggestions

1. FakeWorkspace.access (src/creatidy_kernel/adapters/fake_execution.py:84) still enforces a hardcoded control-plane denylist {"grant","accept","gate","control","credentials"} while core defines WORKER_OPERATIONS as the positive vocabulary. Checking operation in WORKER_OPERATIONS would use one vocabulary and reject any name no grant can confer (carry-over from review #602, deliberately or not — fine to defer).
2. Exception-type consistency remainder: candidate/progress/publish/cancel/lose still raise KeyError on unknown handles where observe now raises ExecutionConflict, and core/execution.py mixes _text ValueError with ExecutionConflict; progress's message "terminal activity cannot progress" also fires for fresh_for < 0.
3. WorkerScope.__post_init__ (src/creatidy_kernel/core/execution.py:186) validates only paths, not operations ⊆ WORKER_OPERATIONS; a hand-built scope could carry arbitrary operation names past access's first check unless denylisted. Synthetic-only concern — the real adapter derives scope from a grant — but a subset check would mirror AuthorityGrant hygiene.

Questions for the Author

1. Contracts.md's authority conformance case "overbroad provider credential" remains unexercised (the partial-launch counterpart was addressed). Confirm this is intentionally deferred to the real provider adapter — the fake has no provider credentials to be overbroad, so I read it as deferred, but the record should say so.

Security / Privacy Notes

Honest boundary labeling throughout: trusted-development mode documented as policy simulation, ISOLATED fails closed, control-plane operations unreachable through worker scope or grants, authentication explicitly trusted-channel, no secrets/network/live effects; secrets scan clean. The remediation's resolved-path comparison removes environment-dependent symlink behavior in scope checks and adds a genuine widening-attack regression test. Informational: AuthorityGrant.__post_init__ performs live filesystem checks (root.is_dir()), acceptable for a synthetic simulation.

Test / Documentation Gaps

• "Overbroad provider credential" conformance case unexercised (question 1; acceptable if confirmed as real-adapter scope).
• Exception-type consistency (non-blocking item 2).

Follow-Up Candidates

None durable enough for an issue; items 1–3 are ordinary polish belonging to the future real adapters or a passing cleanup.

Validation

Reviewer-run against the frozen head 97ac966170: uv run --locked pytest tests/test_fake_authority.py tests/test_fake_execution.py tests/test_k3_integration.py → 12 passed; make check → ruff format/check, basedpyright, lint-imports contract KEPT, 164 tests passed, secrets scan clean, git diff --check clean; make package-check → wheel/sdist rebuild and isolated install passed. make audit not re-run by reviewer: the PR changes no dependency or tooling files, matching the validation rules' relevance condition; the PR reports it passed before the code-only remediation.

Overall Recommendation

APPROVE — for the frozen head 97ac966170 only. Integration and issue closure remain owner-controlled.

## PR Review: [K3] Prove synthetic execution, workspace and authority boundaries (#18) **Repository:** Creatidy/creatidy-kernel **Branch:** issue-6-synthetic-execution -> develop **Frozen head:** 97ac9661709310d701adb790e0afcafc9d33b21d (base 059482d045bd8e772005c9fbc1a9b89ce64ce26f; local checkout verified identical, clean tree, head unchanged before and after review) **Linked issue:** #6 (from PR body "Implements #6") **Reviewer context:** fresh read-only session; no implementation-session context or verdict relied on. Review #602 covered head `3cf0b229` and is stale for this head; this review independently covers the remediation commit `97ac966` ("fix: bound synthetic workspace recovery and path scope", +47/−1) and re-verifies the whole diff. Forgejo rejects an APPROVED-state review from the same `kilo` account as the PR author, so this is posted as COMMENT; the verdict stands on its evidence and formal approval remains with a different account or the owner. ### Summary Adds core execution/authority values, `ports/execution.py` (`Runtime`, `Workspace`) and `ports/authority.py` (`AuthorityBroker`) protocols, deterministic `FakeRuntime`/`FakeWorkspace`/`FakeAuthorityBroker` adapters, three test modules, and honest README/threat-model updates. 11 files, +1115/−3; no dependency or tooling files touched. Dependency direction kept (`adapters -> ports -> core`, lint-imports KEPT); stdlib+pytest only. ### Remediation verification (97ac966 vs 3cf0b22, against comment 11699 and review #602) - Correction item 2 (durable-consumption obligation): documented in the port docstring `src/creatidy_kernel/ports/authority.py:11`. - Correction item 3 / review question 1 (ADR 0004 workspace lifetime): deferral documented in `src/creatidy_kernel/ports/execution.py:31` ("Real adapters enforce workspace lifetime; the fake models only worker-scope expiry"). Field deliberately not added; code and ADR stay aligned via the docstring. - Correction item 4 / review question 2 (partial launch): exercised — `fail_next_materialize` (`src/creatidy_kernel/adapters/fake_execution.py:37`), lost-key semantics in `materialize`, and `tests/test_fake_execution.py:161` proving unknown-not-safe-to-replace. - Review #602 suggestion 1 (symlink/env-dependent scope comparison): fixed — `check_scope` now resolves root, target, and each allowed path, and requires allowed paths to resolve inside the root (`src/creatidy_kernel/core/authority.py:88-96`), with a new regression test proving a symlinked allowed path cannot widen the grant (`tests/test_fake_authority.py:141`). This also closes an escape the old comparison allowed (an allowed path that itself resolves outside the grant root). - Review #602 suggestion 2 (positive `WORKER_OPERATIONS` vocabulary in workspace access): not adopted; see non-blocking item 1. - Review #602 suggestion 3 (exception consistency): partially adopted (`observe` now raises `ExecutionConflict`, `src/creatidy_kernel/adapters/fake_execution.py:171`); see non-blocking item 2. ### Intent / Acceptance Evidence | #6 acceptance criterion | Evidence at frozen head | | --- | --- | | Fake Runtime executes an immutable Attempt via candidate publication; control-plane mutations stay authority-checked | `ExecutionRequest.__post_init__` binds attempt/workspace/context/allocation/agent-definition (src/creatidy_kernel/core/execution.py:188); `FakeRuntime.publish` validates scope attempt/workspace/fence/expiry/operation and binds candidate attempt/spec/artifacts (src/creatidy_kernel/adapters/fake_execution.py:190); `issue`/`revoke` owner-only (src/creatidy_kernel/adapters/fake_authority.py:22,33); candidate does not satisfy the WorkUnit — owner `FinishAttempt` required, then READY (tests/test_k3_integration.py:141) | | Unsupported recovery/identity is unknown/unavailable; duplicate starts and terminal replays safe | `identity_supported=False` blanks resolved/observed (src/creatidy_kernel/adapters/fake_execution.py:143); `reconcile` requires exact `OperationKey` equality and optional exact handle, lost keys → UNKNOWN (src/creatidy_kernel/adapters/fake_execution.py:226); duplicate `start` returns same handle, changed inputs conflict; identical terminal replay no-op, changed candidate conflict (tests/test_fake_execution.py:38,57) | | Quiet healthy activity stays healthy/waiting; recovery targets the exact execution, not global age | per-observation `fresh_until` (WAITING at now=299, fresh_until=300, tests/test_fake_execution.py:98); reconcile keys on the exact effect key — a quiet false child stall cannot redirect recovery (tests/test_fake_execution.py:103) | | Worker grants/acceptance/gate changes, control-state/credential access, workspace escape, stale-fence publication rejected | grant operations ⊆ `WORKER_OPERATIONS` (src/creatidy_kernel/core/authority.py:51); `access` denies control-plane operations and path/symlink/network/mount escapes (src/creatidy_kernel/adapters/fake_execution.py:67-105); stale `scope.fence` publish conflicts; journal rejects fence−1 (`OperationConflict`, tests/test_k3_integration.py:136); replay of a single-use grant rechecks revocation/expiry even when exact (src/creatidy_kernel/adapters/fake_authority.py:46) | | Deterministic synthetic tests preserve replaceable ports and no vendor imports | tests bind ports as `Runtime`/`Workspace` (tests/test_fake_execution.py:83); reviewer-run lint-imports contract KEPT; stdlib+pytest only; uv.lock/pyproject unchanged | | Custom sandbox explicitly excluded; supported trust mode honest | `ISOLATED` fails closed with `UnsupportedExecution` (src/creatidy_kernel/adapters/fake_execution.py:38); docstrings and README/threat-model state trusted-development is not an OS/container boundary | All six criteria are met with observable evidence. ### Architecture / Information Flow Conforms to `adapters -> ports -> core` and ADR 0004/0005 vocabulary. Start receives an immutable Attempt plus explicit workspace/context/allocation/capability references, Operation identity and fence; observe/cancel/candidate/reconcile distinguish FOUND/ABSENT/UNKNOWN; requested/resolved/observed identity recorded separately; single-use grants consumed inside one broker lock with exact replay returning the recorded intent without re-consuming, and no resurrection of consumed authority in the composition (the SQLite journal independently rejects stale fences). Per correction comment 11699: broker atomicity is in-memory only — correctly disclosed in code, PR body, and the port docstring; durable intent-bound consumption remains pinned to the real adapter. Integration uses the actual K2B signatures (`admit_with_intent`/`claim`/`record_transport`/`observe` verified against `src/creatidy_kernel/adapters/sqlite_store.py:349,521,564,585`). ### Blocking Issues None found. ### Non-blocking Suggestions 1. `FakeWorkspace.access` (src/creatidy_kernel/adapters/fake_execution.py:84) still enforces a hardcoded control-plane denylist `{"grant","accept","gate","control","credentials"}` while core defines `WORKER_OPERATIONS` as the positive vocabulary. Checking `operation in WORKER_OPERATIONS` would use one vocabulary and reject any name no grant can confer (carry-over from review #602, deliberately or not — fine to defer). 2. Exception-type consistency remainder: `candidate`/`progress`/`publish`/`cancel`/`lose` still raise `KeyError` on unknown handles where `observe` now raises `ExecutionConflict`, and `core/execution.py` mixes `_text` `ValueError` with `ExecutionConflict`; `progress`'s message "terminal activity cannot progress" also fires for `fresh_for < 0`. 3. `WorkerScope.__post_init__` (src/creatidy_kernel/core/execution.py:186) validates only paths, not `operations ⊆ WORKER_OPERATIONS`; a hand-built scope could carry arbitrary operation names past `access`'s first check unless denylisted. Synthetic-only concern — the real adapter derives scope from a grant — but a subset check would mirror `AuthorityGrant` hygiene. ### Questions for the Author 1. Contracts.md's authority conformance case "overbroad provider credential" remains unexercised (the partial-launch counterpart was addressed). Confirm this is intentionally deferred to the real provider adapter — the fake has no provider credentials to be overbroad, so I read it as deferred, but the record should say so. ### Security / Privacy Notes Honest boundary labeling throughout: trusted-development mode documented as policy simulation, `ISOLATED` fails closed, control-plane operations unreachable through worker scope or grants, authentication explicitly trusted-channel, no secrets/network/live effects; secrets scan clean. The remediation's resolved-path comparison removes environment-dependent symlink behavior in scope checks and adds a genuine widening-attack regression test. Informational: `AuthorityGrant.__post_init__` performs live filesystem checks (`root.is_dir()`), acceptable for a synthetic simulation. ### Test / Documentation Gaps - "Overbroad provider credential" conformance case unexercised (question 1; acceptable if confirmed as real-adapter scope). - Exception-type consistency (non-blocking item 2). ### Follow-Up Candidates None durable enough for an issue; items 1–3 are ordinary polish belonging to the future real adapters or a passing cleanup. ### Validation Reviewer-run against the frozen head 97ac9661709310d701adb790e0afcafc9d33b21d: `uv run --locked pytest tests/test_fake_authority.py tests/test_fake_execution.py tests/test_k3_integration.py` → 12 passed; `make check` → ruff format/check, basedpyright, lint-imports contract KEPT, 164 tests passed, secrets scan clean, `git diff --check` clean; `make package-check` → wheel/sdist rebuild and isolated install passed. `make audit` not re-run by reviewer: the PR changes no dependency or tooling files, matching the validation rules' relevance condition; the PR reports it passed before the code-only remediation. ### Overall Recommendation APPROVE — for the frozen head 97ac9661709310d701adb790e0afcafc9d33b21d only. Integration and issue closure remain owner-controlled.

kilo referenced this pull request 2026-09-26 08:45:59 +00:00

Separate substantive review verdict from Forgejo review state in issue delivery #19

kilo referenced this pull request 2026-09-26 08:50:29 +00:00

[Governance] Separate substantive review verdict from Forgejo state #20

kilo referenced this pull request 2026-09-26 08:51:03 +00:00

Separate substantive review verdict from Forgejo review state in issue delivery #19

adrian.tkacz added 1 commit 2026-09-26 09:54:21 +00:00

Merge branch 'develop' into issue-6-synthetic-execution 73be4bd608

kilo referenced this pull request 2026-09-26 09:58:55 +00:00

[K3] Synthetic execution, workspace, and authority enforcement #6

kilo reviewed 2026-09-26 10:05:08 +00:00

kilo left a comment

Copy link

PR Review: [K3] Prove synthetic execution, workspace and authority boundaries (#18)

Repository: Creatidy/creatidy-kernel
Branch: issue-6-synthetic-execution -> develop
Frozen head: 73be4bd608 (base 0fb435bf36; local clean checkout verified identical; head confirmed unchanged by git ls-remote before and after this review)
Linked issue: #6 (from PR body "Implements #6")
Reviewer context: fresh dispatched read-only reviewer session (Agent Manager local ses_f2558af87ffe7rsTI143sc8bTq); no implementation-session context or verdict relied on. Reviews #602 (3cf0b229) and #603 (97ac966) cover prior heads and are superseded by this full review of the current head. Dispatch-resolution verification (DISPATCH_VERIFIED) belongs to the coordinating delivery session, not this reviewer. The PR body cites validation on 97ac966; the head advanced to 73be4bd by merging develop (0fb435b, governance PR #20). I verified git diff 97ac966..73be4bd touches only .kilo/command/deliver-issue.md and .kilo/command/review-pr.md — src/, tests/, README and threat-model are byte-identical to the previously reviewed code; the branch is current with develop (merge-base equals base tip).

Summary

Adds core execution/authority values, ports/execution.py (Runtime, Workspace) and ports/authority.py (AuthorityBroker) protocols, deterministic FakeRuntime/FakeWorkspace/FakeAuthorityBroker adapters, three test modules (unit + K2B/K1 integration), and honest README/threat-model updates. 11 files, +1134/−3; no dependency or tooling files touched. Dependency direction kept (adapters -> ports -> core, lint-imports KEPT); stdlib+pytest only.

Intent / Acceptance Evidence

#6 acceptance criterion	Evidence at frozen head
Fake Runtime executes an immutable Attempt via candidate publication; control-plane mutations stay authority-checked	ExecutionRequest.__post_init__ binds attempt/workspace/context/allocation/agent-definition (src/creatidy_kernel/core/execution.py:145); FakeRuntime.publish validates scope attempt/workspace/fence/expiry/operation and binds candidate attempt/spec/artifacts (src/creatidy_kernel/adapters/fake_execution.py:190); issue/revoke owner-only (src/creatidy_kernel/adapters/fake_authority.py:20,29); candidate does not satisfy the WorkUnit — owner FinishAttempt required, then READY (tests/test_k3_integration.py:147,150)
Unsupported recovery/identity is unknown/unavailable; duplicate starts and terminal replays safe	identity_supported=False blanks resolved/observed (src/creatidy_kernel/adapters/fake_execution.py:143); reconcile requires exact OperationKey equality and optional exact handle; lost keys and recoverable=False → UNKNOWN (src/creatidy_kernel/adapters/fake_execution.py:226); duplicate start returns same handle, changed inputs conflict; identical terminal replay no-op, changed candidate conflict (tests/test_fake_execution.py:38,57)
Quiet healthy activity stays healthy/waiting; recovery targets the exact execution, not global age	per-observation fresh_until (WAITING at now=299, fresh_until=300, tests/test_fake_execution.py:98); reconcile keys on the exact effect key with full Operation equality — a quiet false child stall cannot redirect recovery (tests/test_fake_execution.py:103)
Worker grants/acceptance/gate changes, control-state/credential access, workspace escape, stale-fence publication rejected	grant operations ⊆ WORKER_OPERATIONS (src/creatidy_kernel/core/authority.py:50); access denies control-plane operations and path/symlink/network/mount escapes (src/creatidy_kernel/adapters/fake_execution.py:67-98); stale scope.fence publish conflicts; journal rejects fence−1 (OperationConflict, tests/test_k3_integration.py:145); exact replay still rechecks revocation/expiry (src/creatidy_kernel/adapters/fake_authority.py:44)
Deterministic synthetic tests preserve replaceable ports and no vendor imports	tests bind ports as Runtime/Workspace (tests/test_fake_execution.py:40); reviewer-run lint-imports contract KEPT; stdlib+pytest only; uv.lock/pyproject unchanged
Custom sandbox explicitly excluded; supported trust mode honest	ISOLATED fails closed with UnsupportedExecution (src/creatidy_kernel/adapters/fake_execution.py:43); README and threat-model state trusted-development is not an OS/container boundary for hostile code

All six criteria are met with observable evidence.

Architecture / Information Flow

Conforms to adapters -> ports -> core and ADR 0004/0005 vocabulary. Start receives an immutable Attempt plus explicit workspace/context/allocation/capability references, Operation identity and fence; observe/cancel/candidate/reconcile distinguish FOUND/ABSENT/UNKNOWN; requested/resolved/observed identity recorded separately. Single-use grants are consumed inside one broker lock with exact replay returning the recorded intent without re-consuming, and exact replay rechecks revocation/expiry (ADR 0005); the in-memory-only scope of that atomicity is honestly disclosed in code, PR body, and the port docstring (src/creatidy_kernel/ports/authority.py:11), with durable intent-bound consumption pinned to the real adapter. The K2B integration uses the actual signatures (admit_with_intent/claim/record_transport/observe verified against src/creatidy_kernel/adapters/sqlite_store.py:349,521,564,585, including the accepted-reference binding and no-regress rules the test relies on). ADR 0004 workspace-lifetime deferral is documented at src/creatidy_kernel/ports/execution.py:31.

Blocking Issues

None found.

Non-blocking Suggestions

Unchanged code from the previously reviewed head; carry-over polish items remain accurate:

1. FakeWorkspace.access (src/creatidy_kernel/adapters/fake_execution.py:84) enforces a hardcoded control-plane denylist while core defines WORKER_OPERATIONS as the positive vocabulary. Checking operation in WORKER_OPERATIONS would use one vocabulary and reject any name no grant can confer.
2. Exception-type consistency: candidate/progress/publish/cancel/lose raise KeyError on unknown handles where observe raises ExecutionConflict; core/execution.py mixes _text ValueError with ExecutionConflict; progress's "terminal activity cannot progress" also fires for fresh_for < 0.
3. WorkerScope.__post_init__ (src/creatidy_kernel/core/execution.py:188) validates only paths, not operations ⊆ WORKER_OPERATIONS; a hand-built scope could carry arbitrary operation names past access's first check unless denylisted. Synthetic-only concern — real adapters derive scope from a grant.

Questions for the Author

1. The PR body still cites validation on 97ac966 while the head is the merge commit 73be4bd (code identical, develop governance docs merged in). Please note the merge in the delivery ledger so the HEAD trail is self-explanatory for the next session.
2. Contracts.md's authority conformance case "overbroad provider credential" remains unexercised (the fake has no provider credentials to be overbroad). Confirm intentional deferral to the real provider adapter.

Security / Privacy Notes

Honest boundary labeling throughout: trusted-development mode documented as policy simulation, ISOLATED fails closed, control-plane operations unreachable through worker scope or grants, authentication explicitly trusted-channel, no secrets/network/live effects; reviewer-run secrets scan clean. The resolved-path scope comparison (src/creatidy_kernel/core/authority.py:88-96) is environment-independent and the symlink-widening regression test holds. Informational: AuthorityGrant.__post_init__ performs live filesystem checks (root.is_dir()), acceptable for a synthetic simulation.

Test / Documentation Gaps

• "Overbroad provider credential" conformance case unexercised (question 2; acceptable if confirmed as real-adapter scope).
• Exception-type consistency (non-blocking item 2).

Follow-Up Candidates

None durable enough for an issue; items 1–3 are ordinary polish belonging to the future real adapters or a passing cleanup.

Validation

Reviewer-run against the frozen head 73be4bd608: focused tests uv run --locked pytest tests/test_fake_authority.py tests/test_fake_execution.py tests/test_k3_integration.py → 12 passed; make check → ruff format/check, basedpyright, lint-imports contract KEPT, 164 tests passed, secrets scan clean, git diff --check clean; make package-check → wheel/sdist rebuild and isolated install passed. make audit not re-run by reviewer: the PR changes no dependency or tooling files, matching the validation rules' relevance condition.

Overall Recommendation

APPROVE — for the frozen head 73be4bd608 only. Integration and issue closure remain owner-controlled.

## PR Review: [K3] Prove synthetic execution, workspace and authority boundaries (#18) **Repository:** Creatidy/creatidy-kernel **Branch:** issue-6-synthetic-execution -> develop **Frozen head:** 73be4bd608a6b391d6a334c4eaa327f491aea1f4 (base 0fb435bf3653f2c3b7d986a0f872334dd278ce59; local clean checkout verified identical; head confirmed unchanged by `git ls-remote` before and after this review) **Linked issue:** #6 (from PR body "Implements #6") **Reviewer context:** fresh dispatched read-only reviewer session (Agent Manager local `ses_f2558af87ffe7rsTI143sc8bTq`); no implementation-session context or verdict relied on. Reviews #602 (3cf0b229) and #603 (97ac966) cover prior heads and are superseded by this full review of the current head. Dispatch-resolution verification (`DISPATCH_VERIFIED`) belongs to the coordinating delivery session, not this reviewer. The PR body cites validation on `97ac966`; the head advanced to `73be4bd` by merging develop (`0fb435b`, governance PR #20). I verified `git diff 97ac966..73be4bd` touches only `.kilo/command/deliver-issue.md` and `.kilo/command/review-pr.md` — `src/`, `tests/`, README and threat-model are byte-identical to the previously reviewed code; the branch is current with develop (merge-base equals base tip). ### Summary Adds core execution/authority values, `ports/execution.py` (`Runtime`, `Workspace`) and `ports/authority.py` (`AuthorityBroker`) protocols, deterministic `FakeRuntime`/`FakeWorkspace`/`FakeAuthorityBroker` adapters, three test modules (unit + K2B/K1 integration), and honest README/threat-model updates. 11 files, +1134/−3; no dependency or tooling files touched. Dependency direction kept (`adapters -> ports -> core`, lint-imports KEPT); stdlib+pytest only. ### Intent / Acceptance Evidence | #6 acceptance criterion | Evidence at frozen head | | --- | --- | | Fake Runtime executes an immutable Attempt via candidate publication; control-plane mutations stay authority-checked | `ExecutionRequest.__post_init__` binds attempt/workspace/context/allocation/agent-definition (src/creatidy_kernel/core/execution.py:145); `FakeRuntime.publish` validates scope attempt/workspace/fence/expiry/operation and binds candidate attempt/spec/artifacts (src/creatidy_kernel/adapters/fake_execution.py:190); `issue`/`revoke` owner-only (src/creatidy_kernel/adapters/fake_authority.py:20,29); candidate does not satisfy the WorkUnit — owner `FinishAttempt` required, then READY (tests/test_k3_integration.py:147,150) | | Unsupported recovery/identity is unknown/unavailable; duplicate starts and terminal replays safe | `identity_supported=False` blanks resolved/observed (src/creatidy_kernel/adapters/fake_execution.py:143); `reconcile` requires exact `OperationKey` equality and optional exact handle; lost keys and `recoverable=False` → UNKNOWN (src/creatidy_kernel/adapters/fake_execution.py:226); duplicate `start` returns same handle, changed inputs conflict; identical terminal replay no-op, changed candidate conflict (tests/test_fake_execution.py:38,57) | | Quiet healthy activity stays healthy/waiting; recovery targets the exact execution, not global age | per-observation `fresh_until` (WAITING at now=299, fresh_until=300, tests/test_fake_execution.py:98); reconcile keys on the exact effect key with full Operation equality — a quiet false child stall cannot redirect recovery (tests/test_fake_execution.py:103) | | Worker grants/acceptance/gate changes, control-state/credential access, workspace escape, stale-fence publication rejected | grant operations ⊆ `WORKER_OPERATIONS` (src/creatidy_kernel/core/authority.py:50); `access` denies control-plane operations and path/symlink/network/mount escapes (src/creatidy_kernel/adapters/fake_execution.py:67-98); stale `scope.fence` publish conflicts; journal rejects fence−1 (`OperationConflict`, tests/test_k3_integration.py:145); exact replay still rechecks revocation/expiry (src/creatidy_kernel/adapters/fake_authority.py:44) | | Deterministic synthetic tests preserve replaceable ports and no vendor imports | tests bind ports as `Runtime`/`Workspace` (tests/test_fake_execution.py:40); reviewer-run lint-imports contract KEPT; stdlib+pytest only; uv.lock/pyproject unchanged | | Custom sandbox explicitly excluded; supported trust mode honest | `ISOLATED` fails closed with `UnsupportedExecution` (src/creatidy_kernel/adapters/fake_execution.py:43); README and threat-model state trusted-development is not an OS/container boundary for hostile code | All six criteria are met with observable evidence. ### Architecture / Information Flow Conforms to `adapters -> ports -> core` and ADR 0004/0005 vocabulary. Start receives an immutable Attempt plus explicit workspace/context/allocation/capability references, Operation identity and fence; observe/cancel/candidate/reconcile distinguish FOUND/ABSENT/UNKNOWN; requested/resolved/observed identity recorded separately. Single-use grants are consumed inside one broker lock with exact replay returning the recorded intent without re-consuming, and exact replay rechecks revocation/expiry (ADR 0005); the in-memory-only scope of that atomicity is honestly disclosed in code, PR body, and the port docstring (src/creatidy_kernel/ports/authority.py:11), with durable intent-bound consumption pinned to the real adapter. The K2B integration uses the actual signatures (`admit_with_intent`/`claim`/`record_transport`/`observe` verified against src/creatidy_kernel/adapters/sqlite_store.py:349,521,564,585, including the accepted-reference binding and no-regress rules the test relies on). ADR 0004 workspace-lifetime deferral is documented at src/creatidy_kernel/ports/execution.py:31. ### Blocking Issues None found. ### Non-blocking Suggestions Unchanged code from the previously reviewed head; carry-over polish items remain accurate: 1. `FakeWorkspace.access` (src/creatidy_kernel/adapters/fake_execution.py:84) enforces a hardcoded control-plane denylist while core defines `WORKER_OPERATIONS` as the positive vocabulary. Checking `operation in WORKER_OPERATIONS` would use one vocabulary and reject any name no grant can confer. 2. Exception-type consistency: `candidate`/`progress`/`publish`/`cancel`/`lose` raise `KeyError` on unknown handles where `observe` raises `ExecutionConflict`; `core/execution.py` mixes `_text` `ValueError` with `ExecutionConflict`; `progress`'s "terminal activity cannot progress" also fires for `fresh_for < 0`. 3. `WorkerScope.__post_init__` (src/creatidy_kernel/core/execution.py:188) validates only paths, not `operations ⊆ WORKER_OPERATIONS`; a hand-built scope could carry arbitrary operation names past `access`'s first check unless denylisted. Synthetic-only concern — real adapters derive scope from a grant. ### Questions for the Author 1. The PR body still cites validation on `97ac966` while the head is the merge commit `73be4bd` (code identical, develop governance docs merged in). Please note the merge in the delivery ledger so the HEAD trail is self-explanatory for the next session. 2. Contracts.md's authority conformance case "overbroad provider credential" remains unexercised (the fake has no provider credentials to be overbroad). Confirm intentional deferral to the real provider adapter. ### Security / Privacy Notes Honest boundary labeling throughout: trusted-development mode documented as policy simulation, `ISOLATED` fails closed, control-plane operations unreachable through worker scope or grants, authentication explicitly trusted-channel, no secrets/network/live effects; reviewer-run secrets scan clean. The resolved-path scope comparison (src/creatidy_kernel/core/authority.py:88-96) is environment-independent and the symlink-widening regression test holds. Informational: `AuthorityGrant.__post_init__` performs live filesystem checks (`root.is_dir()`), acceptable for a synthetic simulation. ### Test / Documentation Gaps - "Overbroad provider credential" conformance case unexercised (question 2; acceptable if confirmed as real-adapter scope). - Exception-type consistency (non-blocking item 2). ### Follow-Up Candidates None durable enough for an issue; items 1–3 are ordinary polish belonging to the future real adapters or a passing cleanup. ### Validation Reviewer-run against the frozen head 73be4bd608a6b391d6a334c4eaa327f491aea1f4: focused tests `uv run --locked pytest tests/test_fake_authority.py tests/test_fake_execution.py tests/test_k3_integration.py` → 12 passed; `make check` → ruff format/check, basedpyright, lint-imports contract KEPT, 164 tests passed, secrets scan clean, `git diff --check` clean; `make package-check` → wheel/sdist rebuild and isolated install passed. `make audit` not re-run by reviewer: the PR changes no dependency or tooling files, matching the validation rules' relevance condition. ### Overall Recommendation APPROVE — for the frozen head 73be4bd608a6b391d6a334c4eaa327f491aea1f4 only. Integration and issue closure remain owner-controlled.

kilo referenced this pull request 2026-09-26 10:07:17 +00:00

[K3] Synthetic execution, workspace, and authority enforcement #6

adrian.tkacz approved these changes 2026-09-26 10:09:27 +00:00

adrian.tkacz merged commit e41f6928ba into develop 2026-09-26 10:09:33 +00:00

adrian.tkacz deleted branch issue-6-synthetic-execution 2026-09-26 10:09:33 +00:00

adrian.tkacz referenced this pull request from a commit 2026-09-26 10:09:34 +00:00

Merge pull request '[K3] Prove synthetic execution, workspace and authority boundaries' (#18) from issue-6-synthetic-execution into develop

kilo referenced this pull request 2026-10-04 22:29:51 +00:00

[Kernel][G13] Prove public installation, contract compatibility and safe state/profile migration #62

kilo referenced this pull request 2026-10-04 22:58:10 +00:00

Align Creatidy Kernel with the shared harness, routing and observability architecture #46

kilo referenced this pull request 2026-10-27 00:33:55 +00:00

Revise unified work coordination architecture and realign the canonical backlog #103

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

Subscribe

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!18

No description provided.