feat(agent-runtime): gate procedural guidance by execution lifecycle - #586
Conversation
Stacked on #585. Define the negative and running-path behavior before adding the adapter: no advice before start, during cancellation, or after terminal state; exact execution identity binding; malformed/accessor input rejection; and no request evaluation while guidance is suppressed.
Stacked on #585. Bind procedural advice to one canonical execution, suppress it before start, during cancellation, and after terminal states, and preserve the advisory-only authority boundary while the execution is running.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAgent Runtime에 ChangesProcedural guidance
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant guideProceduralExecution
participant ExecutionLifecycle
participant ProceduralSession
Caller->>guideProceduralExecution: lifecycle, session, request 전달
guideProceduralExecution->>ExecutionLifecycle: lifecycle 스냅샷 검증
guideProceduralExecution->>ProceduralSession: 세션 admission brand 검증
guideProceduralExecution->>ProceduralSession: running 상태에서 context 요청
ProceduralSession-->>guideProceduralExecution: advisory_only guidance
guideProceduralExecution-->>Caller: guidance 또는 비가용 결과
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review Please review exact head This child intentionally does not alter the existing lifecycle state machine or graph implementation. The test commit precedes the production commit, but the RED commit was not executed in the authoring environment; native exact-head CI remains authoritative. Keep Draft until all current-head deterministic/security checks and independent review pass. |
|
The review will verify lifecycle authority, suppression of graph reads after cancellation or terminal states, identity hardening against accessors and proxies, and preservation of the parent
|
|
Coordination update for the resumed procedural-graph writer: locally reproduced 16 failing cases against exact-source graph Fresh read observed parent #585 advance to |
…al sessions Preserve the concurrently published assertProceduralSession owner and its regressions, ADR-0017, and Unreleased entry. Reuse runtime-shared execution identity rather than a narrower graph-node grammar. Add hostile lookalike, proxy, control-character and length-bound tests. Observed RED on the predecessor accepted runtime IDs but rejected graph sessions for run@workflow+attempt=1 and _run:attempt#1. Integrated local compiled-source battery: 140/140 assertions pass across parent and child candidate sources; native exact-head repository CI remains mandatory. Refs #584, #585, #586 and ContextualWisdomLab/.github#2067.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review on fa4b0fd7598a1308015bbd0f4a897c1c9e19bc15: the lifecycle gate now rejects structural session lookalikes before reading their properties/callbacks, uses the parent module's runtime admission brand, preserves the Agent Runtime owner's canonical execution-ID grammar through the latest parent integration, and records its behavior under Unreleased. Exact compare from parent a99b8615... is limited to this child's lifecycle adapter, focused regression, and one changelog line. The predecessor bypass was reproduced locally and the causal repair returned ProceduralExecutionError:invalid_procedural_session while a genuine session still returned running advisory context. Keep Draft until this exact head re-earns native CI/reviewer/security evidence; parent #585 must integrate first and no predecessor check transfers.
…out evidence Keep the concurrently integrated session brand, canonical execution identities, original child tests, both Unreleased entries, and Proposed ADR-0017. Propagate unknown-procedure and context-budget abstention as unavailable instead of advertising a successful running context. Remove only identity checks made unreachable by the admitted frozen session closure; retain lifecycle/session identity equality and reject foreign sessions before any property access. New hostile session/lifecycle assertions reproduce the two availability failures against the incoming exact source. Final scoped adapted run: 140 passed, 0 failed, 0 skipped. This is not native full-repository, Cloudflare, or hosted CI evidence. Document fresh-lifecycle/replay limits and source-vs-CWL rollout responsibilities. Refs #586, #584 and ContextualWisdomLab/.github#2067.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head stacked review on fd9eb69c1fcf05a9acc5d1944be251b768038efb: ordinary two-parent convergence now consumes repaired parent #585 exact 80f96aeb1fe83370d09e45e4177eb95ebefc64b2, and exact parent→child compare remains five paths only. The lifecycle adapter requires the parent module's admitted session before session property/callback access, binds lifecycle/session execution identity, suppresses guidance before start/after cancellation/at terminal states, and preserves unknown_procedure/context_budget_exceeded as unavailable rather than successful guidance. The doctoring record explicitly says the caller must supply fresh authenticated lifecycle state and does not misrepresent this pure adapter as durable revocation authority. I found no new child-specific source finding in this pass. Keep Draft and keep the feature-branch base until #585 integrates; under live AGENTS/ruleset semantics the current absence of a central Security Scan on this stacked base is not passing evidence and not itself a routing defect. After parent integration, retarget/non-force reconverge to fresh protected main and earn all four exact-head gates. COMMENT only, not self-approval.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review on 1a29a0158a7ccb29616318f0826e2d1d186f32ea: the running-path error taxonomy is now repaired so malformed procedural-neighborhood input fails closed as invalid_procedural_request rather than being misreported as lifecycle-authority corruption. The adapter still requires a module-admitted session, preserves same-execution identity, suppresses accepted/cancellation/terminal states before request inspection, and grants advisory-only context. This is not an approval and does not supersede the required stack order or fresh exact-head checks after retarget to protected main.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review on 4730a0dcdffd818e5f17a1c8e9f2be9da2f3d373: GitHub compare against parent 8b63edbb7d88bfc7536b3f66bd030e2c8b5d74af remains exactly the five owned execution-gating paths. The lifecycle adapter requires a module-admitted procedural session, preserves exact execution identity, suppresses pre-start/cancellation/terminal requests, propagates graph abstention as unavailable, and now separates malformed running procedural requests (invalid_procedural_request) from lifecycle-authority failures. No provider/tool/Policy/Approval authority is added. This is not an approval; #585 must integrate first, then this child must be non-force reconverged to fresh protected main and earn its own exact-head gates.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review on 6bb1c7c0f4569d7026523904500bce05f2013fed found a remaining lifecycle-authority mismatch. guideProceduralExecution() structurally validates the supplied {executionId,state} record but does not prove that it is the current Agent Runtime lifecycle authority. A caller holding a genuine module-admitted procedural session can supply a fresh plain {executionId:"run-1",state:"running"} record and receive available:true; the current implementation itself documents that a replayed old running snapshot cannot be detected. That means the PR body's unconditional claims that guidance is available "only" while the execution is running and that "once cancellation is requested, no additional planning context is produced" are not enforced by this adapter against stale/forged lifecycle snapshots.
This is repair-not-close. Either bind this gate to a canonical current-lifecycle capability/admission boundary owned by Agent Runtime (without inventing foreign authority), with a regression that demonstrates stale running cannot reopen guidance after cancellation/terminal transition, or narrow the PR/ADR/product claims so this function is explicitly a pure projection that requires already-authenticated fresh lifecycle authority and cannot itself enforce cancellation freshness. Do not solve it by adding mutable sibling authority or by weakening the session brand. #585 remains the prerequisite and should still merge first.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Follow-up on the current exact 6bb1c7c0f4569d7026523904500bce05f2013fed: the lifecycle-freshness finding is repaired at the claim/authority boundary without pretending the source gained a revocation capability. The PR body now matches the implementation and doctoring record: guideProceduralExecution() is a pure projection that requires a caller-supplied fresh authenticated Agent Runtime lifecycle snapshot; it does not authenticate freshness and cannot detect replay of an old canonical running snapshot. Cancellation/terminal suppression is therefore conditional on the supplied current authority, while durable current-state/revocation enforcement remains later State/Checkpoint + Policy/Approval work under #584/ADR-0017.
No source head changed for this documentation/PR-authority repair, so no predecessor check is being promoted to cover a new code revision. I found no additional child-specific source finding in this pass. #586 remains Draft and stacked; #585 must integrate first, then the child must non-force reconverge/retarget to protected main and acquire its own unchanged-head application CI, reviewer-ci, central Security Scan and patch-validator-image evidence. COMMENT only, not self-approval.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head stacked review on 761734e6f46ae8b564ad75e4110749335eb1fe44: ordinary two-parent convergence incorporates parent #585 exact aa954e60a4ed1a940dd4eb118bc74854ac0b2491 without rewriting child history. Because the Proposed adoption record is now parent-owned at the identical blob, exact parent→child compare cleanly reduces from five paths to four: CHANGELOG.md, src/agent-runtime/procedural-execution.ts, and the two focused execution tests. The child still requires a module-admitted procedural session, preserves exact execution identity, treats the supplied lifecycle snapshot as an explicit fresh-authenticated precondition rather than self-proven revocation authority, suppresses accepted/cancellation/terminal states for that supplied authority, propagates graph abstention as unavailable, and grants advisory-only context. I found no new child-specific source finding in this convergence. Keep Draft; #585 must integrate first, then retarget/non-force reconverge to protected main and earn all four exact-head gates. COMMENT only, not self-approval.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head stacked review on 94d8f6e42f6ed76b3639854b75425a022fea56ec: the merge tree is based on current parent #585 exact c016ee1073fb0ea1d5152100cc90ff3678ed0d84 and overlays only the four child-owned blobs, so the new parent documentation regression gate is retained. Exact parent→child compare confirms no parent-behind delta and only CHANGELOG.md, procedural-execution.ts, and the two focused execution tests differ. The child authority boundary is unchanged: module-admitted session, same execution, caller-supplied fresh authenticated lifecycle snapshot, bounded advisory-only context, and no self-claimed revocation freshness. I found no new child-specific source finding in this pass. Keep Draft and parent-first stack order; after #585 integrates, retarget/non-force reconverge to fresh protected main and re-earn all four gates. COMMENT only, not self-approval.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review on 5351723f4ce3c2d41d463555986ba49e6c8f0f20 after #585 normal integration. This is an ordinary two-parent reconvergence: the previous child exact 94d8f6e... is preserved as first parent and protected main@3f5aad19e6bb9231ec6bde9724ad0bd51752bf9f as second parent, with the child tree unchanged. Fresh compare against protected main is still exactly four paths (CHANGELOG.md, src/agent-runtime/procedural-execution.ts, and the two focused execution tests) and the branch is no longer behind protected source. The reviewed adapter still requires module-admitted procedural session provenance, same canonical execution identity, and caller-supplied fresh authenticated lifecycle state; accepted/cancellation/terminal snapshots suppress request evaluation, running-state graph abstention remains unavailable, and output is advisory-only with no tool/retry/Policy/Approval/transition authority. I found no additional child-specific source finding in this pass and there are no unresolved inline review threads. This COMMENT is not self-approval. All predecessor workflow evidence is invalidated by the reconvergence; keep Draft until unchanged exact 5351723... earns application CI, reviewer-ci, required central Security Scan and patch-validator-image SUCCESS.
1 participant
Scope and current protected lineage
#585 integrated normally as protected
main@3f5aad19e6bb9231ec6bde9724ad0bd51752bf9f. This PR is now directly based onmain; current exact head is5351723f4ce3c2d41d463555986ba49e6c8f0f20.The current head is an ordinary/non-force two-parent reconvergence whose first parent is previous child
94d8f6e42f6ed76b3639854b75425a022fea56ecand second parent is protectedmain@3f5aad19e6bb9231ec6bde9724ad0bd51752bf9f. The tree is unchanged from the previous child exact. Fresh protected-main→child compare remains exactly four paths:CHANGELOG.md,src/agent-runtime/procedural-execution.ts,test/procedural-execution-integrity.test.mjs, andtest/procedural-execution.test.mjs. No force update or destructive rebase was used.This child does not modify existing execution-lifecycle transitions, procedural graph semantics, provider routing, credentials, workflows, persistence, Policy/Approval, isolation/security owner truth, or product-domain truth.
Behavior and authority boundary
running.accepted,cancellation_requested,succeeded,failed, andcancelledsuppress planning context for the supplied current snapshot and do not inspect the procedural request.runningsnapshot cannot be detected by this adapter; current-state/revocation enforcement remains later State/Checkpoint + Policy/Approval work under [Agent Runtime] Procedural graph core and guarded offline candidate evaluation #584/ADR-0017.runningisinvalid_procedural_request; unknown procedures and exceeded budgets remain unavailable.advisory_onlyand grants no tool/retry/approval/transition authority.TDD / repair lineage
The child retains focused repairs for structural-session admission, graph-abstention propagation, malformed-running-request taxonomy and caller-minted execution-error provenance. The lifecycle-freshness finding was repaired at the claim boundary rather than by inventing authority: current/fresh lifecycle state is an explicit caller precondition, not something this pure function proves.
As #585 advanced, ordinary two-parent commits preserved child history and incorporated parent repairs. After #585 integrated,
5351723f4ce3c2d41d463555986ba49e6c8f0f20reconverged the existing four-path child tree onto fresh protectedmain@3f5aad19.... Exact protected-main→child compare confirms the same four paths and no behind delta.Every source mutation or reconvergence invalidates predecessor workflow evidence.
Required verification
Keep Draft until unchanged exact
5351723f4ce3c2d41d463555986ba49e6c8f0f20has terminal application CI, reviewer-ci, required central Security Scan and patch-validator-image SUCCESS, plus current-head review with zero valid unresolved findings and a fresh live-base/ruleset read. Formal independent approval is required only if fresh live governance actually requires it; do not manufacture an approval gate, self-approve, bypass gates, force-push/destructively rebase or reuse predecessor GREEN.Summary by CodeRabbit
새로운 기능
문서