[K5] Add bounded Forge port and Forgejo reference adapter #22
No reviewers
Labels
No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Creatidy/creatidy-kernel!22
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "issue-9-forge-port"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #9 after owner integration; keep issue open during review.
Forge port and Forgejo reference adapter for opaque identities, normalized observations/receipts, conditional effects, and shared fake/reference conformance. No merge or deploy capability.
Owner-authorized asymmetric PR authority: exact accepted candidate commit SHA is materialized as a Kernel snapshot ref by one absent-target atomic Git CAS under an externally enforced, bound write-once isolation capability held through POST and verification. No pre-read or adoption of existing same-SHA ref; the mutable working branch is not the PR head. Without isolation,
pris unsupported before writes. Moving target base SHA is receipt provenance, not a changed repository/branch authority identity. No live server-side isolation provider or deployed protection is claimed. Unreadable snapshot verification and malformed producer evidence remain UNKNOWN, not authority conflict or acceptance.Current HEAD validation:
make check(471 passed, formatting/lint/typing/import/secrets/diff),make package-checkpassed. Earliermake auditreported no known vulnerabilities; no dependencies/tooling changed. No live Forgejo integration was run. Delivery ledger begins #9 (comment); owner decision and remediation 10 are #9 (comment) and #9 (comment).PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch: issue-9-forge-port -> develop
Frozen head:
ec8f83c05cLinked issue: #9
Summary
The port direction, inward dependency boundary, fail-closed receipts, conditional single-ref push, and absence of merge capability align with the selected issue and ADRs. The focused suite and repository gates pass, but producer-shape and trusted-transport gaps leave core acceptance criteria unmet.
Intent / Acceptance Evidence
Architecture / Information Flow
Dependency direction remains
adapters -> ports -> core, verified by import-linter. Forgejo payloads do not enter core. The main boundary defects are at the real producer schema and credential-bearing Git process.Blocking Issues
Blocking defect: real Forgejo check observations always become incomplete.
src/creatidy_kernel/adapters/forgejo.py:187-200requires ashafield on each item returned byGET /repos/{owner}/{repo}/commits/{ref}/statuses. The exact Forgejo instance Swagger forCommitStatusListdefinescontext, timestamps, creator, description,id,status,target_url, andurl, but nosha; the requested{ref}already binds the subject. The synthetic fixture addsshaattests/test_forge.py:58-63, masking the producer mismatch. A real successful response therefore raises inside_field(data, "sha")and returnsPage((), None, False), so exact-head CI evidence cannot be observed. Build the observation revision from the requested subject and test a producer-shaped Forgejo payload.Security/privacy note and blocking defect: the conditional push executes inherited Git hooks/config in the credential-bearing controller.
src/creatidy_kernel/adapters/forgejo_transport.py:117-152runs both Git commands in the supplied worktree, inherits the full environment, and does not disable hooks or isolate system/global/local configuration. A configuredpre-pushhook can execute arbitrary code with controller authority, andurl.*.insteadOfcan redirect the lexically validated URL afterConditionalGitTransport.__init__checks it at lines 91-108. This conflicts withdocs/architecture/threat-model.md:23and means exact-destination authorization is not enforced by the transport. Use a controlled Git configuration/hooks boundary or a trusted dedicated Git repository and add malicious-hook and URL-rewrite fixtures.Blocking defect: branch reconciliation can turn a deliberately uncertain source race into acceptance. The initial branch path returns
UNKNOWNwhen the exact source moves after the push (src/creatidy_kernel/adapters/forgejo.py:238-258), butreconcile()checks only the target revision at lines 303-309. The fake has the same gap atsrc/creatidy_kernel/adapters/fake_forge.py:272-278. After a lost reply and source movement, reconciliation therefore upgrades the same effect toACCEPTEDdespite no proof that the source precondition held at mutation time. Preserve that uncertainty by rechecking the branch source during reconciliation and add the race to the shared suite.Blocking conformance gap: the fake accepts revision providers rejected by the Forgejo adapter.
src/creatidy_kernel/adapters/fake_forge.py:204-230strips an optionalforgejo:prefix without validating the provider, whereasForgejoForge._authorized()rejects foreign providers atsrc/creatidy_kernel/adapters/forgejo.py:212-222. Agithub:revision can therefore pass fake tests while being impossible through the reference adapter. Enforce equivalent configured-provider constraints and cover foreign-provider effects in the shared suite.Non-blocking Suggestions
None beyond the blocking remediation.
Questions for the Author
None.
Security / Privacy Notes
The HTTP transport correctly requires HTTPS, rejects embedded credentials, disables redirects, bounds response bytes, and obtains its token lazily. The Git hook/config inheritance above remains the material security boundary issue.
Test / Documentation Gaps
Add producer-shaped
CommitStatusfixtures, malicious Git hook/config fixtures, a lost-reply plus moved-source reconciliation case, and cross-provider conformance cases. No documentation expansion is required.Follow-Up Candidates
None. These defects are required by issue #9 and belong in this PR.
Overall Recommendation
REQUEST_CHANGES
The recommendation applies only to frozen HEAD
ec8f83c05ceb38162845b854ef7b0f4b85be62d2. Blocking findings: four. Validation independently rerun:uv run --locked pytest tests/test_forge.py tests/test_forge_transports.py(22 passed),make check(201 passed),make package-check(passed), and frozen diff whitespace checks (passed). The sharedkiloidentity authored the PR, so this is intentionally persisted asCOMMENT; it is not a formal ForgejoREQUEST_CHANGESstate.PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch: issue-9-forge-port -> develop
Frozen head:
36351d0081Linked issue: #9
Summary
The cycle-1 remediation addresses the prior status-payload, inherited Git config/hooks, moved-source, and provider-conformance findings. Dependency direction remains
adapters -> ports -> core, exact-head CI run #1960 is green, and no merge capability is exposed. The replacement head still has blocking ambiguity in operation-key reconciliation, PR repository validation, absence/pagination semantics, and stale-result classification.Intent / Acceptance Evidence
Architecture / Information Flow
The code preserves the accepted inward dependency boundary and keeps Forgejo payloads out of core. The isolated bare Git push materially improves the credential boundary. Remaining defects are at the untrusted producer/reconciliation boundary required by ADR 0003, ADR 0005, and
docs/architecture/contracts.md.Blocking Issues
Blocking defect: different operation keys can generate the same durable PR marker.
src/creatidy_kernel/adapters/forgejo.py:266-267,313-314andsrc/creatidy_kernel/adapters/fake_forge.py:262,299concatenateeffect_keyandrequest_digestwith:.OperationKeypermits arbitrary nonempty strings, so("a:b", "c")and("a", "b:c")collide. With otherwise identical PR metadata, reconciliation can acknowledge one logical effect using another effect's artifact. Use an unambiguous canonical encoding or constrain and test the key grammar.Blocking defect: accepted PR receipts do not validate the repository identities in the Forgejo response.
src/creatidy_kernel/adapters/forgejo.py:147-158,280-297,327-352validates number, refs, and SHAs but ignoreshead.repoandbase.repo, then manufactures a change handle under the requested repository. A malformed, stale, or cross-repository response with matching names/SHAs can therefore be accepted for the wrong artifact. Validate producer-shaped repository identity on creation and reconciliation; if fork PRs are intended, represent their head repository explicitly.Blocking contract gap: authoritative absence is never represented.
src/creatidy_kernel/adapters/forgejo.py:97-102,112-117,130-139maps every direct 404 toUNKNOWN;src/creatidy_kernel/adapters/fake_forge.py:117-133also maps known-missing in-memory branches and changes toUNKNOWN.Presence.ABSENTis consequently unused, andtests/test_forge.py:52-57codifies the omission. Define which observations establish authoritative absence and separately coverABSENT, permission-limitedINACCESSIBLE, and transport/eventual-consistencyUNKNOWN.Blocking defect: hostile or incidental stderr can turn an unknown push into
STALE.src/creatidy_kernel/adapters/forgejo_transport.py:190-195concatenates stdout and stderr and classifies any output containingstale infoas a CAS rejection. Remote sideband/error text is not a validated porcelain receipt; a failed or uncertain push whose stderr contains that phrase is incorrectly made retryable as stale. Parse an exact structured porcelain rejection for the authorized target ref and test misleading stderr asUNKNOWN.Blocking pagination defect: a short page is treated as authoritative completion without producer pagination metadata.
src/creatidy_kernel/adapters/forgejo.py:160-200setscomplete=Truewhenever the payload is shorter than the requested size, while the HTTP transport discards response headers. If Forgejo applies a lower server-side cap, page one is falsely terminal and later PRs/checks cannot be observed. Preserve authoritative pagination metadata or continue safely to a terminal page; add a server-cap fixture.Blocking boundedness defect: PR reconciliation can issue unbounded authenticated requests.
src/creatidy_kernel/adapters/forgejo.py:313-356scans every PR page and fetches each PR individually with no total page/request/time budget. A large or adversarial repository can consume unbounded controller time and API quota despite per-request timeouts. Add a configurable reconciliation budget and returnUNKNOWNwithout treating budget exhaustion as absence.Non-blocking Suggestions
None beyond the required remediation.
Questions for the Author
None.
Security / Privacy Notes
The remediation now isolates Git hooks/configuration, constrains the remote, disables redirects, and keeps credentials outside the workload-controlled worktree. Findings 4 and 6 remain availability/authority-boundary issues because forge output is untrusted.
Test / Documentation Gaps
Add shared collision, repository-mismatch, authoritative-absence, server-cap, scan-budget, and misleading-stderr fixtures. Also align malformed/zero/negative change-handle behavior between the fake and Forgejo adapter; the current fake can accept synthetic identities the reference adapter rejects. No documentation expansion is required.
Follow-Up Candidates
None. These gaps are part of issue #9's exact-effect, pagination, conformance, and reconciliation acceptance criteria.
Overall Recommendation
REQUEST_CHANGES
This recommendation applies only to frozen HEAD
36351d00811113f4d22fa8aebb6f5ff55e2a2056. Blocking findings: six. Exact-head Forgejo Actions run #1960 reports success; PR evidence reportsmake checkwith 211 passing tests andmake package-checkpassing. I did not execute untrusted PR code in the reviewer context. The server's workflow-job endpoint returned 404, so job logs were not independently inspected. Live Forgejo integration remains unrun. The sharedkiloidentity authored the PR, so this review is intentionally persisted asCOMMENT; it is not a formal ForgejoREQUEST_CHANGESstate.Architecture / Model-Correction Review
Frozen HEAD:
36351d00811113f4d22fa8aebb6f5ff55e2a2056Base:
develop@8877b97b83629e942e1b8725dfd02bb70c658b27Scope: issue #9, ADR 0004/0005,
docs/architecture/contracts.md, whole PR, and cycle-2 F5-F10. This is not a replacement full-review verdict and is not a formal APPROVE.Verdict
BOUNDED_CORRECTION
All six blockers are remediable inside issue #9's accepted Forge boundary. They require no merge capability, fork-universal model, new Program semantics, owner authority, or architecture/policy decision.
Finding Disposition
base.repo.full_nameand, for this same-repository PR operation,head.repo.full_nameagainst the authorized repository before acknowledging creation or reconciliation. Treat missing/mismatched/fork-shaped data as unsupported/unknown; do not add universal fork support or a new core field.ABSENTwould be invalid. PreserveUNKNOWNwhen 404 may hide permission or consistency state. Add an explicit adapter/transport evidence path for authoritative direct absence and test it separately fromINACCESSIBLEandUNKNOWN; make the fake model the same distinctions.UNKNOWN.UNKNOWN.Bounded Instructions
FOUND/ABSENT/INACCESSIBLE/UNKNOWN; refine transport/adapter evidence rather than inventing a universal forge model.adapters -> ports -> coredirection.No genuine owner authority, security/privacy choice, scope expansion, or policy decision is materially required for these corrections.
PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch: issue-9-forge-port -> develop
Frozen head:
416e817499Linked issue: #9
Summary
The cycle-2 remediation fixes the prior marker, same-repository PR, server-cap pagination, stale-output, and basic reconciliation findings. Dependency direction remains
adapters -> ports -> core, exact-head CI is green, and no merge capability is exposed. The replacement head still has blocking transport-boundary, finite-budget, and absence/conformance gaps.Intent / Acceptance Evidence
Architecture / Information Flow
The code preserves the accepted inward dependency boundary. The remaining defects sit at the trusted-controller/untrusted-worktree and external-forge boundaries governed by ADR 0003, ADR 0005,
docs/architecture/contracts.md, and the threat model.Blocking Issues
Security/privacy note and blocking defect: workload Git metadata can redirect the credential-bearing controller to an arbitrary object database.
src/creatidy_kernel/adapters/forgejo_transport.py:106-107,163-175only verifies thatworktreeis a directory, then trustsgit rev-parse --git-path objectsand exports the result asGIT_ALTERNATE_OBJECT_DIRECTORIES. A worker-controlled.gitfile, linked-worktree/common-dir metadata, symlink, or alternates file can escape to another controller-readable repository; the controller then reads and can push reachable objects with forge credentials. This also has a validation/use race.tests/test_forge_transports.py:102-165covers hooks and config rewrites but not Git-dir/object-dir indirection. Canonically validate and pin the repository/object source, or require a separately trusted object store, with malicious indirection/symlink fixtures.Blocking defect: successful pushes are acknowledged from a tool exit code, not a validated domain receipt.
src/creatidy_kernel/adapters/forgejo_transport.py:196-198maps any zerogit pushreturn code toACCEPTEDwithout parsing the successful porcelain record for the exact source object, destination ref, and single requested update.tests/test_forge_transports.py:50-54codifies the gap by accepting arbitrary stdoutok. ADR 0003 explicitly says acknowledgement requires a validated domain receipt, not a tool exit code. Validate the exact success/up-to-date porcelain record and treat malformed, missing, extra-ref, or contradictory output asUNKNOWN.Blocking boundedness defect:
max_reconcile_requestsis exceeded on the successful-match path.src/creatidy_kernel/adapters/forgejo.py:339-355counts the list and detail requests, but lines 381-384 perform two additionalbranch()GETs without checking or incrementing the budget. With one matching PR andmax_reconcile_requests=2, reconciliation performs four authenticated requests and can returnACCEPTED. The current test only exercises a limit of one attests/test_forge.py:358-359. Make every network observation consume the same finite budget and test the exact-boundary acceptance path.Blocking acceptance gap: authoritative repository absence is discarded.
HTTPTransport.authoritative_absence()is part of the boundary and is used for branch/change 404s, butForgejoForge.identity()atsrc/creatidy_kernel/adapters/forgejo.py:104-117maps every non-200/non-403 response toUNKNOWN, even when the injected transport explicitly attests authoritative absence.FakeForge.identity()likewise cannot represent known repository absence. This leaves identity absence outside the issue's observation/absence model and shared conformance. Apply the same explicit-evidence rule to repository identity and add sharedABSENTversusINACCESSIBLEversusUNKNOWNcases.Security/privacy note and blocking boundedness defect: hostile Git output is captured without a byte cap. Every command at
src/creatidy_kernel/adapters/forgejo_transport.py:155-158, including network-facinggit push, usescapture_output=True. A hostile forge can emit unbounded sideband/stdout/stderr during the timeout and exhaust controller memory. Bound captured output while retaining enough structured porcelain for validation, and add an output-flood fixture.Blocking boundedness defect: the HTTPS limits are not end-to-end. At
src/creatidy_kernel/adapters/forgejo_transport.py:42-50,63,75-85, urllib's timeout is a per-socket-operation timeout rather than a total deadline, so a trickling peer can keepresponse.read(max_bytes + 1)alive indefinitely; request JSON is also serialized and sent without a request-size limit. Add a total operation deadline and outbound byte bound, with slow-response and oversized-request fixtures.Non-blocking Suggestions
Align malformed pagination cursors in the shared fake/reference suite.
ForgejoForge._page()returns an incomplete page for invalid/zero cursors, whileFakeForge._page()raises fromint(cursor)or its page check (src/creatidy_kernel/adapters/fake_forge.py:177-187).Questions for the Author
None.
Security / Privacy Notes
The isolated bare push repository, disabled hooks/config/helpers, redirect refusal, exact stale-output parsing, and same-repository PR validation are material improvements. Findings 1, 5, and 6 remain controller trust/resource-boundary defects.
Test / Documentation Gaps
Add producer-shaped success-porcelain, Git-dir/object-dir escape, exact request-budget, repository-absence, output-flood, slow-response, outbound-size, and shared malformed-cursor cases. No documentation expansion is required.
Follow-Up Candidates
None. These defects belong to issue #9's exact receipt, bounded transport/reconciliation, absence, and shared-conformance criteria.
Overall Recommendation
REQUEST_CHANGES
This recommendation applies only to frozen HEAD
416e81749916746cf41218538487ae28d19fc102. Blocking findings: six. Forgejo Actions run #1963 reports success for the exact head, andgit diff --check origin/develop...HEADpassed. I did not execute untrusted PR code in the reviewer context. Forgejo's workflow-job endpoint returned 404, so job logs were not independently inspected; live Forgejo integration remains unrun. The sharedkiloidentity authored the PR, so this review is intentionally persisted asCOMMENT; it is not a formal ForgejoREQUEST_CHANGESstate.Architecture / Model-Correction Review
Frozen HEAD:
416e81749916746cf41218538487ae28d19fc102Base:
develop@8877b97b83629e942e1b8725dfd02bb70c658b27Scope: issue #9, the whole PR, accepted ADR 0004/0005,
docs/architecture/contracts.md, the threat model, and cycle-3 F11-F16. This is not a replacement full-PR verdict and is not a formal APPROVE.Verdict
BOUNDED_CORRECTION
No material owner architecture, security, scope, or policy decision is required. Issue #9 already requires authorized conditional push, validated domain receipts, safe absence distinctions, bounded uncertain-operation reconciliation, and fake/reference conformance. ADR 0004 and the threat model already place credentials and enforcement in the trusted controller/OS boundary; ADR 0005 explicitly says repeated defects or remediation rounds do not themselves create a Human Gate.
Finding Disposition
No F11-F16 finding is invalid as a defect. Some overly local remediations would be invalid, as constrained below.
git rev-parseresult alone. The current transport derivesGIT_ALTERNATE_OBJECT_DIRECTORIESfrom workload-controlled Git metadata. The bounded correction is to stop discovering the credential-bearing push source through the workload worktree and require an explicit controller-owned, pinned object source whose trust/non-worker-writability is established by the existing Workspace/controller boundary. Locally validate the exact full OID exists and is a commit in that source, reject symlink/common-dir/alternates indirection outside the trusted source, and fail closed when the source cannot be established. The Forge adapter need not implement a new sandbox, provenance service, artifact store, or private-product integration; trusted-development mode may supply a controller-created bare repository, while hostile-worker operation remains unsupported until the accepted external sandbox/Workspace boundary supplies the source.UNKNOWN. Keep exact structured stale parsing separate.UNKNOWN; it cannot returnACCEPTEDor prove absence.authoritative_absence(path)evidence rule to repository identity and make the fake represent repositoryABSENT,INACCESSIBLE, andUNKNOWNdistinctly. Real Forgejo 404 remainsUNKNOWNwhen the transport cannot attest absence; blanket 404-to-ABSENTmapping would be invalid.capture_output=Truewith bounded concurrent stdout/stderr collection. On output-limit or timeout breach, terminate the process and returnUNKNOWN, because the remote effect may already have occurred. Preserve enough bounded porcelain bytes for F12 validation.UNKNOWN; oversize input rejected before I/O is a local refusal, not a remote receipt.Bounded Instructions
adapters -> ports -> core, and expose no merge operation.resolve(), or a caller assertion alone proves hostile-worker isolation, and do not add a mandatory external service or private repository dependency..git/common-dir/symlink/alternates escape, wrong-type or missing OID, exact and malformed success porcelain, extra update records, exact request-budget boundaries, repository absence classes, Git output floods, slow/trickling HTTP responses, and oversized outbound requests.Owner Decision
None. The trusted-source precondition and finite transport limits are consequences of already accepted architecture. If no trusted object source or truly bounded transport is available in a deployment, the correct existing decision is fail-closed unsupported operation, not an owner policy exception.
Exact-head Forgejo Actions run #1963 reports success. I did not execute PR code or tests; the workflow-job endpoint returned 404, so job logs were not independently inspected. This review is persisted as
COMMENTonly and does not approve or merge the PR.PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch: issue-9-forge-port -> develop
Frozen head:
0a42123cf2Linked issue: #9
Summary
The cycle-3 remediation materially fixes the trusted object-source seam, exact success receipt, request-budget accounting, repository absence evidence, bounded Git output, and total HTTP deadline. Dependency direction remains
adapters -> ports -> core, exact-head CI is green, and no merge capability is exposed. The current head still has blocking idempotency/replay defects and incomplete transport/resource guarantees.Intent / Acceptance Evidence
Architecture / Information Flow
The change preserves
adapters -> ports -> core, keeps provider payloads out of core, and does not add private/runtime dependencies. The remaining defects are at the exact-effect reconciliation and credential-bearing transport boundaries governed by ADR 0003, ADR 0005,docs/architecture/contracts.md, and the threat model.Blocking Issues
Blocking defect: PR reconciliation accepts one exact marker without proving the operation key is unique.
src/creatidy_kernel/adapters/forgejo.py:369-398returnsACCEPTEDon the first exact matching PR and never examines the rest of that page or later pages.src/creatidy_kernel/adapters/fake_forge.py:334-358has the same early return. If PR A exactly matches and PR B reuses the marker with different metadata/subjects, ordering A first hides the conflict and acknowledges an ambiguous duplicate effect. Scan the complete bounded search, returnUNKNOWNfor multiple/conflicting uses, and returnUNKNOWNif the budget cannot prove uniqueness.Blocking defect: replay bypasses the branch-creation absence invariant.
Effect.__post_init__permitsaction="branch"with a non-nullexpected(src/creatidy_kernel/core/forge.py:99-109). Bothapply()implementations route retries to reconciliation before enforcingexpected is None(fake_forge.py:251-260,forgejo.py:251-261), and both reconcile paths accept when the target already equals the requested revision (fake_forge.py:320-330,forgejo.py:327-337). An authorized malformed branch effect withdelivery_attempts=2can therefore receiveACCEPTEDeven though branch creation was never a legal effect. Enforce action-specific invariants before dispatch/reconciliation and add the malformed replay to the shared suite.Security/privacy note and blocking defect: Git timeout/output-limit handling kills only the immediate process.
run_git_bounded()starts no process group and callsprocess.kill()only on the parent PID (src/creatidy_kernel/adapters/forgejo_transport.py:128-159).git-remote-https, askpass, or another descendant can survive with credential-bearing environment/network activity after the adapter returnsUNKNOWN. The test attests/test_forge_transports.py:238-249uses childless processes and cannot detect this. Start an isolated process group/session and terminate/reap the entire group on every exceptional path.Blocking stale-semantics defect: producer-shaped stale porcelain is rejected by the parser. Success parsing correctly expects
To <remote>, one status line, andDone, but stale parsing atforgejo_transport.py:326-327accepts only the unframed status line. Realgit push --porcelain --force-with-leaserejection output uses the same three-line framing, so an ordinary lease race becomesUNKNOWN, notSTALE.tests/test_forge_transports.py:114-130codifies the synthetic one-line form. Parse the exact framed single-ref rejection while continuing to reject misleading stderr/extra records.Blocking HTTP boundary defect: the outbound limit is post-allocation and its refusal escapes the Forge receipt boundary.
forgejo_transport.py:88-95fully materializes the JSON string and encoded bytes before checkingmax_bytes, so a large worker-derived PR body can consume unbounded memory/CPU before rejection. The resultingValueErroris not caught byForgejoForge.apply()atforgejo.py:294-303, so an otherwise valid authorized effect escapes without a normalized receipt or explicit supported-domain exception. Bound serialization before full allocation and normalize the pre-I/O refusal.Blocking boundedness defect: trusted-source validation recursively walks the whole object database without a budget.
forgejo_transport.py:202-204walks the entire bare repository at construction and lines 262-278 repeat that traversal before every push, outside the subprocess timeout and without entry/depth/time limits. A large object store can therefore consume unbounded controller time and filesystem work despite the transport's finite-limit contract. Replace the full walk with a bounded/pinned validation strategy or enforce an explicit traversal budget that fails closed.Blocking observation gap: authentication failures are collapsed into generic uncertainty.
ForgejoForge.identity(),branch(), andchange()treat only HTTP 403 asINACCESSIBLE(forgejo.py:104-155). Forgejo 401 responses for missing/invalid/expired credentials becomeUNKNOWN, despite being explicit authentication/permission evidence. Handle producer-shaped 401 asINACCESSIBLEand add shared/reference cases distinct from authoritative absence and transport uncertainty.Non-blocking Suggestions
None beyond the required remediation.
Questions for the Author
None.
Security / Privacy Notes
The controller-owned bare source, disabled ambient Git configuration/hooks, exact one-ref success parser, redirect refusal, bounded captured bytes, and signal-backed main-thread HTTP deadline are material improvements. The surviving process-tree and unbounded local-allocation/traversal issues remain controller resource and credential-boundary defects.
Test / Documentation Gaps
Add duplicate-marker ordering/across-page cases, malformed branch replay, a descendant-process timeout/flood fixture, producer-shaped framed stale output, pre-allocation outbound limits, traversal-budget exhaustion, and HTTP 401 classifications. Use production-valid 40/64-character object IDs in shared Forgejo operation fixtures and cover malformed same-provider revisions. No documentation expansion is required.
Follow-Up Candidates
None. These findings belong to issue #9's required conformance, exact-effect reconciliation, distinct stale/inaccessible semantics, normalized receipts, and bounded transport behavior.
Overall Recommendation
REQUEST_CHANGES
This recommendation applies only to frozen HEAD
0a42123cf23a9cd3d03a9637290375ef09aa31e7. Blocking findings: seven. Forgejo Actions run #1964 reports success for the exact head, andgit diff --check origin/develop...HEADpassed. I did not execute untrusted PR code or tests in the reviewer context. Forgejo's workflow-job and artifact endpoints returned 404, so logs/artifacts were not independently inspected; live Forgejo integration remains unrun. The authenticatedkiloidentity authored the PR, so this review is intentionally persisted asCOMMENT; it is not a formal ForgejoREQUEST_CHANGESstate.Architecture / Model-Correction Review
Frozen HEAD:
0a42123cf23a9cd3d03a9637290375ef09aa31e7Base:
develop@8877b97b83629e942e1b8725dfd02bb70c658b27Scope: issue #9, the complete PR, accepted ADR 0004/0005,
docs/architecture/contracts.md, the threat model, and cycle-4 F17-F23. This is not a full PR approval and is not a merge recommendation.Verdict
BOUNDED_CORRECTION
The current shape should be boundedly simplified around its existing seams. No material owner authority, architecture, security, scope, or policy decision is required. Issue #9 and the accepted architecture already require canonical effect identity, replay-safe exact-effect reconciliation, normalized producer receipts, finite transport/resource behavior, and inaccessible/absent/unknown distinctions. ADR 0005 explicitly rejects remediation count as a Human Gate by itself.
Finding Disposition
UNKNOWNfor any duplicate/conflict or incomplete/budget-exhausted scan. Accept only one exact candidate after authoritative scan completion and exact current head/base checks. Apply the same cardinality rule in the fake.branchwith non-nullexpectedis an invalid effect shape that replay currently bypasses. Enforcebranch.expected is Nonebefore first delivery or reconciliation, preferably inEffect.__post_init__. It would be invalid scope expansion to require forensic proof of historical absence for every legitimate lost-reply branch creation; exact stable-key/postcondition reconciliation remains allowed.STALEonly from the exact producer-shaped three-line porcelain frame for the configured destination and single authorized ref. Malformed, extra, contradictory, or stderr-only evidence remainsUNKNOWN.REJECTEDwithout network I/O. Network/deadline ambiguity remainsUNKNOWN. Do not add a provider-specific arbitrary body limit to core or catch unrelatedValueErrors broadly.os.walk. Retain finite checks for the explicit controller-owned bare source, pinned top-level identity,commondir/alternates indirection, exact repository binding, and commit OID/type. An exhaustive path walk cannot establish hostile-worker isolation and must not grow into a provenance service or filesystem verifier.INACCESSIBLE; preserve explicit authoritative 404 asABSENT, concealed/non-authoritative 404 asUNKNOWN, and malformed/transport failures asUNKNOWN.No F17-F23 finding is wholly invalid. F18 and F22 are overstated if interpreted as demands for historical-absence proof or hostile-source certification; those interpretations should not drive remediation.
One Coherent Minimal Correction
UNKNOWN.The public
Forgeprotocol and existingEffect,Receipt,Page, andPresenceconcepts are sufficient. Preserve same-repository PR scope, no merge operation, no fork-universal behavior,adapters -> ports -> core, and no new service/private dependency.Evidence Boundary
The frozen head and base were reverified immediately before this review. Forgejo reports exact-head Actions run #1964 successful and the PR reports
make check/make package-checkpassing, but those results do not negate the model defects. I did not execute untrusted PR code, edit the branch, or merge. This review is persisted asCOMMENTonly and never constitutes full PR approval.PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch: issue-9-forge-port -> develop
Frozen head:
48ec096862Linked issue: #9
Summary
The cycle-4 remediation fixes the previously reported branch-effect shape, duplicate-marker scan cardinality on a static result set, framed stale parsing, local request refusal, HTTP 401 classification, recursive source walk, and timeout/output-overflow process-group cleanup. Dependency direction remains
adapters -> ports -> core, exact-head CI is green, and no merge capability is exposed. Three blocking defects remain at the exact-effect and hostile/uncertain transport boundaries.Intent / Acceptance Evidence
Architecture / Information Flow
The change preserves the accepted inward dependency boundary and keeps Forgejo payloads outside core. The remaining findings are confined to the reference adapter/transport and are required by ADR 0003, ADR 0005,
docs/architecture/contracts.md, and the threat model; they require no new service, merge capability, or architecture decision.Blocking Issues
Blocking exact-effect defect:
up to datecan bypass the expected-old or absent-target precondition.src/creatidy_kernel/adapters/forgejo_transport.py:321-337accepts=\t<sha>:<ref>\t[up to date]asACCEPTED. Git can report this no-op when the remote ref already equals the requested revision without proving that--force-with-lease=<ref>:<expected>matched; for branch creation it likewise does not prove the target was absent. A first delivery can therefore acknowledge an operation whose required CAS precondition never held. Treat an up-to-date first attempt asUNKNOWNunless the exact precondition can be proven independently; add stale-expected and pre-existing-same-revision branch fixtures.Security/privacy note and blocking process-lifecycle defect: a normal nonzero Git exit can leave credential-bearing descendants alive.
run_git_bounded()creates a new session atsrc/creatidy_kernel/adapters/forgejo_transport.py:145-147, but kills the process group only from the exception path at lines 170-176. If Git exits nonzero after a descendant closes its inherited output descriptors, collection completes normally and lines 177-182 return without terminating that descendant. Existing descendant tests attests/test_forge_transports.py:281-294cover only timeout/output overflow. Reap/terminate the command's process group on every completed non-success path and add a prompt-parent-failure fixture whose child closes stdout/stderr before waiting.Blocking reconciliation race: offset pagination does not prove marker uniqueness on a changing PR list.
_page()advances numeric pages until an empty result (src/creatidy_kernel/adapters/forgejo.py:184-230), andreconcile()treats that traversal as a complete uniqueness proof before returningACCEPTED(src/creatidy_kernel/adapters/forgejo.py:337-412). Forgejo's PR list is mutable: if a conflicting marker-bearing PR is inserted ahead of already-read pages, page offsets shift and the new PR can remain entirely behind the cursor while the later empty page still marks the scan complete. The static ordering/across-page fixtures attests/test_forge.py:374-395cannot expose this. Use a stable high-water/cursor strategy or require a bounded stable rescan; any mutation, incomplete stability proof, or exhausted budget must remainUNKNOWN.Non-blocking Suggestions
None beyond the blocking remediation.
Questions for the Author
None.
Security / Privacy Notes
HTTPS-only origin binding, redirect refusal, total HTTP deadline, request/response/output bounds, controlled Git configuration, explicit controller-owned bare source, exact framed stale parsing, and timeout/overflow group cleanup are material improvements. Finding 2 remains a credential/process containment gap.
Test / Documentation Gaps
Add producer-shaped cases for pre-existing same-revision targets with mismatched lease/required absence, nonzero parent exit with a detached-output descendant in the same process group, and a duplicate marker inserted ahead of the active pagination cursor. No documentation expansion is required.
Follow-Up Candidates
None. These defects belong to issue #9's exact authorized effect, uncertainty reconciliation, pagination, and bounded transport acceptance criteria.
Overall Recommendation
REQUEST_CHANGES
This recommendation applies only to frozen HEAD
48ec0968628739430be04f62e08db903664f1b9f. Blocking findings: three. Forgejo Actions run #1965 reports success for the exact head; the PR reportsmake checkwith 274 passing tests andmake package-checkpassing. I did not execute untrusted PR code in the reviewer context. Forgejo's workflow-job and artifact endpoints returned 404, so logs/artifacts were not independently inspected; live Forgejo integration remains unrun. The authenticatedkiloidentity authored the PR, so this review is intentionally persisted asCOMMENT; it is not a formal ForgejoREQUEST_CHANGESstate.PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch: issue-9-forge-port -> develop
Frozen head:
b35619fa2754b87fa44a9642665de30c99513741Linked issue: #9
Summary
The change establishes the intended inward dependency direction, an opaque Forge port, an independent fake, bounded HTTPS/Git transports, conditional effects, and no merge capability. Three blocking correctness/conformance defects remain at this exact head.
Intent / Acceptance Evidence
Referencestays qualified and payload parsing is adapter-local, but Forgejo Git revision payloads are not validated before acceptance.Architecture / Information Flow
The dependency direction remains
adapters -> ports -> core. Core does not depend on Forgejo, private repositories, cloud services, or live infrastructure. The concrete transports are controller-facing and bounded, but adapter validation must close the malformed-revision trust boundary before remote evidence can become domain acceptance.Blocking Issues
Malformed Forgejo revision identities can be accepted as exact domain evidence.
src/creatidy_kernel/adapters/forgejo.py:86-90validates only theforgejo:prefix. Branch and PR observations accept any nonempty revision string atforgejo.py:128-138andforgejo.py:160-178; PR creation can then returnACCEPTEDatforgejo.py:283-322when those malformed values match the effect. The fake repeats the weak validation atsrc/creatidy_kernel/adapters/fake_forge.py:163-180andfake_forge.py:237-249, whileforgejo_transport.py:258-265correctly requires a full 40- or 64-character hexadecimal object ID. Validate revision payloads using that full-OID grammar before producing observations or receipts, with shared malformed-revision coverage.Unsupported capabilities cannot be represented or rejected explicitly.
ForgejoForge.capabilities()always advertises all operations atsrc/creatidy_kernel/adapters/forgejo.py:75-76; theUnsupportedForgechecks atforgejo.py:244-256test that same constant set and are unreachable.FakeForgeis likewise fixed atsrc/creatidy_kernel/adapters/fake_forge.py:123-124. Configurations without PR or conditional-push support therefore advertise support and degrade into generic rejection/unknown outcomes. Derive capabilities from configured/probed transport support and cover supported/unsupported fixtures in the shared suite.Oversized numeric cursors violate fake/reference conformance.
ForgejoForge._page()catches integer conversion failure atsrc/creatidy_kernel/adapters/forgejo.py:180-186, whileFakeForge._page()performs unguardedint(cursor)atsrc/creatidy_kernel/adapters/fake_forge.py:182-192. Python rejects decimal strings beyond its configured digit limit, so Forgejo fails closed but the fake raises. The same issue exists for known PR references atfake_forge.py:326-337. Bound input length or catch conversion failures consistently, and add long numeric cursor/reference cases.Non-blocking Suggestions
None beyond the direct issue work above.
Questions for the Author
None.
Security / Privacy Notes
No merge operation, unconditional push, redirects, ambient credential helper, worker-controlled Git hooks, secrets, or private-runtime dependency is introduced. Git and HTTP operations have finite bounds and controlled environments. Blocking issue 1 remains a trust-boundary defect because malformed remote responses can be elevated into accepted domain evidence.
Test / Documentation Gaps
README.md:9-18still says the package does not contact providers or perform live effects, although this PR adds concrete HTTPS and credential-bearing Git transports. Distinguish not-autonomously-invoked from not-implemented.README.md:54-56still describes the Forge port as future integration.Follow-Up Candidates
None. These defects and documentation corrections are directly within issue #9.
Overall Recommendation
REQUEST_CHANGES for exact frozen HEAD
b35619fa2754b87fa44a9642665de30c99513741.The authenticated Forgejo account (
kilo) is also the PR author, so this substantive request-changes verdict is persisted asCOMMENT; it is not a formal ForgejoREQUEST_CHANGESreview.PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch:
issue-9-forge-port->developFrozen head:
7a33307fb7efcc1174a6e75b91a0758ea2cd02c6Linked issue: #9
Summary
The architecture direction, opaque qualified references, explicit capability negotiation, fail-closed observations, bounded transports, conservative uncertain replay, and absence of merge automation are aligned with issue #9. CI run #1967 succeeded for this exact head, and the PR reports
make check(365 passed) plusmake package-checkpassing. Live Forgejo integration was not run.Three blocking correctness/authority defects remain on the frozen head.
Intent / Acceptance Evidence
Architecture / Information Flow
Dependency direction remains
adapters -> ports -> core; Forgejo payloads do not leak into core. No merge API, deployment behavior, GitHub canonical behavior, or universal SDK was added.Blocking Issues
Security/authority: Git remote binding compares raw URL spelling rather than a canonical remote target.
src/creatidy_kernel/adapters/forgejo_transport.py:209-214compares the raw URL path directly to the opaque reference payload. A reference such asforgejo:team/proj%65ctcan bind Git to/team/proj%65ct.git, which a server interprets asteam/project, while the HTTP adapter quotes%and addresses/repos/team/proj%2565ct. Git and HTTP can therefore act on different repositories for one supposedly exact repository reference. Parse and canonicalize the URL path, or reject percent escapes and other noncanonical repository components before accepting the transport.Idempotency: a PR receipt can be accepted with a reference that cannot later be observed or reconciled.
src/creatidy_kernel/adapters/forgejo.py:199-205acceptsnumberwhenisinstance(number, int)andnumber >= 0; this admits0,True, and arbitrarily long integers. Reference parsing later requires 1-18 nonzero ASCII digits (_number, used bychange/reconcile). A malformed producer response can therefore yield anACCEPTEDreceipt whose reference cannot be replayed. Apply the same invariant to producer payloads and reference parsing, usingtype(number) is int.Conformance: the fake accepts branch names that production rejects. Core validation at
src/creatidy_kernel/core/forge.py:97-103checks only emptiness/surrounding whitespace.FakeForgewrites the branch directly atsrc/creatidy_kernel/adapters/fake_forge.py:294-299, whileConditionalGitTransportrejects invalid refs withgit check-ref-format --branchatsrc/creatidy_kernel/adapters/forgejo_transport.py:305-307. For example,bad..namesucceeds in fake-backed tests but fails in production, so the shared fake/reference conformance requirement is unmet. Validate the same branch grammar before either adapter performs an effect and add shared regression coverage.Non-blocking Suggestions
None.
Questions for the Author
None.
Security / Privacy Notes
Blocking issue 1 is an exact-scope authority violation: the same logical repository handle can identify different Git and HTTP targets. No secret exposure was observed in this review.
Test / Documentation Gaps
Add shared adversarial cases for percent-escaped repository paths, producer PR numbers
0/True/more than 18 digits, and Git-invalid branch names. Documentation otherwise matches the implemented opt-in transport behavior.Follow-Up Candidates
None. All missing work is required by issue #9 and belongs in this PR.
Overall Recommendation
REQUEST_CHANGES
Three blocking findings remain on exact HEAD
7a33307fb7efcc1174a6e75b91a0758ea2cd02c6. This review is persisted asCOMMENTbecause the authenticated sharedkiloaccount is also the PR author;COMMENTis not formal platform approval or a formal requested-changes state.PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch: issue-9-forge-port -> develop
Frozen head:
08f994f4abf31601b03dc551633a9181cf6585ecLinked issue: #9
Summary
The port direction, inward dependency layering, conservative PR replay, and lack of merge support align with issue #9. Exact-head CI run #1968 succeeded. The current head still has blocking exact-scope, fail-closed producer-validation, rejection-classification, and transport-boundary defects.
Intent / Acceptance Evidence
Reference; adapters interpret Forgejo payloadsUNKNOWNArchitecture / Information Flow
Dependency direction remains
adapters -> ports -> core. Core does not import Forgejo payloads, live Forgejo, or private infrastructure. The blocking issues occur at untrusted producer and credential-bearing adapter boundaries, where ADR 0003/0005 anddocs/architecture/contracts.mdrequire exact subjects, domain rejection distinct from uncertainty, and bounded fail-closed behavior.Blocking Issues
Exact repository authority is not bound across transports (
src/creatidy_kernel/adapters/forgejo_transport.py:199-212,src/creatidy_kernel/adapters/forgejo.py:76-87).ConditionalGitTransportvalidates only the rawowner/reposuffix. AForgejoForgecan observeforgejo:team/projectthroughhttps://canonical.example/api/v1while pushing the same authorized effect tohttps://other.example/team/project.git. Neither the opaque reference norauthorize(effect)distinguishes those installations. Composition must reject mismatched origins or the reference/configuration must carry an equivalent exact forge-instance binding.Malformed synthetic PRs can escape fail-closed handling (
src/creatidy_kernel/adapters/fake_forge.py:165-177).change()indexespull['number']before_pull()validates the record. A producer-shaped record missingnumberraisesKeyErrorinstead of returningPresence.UNKNOWN, violating fake/reference conformance. The malformed-record test covers invalid values but not a missing field.Boolean check IDs fabricate exact passed-check evidence (
src/creatidy_kernel/adapters/fake_forge.py:235-256,src/creatidy_kernel/adapters/forgejo.py:221-243). Both adapters useisinstance(check_id, int), so JSONtruebecomes the exact referenceforgejo:team/project@Trueand may carryPASSED. Requiretype(check_id) is intand the producer's finite integer range; add shared malformed-ID cases.Known Forgejo PR refusals are classified as uncertain (
src/creatidy_kernel/adapters/forgejo.py:318-334). Only 403/422 becomeREJECTED; documented non-creation responses such as 409 conflict, 413 quota exceeded, and 423 archived repository becomeUNKNOWN. These responses establish a domain refusal rather than an uncertain transport outcome. Map documented deterministic refusals explicitly and test replay behavior.Advertised HTTP capabilities can be deterministically unusable (
src/creatidy_kernel/adapters/forgejo_transport.py:51-59,76-90;src/creatidy_kernel/adapters/forgejo.py:88-101).supports_readsandsupports_prare always true, but_deadline()refuses every operation outside the main thread or withoutsetitimer, before token access or network I/O. PR apply then reportsUNKNOWN, falsely implying a remote effect may have occurred, instead of explicit unsupported/local refusal. Capability negotiation must reflect the execution context or dispatch must reject before an uncertain receipt is possible.A successful Git parent can leave credential-bearing descendants alive (
src/creatidy_kernel/adapters/forgejo_transport.py:146-190). Process-group cleanup occurs for exceptions and nonzero exit only. A zero-exit Git/askpass process can spawn a child with closed output descriptors; selector draining and parent success return while the descendant survives past the configured deadline. Ensure the process group is settled/terminated on every completion path and add the corresponding successful-parent fixture.Bounded hostile JSON can raise uncaught parser exceptions (
src/creatidy_kernel/adapters/forgejo_transport.py:127-143).json.loads()catches only decoding errors. Deep nesting can raiseRecursionError, and pathological numeric literals can raiseValueError; both escape rather than becoming malformed/unknown producer data. Normalize all bounded parse failures without hiding transport outcomes.Reference/path validation is internally inconsistent and unbounded (
src/creatidy_kernel/adapters/forge_refs.py:10-35,src/creatidy_kernel/adapters/forgejo_transport.py:96-109).repository_path()permitsteam/pro..ject, while the HTTP transport rejects every path containing..; the Git side accepts the same repository, creating another split boundary. Repository and branch values also have no finite length limit, allowing unbounded URL quoting/path allocation before credential access. Define one canonical finite grammar and exercise it across fake, HTTP, and Git transports.Git object IDs are accepted case-insensitively but compared case-sensitively (
src/creatidy_kernel/adapters/forgejo.py:65-69,108-115,155-164,287-317). An authorized uppercase full OID and the equivalent lowercase Forgejo observation compare as differentReferencevalues, producing falseSTALE/UNKNOWNresults. Canonicalize adapter-owned OIDs (or consistently reject noncanonical spelling) and test uppercase input against lowercase producer payloads.Non-blocking Suggestions
None beyond the blocking corrections above.
Questions for the Author
None. The findings are bounded implementation corrections within issue #9 and the accepted architecture.
Security / Privacy Notes
The cross-origin mutation path and surviving successful-process descendants affect exact authority and scoped-credential containment. No secret material was inspected, and no live Forgejo integration or PR code was executed during this review.
Test / Documentation Gaps
Add producer-shaped cases for missing PR fields, boolean/out-of-range check IDs, lowercase producer OIDs against uppercase inputs, all documented PR refusal statuses, mismatched HTTP/Git origins, non-main-thread capability behavior, successful-parent descendants, hostile bounded JSON, oversized references, and consecutive-dot repository names.
Follow-Up Candidates
None. These are required issue #9 acceptance corrections, not distinct follow-up work.
Overall Recommendation
REQUEST_CHANGES for frozen HEAD
08f994f4abf31601b03dc551633a9181cf6585ec.Because the authenticated reviewer is the shared
kiloPR-author account, this substantive verdict is persisted as a ForgejoCOMMENT, not a formal platformREQUEST_CHANGESor approval.Architecture / Model-Correction Review
Frozen HEAD:
08f994f4abf31601b03dc551633a9181cf6585ecBase:
develop@8877b97b83629e942e1b8725dfd02bb70c658b27Scope: issue #9, the complete PR, accepted ADR 0003/0004/0005,
docs/architecture/contracts.md, the threat model, and full review F33-F41. This is not full PR approval and is not a merge recommendation.Verdict
BOUNDED_CORRECTION
One coherent correction remains feasible within the final three authorized cycles. No new owner authority, architecture, security policy, service, merge capability, or scope expansion is required. The accepted architecture already requires exact repository authority, finite external operations, validated producer evidence, explicit unsupported capabilities, and shared fake/reference conformance. Repeated remediation is not itself a Human Gate under ADR 0005.
Finding Disposition
type(value) is int), bounded JSON parse failures, and canonical Git OIDs. The smallest safe OID rule is lowercase full 40/64 hex at the boundary; uppercase/noncanonical input may be rejected rather than supported.REJECTED; authentication remainsINACCESSIBLE, malformed/unrecognized responses remainUNKNOWN, and transient or ambiguous statuses must not be guessed into rejection. Do not create a universal HTTP-status policy.UnsupportedForgebefore token access or network I/O. Non-main-thread and no-setitimerlive use may safely remain unsupported; do not weaken the deadline or report remote uncertainty for a deterministic local refusal.UNKNOWN; it is not acceptance.pro..jector consistently reject it; split behavior is the defect.One Coherent Minimal Correction
ForgejoForgerejects mismatched composition before capabilities or credentials are available. The synthetic transport exposes the same binding for shared tests.UNKNOWN/incomplete evidence, never an exception or accepted receipt.UNKNOWN.REJECTED; preserve all other ambiguity asUNKNOWN. Add one shared adversarial matrix covering mismatched origins, missing PR fields, bool/out-of-range IDs, hostile JSON, canonical OIDs, finite reference limits, context-disabled HTTP, successful-parent descendants, and refusal statuses.Safe Unsupported / Fail-Closed Boundary
It is safe for this issue to leave live HTTP unsupported where a true total deadline cannot be enforced, reject mismatched or noncanonical instance/repository configuration at construction, reject uppercase or oversized identifiers, and return
UNKNOWNfor unrecognized producer/status/process evidence. It is not safe to advertise unusable capabilities, infer cross-instance equality fromowner/repo, accept a parent exit as process-group completion, or broadenREJECTEDto ambiguous transport responses.The existing public
Forgeconcepts andadapters -> ports -> coredirection are sufficient. Preserve same-repository PR scope, the trusted controller-owned object source, conservative known-reference replay, and no merge operation. This correction does not require a new service, universal Forge SDK, custom sandbox, secrets manager, private dependency, or relaxed issue criterion.I reverified the PR immediately before persistence: HEAD and base remain the frozen commits above. I did not execute PR code, edit the branch, launch another reviewer, or merge. Persisted state is
COMMENTonly.PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch:
issue-9-forge-port->developFrozen head:
a62e8a7159c50af0c6adfffa92bc4101abb7f4bdLinked issue: #9
Summary
The dependency direction, opaque core references, conditional Git update, canonical Forge binding, fail-closed observation model, and absence of merge automation are sound. Three blocking defects remain at this exact head.
Intent / Acceptance Evidence
Referencestays provider-qualified; Forgejo interpretation remains in adaptersArchitecture / Information Flow
adapters -> ports -> coreis preserved. HTTPS origin/repository binding, redirect refusal, controlled Git configuration, exact conditional push, bounded subprocess capture, process-group cleanup, scoped credentials, and conservative 404 handling are positive controls.Blocking Issues
PR creation can perform an effect outside the authorized exact revision scope (
src/creatidy_kernel/adapters/forgejo.py:322-360). The adapter observeshead@Aandbase@B, then POSTs only mutable branch names. If either branch moves before Forgejo handles the POST, Forgejo can create a PR for different revisions. Post-creation payload checks can returnUNKNOWN, but the unauthorized artifact already exists. This violates the exact-scope acceptance criterion. Theprcapability needs a race-free creation mechanism or must not be advertised when exact revision binding cannot be enforced.Fake/reference conformance diverges for unavailable reads and repository binding (
src/creatidy_kernel/adapters/fake_forge.py:132-152,src/creatidy_kernel/adapters/forgejo.py:103-107,128-129,265-267).FakeForge.capabilities()removes read capabilities whensupports_readsis false, yet its observation methods still return state;ForgejoForgeraisesUnsupportedForge. A valid repository outside the configured binding returnsUNKNOWNin the fake but raisesForgeConflictin the reference adapter. Shared tests do not cover these cases, allowing fake-tested callers to depend on behavior unavailable in production.PR size limits occur after unbounded marker/body construction (
src/creatidy_kernel/core/forge.py:74-118,src/creatidy_kernel/adapters/forgejo.py:328-334,src/creatidy_kernel/adapters/forgejo_transport.py:124-134).Effect/OperationKeytext fields have no finite bound.effect_marker()serializes and Base64-encodes the complete operation identity, andapply()concatenates the PR body before the transport enforcesmax_bytes. Large authorized inputs can consume controller memory/CPU before deterministic local refusal. Validate finite inputs before encoding/concatenation and return the existing local rejection without network I/O.Non-blocking Suggestions
None.
Questions for the Author
None.
Security / Privacy Notes
No credential disclosure or private-state dependency was found. The first and third blockers affect authority and trusted-controller resource boundaries.
Test / Documentation Gaps
Add shared conformance cases for unsupported reads and valid cross-binding repositories. Add adversarial tests proving oversized operation/title/body inputs are rejected before marker/body allocation. A live Forgejo race integration was not run; post-creation validation is insufficient by inspection.
Follow-Up Candidates
None. These findings are required by issue #9 and belong in this PR.
Overall Recommendation
REQUEST_CHANGES
Three blocking findings remain for frozen HEAD
a62e8a7159c50af0c6adfffa92bc4101abb7f4bd. This review is persisted asCOMMENTbecause the authenticated sharedkiloidentity is also the PR author; this is not a formal Forgejo requested-changes state.Architecture / Model-Correction Review
Frozen head:
a62e8a7159c50af0c6adfffa92bc4101abb7f4bdFrozen base:
develop@8877b97b83629e942e1b8725dfd02bb70c658b27Selected issue: #9
Verdict:
GENUINE_OWNER_DECISION_REQUIREDF42 is not a bounded implementation correction under the current contract. Issue #9 simultaneously requires authorized Forgejo PR effects, only authorized exact-scope effects, shared fake/reference required-operation conformance, and tested PR opening. At this head,
ForgejoForge.apply()observes exact head/base revisions, then callsPOST /pullswith only mutable branch names (src/creatidy_kernel/adapters/forgejo.py:322-335). Either ref may move before Forgejo resolves the POST. Response and branch checks at lines 344-360 can classify the outcomeUNKNOWN, but cannot prevent or undo the already-created out-of-scope artifact.ADR 0003 permits an effect only with external idempotency/CAS or an enforced exclusive capability and says unsafe automatic operations remain unsupported otherwise. ADR 0005 binds dangerous authority to the exact effect/input and rejects observe-then-unconditional mutation races. ADR 0004 and
contracts.mdrequire capability-gated conditional mutations rather than weakening correctness for an adapter.No legitimate bounded race-free mechanism is present for this Forgejo endpoint. A pre/post check is observational, cleanup is too late, and a temporary/protected branch is still a mutable name unless a new, proven exclusive authority/locking mechanism prevents every competing writer through creation. Adding such exclusivity or selecting another API/effect model changes authority/architecture and requires owner authorization.
Disabling the live
prcapability while retaining synthetic PR opening and fail-closed reference behavior would safely avoid the unauthorized effect, but it would materially change selected scope: the Forgejo reference adapter would no longer implement the issue's in-scope authorized PR operation or satisfy the same required-operation contract as the fake. That is an owner decision to revise issue #9, not an inferred remediation.F43 (fake/reference conformance) and F44 (pre-encoding bounds) remain bounded defects if this scope continues, but neither resolves or overrides F42. This COMMENT is a correction-only architecture verdict, not full PR approval.
PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch: issue-9-forge-port -> develop
Frozen head:
cda6861c15b567de683b81cb6989826f3cdd9bd7Linked issue: #9
[K5] Forge port and Forgejo adapterSummary
The port and adapter preserve the intended inward dependency direction, opaque qualified references, conditional Git effects, exact PR-head verification, base repository/branch identity, and absence of merge/deploy capability. Exact-head Forgejo workflow run 1973 succeeded. Three blocking issues remain in snapshot authority/recovery and fake/reference conformance.
Intent / Acceptance Evidence
UNKNOWNcore/forge.py,forge_refs.py, binding and codec testsForgeConflictArchitecture / Information Flow
adapters -> ports -> coreis preserved. Forgejo payload parsing remains at the adapter boundary, and the core owns normalized references, observations, effects, and receipts. The optional live transports are injected and are not invoked autonomously.Blocking Issues
A fresh PR delivery adopts a pre-existing same-SHA snapshot instead of proving absent-to-exact creation. In
src/creatidy_kernel/adapters/forgejo.py:362-368andsrc/creatidy_kernel/adapters/fake_forge.py:389-393, an existing snapshot is accepted whenever its current SHA equals the authorized candidate, socompare_and_push(..., expected=None, ...)is skipped. The isolation guard prevents mutation while held, but it does not prove that this delivery created the ref or that the existing ref is Kernel-controlled. This contradicts issue #9 and the PR description's explicit absent-to-exact conditional-creation requirement.tests/test_forge.py:426-444currently pre-seeds the ref and expects a first delivery to adopt it, encoding the unsafe behavior. A first delivery must require absent-target CAS and treat any pre-existing ref as contention; uncertain later delivery must remain reconciliation-only and must not POST without a trusted known PR reference.Unavailable snapshot verification reads are misclassified as immutable authority conflicts.
src/creatidy_kernel/adapters/forgejo.py:375-376,402-403, and442-443compare only.revision.UNKNOWNorINACCESSIBLEobservations haverevision=None, so transient, permission-limited, or eventually consistent reads raiseForgeConflictjust like a positively observed mismatched SHA. Issue #9 explicitly requires unavailable reads, unknown effects, eventual consistency, and actual head mismatch to remain distinct. OnlyPresence.FOUNDwith a different revision establishes the authority conflict; a non-authoritative read must produce anUNKNOWNreceipt. The current read-failure test attests/test_forge.py:948-957does not enter creation's pre/post snapshot-verification or known-reference reconciliation paths.The fake/reference receipt contract diverges for malformed immediate PR evidence.
src/creatidy_kernel/adapters/fake_forge.py:412-427calls_pull()without normalizingValueError, whilesrc/creatidy_kernel/adapters/forgejo.py:391-408maps equivalent malformed producer evidence toReceipt(UNKNOWN, ...). Existing malformed-response coverage attests/test_forge.py:256-276is Forgejo-only; the shared test attests/test_forge.py:226-240corrupts stored state only after an accepted creation. This violates the explicit shared fake/reference required-operation/receipt conformance criterion and can make controller behavior differ by adapter.Non-blocking Suggestions
None.
Questions for the Author
None. The issue and owner decision specify the required authority model sufficiently.
Security / Privacy Notes
No credential disclosure, merge authority, deploy authority, ambient Git configuration use, or cross-origin redirect behavior was found in the reviewed diff. The first blocking issue is an authority/provenance gap in the snapshot namespace rather than a content-SHA mismatch.
Test / Documentation Gaps
Add shared fake/reference cases for: same-SHA pre-existing snapshot contention on first delivery; unavailable snapshot reads before POST, after POST, and during known-reference reconciliation; malformed immediate PR creation evidence. No documentation-only correction can replace these behavioral fixes.
Follow-Up Candidates
None. These are required issue #9 acceptance work, not distinct follow-ups.
Overall Recommendation
REQUEST_CHANGESThree blocking findings remain at exact head
cda6861c15b567de683b81cb6989826f3cdd9bd7. The review was read-only; PR code was not executed in the reviewer context. Exact-head Forgejo CI reports success, but the passing suite currently does not detect these authority/recovery defects.Persisted review-state limitation: Forgejo rejected formal
REQUEST_CHANGESbecause the authenticated reviewer is the PR author. This review is therefore stored asCOMMENT; the substantive recommendation remainsREQUEST_CHANGESand is not a formal approval.PR Review: [K5] Add bounded Forge port and Forgejo reference adapter (#22)
Repository: Creatidy/creatidy-kernel
Branch:
issue-9-forge-port->developFrozen head:
4f412d8022e4f1f39a1bcebf3f6afb17a89a31faLinked issue: #9
Summary
Fresh, read-only whole-PR review of the exact frozen head found no blocking defects. The provider-neutral core contract, Forgejo reference adapter, bounded transports, fake/reference conformance coverage, and documentation changes align with issue #9 and ADRs 0003/0004/0005. Exact-head CI run #1982 succeeded.
Intent / Acceptance Evidence
tests/test_forge.pyparameterizes fake and Forgejo adapters, including unavailable reads, binding, pagination, malformed evidence, receipts, and effects.Referencevalues; Forgejo parsing/validation remains at the adapter boundary.pris unsupported before writes unless a binding-matched externalSnapshotIsolationadvertises write-once enforcement.UNKNOWN; known-reference reconciliation revalidates exact head, base identity, body, title, snapshot, and repository.Architecture / Information Flow
Dependency direction remains
adapters -> ports -> core. Forgejo payloads and transport details do not leak into core, external success is normalized into domain receipts, and reads or malformed evidence cannot manufacture authority or acceptance.Blocking Issues
None.
Non-blocking Suggestions
README.md:16-18says the included HTTPS and conditional Git transports can perform authorized branch, push, and PR effects when configured, but PR capability additionally requires a separately supplied, binding-matched write-onceSnapshotIsolation; no live isolation provider ships here. Clarifying that prerequisite would prevent integrators from expecting the bundled transports alone to enablepr.Questions for the Author
None.
Security / Privacy Notes
The exact-head snapshot path fails closed without externally enforced isolation. No merge/deploy authority, secret material, private dependency, or live provider requirement was introduced.
Test / Documentation Gaps
No blocking test gap. No live Forgejo integration was run; offline shared conformance and exact-head CI are the available evidence. The README wording above is the only identified documentation gap.
Follow-Up Candidates
None. The README clarification is small and may be handled directly without a separate issue.
Overall Recommendation
APPROVE
No blocking findings for frozen HEAD
4f412d8022e4f1f39a1bcebf3f6afb17a89a31fa.Forgejo identity limitation: formal
APPROVEDsubmission was rejected because the authenticated account is treated as the PR author. This persistedCOMMENTpreserves the substantive verdict but is not a formal approval.