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

[K8] Prove optional Scarcity Router allocation with durable recovery #28

Merged
adrian.tkacz merged 2 commits from feat/k8-scarcity-allocator into develop 2026-09-28 11:09:49 +00:00
Conversation 5 Commits 2 Files changed 21 +2694 -108
kilo commented 2026-09-28 08:27:39 +00:00
Member
Copy link

READY_FOR_OWNER_MERGE

Exact approved head: 0dbbff381306596bb947155cda518d70570eda00; base develop at 367b617c87569080e99c46af7cd4cfefd116e4e2. Both re-verified after review. PR remains unmerged; owner integration only. Keep #11 open until integration.

Fresh independent whole-PR review by owner-selected Z.ai GLM-5.3: APPROVE, zero blocking findings. Review #654 is pinned to this head and nonstale: #28 (comment)

Forgejo persists the review as COMMENT, not formal approval, because the shared account is also the PR author. The substantive verdict is explicitly APPROVE. Exact-head CI and all repository gates pass.

Final delivery ledger: #11 (comment)

Review Provenance

  • Automated review routing previously returned NO_SELECTION. That was not a substantive review and consumed no remediation cycle.
  • Owner explicitly authorized OWNER_MODEL_OVERRIDE for this boundary: Z.ai / GLM-5.3 only, no fallback. This is not a Scarcity Router selection.
  • One fresh context dispatched through Agent Manager request am-1790589694495-94a2yu, session ses_f18898d77ffeGgpvhUgPGovCuw. Actual harness resolution: GLM-5.3 (zai-coding-plan), independently checked by reviewer. No explicit variant override or unreported variant claim.
  • Dispatch evidence: #28 (comment)
  • No Router call or repository/config/deployment/policy/runtime-state/issue/PR modification was made under the override. The previous policy discrepancy remains outside Kernel scope.

Scope And Baselines

Implements selected #11, preserving K8 as an optional external-consumer ResourceAllocator proof. Parent #3 changes were limited to K8 wording. Same issue, branch and PR throughout.

Upstream public-contract/reuse baseline: Scarcity Router 81b8e6393a1b2df8a6c7a9c6c3dcb8a7f6884e4e, merged D-057 / #148 / PR #149. Nine upstream effort-serialization tests passed at that pinned source. Public candidate effort is explicit catalog-sourced string/null, independent of opaque variant. No Router Python dependency or internal imports.

Changes

  • Optional public POST /v1/select allocator with exact independently configured Runtime bindings, bounded opaque provenance, explicit refusal/errors and no fallback selector.
  • Apache-2.0 transport/parser ADAPT with pinned source, modifications and distributed NOTICE: verified TLS, loopback HTTP, bearer authentication, redirect refusal, bounded bytes/depth/deadline, strict JSON/envelope handling and credential-safe failures.
  • Generic atomic Prepare binds original Allocation/context bytes with the immutable Attempt and operation intent. Recovery uses original bytes with zero re-selection, including Router outage or changed recommendations; corrupt/missing evidence fails closed.
  • Exact Codex thread configuration and turn effort, independent support bindings and native receipt validation. Requested/resolved/observed identities remain separate; unknown generation attestation stays unknown.
  • Native handle/fence/configuration snapshot is durably published together before accepting the receipt, closing the reviewed crash window. Later known identity contradictions are rejected, including unconfigured-effort cases.
  • Replaceable allocator reference composition exports evidence and retains verification, acceptance, explicit accepted-predecessor input and the separate existing K5/K7 AGit Forge effect.

Separate Acceptance Evidence

Fact Evidence
Router recommendation Producer-shaped redacted D-057 fixture; 94 adapter cases including selected/null, null vs none, malformed/version/identity/auth/network/runtime failure and transport safety
Kernel durable Allocation Crash-boundary matrix, real transaction rollback, stale caller state, original-byte/digest checks and zero recovery re-selection
Runtime requested/resolved/observed Exact native-shaped dispatch/receipt assertions; fresh controller/runtime reconstruction around receipt evidence, receipt acceptance and identity publication
Program acceptance Finite two-WorkUnit proof preserves spec, verification and explicit first accepted output as successor input; no preparation on invalid recommendation or duplicate native dispatch during supported recovery
Forge delivery Existing AGit and exact-head receipt semantics unchanged; fixture evidence, no automatic merge or deployment

Validation

  • make check: PASS, 757 tests, formatting/lint/BasedPyright/import boundary/secret scan/diff checks.
  • make package-check: PASS, LICENSE/NOTICE distribution, sdist rebuild equality, zero-dependency isolated install and reference replay/export.
  • make audit: PASS, no known vulnerabilities.
  • Reviewer reports independent reruns of all three gates at the same frozen head.
  • Forgejo CI #2039 / actions run62: SUCCESS at the exact approved SHA, reconfirmed after review: https://forgejo.creatidy.com/Creatidy/creatidy-kernel/actions/runs/62

Review #653 requested changes on old head 31bf956; the receipt/identity defect was remediated in 0dbbff3 and reviewed afresh. Remediation record: #28 (comment)

Boundaries And Limitations

Kernel retains Program authority, execution/recovery, verification/acceptance/outcomes and Forge effects. No gateway/prompt proxy, shadow policy, access-mode inference, reservations, adaptive rerouting, main/release/deployment changes. Acceptance tests use no live provider execution or paid inference.

Only the bounded reference capability/context mapping is supported. Runtime support is separately configured evidence, not inferred from names. OS DNS retains its resolver timeout; response I/O is bounded. Conservative echoed-token checks may refuse public/short-token collisions as documented. Trusted-development fixtures do not claim hostile-worker isolation. Reviewer's strictness notes are nonblocking; no speculative follow-up issues created.

Final ledger: valid full reviews=2; remediation_used=1/10, remaining9. Terminal state READY_FOR_OWNER_MERGE. Worktree clean; review polling cancelled. Any new implementation head requires a fresh review.

## READY_FOR_OWNER_MERGE Exact approved head: `0dbbff381306596bb947155cda518d70570eda00`; base `develop` at `367b617c87569080e99c46af7cd4cfefd116e4e2`. Both re-verified after review. **PR remains unmerged; owner integration only. Keep #11 open until integration.** Fresh independent whole-PR review by owner-selected **Z.ai GLM-5.3**: **APPROVE, zero blocking findings**. Review #654 is pinned to this head and nonstale: https://forgejo.creatidy.com/Creatidy/creatidy-kernel/pulls/28#issuecomment-12607 Forgejo persists the review as **COMMENT**, not formal approval, because the shared account is also the PR author. The substantive verdict is explicitly APPROVE. Exact-head CI and all repository gates pass. Final delivery ledger: https://forgejo.creatidy.com/Creatidy/creatidy-kernel/issues/11#issuecomment-12608 ## Review Provenance - Automated review routing previously returned NO_SELECTION. That was not a substantive review and consumed no remediation cycle. - Owner explicitly authorized **OWNER_MODEL_OVERRIDE** for this boundary: Z.ai / GLM-5.3 only, no fallback. This is not a Scarcity Router selection. - One fresh context dispatched through Agent Manager request `am-1790589694495-94a2yu`, session `ses_f18898d77ffeGgpvhUgPGovCuw`. Actual harness resolution: `GLM-5.3 (zai-coding-plan)`, independently checked by reviewer. No explicit variant override or unreported variant claim. - Dispatch evidence: https://forgejo.creatidy.com/Creatidy/creatidy-kernel/pulls/28#issuecomment-12598 - No Router call or repository/config/deployment/policy/runtime-state/issue/PR modification was made under the override. The previous policy discrepancy remains outside Kernel scope. ## Scope And Baselines Implements selected #11, preserving K8 as an optional external-consumer ResourceAllocator proof. Parent #3 changes were limited to K8 wording. Same issue, branch and PR throughout. Upstream public-contract/reuse baseline: Scarcity Router `81b8e6393a1b2df8a6c7a9c6c3dcb8a7f6884e4e`, merged D-057 / #148 / PR #149. Nine upstream effort-serialization tests passed at that pinned source. Public candidate effort is explicit catalog-sourced string/null, independent of opaque variant. No Router Python dependency or internal imports. ## Changes - Optional public `POST /v1/select` allocator with exact independently configured Runtime bindings, bounded opaque provenance, explicit refusal/errors and no fallback selector. - Apache-2.0 transport/parser ADAPT with pinned source, modifications and distributed NOTICE: verified TLS, loopback HTTP, bearer authentication, redirect refusal, bounded bytes/depth/deadline, strict JSON/envelope handling and credential-safe failures. - Generic atomic Prepare binds original Allocation/context bytes with the immutable Attempt and operation intent. Recovery uses original bytes with zero re-selection, including Router outage or changed recommendations; corrupt/missing evidence fails closed. - Exact Codex thread configuration and turn effort, independent support bindings and native receipt validation. Requested/resolved/observed identities remain separate; unknown generation attestation stays unknown. - Native handle/fence/configuration snapshot is durably published together before accepting the receipt, closing the reviewed crash window. Later known identity contradictions are rejected, including unconfigured-effort cases. - Replaceable allocator reference composition exports evidence and retains verification, acceptance, explicit accepted-predecessor input and the separate existing K5/K7 AGit Forge effect. ## Separate Acceptance Evidence | Fact | Evidence | | --- | --- | | Router recommendation | Producer-shaped redacted D-057 fixture; 94 adapter cases including selected/null, null vs none, malformed/version/identity/auth/network/runtime failure and transport safety | | Kernel durable Allocation | Crash-boundary matrix, real transaction rollback, stale caller state, original-byte/digest checks and zero recovery re-selection | | Runtime requested/resolved/observed | Exact native-shaped dispatch/receipt assertions; fresh controller/runtime reconstruction around receipt evidence, receipt acceptance and identity publication | | Program acceptance | Finite two-WorkUnit proof preserves spec, verification and explicit first accepted output as successor input; no preparation on invalid recommendation or duplicate native dispatch during supported recovery | | Forge delivery | Existing AGit and exact-head receipt semantics unchanged; fixture evidence, no automatic merge or deployment | ## Validation - `make check`: PASS, **757 tests**, formatting/lint/BasedPyright/import boundary/secret scan/diff checks. - `make package-check`: PASS, LICENSE/NOTICE distribution, sdist rebuild equality, zero-dependency isolated install and reference replay/export. - `make audit`: PASS, no known vulnerabilities. - Reviewer reports independent reruns of all three gates at the same frozen head. - Forgejo CI #2039 / actions run62: SUCCESS at the exact approved SHA, reconfirmed after review: https://forgejo.creatidy.com/Creatidy/creatidy-kernel/actions/runs/62 Review #653 requested changes on old head `31bf956`; the receipt/identity defect was remediated in `0dbbff3` and reviewed afresh. Remediation record: https://forgejo.creatidy.com/Creatidy/creatidy-kernel/pulls/28#issuecomment-12583 ## Boundaries And Limitations Kernel retains Program authority, execution/recovery, verification/acceptance/outcomes and Forge effects. No gateway/prompt proxy, shadow policy, access-mode inference, reservations, adaptive rerouting, main/release/deployment changes. Acceptance tests use no live provider execution or paid inference. Only the bounded reference capability/context mapping is supported. Runtime support is separately configured evidence, not inferred from names. OS DNS retains its resolver timeout; response I/O is bounded. Conservative echoed-token checks may refuse public/short-token collisions as documented. Trusted-development fixtures do not claim hostile-worker isolation. Reviewer's strictness notes are nonblocking; no speculative follow-up issues created. Final ledger: valid full reviews=2; remediation_used=1/10, remaining9. Terminal state **READY_FOR_OWNER_MERGE**. Worktree clean; review polling cancelled. Any new implementation head requires a fresh review.
feat: add durable optional Scarcity Router allocation
All checks were successful
ci / check (pull_request) Successful in 43s
Details
31bf956119
kilo reviewed 2026-09-28 08:36:16 +00:00
kilo left a comment
Copy link

PR Review: [K8] Prove optional Scarcity Router allocation with durable recovery (#28)

Repository: Creatidy/creatidy-kernel
Branch: feat/k8-scarcity-allocator -> develop
Frozen head: 31bf956119b0c5ead312b19ea408295a6b3b5d03
Linked issue: #11

Forgejo identity limitation: the authenticated account is also the PR author, so Forgejo rejected formal REQUEST_CHANGES. This stored COMMENT preserves the independent review's substantive REQUEST_CHANGES verdict; it is not a formal changes-request state.

Summary

The public D-057 contract gate, attribution, dependency direction, immutable Allocation preparation, no-reselection behavior, exact non-null effort dispatch, and happy-path two-WorkUnit proof are well covered. The exact frozen head is not ready to integrate because native runtime recovery loses required resolved identity at one committed crash boundary.

Intent / Acceptance Evidence

Issue #11 requirement Review evidence Status
Revision-pinned public contract and reuse record Issue comments #12454/#12541, docs/architecture/reuse.md, NOTICE Satisfied
Recommendation-only optional adapter, no fallback or Router dependency Adapter/diff and producer-shaped fixture Satisfied
Immutable Allocation survives restart without reselection Atomic preparation and allocation recovery tests Satisfied
Exact runtime configuration and durable requested/resolved/observed evidence Receipt-to-identity crash window described below Blocking
Finite two-WorkUnit proof and accepted predecessor input Happy-path reference tests Satisfied, but native restart coverage is incomplete
Forge path unchanged; no merge/deploy Diff inspection Satisfied
Repository gates PR evidence plus Forgejo Actions run 61 at this exact SHA reports success Reported/persisted CI success; reviewer did not execute PR code

Architecture / Information Flow

The change preserves adapters -> ports -> core, keeps Router policy and gateway behavior outside Kernel, and separates recommendation, durable Allocation, runtime identity, Program acceptance, and Forge delivery. Allocation/context bytes are durably bound before dispatch and existing Attempts do not reselect.

Blocking Issues

  1. A crash after the accepted runtime receipt but before identity publication permanently loses Codex resolution evidence. ports/application.py:188-192 records the accepted handle and exposes the receipt crash boundary before the first identity artifact is written at ports/application.py:226-232. On process restart, CodexRuntime.restore() reconstructs _Run only from thread/turn IDs (adapters/codex_runtime.py:247-260); thread/read recovers status but not model/provider/effort (adapters/codex_runtime.py:263-299). The restored observation therefore has all resolved* fields unset, and the application returns identity_unavailable because no prior durable identity exists. reference_live.py:248-263 then stops, so a completed first WorkUnit can never be accepted and the second WorkUnit is never created. This violates the issue's restart/two-WorkUnit criteria and the runtime contract's durable identity requirement. Persist recoverable resolution evidence atomically with the accepted receipt, or make native reconciliation re-establish and validate it before acceptance.

Non-blocking Suggestions

  • ports/allocation.py:196-200 accepts every resolved/observed effort when the Allocation effort is None. This allows malformed values or contradictory later observations to pass independently of the persisted resolution. Validate the identity shape before publication and compare any non-null current observation with the immutable recorded resolution.
  • adapters/scarcity_router.py:135-145,292-295,397-398 accepts short visible-ASCII bearer tokens but rejects any response containing the token as a substring anywhere. Valid tokens such as openai, high, or 1 can collide with ordinary response content and make every authenticated selection fail. Either constrain credentials so accidental collisions are not accepted configurations or narrow the leak check.

Questions for the Author

None.

Security / Privacy Notes

No blocking credential disclosure, TLS bypass, redirect-following, policy-shadowing, or execution-authority issue was found. Residual transport evidence is synthetic: certificate/hostname failure is asserted through a mocked SSL context rather than a real TLS failure, and OS DNS resolution remains outside the adapter deadline as documented.

Test / Documentation Gaps

  • tests/test_allocation_recovery.py:152-188 covers receipt, but reuses the same in-memory runtime object after reopening SQLite, preserving resolution state that a process restart loses.
  • tests/test_reference_scarcity.py:174-200 reconstructs the native path only after deliberately crashing after the identity artifact is durable. Add a native process-reconstruction regression that crashes immediately after receipt and before identity publication.
  • No documentation gap beyond the behavioral correction above was found.

Follow-Up Candidates

None. The missing receipt-boundary recovery is required by issue #11 and belongs in this PR.

Overall Recommendation

REQUEST_CHANGES

Blocking findings: 1. The review is limited to frozen head 31bf956119b0c5ead312b19ea408295a6b3b5d03; any remediation requires a fresh exact-head review.

## PR Review: [K8] Prove optional Scarcity Router allocation with durable recovery (#28) **Repository:** Creatidy/creatidy-kernel **Branch:** feat/k8-scarcity-allocator -> develop **Frozen head:** `31bf956119b0c5ead312b19ea408295a6b3b5d03` **Linked issue:** #11 > **Forgejo identity limitation:** the authenticated account is also the PR author, so Forgejo rejected formal `REQUEST_CHANGES`. This stored `COMMENT` preserves the independent review's substantive `REQUEST_CHANGES` verdict; it is not a formal changes-request state. ### Summary The public D-057 contract gate, attribution, dependency direction, immutable Allocation preparation, no-reselection behavior, exact non-null effort dispatch, and happy-path two-WorkUnit proof are well covered. The exact frozen head is not ready to integrate because native runtime recovery loses required resolved identity at one committed crash boundary. ### Intent / Acceptance Evidence | Issue #11 requirement | Review evidence | Status | | --- | --- | --- | | Revision-pinned public contract and reuse record | Issue comments #12454/#12541, `docs/architecture/reuse.md`, `NOTICE` | Satisfied | | Recommendation-only optional adapter, no fallback or Router dependency | Adapter/diff and producer-shaped fixture | Satisfied | | Immutable Allocation survives restart without reselection | Atomic preparation and allocation recovery tests | Satisfied | | Exact runtime configuration and durable requested/resolved/observed evidence | Receipt-to-identity crash window described below | **Blocking** | | Finite two-WorkUnit proof and accepted predecessor input | Happy-path reference tests | Satisfied, but native restart coverage is incomplete | | Forge path unchanged; no merge/deploy | Diff inspection | Satisfied | | Repository gates | PR evidence plus Forgejo Actions run 61 at this exact SHA reports success | Reported/persisted CI success; reviewer did not execute PR code | ### Architecture / Information Flow The change preserves `adapters -> ports -> core`, keeps Router policy and gateway behavior outside Kernel, and separates recommendation, durable Allocation, runtime identity, Program acceptance, and Forge delivery. Allocation/context bytes are durably bound before dispatch and existing Attempts do not reselect. ### Blocking Issues 1. **A crash after the accepted runtime receipt but before identity publication permanently loses Codex resolution evidence.** `ports/application.py:188-192` records the accepted handle and exposes the `receipt` crash boundary before the first identity artifact is written at `ports/application.py:226-232`. On process restart, `CodexRuntime.restore()` reconstructs `_Run` only from thread/turn IDs (`adapters/codex_runtime.py:247-260`); `thread/read` recovers status but not model/provider/effort (`adapters/codex_runtime.py:263-299`). The restored observation therefore has all `resolved*` fields unset, and the application returns `identity_unavailable` because no prior durable identity exists. `reference_live.py:248-263` then stops, so a completed first WorkUnit can never be accepted and the second WorkUnit is never created. This violates the issue's restart/two-WorkUnit criteria and the runtime contract's durable identity requirement. Persist recoverable resolution evidence atomically with the accepted receipt, or make native reconciliation re-establish and validate it before acceptance. ### Non-blocking Suggestions - `ports/allocation.py:196-200` accepts every resolved/observed effort when the Allocation effort is `None`. This allows malformed values or contradictory later observations to pass independently of the persisted resolution. Validate the identity shape before publication and compare any non-null current observation with the immutable recorded resolution. - `adapters/scarcity_router.py:135-145,292-295,397-398` accepts short visible-ASCII bearer tokens but rejects any response containing the token as a substring anywhere. Valid tokens such as `openai`, `high`, or `1` can collide with ordinary response content and make every authenticated selection fail. Either constrain credentials so accidental collisions are not accepted configurations or narrow the leak check. ### Questions for the Author None. ### Security / Privacy Notes No blocking credential disclosure, TLS bypass, redirect-following, policy-shadowing, or execution-authority issue was found. Residual transport evidence is synthetic: certificate/hostname failure is asserted through a mocked SSL context rather than a real TLS failure, and OS DNS resolution remains outside the adapter deadline as documented. ### Test / Documentation Gaps - `tests/test_allocation_recovery.py:152-188` covers `receipt`, but reuses the same in-memory runtime object after reopening SQLite, preserving resolution state that a process restart loses. - `tests/test_reference_scarcity.py:174-200` reconstructs the native path only after deliberately crashing *after* the identity artifact is durable. Add a native process-reconstruction regression that crashes immediately after receipt and before identity publication. - No documentation gap beyond the behavioral correction above was found. ### Follow-Up Candidates None. The missing receipt-boundary recovery is required by issue #11 and belongs in this PR. ### Overall Recommendation **REQUEST_CHANGES** Blocking findings: 1. The review is limited to frozen head `31bf956119b0c5ead312b19ea408295a6b3b5d03`; any remediation requires a fresh exact-head review.
fix: persist runtime resolution with durable receipt
All checks were successful
ci / check (pull_request) Successful in 44s
Details
0dbbff3813
kilo commented 2026-09-28 08:53:56 +00:00
Author
Member
Copy link

Remediation 1 complete; fresh whole-PR review pending

New head 0dbbff381306596bb947155cda518d70570eda00, same base 367b617c87569080e99c46af7cd4cfefd116e4e2. Resolves review #653's blocking receipt/identity crash window without a schema change or extra execution authority.

The generic application now atomically publishes the native handle, delivery fence and actual configuration snapshot in one immutable runtime-receipt artifact before accepting that receipt in operation state. Restart can replay publication-to-acceptance recovery and reconstruct identity after acceptance, with zero new allocation or native starts. The separate exported identity remains original runtime evidence, not a copy of requested configuration. Existing records genuinely lacking resolution still fail closed.

Discriminating native fixtures recreate CodexRuntime/controller state at (1) receipt evidence published but not accepted, (2) accepted receipt before identity publication, and (3) identity publication. Each is crossed with Router outage and changed recommendation; the original Attempt is accepted unchanged, followed by the second WorkUnit and existing Forge effect. The external native fixture retains server-side state, not the discarded adapter's resolution cache.

Also addressed the nonblocking null-effort identity concern: malformed identity fields and contradictory non-null later resolution/observations are rejected even for unconfigured Allocation effort. Tests prove unknown values do not erase the original known resolution.

Credential-collision suggestion disposition: retain conservative refusal rather than weakening protection against credential echoes or inventing a new upstream token syntax. README now explicitly documents that short/public-name credentials can collide with public decision strings and must not be used as opaque secrets. This is a safe-refusal limitation, not a successful authentication guarantee or disclosure.

Validation at new head: make check PASS (757 tests, all lint/type/import/secret/diff gates); make package-check PASS; make audit PASS. No live inference. Prior 737-test evidence remains historical to head 31bf956.

Owner now requires GLM-5.3/Z.ai instead of GPT-Sol; next review routing preserves L4 and reasoning/coding/tool_use 5/5/5 with reasoning/tool use required and constrains provider/model accordingly. Review count 1, remediation_used 1/10; prior REQUEST_CHANGES verdict is not treated as approval of this new head. No merge.

## Remediation 1 complete; fresh whole-PR review pending New head `0dbbff381306596bb947155cda518d70570eda00`, same base `367b617c87569080e99c46af7cd4cfefd116e4e2`. Resolves review #653's blocking receipt/identity crash window without a schema change or extra execution authority. The generic application now atomically publishes the native handle, delivery fence and actual configuration snapshot in one immutable `runtime-receipt` artifact **before** accepting that receipt in operation state. Restart can replay publication-to-acceptance recovery and reconstruct identity after acceptance, with zero new allocation or native starts. The separate exported identity remains original runtime evidence, not a copy of requested configuration. Existing records genuinely lacking resolution still fail closed. Discriminating native fixtures recreate CodexRuntime/controller state at (1) receipt evidence published but not accepted, (2) accepted receipt before identity publication, and (3) identity publication. Each is crossed with Router outage and changed recommendation; the original Attempt is accepted unchanged, followed by the second WorkUnit and existing Forge effect. The external native fixture retains server-side state, not the discarded adapter's resolution cache. Also addressed the nonblocking null-effort identity concern: malformed identity fields and contradictory non-null later resolution/observations are rejected even for unconfigured Allocation effort. Tests prove unknown values do not erase the original known resolution. Credential-collision suggestion disposition: retain conservative refusal rather than weakening protection against credential echoes or inventing a new upstream token syntax. README now explicitly documents that short/public-name credentials can collide with public decision strings and must not be used as opaque secrets. This is a safe-refusal limitation, not a successful authentication guarantee or disclosure. Validation at new head: `make check` PASS (**757 tests**, all lint/type/import/secret/diff gates); `make package-check` PASS; `make audit` PASS. No live inference. Prior 737-test evidence remains historical to head `31bf956`. Owner now requires GLM-5.3/Z.ai instead of GPT-Sol; next review routing preserves L4 and reasoning/coding/tool_use 5/5/5 with reasoning/tool use required and constrains provider/model accordingly. Review count 1, remediation_used 1/10; prior REQUEST_CHANGES verdict is not treated as approval of this new head. No merge.
kilo commented 2026-09-28 10:02:04 +00:00
Author
Member
Copy link

OWNER_MODEL_OVERRIDE: fresh review dispatched

Owner override supersedes the automated-routing blocker for this review boundary only; it does not change implementation, Router policy or the historical routing result. Head remains 0dbbff381306596bb947155cda518d70570eda00, base 367b617c87569080e99c46af7cd4cfefd116e4e2.

Actual dispatch: one fresh local Agent Manager session, exact catalog model GLM-5.3, provider zai-coding-plan (Z.ai). No variant override was specified by the owner or invented by the dispatcher. No fallback.

Agent Manager request am-1790589694495-94a2yu returned explicit harness resolution:

Requested 1 Agent Manager local session.
Resolved models:
- session: GLM-5.3 (zai-coding-plan)

This establishes DISPATCH_VERIFIED for the required model/provider; no unreported reasoning variant is claimed. The initial prompt supplies only the owner review mandate, frozen head/base, current issue acceptance checklist and read-only workflow, not the implementer conversation or a prior verdict as authority. Full-PR review is required, not a remediation-only pass.

Distinct evidence states: automated routing NO_SELECTION (historical); explicit override OWNER_MODEL_OVERRIDE; actual dispatch REQUESTED; model/provider DISPATCH_VERIFIED; reviewer completion PENDING; substantive verdict PENDING; current-head Forgejo review state absent. Existing exact-head local gates and CI #2039 remain green. No Router calls/mutations, code changes, merge or deployment in this boundary. Review budget remains remediation_used=1/10 until a valid substantive review requires further remediation.

## OWNER_MODEL_OVERRIDE: fresh review dispatched Owner override supersedes the automated-routing blocker for this review boundary only; it does not change implementation, Router policy or the historical routing result. Head remains `0dbbff381306596bb947155cda518d70570eda00`, base `367b617c87569080e99c46af7cd4cfefd116e4e2`. Actual dispatch: one fresh local Agent Manager session, exact catalog model **GLM-5.3**, provider **zai-coding-plan** (Z.ai). No variant override was specified by the owner or invented by the dispatcher. No fallback. Agent Manager request `am-1790589694495-94a2yu` returned explicit harness resolution: ```text Requested 1 Agent Manager local session. Resolved models: - session: GLM-5.3 (zai-coding-plan) ``` This establishes **DISPATCH_VERIFIED** for the required model/provider; no unreported reasoning variant is claimed. The initial prompt supplies only the owner review mandate, frozen head/base, current issue acceptance checklist and read-only workflow, not the implementer conversation or a prior verdict as authority. Full-PR review is required, not a remediation-only pass. Distinct evidence states: automated routing NO_SELECTION (historical); explicit override OWNER_MODEL_OVERRIDE; actual dispatch REQUESTED; model/provider DISPATCH_VERIFIED; reviewer completion PENDING; substantive verdict PENDING; current-head Forgejo review state absent. Existing exact-head local gates and CI #2039 remain green. No Router calls/mutations, code changes, merge or deployment in this boundary. Review budget remains remediation_used=1/10 until a valid substantive review requires further remediation.
kilo reviewed 2026-09-28 10:13:58 +00:00
kilo left a comment
Copy link

PR Review: [K8] Prove optional Scarcity Router allocation with durable recovery (#28)

Repository: Creatidy/creatidy-kernel
Branch: feat/k8-scarcity-allocator -> develop
Frozen head: 0dbbff381306596bb947155cda518d70570eda00 (re-verified via Git transport and Forgejo MCP before and after review; merge-base = base)
Base: develop at 367b617c87569080e99c46af7cd4cfefd116e4e2
Linked issue: #11
Reviewer dispatch: OWNER_MODEL_OVERRIDE (issue #11 comment 12594), not a Scarcity Router selection. Harness resolution recorded in PR comment 12598: session: GLM-5.3 (zai-coding-plan) for Agent Manager request am-1790589694495-94a2yu; no variant override, no fallback. Independently validated by this reviewer session.

Forgejo identity limitation: the authenticated account is also the PR author, so a formal APPROVED review state cannot be recorded for this PR (precedent: review #653). This COMMENT preserves the independent reviewer's substantive APPROVE verdict; it is not a formal Forgejo approval state.

Summary

Fresh whole-PR review of the remediated head. The optional ScarcityRouterAllocator consumes only the public D-057 POST /v1/select contract with exact explicit effort; generic Prepare now binds original allocation/context bytes atomically with the immutable Attempt and operation intent; recovery reconstructs original bytes with zero re-selection at every committed boundary; the native handle plus configuration snapshot are durably published together before receipt acceptance, closing review #653's blocking crash window; requested/resolved/observed identities remain distinct evidence; the K7 two-WorkUnit scenario, verification/acceptance/successor input, and the AGit Forge path are unchanged. Attribution (NOTICE) and packaging are correct. No blocking findings.

Intent / Acceptance Evidence

Issue #11 criterion Review evidence Status
Revision-pinned public-contract gate + reuse evidence before adapter work Issue comments 12454 (gate FAIL, GENUINE_OWNER_DECISION_REQUIRED, no workaround) and 12541 (Router 81b8e639 D-057 verified, 9 upstream tests); docs/architecture/reuse.md, NOTICE Satisfied
Adapter over stable public data; explicit incompatibility failure, no weakened requirements adapters/scarcity_router.py consumes only /v1/select; unsupported capabilities/runtime mismatch raise AllocationUnavailable Satisfied
Offline failure matrix: valid; selected=null; malformed; schema version; missing identity; unreachable; auth; exact-effort unavailable; runtime incompatibility; no fallback tests/test_scarcity_router.py (exactly 94 cases, counted): 9 HTTP statuses incl. 401/403, 27 malformed-field cases, 10 strict-JSON cases, null-vs-"none"-vs-missing effort, alternatives never used as fallback Satisfied
NEW Attempts bind durable Allocation; restart reconstructs without selection; Router-outage-after-dispatch and changed-recommendation-after-restart tested ports/application.py select-only-when-Attempt-absent + atomic admit_with_intent with original bytes; tests/test_allocation_recovery.py 8 boundaries x 2 replacements with calls == 0; tests/test_reference_scarcity.py 3 native boundaries x outage/different Satisfied
Exact execution config reaches Runtime; requested/resolved/observed separate; no silent effort omission/defaulting adapters/codex_runtime.py thread config.model_reasoning_effort + turn effort; receipt equality enforced before turn; resolver omission raises ExecutionConflict; supported_efforts trusted bindings; observed attestation refused Satisfied
K7 two-WorkUnit scenario with Router allocator; first accepted output is explicit second input test_router_native_two_work_unit_proof: 2 router calls, 2 native starts/turns, 1 PR, first\n/first\nsecond\n, predecessor binding enforced by runtime and store Satisfied
Forge delivery remains K5/K7 AGit; no automatic merge/deploy adapters/reference_forge.py untouched by the diff; exports assert automatic_merge/automatic_deploy false Satisfied
No shadow routing or reservation/capacity claims; provenance only from public contract No local ranking (single exact binding match); opaque containers type-checked, never interpreted; disclaimers in code/README Satisfied
FixedAllocator conformance; K1-K7 offline tests pass Router-absent; producer-shaped redacted fixtures; no paid inference tests/fixtures/scarcity_router_selected.json synthetic producer-shaped; full suite offline; FixedAllocator unchanged and still default Satisfied
make check, make package-check, make audit; independent review; issue open, PR unmerged Re-executed by this reviewer at the frozen head (below); review #653 superseded by this fresh whole-PR review; PR open/unmerged Satisfied

Reviewer-executed validation at frozen head 0dbbff381306596bb947155cda518d70570eda00: make check PASS (757 tests, ruff format/check, basedpyright, lint-imports 1 contract kept, secret scan, git diff --check); make package-check PASS (wheel LICENSE/NOTICE equality, sdist rebuild, zero-dependency isolated install, reference replay/export); make audit PASS (no known vulnerabilities). Forgejo CI run #2039 (actions run 62) SUCCESS at the exact SHA, verified via MCP.

Architecture / Information Flow

adapters -> ports -> core preserved: ports/allocation.py imports only core + ports.program_store; adapters/scarcity_router.py imports core resources and the ResourceAllocator port; zero new runtime dependencies (stdlib http.client/ssl/socket/threading only; dependencies = [] still enforced). Router policy, ranking, quota semantics, and the execution gateway remain outside Kernel; the adapter maps only the bounded reference capability/context request and treats opaque decision records as untrusted provenance. Recommendation, durable Allocation, Runtime identities, Program acceptance, and Forge effects remain distinct evidence chains. The prior K7 select-before-Attempt-lookup defect is corrected: selection occurs only when no Attempt exists, and Prepare commits the Attempt and the operation intent carrying the original allocation/context bytes in one transaction; artifacts are later finalized from those original bytes with digest verification, and missing/corrupt evidence fails closed (CorruptHistory/ValueError) rather than reselecting.

Blocking Issues

None (0). The single blocker from review #653 — loss of resolved provider/model/effort when the process stopped after the accepted native receipt but before identity publication — is remediated at this head: runtime-receipt (handle, fence, identity snapshot) is finalized before receipt acceptance, replay is fence/reference/identity-validated on recovery, and identity publication uses the durable receipt snapshot with require_resolved=True. Regressions prove completion after that exact crash (test_codex_receipt_crash_recovers_original_resolution_before_identity_publication) and no duplicate native starts across receipt-evidence/receipt/identity boundaries crossed with Router outage and changed recommendation, while records genuinely lacking resolution still fail closed.

Non-blocking Suggestions

  • _decision pins selector_mode == "balanced", a closed selection reason-code set, and a canonically identical requirement echo. Future additive Router evolution (new selector mode or reason code) will be refused until the adapter is re-reviewed. This is deliberate bounded-consumer strictness (and reuse.md already requires explicit public-contract review on upstream change); no change requested.
  • Candidate identity grammar is lowercase [a-z0-9][a-z0-9._:-]{0,63}. Current public identifiers fit; identifiers outside this grammar would be refused. Fail-closed; acceptable.
  • Auth, network, and HTTP failures collapse into one ScarcityRouterUnavailable message; the status class is not surfaced on the exception (only null-selection retains decision provenance). The port contract does not require finer granularity; an operator-facing diagnostic channel could be added later.
  • __post_init__ swallows TypeError/OverflowError into "invalid ... configuration", making construction-time type errors indistinguishable from invalid configuration. Cosmetic.
  • The echoed-credential substring check remains conservatively refusal-prone for public/short tokens; the limitation and guidance are now documented in README. Reasonable safety disposition, consistent with review #653's discussion.

Questions for the Author

None.

Security / Privacy Notes

Verified TLS defaults asserted (CERT_REQUIRED, hostname check), HTTP restricted to literal loopback, redirects never followed, error bodies never inspected or echoed, bearer credential excluded from repr, exceptions, and durable provenance (unicode-escaped echo case covered), bounded response bytes/JSON depth/total deadline with trickle and truncated-Content-Length coverage, duplicate-key and nonfinite-number rejection, and no raw hostile content in exception chains. No secrets introduced; repository secret scan passes. Residual, previously documented limitations unchanged: TLS certificate failure is evidenced via a mocked context rather than a live TLS failure, and OS DNS resolution remains outside the adapter deadline.

Test / Documentation Gaps

None blocking. Adapter case count (94), total test count (757), and all gate/CI claims in the PR body were independently reproduced or verified. README/reuse.md/NOTICE statements match the actual code behavior at this head; planned/live-capability wording is accurate (no live-provider execution claimed).

Follow-Up Candidates

None. The strictness notes above are documentation-level positions already recorded in docs/architecture/reuse.md and README.

Overall Recommendation

APPROVE — blocking findings: 0, at frozen head 0dbbff381306596bb947155cda518d70570eda00 only. Per the owner override in issue comment 12594, unchanged green validation/CI plus this zero-blocker APPROVE satisfies the review condition for READY_FOR_OWNER_MERGE; merge remains an owner decision and must not be performed by automation. Any new head requires a fresh review.

## PR Review: [K8] Prove optional Scarcity Router allocation with durable recovery (#28) **Repository:** Creatidy/creatidy-kernel **Branch:** feat/k8-scarcity-allocator -> develop **Frozen head:** `0dbbff381306596bb947155cda518d70570eda00` (re-verified via Git transport and Forgejo MCP before and after review; merge-base = base) **Base:** `develop` at `367b617c87569080e99c46af7cd4cfefd116e4e2` **Linked issue:** #11 **Reviewer dispatch:** OWNER_MODEL_OVERRIDE (issue #11 comment 12594), not a Scarcity Router selection. Harness resolution recorded in PR comment 12598: `session: GLM-5.3 (zai-coding-plan)` for Agent Manager request `am-1790589694495-94a2yu`; no variant override, no fallback. Independently validated by this reviewer session. > **Forgejo identity limitation:** the authenticated account is also the PR author, so a formal `APPROVED` review state cannot be recorded for this PR (precedent: review #653). This `COMMENT` preserves the independent reviewer's substantive **APPROVE** verdict; it is not a formal Forgejo approval state. ### Summary Fresh whole-PR review of the remediated head. The optional `ScarcityRouterAllocator` consumes only the public D-057 `POST /v1/select` contract with exact explicit effort; generic Prepare now binds original allocation/context bytes atomically with the immutable Attempt and operation intent; recovery reconstructs original bytes with zero re-selection at every committed boundary; the native handle plus configuration snapshot are durably published together before receipt acceptance, closing review #653's blocking crash window; requested/resolved/observed identities remain distinct evidence; the K7 two-WorkUnit scenario, verification/acceptance/successor input, and the AGit Forge path are unchanged. Attribution (NOTICE) and packaging are correct. No blocking findings. ### Intent / Acceptance Evidence | Issue #11 criterion | Review evidence | Status | | --- | --- | --- | | Revision-pinned public-contract gate + reuse evidence before adapter work | Issue comments 12454 (gate FAIL, GENUINE_OWNER_DECISION_REQUIRED, no workaround) and 12541 (Router `81b8e639` D-057 verified, 9 upstream tests); `docs/architecture/reuse.md`, `NOTICE` | Satisfied | | Adapter over stable public data; explicit incompatibility failure, no weakened requirements | `adapters/scarcity_router.py` consumes only `/v1/select`; unsupported capabilities/runtime mismatch raise `AllocationUnavailable` | Satisfied | | Offline failure matrix: valid; `selected=null`; malformed; schema version; missing identity; unreachable; auth; exact-effort unavailable; runtime incompatibility; no fallback | `tests/test_scarcity_router.py` (exactly 94 cases, counted): 9 HTTP statuses incl. 401/403, 27 malformed-field cases, 10 strict-JSON cases, null-vs-`"none"`-vs-missing effort, alternatives never used as fallback | Satisfied | | NEW Attempts bind durable Allocation; restart reconstructs without selection; Router-outage-after-dispatch and changed-recommendation-after-restart tested | `ports/application.py` select-only-when-Attempt-absent + atomic `admit_with_intent` with original bytes; `tests/test_allocation_recovery.py` 8 boundaries x 2 replacements with `calls == 0`; `tests/test_reference_scarcity.py` 3 native boundaries x outage/different | Satisfied | | Exact execution config reaches Runtime; requested/resolved/observed separate; no silent effort omission/defaulting | `adapters/codex_runtime.py` thread `config.model_reasoning_effort` + turn `effort`; receipt equality enforced before turn; resolver omission raises `ExecutionConflict`; `supported_efforts` trusted bindings; observed attestation refused | Satisfied | | K7 two-WorkUnit scenario with Router allocator; first accepted output is explicit second input | `test_router_native_two_work_unit_proof`: 2 router calls, 2 native starts/turns, 1 PR, `first\n`/`first\nsecond\n`, predecessor binding enforced by runtime and store | Satisfied | | Forge delivery remains K5/K7 AGit; no automatic merge/deploy | `adapters/reference_forge.py` untouched by the diff; exports assert `automatic_merge`/`automatic_deploy` false | Satisfied | | No shadow routing or reservation/capacity claims; provenance only from public contract | No local ranking (single exact binding match); opaque containers type-checked, never interpreted; disclaimers in code/README | Satisfied | | FixedAllocator conformance; K1-K7 offline tests pass Router-absent; producer-shaped redacted fixtures; no paid inference | `tests/fixtures/scarcity_router_selected.json` synthetic producer-shaped; full suite offline; FixedAllocator unchanged and still default | Satisfied | | `make check`, `make package-check`, `make audit`; independent review; issue open, PR unmerged | Re-executed by this reviewer at the frozen head (below); review #653 superseded by this fresh whole-PR review; PR open/unmerged | Satisfied | **Reviewer-executed validation at frozen head `0dbbff381306596bb947155cda518d70570eda00`:** `make check` PASS (757 tests, ruff format/check, basedpyright, lint-imports 1 contract kept, secret scan, `git diff --check`); `make package-check` PASS (wheel LICENSE/NOTICE equality, sdist rebuild, zero-dependency isolated install, reference replay/export); `make audit` PASS (no known vulnerabilities). Forgejo CI run #2039 (actions run 62) SUCCESS at the exact SHA, verified via MCP. ### Architecture / Information Flow `adapters -> ports -> core` preserved: `ports/allocation.py` imports only core + `ports.program_store`; `adapters/scarcity_router.py` imports core resources and the ResourceAllocator port; zero new runtime dependencies (stdlib `http.client`/`ssl`/`socket`/`threading` only; `dependencies = []` still enforced). Router policy, ranking, quota semantics, and the execution gateway remain outside Kernel; the adapter maps only the bounded `reference` capability/context request and treats opaque decision records as untrusted provenance. Recommendation, durable Allocation, Runtime identities, Program acceptance, and Forge effects remain distinct evidence chains. The prior K7 select-before-Attempt-lookup defect is corrected: selection occurs only when no Attempt exists, and Prepare commits the Attempt and the operation intent carrying the original allocation/context bytes in one transaction; artifacts are later finalized from those original bytes with digest verification, and missing/corrupt evidence fails closed (`CorruptHistory`/`ValueError`) rather than reselecting. ### Blocking Issues None (0). The single blocker from review #653 — loss of resolved provider/model/effort when the process stopped after the accepted native receipt but before identity publication — is remediated at this head: `runtime-receipt` (handle, fence, identity snapshot) is finalized before receipt acceptance, replay is fence/reference/identity-validated on recovery, and identity publication uses the durable receipt snapshot with `require_resolved=True`. Regressions prove completion after that exact crash (`test_codex_receipt_crash_recovers_original_resolution_before_identity_publication`) and no duplicate native starts across receipt-evidence/receipt/identity boundaries crossed with Router outage and changed recommendation, while records genuinely lacking resolution still fail closed. ### Non-blocking Suggestions - `_decision` pins `selector_mode == "balanced"`, a closed selection reason-code set, and a canonically identical requirement echo. Future additive Router evolution (new selector mode or reason code) will be refused until the adapter is re-reviewed. This is deliberate bounded-consumer strictness (and `reuse.md` already requires explicit public-contract review on upstream change); no change requested. - Candidate identity grammar is lowercase `[a-z0-9][a-z0-9._:-]{0,63}`. Current public identifiers fit; identifiers outside this grammar would be refused. Fail-closed; acceptable. - Auth, network, and HTTP failures collapse into one `ScarcityRouterUnavailable` message; the status class is not surfaced on the exception (only null-selection retains decision provenance). The port contract does not require finer granularity; an operator-facing diagnostic channel could be added later. - `__post_init__` swallows `TypeError`/`OverflowError` into "invalid ... configuration", making construction-time type errors indistinguishable from invalid configuration. Cosmetic. - The echoed-credential substring check remains conservatively refusal-prone for public/short tokens; the limitation and guidance are now documented in README. Reasonable safety disposition, consistent with review #653's discussion. ### Questions for the Author None. ### Security / Privacy Notes Verified TLS defaults asserted (`CERT_REQUIRED`, hostname check), HTTP restricted to literal loopback, redirects never followed, error bodies never inspected or echoed, bearer credential excluded from `repr`, exceptions, and durable provenance (unicode-escaped echo case covered), bounded response bytes/JSON depth/total deadline with trickle and truncated-Content-Length coverage, duplicate-key and nonfinite-number rejection, and no raw hostile content in exception chains. No secrets introduced; repository secret scan passes. Residual, previously documented limitations unchanged: TLS certificate failure is evidenced via a mocked context rather than a live TLS failure, and OS DNS resolution remains outside the adapter deadline. ### Test / Documentation Gaps None blocking. Adapter case count (94), total test count (757), and all gate/CI claims in the PR body were independently reproduced or verified. README/reuse.md/NOTICE statements match the actual code behavior at this head; planned/live-capability wording is accurate (no live-provider execution claimed). ### Follow-Up Candidates None. The strictness notes above are documentation-level positions already recorded in `docs/architecture/reuse.md` and README. ### Overall Recommendation **APPROVE** — blocking findings: **0**, at frozen head `0dbbff381306596bb947155cda518d70570eda00` only. Per the owner override in issue comment 12594, unchanged green validation/CI plus this zero-blocker APPROVE satisfies the review condition for `READY_FOR_OWNER_MERGE`; merge remains an owner decision and must not be performed by automation. Any new head requires a fresh review.
adrian.tkacz deleted branch feat/k8-scarcity-allocator 2026-09-28 11:09:49 +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!28
No description provided.