feat: integrate prepared ITSM closed loop - #40
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
9 issues found across 15 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/ofw/evolution/integration.py">
<violation number="1" location="src/ofw/evolution/integration.py:104">
P2: When a later Harbor trial contains a malformed checksum or out-of-range metric, strict outcome construction fails after earlier trials have been stored. Validate all trial-derived receipt fields before writing, and map the boundary failure to a typed `CandidateFailure` to avoid partial evaluation state.</violation>
<violation number="2" location="src/ofw/evolution/integration.py:124">
P2: The shared integration is not usable with the actual prepared baseline runner. `HarborBaselineRunner.summarize()` returns `BaselineSummary`, but this integration requires `ExperimentSummary.trials`, so baseline runs still cannot produce trace-linked outcome receipts. Wire baseline polling through the generalized runner or add a real baseline adapter that returns `ExperimentSummary`.</violation>
<violation number="3" location="src/ofw/evolution/integration.py:216">
P1: When a trial includes an explicit authoritative `abstain` or `error`, this branch can discard it because `exception` and reward are checked first. Check the explicit verdict before exception and reward so the reducer follows the ITSM authority contract.</violation>
</file>
<file name="src/ofw/mcp.py">
<violation number="1" location="src/ofw/mcp.py:112">
P2: The new instruction tells the agent the evolution controller owns publication, rollback, budgets, and stopping, but the controller tool surface can only do publication and stopping: `EvolutionAdvanceAction` has no ROLLBACK action and `advance_evolution` exposes no rollback or budget path (`RollbackRequest` is only used inside `publication.py`). An agent that defers rollback or budgeting to the controller has no tool to invoke it, so the guidance is misleading. Either trim the claim to what the controller can actually do (publication and stopping) or wire up the missing rollback/budget actions before asserting ownership.</violation>
</file>
<file name="src/ofw/preparation/templates/base.md">
<violation number="1" location="src/ofw/preparation/templates/base.md:48">
P3: Step 4's phrase "an exact current non-pass to candidate pass improvement" is ungrammatical and hard for the agent to act on. Restate the rule plainly: the accepted candidate's evidence may be reused as current evidence only when it shows an exact improvement from the current non-passing run to a candidate pass.</violation>
<violation number="2" location="src/ofw/preparation/templates/base.md:54">
P3: The package boundary asks the agent to report a "stop receipt", but the system has no such object: advance_evolution's stop emits an EVOLUTION_STOPPED event carrying a stop reason and returns an observation. Use "stop reasons" to match the controller loop in this file and itsm.md, so the agent reports what actually exists.</violation>
</file>
<file name="src/ofw/evolution/controller.py">
<violation number="1" location="src/ofw/evolution/controller.py:802">
P2: The new accepted-side authority check only validates the commit, not the content tree. `_accepted_source` returns `(content_commit, _tree_content_identity(current.content_tree))`, but here the tree is discarded (`expected_commit, _ = ...`) and only `accepted.evaluated_commit` is compared. A caller-supplied accepted receipt that carries the current accepted commit but a mismatched `evaluated_tree` passes the STALE_RECEIPT gate and flows into `decide_promotion`, so the accepted evidence is not actually bound to the authoritative content tree the PR description claims to enforce. Note the tree value returned by `_accepted_source` is a `sha256:` content identity, not a git hash, so it cannot be compared to `evaluated_tree` directly; `_accepted_source` must expose the raw `content_tree` for this check to work.</violation>
</file>
<file name="tests/test_closed_loop_acceptance.py">
<violation number="1" location="tests/test_closed_loop_acceptance.py:257">
P3: The rejection branch only asserts the gate status is REJECT; it never verifies that the accepted publication remains unchanged, even though 'rejected candidates leave the accepted publication unchanged' is a stated PR guarantee and this is the test meant to cover rejection. Add an assertion after the REJECT decision that `current_accepted(root, ...).content_commit` still equals `initial` (and that the rejected candidate worktree/commit was not published) before proceeding to the accepted candidate.</violation>
</file>
<file name="tests/test_closed_loop.py">
<violation number="1" location="tests/test_closed_loop.py:186">
P3: This test is named to verify that a baseline run is reduced by the same reducer as an accepted run, but it only constructs an ACCEPTED-side receipt via evaluate and asserts the side. It neither adapts a BaselineRun through baseline_run_for_evidence nor compares the reduced outcome_receipts/blockers with an accepted run, so the claim in the test name is untested and a reader gets false confidence. Run both paths and assert their reductions are equal.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| def _authoritative_result( | ||
| trial: ExperimentTrial, | ||
| ) -> tuple[VerifierVerdict, float | None] | EvaluatedRunBlocker: | ||
| if trial.exception: |
There was a problem hiding this comment.
P1: When a trial includes an explicit authoritative abstain or error, this branch can discard it because exception and reward are checked first. Check the explicit verdict before exception and reward so the reducer follows the ITSM authority contract.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ofw/evolution/integration.py, line 216:
<comment>When a trial includes an explicit authoritative `abstain` or `error`, this branch can discard it because `exception` and reward are checked first. Check the explicit verdict before exception and reward so the reducer follows the ITSM authority contract.</comment>
<file context>
@@ -0,0 +1,289 @@
+def _authoritative_result(
+ trial: ExperimentTrial,
+) -> tuple[VerifierVerdict, float | None] | EvaluatedRunBlocker:
+ if trial.exception:
+ return _blocker(trial, CandidateBlockerCode.UNVERIFIED.value, "agent_exception")
+ reward = _reward_result(trial)
</file context>
| ) | ||
| if match.trace_id is None: | ||
| return _trace_blocker(trial, match.blocker) | ||
| outcome = _outcome(trial, request.controls, match.trace_id, result) |
There was a problem hiding this comment.
P2: When a later Harbor trial contains a malformed checksum or out-of-range metric, strict outcome construction fails after earlier trials have been stored. Validate all trial-derived receipt fields before writing, and map the boundary failure to a typed CandidateFailure to avoid partial evaluation state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ofw/evolution/integration.py, line 104:
<comment>When a later Harbor trial contains a malformed checksum or out-of-range metric, strict outcome construction fails after earlier trials have been stored. Validate all trial-derived receipt fields before writing, and map the boundary failure to a typed `CandidateFailure` to avoid partial evaluation state.</comment>
<file context>
@@ -0,0 +1,289 @@
+ )
+ if match.trace_id is None:
+ return _trace_blocker(trial, match.blocker)
+ outcome = _outcome(trial, request.controls, match.trace_id, result)
+ try:
+ submission = self._outcome_store.store(outcome)
</file context>
| "evidence-backed hypotheses and isolated candidates. Never infer outcomes, mutate " | ||
| "traces, copy trace payloads into local storage, or broaden candidate edit authority." | ||
| "traces, copy trace payloads into local storage, or broaden candidate edit authority. " | ||
| "The evolution controller owns publication, rollback, budgets, and stopping; after " |
There was a problem hiding this comment.
P2: The new instruction tells the agent the evolution controller owns publication, rollback, budgets, and stopping, but the controller tool surface can only do publication and stopping: EvolutionAdvanceAction has no ROLLBACK action and advance_evolution exposes no rollback or budget path (RollbackRequest is only used inside publication.py). An agent that defers rollback or budgeting to the controller has no tool to invoke it, so the guidance is misleading. Either trim the claim to what the controller can actually do (publication and stopping) or wire up the missing rollback/budget actions before asserting ownership.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ofw/mcp.py, line 112:
<comment>The new instruction tells the agent the evolution controller owns publication, rollback, budgets, and stopping, but the controller tool surface can only do publication and stopping: `EvolutionAdvanceAction` has no ROLLBACK action and `advance_evolution` exposes no rollback or budget path (`RollbackRequest` is only used inside `publication.py`). An agent that defers rollback or budgeting to the controller has no tool to invoke it, so the guidance is misleading. Either trim the claim to what the controller can actually do (publication and stopping) or wire up the missing rollback/budget actions before asserting ownership.</comment>
<file context>
@@ -108,7 +108,9 @@
"evidence-backed hypotheses and isolated candidates. Never infer outcomes, mutate "
- "traces, copy trace payloads into local storage, or broaden candidate edit authority."
+ "traces, copy trace payloads into local storage, or broaden candidate edit authority. "
+ "The evolution controller owns publication, rollback, budgets, and stopping; after "
+ "rollback, require a fresh Harbor run for attribution."
),
</file context>
| raise EvolutionControllerFailure( | ||
| EvolutionControllerErrorCode.STALE_RECEIPT, candidate.receipt_id | ||
| ) | ||
| expected_commit, _ = self._accepted_source(request, policy, state) |
There was a problem hiding this comment.
P2: The new accepted-side authority check only validates the commit, not the content tree. _accepted_source returns (content_commit, _tree_content_identity(current.content_tree)), but here the tree is discarded (expected_commit, _ = ...) and only accepted.evaluated_commit is compared. A caller-supplied accepted receipt that carries the current accepted commit but a mismatched evaluated_tree passes the STALE_RECEIPT gate and flows into decide_promotion, so the accepted evidence is not actually bound to the authoritative content tree the PR description claims to enforce. Note the tree value returned by _accepted_source is a sha256: content identity, not a git hash, so it cannot be compared to evaluated_tree directly; _accepted_source must expose the raw content_tree for this check to work.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ofw/evolution/controller.py, line 802:
<comment>The new accepted-side authority check only validates the commit, not the content tree. `_accepted_source` returns `(content_commit, _tree_content_identity(current.content_tree))`, but here the tree is discarded (`expected_commit, _ = ...`) and only `accepted.evaluated_commit` is compared. A caller-supplied accepted receipt that carries the current accepted commit but a mismatched `evaluated_tree` passes the STALE_RECEIPT gate and flows into `decide_promotion`, so the accepted evidence is not actually bound to the authoritative content tree the PR description claims to enforce. Note the tree value returned by `_accepted_source` is a `sha256:` content identity, not a git hash, so it cannot be compared to `evaluated_tree` directly; `_accepted_source` must expose the raw `content_tree` for this check to work.</comment>
<file context>
@@ -788,15 +789,22 @@ def _validate_decision(
raise EvolutionControllerFailure(
EvolutionControllerErrorCode.STALE_RECEIPT, candidate.receipt_id
)
+ expected_commit, _ = self._accepted_source(request, policy, state)
+ if accepted.evaluated_commit != expected_commit:
+ raise EvolutionControllerFailure(
</file context>
| ) | ||
|
|
||
|
|
||
| class PreparedExperimentIntegration: |
There was a problem hiding this comment.
P2: The shared integration is not usable with the actual prepared baseline runner. HarborBaselineRunner.summarize() returns BaselineSummary, but this integration requires ExperimentSummary.trials, so baseline runs still cannot produce trace-linked outcome receipts. Wire baseline polling through the generalized runner or add a real baseline adapter that returns ExperimentSummary.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ofw/evolution/integration.py, line 124:
<comment>The shared integration is not usable with the actual prepared baseline runner. `HarborBaselineRunner.summarize()` returns `BaselineSummary`, but this integration requires `ExperimentSummary.trials`, so baseline runs still cannot produce trace-linked outcome receipts. Wire baseline polling through the generalized runner or add a real baseline adapter that returns `ExperimentSummary`.</comment>
<file context>
@@ -0,0 +1,289 @@
+ )
+
+
+class PreparedExperimentIntegration:
+ """Poll one prepared Harbor run and reduce its terminal evidence."""
+
</file context>
| Report the hypothesis, candidate, commit, blocker, gate, and outcome receipts. Stop before | ||
| publication: do not publish, merge, push, or install the candidate. | ||
| Report the hypothesis, candidate, commit, blocker, gate, outcome, publication, rollback, and | ||
| stop receipts. Do not merge, push, deploy, or add a second harness or agent runtime plane. |
There was a problem hiding this comment.
P3: The package boundary asks the agent to report a "stop receipt", but the system has no such object: advance_evolution's stop emits an EVOLUTION_STOPPED event carrying a stop reason and returns an observation. Use "stop reasons" to match the controller loop in this file and itsm.md, so the agent reports what actually exists.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ofw/preparation/templates/base.md, line 54:
<comment>The package boundary asks the agent to report a "stop receipt", but the system has no such object: advance_evolution's stop emits an EVOLUTION_STOPPED event carrying a stop reason and returns an observation. Use "stop reasons" to match the controller loop in this file and itsm.md, so the agent reports what actually exists.</comment>
<file context>
@@ -44,9 +44,11 @@ and ledger truth; never append events or perform generic transitions yourself.
-Report the hypothesis, candidate, commit, blocker, gate, and outcome receipts. Stop before
-publication: do not publish, merge, push, or install the candidate.
+Report the hypothesis, candidate, commit, blocker, gate, outcome, publication, rollback, and
+stop receipts. Do not merge, push, deploy, or add a second harness or agent runtime plane.
</file context>
| stop receipts. Do not merge, push, deploy, or add a second harness or agent runtime plane. | |
| stop reasons. Do not merge, push, deploy, or add a second harness or agent runtime plane. |
| evidence as current evidence only when it contains an exact current non-pass to candidate | ||
| pass improvement, and run Harbor again after rollback for fresh attribution. |
There was a problem hiding this comment.
P3: Step 4's phrase "an exact current non-pass to candidate pass improvement" is ungrammatical and hard for the agent to act on. Restate the rule plainly: the accepted candidate's evidence may be reused as current evidence only when it shows an exact improvement from the current non-passing run to a candidate pass.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/ofw/preparation/templates/base.md, line 48:
<comment>Step 4's phrase "an exact current non-pass to candidate pass improvement" is ungrammatical and hard for the agent to act on. Restate the rule plainly: the accepted candidate's evidence may be reused as current evidence only when it shows an exact improvement from the current non-passing run to a candidate pass.</comment>
<file context>
@@ -44,9 +44,11 @@ and ledger truth; never append events or perform generic transitions yourself.
4. Pass the existing `PromotionDecision` to `advance_evolution`. Accepted candidates remain
- `AwaitingPublication` until PR5; do not publish, merge, push, or install.
+ `AwaitingPublication` until the controller records publication; reuse accepted candidate
+ evidence as current evidence only when it contains an exact current non-pass to candidate
+ pass improvement, and run Harbor again after rollback for fresh attribution.
</file context>
| evidence as current evidence only when it contains an exact current non-pass to candidate | |
| pass improvement, and run Harbor again after rollback for fresh attribution. | |
| evidence as current evidence only when it shows an exact improvement from the current | |
| non-passing run to a candidate pass, and run Harbor again after rollback for fresh attribution. |
| ) | ||
| ) | ||
| assert rejected is not None | ||
| assert decide_promotion(policy, baseline, rejected).status is PromotionStatus.REJECT |
There was a problem hiding this comment.
P3: The rejection branch only asserts the gate status is REJECT; it never verifies that the accepted publication remains unchanged, even though 'rejected candidates leave the accepted publication unchanged' is a stated PR guarantee and this is the test meant to cover rejection. Add an assertion after the REJECT decision that current_accepted(root, ...).content_commit still equals initial (and that the rejected candidate worktree/commit was not published) before proceeding to the accepted candidate.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_closed_loop_acceptance.py, line 257:
<comment>The rejection branch only asserts the gate status is REJECT; it never verifies that the accepted publication remains unchanged, even though 'rejected candidates leave the accepted publication unchanged' is a stated PR guarantee and this is the test meant to cover rejection. Add an assertion after the REJECT decision that `current_accepted(root, ...).content_commit` still equals `initial` (and that the rejected candidate worktree/commit was not published) before proceeding to the accepted candidate.</comment>
<file context>
@@ -0,0 +1,329 @@
+ )
+ )
+ assert rejected is not None
+ assert decide_promotion(policy, baseline, rejected).status is PromotionStatus.REJECT
+
+ accepted_candidate = PreparedExperimentIntegration(
</file context>
| receipt = integration.poll(request) | ||
|
|
||
| assert receipt is not None | ||
| assert receipt.side is RunSide.ACCEPTED |
There was a problem hiding this comment.
P3: This test is named to verify that a baseline run is reduced by the same reducer as an accepted run, but it only constructs an ACCEPTED-side receipt via evaluate and asserts the side. It neither adapts a BaselineRun through baseline_run_for_evidence nor compares the reduced outcome_receipts/blockers with an accepted run, so the claim in the test name is untested and a reader gets false confidence. Run both paths and assert their reductions are equal.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_closed_loop.py, line 186:
<comment>This test is named to verify that a baseline run is reduced by the same reducer as an accepted run, but it only constructs an ACCEPTED-side receipt via evaluate and asserts the side. It neither adapts a BaselineRun through baseline_run_for_evidence nor compares the reduced outcome_receipts/blockers with an accepted run, so the claim in the test name is untested and a reader gets false confidence. Run both paths and assert their reductions are equal.</comment>
<file context>
@@ -0,0 +1,464 @@
+ receipt = integration.poll(request)
+
+ assert receipt is not None
+ assert receipt.side is RunSide.ACCEPTED
+ assert receipt.run_id == "run-1"
+ assert runner.calls == 1
</file context>
1 participant
Summary
This PR implements the prepared ITSM closed-loop integration on top of PR5:
Scope boundaries
Prepared ITSM only. This does not add live-agent scenarios, installation/update manifests, platform adapters, generic harness/runtime/E2B/canary planes, System Book functionality, remote deployment, or another controller.
Rejected or inconclusive candidates leave the accepted publication unchanged; rejected candidate worktrees remain auditable.
Verification
uv run ruff check src tests plugins/openflywheel/scripts/mcp_server.pyuv run mypy src tests plugins/openflywheel/scripts/mcp_server.pyuv run pytest --cov=ofw --cov-report=term-missing --cov-fail-under=90 -q— 404 passed, 2 skipped, 93.93% coverageuvx --from radon radon cc -s -a src tests plugins/openflywheel/scripts/mcp_server.py— changed integration production functions/classes ≤5PR5 base was verified at
5ca70881b3f72e1a1015a700a78c3bc37a034948before commit. No remote deployment or merge was performed.Summary by cubic
Completes the prepared ITSM closed loop: candidates previously stopped before publication, and now the controller owns publication, rollback, budgets, and stopping.
What changed
Scope and side effects
Written for commit f851f5f. Summary will update on new commits.