diff --git a/packages/gittensory-engine/src/index.ts b/packages/gittensory-engine/src/index.ts index 880938724b..f6800de26f 100644 --- a/packages/gittensory-engine/src/index.ts +++ b/packages/gittensory-engine/src/index.ts @@ -217,6 +217,17 @@ export { type SelfReviewSlopInput, type SelfReviewVerdict, } from "./miner/self-review-adapter.js"; +export { + decideNextAction, + decideNextActionWithReason, + deriveSelfReviewOutcome, + type AbandonReason, + type HandoffPacket, + type IterateLoopAction, + type IterateLoopDecision, + type IterationState, + type SelfReviewOutcome, +} from "./miner/iterate-policy.js"; export { codingAgentModeExecutes, isGlobalMinerCodingAgentPause, diff --git a/packages/gittensory-engine/src/miner/iterate-policy.ts b/packages/gittensory-engine/src/miner/iterate-policy.ts new file mode 100644 index 0000000000..d3dab777b2 --- /dev/null +++ b/packages/gittensory-engine/src/miner/iterate-policy.ts @@ -0,0 +1,159 @@ +// Iterate-loop stop/abandon/handoff policy (#2335): the explicit POLICY the orchestration loop's control flow +// (#2333, sibling issue) consults each iteration to decide among exactly three outcomes. Deliberately split +// from the loop MECHANICS (#2333) so the actual thresholds/rules are one small, individually-reviewable, pure +// artifact -- `decideNextAction` needs no driver, no worktree, no IO to test. +// +// STRATEGIC CONSTRAINTS this policy encodes (gittensory-miner-autonomy-roadmap): +// - "never auto-submit (P4) before governor+caps (P5)" -- a mandatory clean predicted-gate PASS is the ONLY +// path to `"handoff"`; an ambiguous or errored self-review downgrades to abandon, never optimistically +// hands off. +// - "disengage SILENTLY on rejection" (the Matplotlib "MJ Rathbun" cautionary tale) -- a rejection signal +// wins over EVERYTHING else, including a self-review that would otherwise pass. Continuing to submit to a +// repo that has already shown it does not want automated contributions is the exact anti-pattern this +// guards against, regardless of how good any individual attempt looks. +// - "reward MERGED net-positive (never submission volume)" -- the no-progress detector and iteration ceiling +// exist so a stuck loop stops wasting turns chasing a submission that was never going to land, rather than +// grinding toward *a* submission for its own sake. +// +// AUTONOMY DIAL (not yet wired): `src/settings/autonomy.ts`'s `resolveAutonomy`/`isActingAutonomyLevel` is the +// existing reusable deny-by-default pattern this policy's eventual live autonomy-level check should consult +// once `.gittensory-miner.yml` defines autonomy fields -- that wiring is explicitly left to a later phase; this +// module's `decideNextAction` is autonomy-level-agnostic today. + +import type { SelfReviewVerdict } from "./self-review-adapter.js"; + +/** The three outcomes `decideNextAction` may reach. */ +export type IterateLoopAction = "continue" | "handoff" | "abandon"; + +/** Every distinct reason `decideNextAction` can abandon for -- kept as a closed literal union so a caller + * recording the decision (the attempt-log primitive, per #2333) has a stable, exhaustive vocabulary. */ +export type AbandonReason = "rejection_signaled" | "self_review_ambiguous" | "max_iterations_reached" | "no_progress"; + +/** + * The self-review outcome as the policy needs it -- narrower than the full {@link SelfReviewVerdict} (self- + * review-adapter.ts, #2334) so `IterationState` stays a minimal, cheap-to-construct synthetic fixture in + * tests. `"ambiguous"` is NOT something {@link deriveSelfReviewOutcome} ever produces from a successfully + * returned verdict (a real verdict object always has a definite conclusion) -- it is constructed directly by + * the caller's own error handling when the self-review call itself throws (e.g. a calculator inside it errors), + * per #2333's own "self-review itself errors" abandon trigger. + */ +export type SelfReviewOutcome = + | { readonly kind: "pass" } + | { readonly kind: "fail"; readonly blockerCodes: readonly string[] } + | { readonly kind: "ambiguous"; readonly reason?: string | undefined }; + +/** Derive the policy-relevant {@link SelfReviewOutcome} from a real, successfully computed + * {@link SelfReviewVerdict}. Only ever returns `"pass"` or `"fail"` -- see that variant's own doc comment for + * why `"ambiguous"` is constructed elsewhere. */ +export function deriveSelfReviewOutcome(verdict: SelfReviewVerdict): SelfReviewOutcome { + if (verdict.passesPredictedGate) return { kind: "pass" }; + return { kind: "fail", blockerCodes: verdict.predictedGateVerdict.blockers.map((blocker) => blocker.code) }; +} + +/** + * Everything `decideNextAction` needs for one iteration's decision. Deliberately minimal and synthetic- + * fixture-friendly -- no driver, no worktree, no IO. + */ +export type IterationState = { + /** 1-indexed count of iterations attempted so far, INCLUDING this one. */ + iterationNumber: number; + /** Hard ceiling enforced INSIDE this policy (#2333's own deliverable: not left to an external caller to + * remember to enforce). `iterationNumber >= maxIterations` abandons regardless of self-review outcome. */ + maxIterations: number; + selfReview: SelfReviewOutcome; + /** The prior iteration's `fail` blocker codes, for the no-progress detector -- `null` when there is no prior + * iteration to compare (the first iteration, or the prior iteration did not reach a `fail` outcome). */ + previousBlockerCodes: readonly string[] | null; + /** True when the target repo (or this contributor's history with it) has signaled it does not want + * automated/AI-authored contributions -- an explicit AI-usage-policy ban, or a prior submission from this + * same miner was closed/rejected on this exact repo. The caller resolves this (e.g. via the AI-policy-map + * signals or the rejection-state-machine primitive already shipped in `packages/gittensory-miner/lib/`) and + * passes it in; this policy does not compute it itself. */ + rejectionSignaled: boolean; +}; + +/** Forward-looking INTERFACE for Phase 4 (submission), not an implementation of it -- Phase 4 lands as a later, + * separate issue. Gives it a stable target instead of reverse-engineering the shape from the loop's internals. */ +export type HandoffPacket = { + /** Absolute path to (or a branch ref identifying) the worktree holding the passing attempt's changes. */ + worktreePath: string; + branchRef?: string | undefined; + /** Human-readable summary of the final diff, for the submission's own PR description. */ + diffSummary: string; + /** The PASSING self-review verdict that authorized this handoff -- always has `passesPredictedGate: true` + * (constructing a packet from anything else is a caller bug, not something this type can prevent statically, + * since `decideNextAction` is the actual enforcement point). */ + selfReviewVerdict: SelfReviewVerdict; + /** Reference into the attempt-log primitive (`packages/gittensory-engine/src/miner/attempt-log.ts`) for this + * attempt's full decision trail. */ + attemptLogReference: string; +}; + +export type IterateLoopDecision = { + action: IterateLoopAction; + /** Machine-stable, human-readable reason -- always populated, including for `"continue"` and `"handoff"`, so + * every decision (not just abandons) has an auditable reason string for the attempt-log. */ + reason: string; + /** Populated only when `action === "abandon"`. */ + abandonReason?: AbandonReason | undefined; +}; + +function blockerSetsEqual(current: readonly string[], previous: readonly string[]): boolean { + if (current.length !== previous.length) return false; + const currentSet = new Set(current); + const previousSet = new Set(previous); + if (currentSet.size !== previousSet.size) return false; + for (const code of currentSet) if (!previousSet.has(code)) return false; + return true; +} + +/** + * Decide the next action for one iteration. Pure; identical inputs always yield the identical decision. + * + * Precedence (each check short-circuits the ones below it): + * 1. `rejectionSignaled` -- ALWAYS abandons, even over an otherwise-passing self-review (disengage silently). + * 2. `selfReview.kind === "ambiguous"` -- abandons; never optimistically continues or hands off on ambiguity. + * 3. `selfReview.kind === "pass"` -- the ONLY path to `"handoff"`. + * 4. `iterationNumber >= maxIterations` -- abandons at the hard ceiling regardless of whether the blocker set + * was still changing (genuine incremental progress does not buy unlimited iterations). + * 5. The current `fail` blocker set is identical to `previousBlockerCodes` -- abandons (no progress, stop + * wasting turns). + * 6. Otherwise -- continue. + */ +export function decideNextActionWithReason(state: IterationState): IterateLoopDecision { + if (state.rejectionSignaled) { + return { action: "abandon", abandonReason: "rejection_signaled", reason: "Repo or contributor has signaled it does not want automated contributions; disengaging silently rather than retry-hammering." }; + } + if (state.selfReview.kind === "ambiguous") { + return { + action: "abandon", + abandonReason: "self_review_ambiguous", + reason: `Self-review could not conclusively determine pass/fail${state.selfReview.reason ? `: ${state.selfReview.reason}` : "."} Downgrading to abandon rather than optimistically handing off.`, + }; + } + if (state.selfReview.kind === "pass") { + return { action: "handoff", reason: "Self-review reached a clean predicted-gate pass." }; + } + if (state.iterationNumber >= state.maxIterations) { + return { + action: "abandon", + abandonReason: "max_iterations_reached", + reason: `Reached the iteration ceiling (${state.maxIterations}) without a clean predicted-gate pass.`, + }; + } + if (state.previousBlockerCodes !== null && blockerSetsEqual(state.selfReview.blockerCodes, state.previousBlockerCodes)) { + return { + action: "abandon", + abandonReason: "no_progress", + reason: `Blocker set unchanged from the prior iteration (${state.selfReview.blockerCodes.join(", ") || "no blockers listed"}); stopping rather than repeating an attempt that is not converging.`, + }; + } + return { action: "continue", reason: "Self-review still failing but the blocker set changed since the prior iteration; continuing." }; +} + +/** The bare `decideNextAction(state) -> "continue" | "handoff" | "abandon"` signature this issue's deliverable + * calls for. For the WHY behind a decision (the attempt-log needs a reason string, not just the action), use + * {@link decideNextActionWithReason} -- this is a thin projection over the same logic. */ +export function decideNextAction(state: IterationState): IterateLoopAction { + return decideNextActionWithReason(state).action; +} diff --git a/packages/gittensory-engine/test/iterate-policy.test.ts b/packages/gittensory-engine/test/iterate-policy.test.ts new file mode 100644 index 0000000000..8b79eab7a7 --- /dev/null +++ b/packages/gittensory-engine/test/iterate-policy.test.ts @@ -0,0 +1,168 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; + +import { + decideNextAction, + decideNextActionWithReason, + deriveSelfReviewOutcome, + type IterationState, + type SelfReviewVerdict, +} from "../dist/index.js"; + +function baseState(overrides: Partial = {}): IterationState { + return { + iterationNumber: 1, + maxIterations: 5, + selfReview: { kind: "fail", blockerCodes: ["missing_linked_issue"] }, + previousBlockerCodes: null, + rejectionSignaled: false, + ...overrides, + }; +} + +test("barrel: the public entrypoint re-exports the iterate-loop policy (#2335)", () => { + assert.equal(typeof decideNextAction, "function"); + assert.equal(typeof decideNextActionWithReason, "function"); + assert.equal(typeof deriveSelfReviewOutcome, "function"); +}); + +test("continue: a still-failing self-review whose blocker set changed since the prior iteration", () => { + const state = baseState({ + selfReview: { kind: "fail", blockerCodes: ["missing_test_evidence"] }, + previousBlockerCodes: ["missing_linked_issue"], + }); + assert.equal(decideNextAction(state), "continue"); + const decision = decideNextActionWithReason(state); + assert.equal(decision.action, "continue"); + assert.ok(decision.reason.length > 0, "even a continue decision must carry a populated reason string"); + assert.equal(decision.abandonReason, undefined); +}); + +test("continue: a failing first iteration with no prior blocker set to compare against", () => { + const state = baseState({ previousBlockerCodes: null }); + assert.equal(decideNextAction(state), "continue"); +}); + +test("handoff: the ONLY path is a clean self-review pass", () => { + const state = baseState({ selfReview: { kind: "pass" } }); + const decision = decideNextActionWithReason(state); + assert.equal(decision.action, "handoff"); + assert.ok(decision.reason.length > 0); +}); + +test("abandon (rejection_signaled): wins over EVERYTHING else, including an otherwise-passing self-review", () => { + const state = baseState({ selfReview: { kind: "pass" }, rejectionSignaled: true }); + const decision = decideNextActionWithReason(state); + assert.equal(decision.action, "abandon"); + assert.equal(decision.abandonReason, "rejection_signaled"); +}); + +test("abandon (self_review_ambiguous): never optimistically continues or hands off on ambiguity", () => { + const state = baseState({ selfReview: { kind: "ambiguous", reason: "predicted-gate calculator threw" } }); + const decision = decideNextActionWithReason(state); + assert.equal(decision.action, "abandon"); + assert.equal(decision.abandonReason, "self_review_ambiguous"); + assert.match(decision.reason, /predicted-gate calculator threw/); +}); + +test("abandon (self_review_ambiguous): the reason is optional and the decision still abandons without it", () => { + const decision = decideNextActionWithReason(baseState({ selfReview: { kind: "ambiguous" } })); + assert.equal(decision.action, "abandon"); + assert.equal(decision.abandonReason, "self_review_ambiguous"); +}); + +test("abandon (max_iterations_reached): the hard ceiling stops the loop even if the blocker set is still changing", () => { + const state = baseState({ + iterationNumber: 5, + maxIterations: 5, + selfReview: { kind: "fail", blockerCodes: ["a_brand_new_blocker_never_seen_before"] }, + previousBlockerCodes: ["missing_linked_issue"], + }); + const decision = decideNextActionWithReason(state); + assert.equal(decision.action, "abandon"); + assert.equal(decision.abandonReason, "max_iterations_reached"); +}); + +test("abandon (max_iterations_reached): fires at or beyond the ceiling, not only exactly at it", () => { + assert.equal(decideNextAction(baseState({ iterationNumber: 6, maxIterations: 5 })), "abandon"); +}); + +test("continue: one iteration below the ceiling still continues", () => { + const state = baseState({ iterationNumber: 4, maxIterations: 5, previousBlockerCodes: ["something_else"] }); + assert.equal(decideNextAction(state), "continue"); +}); + +test("abandon (no_progress): an identical blocker set to the prior iteration stops wasting turns", () => { + const state = baseState({ + iterationNumber: 2, + maxIterations: 10, + selfReview: { kind: "fail", blockerCodes: ["missing_linked_issue", "missing_test_evidence"] }, + previousBlockerCodes: ["missing_linked_issue", "missing_test_evidence"], + }); + const decision = decideNextActionWithReason(state); + assert.equal(decision.action, "abandon"); + assert.equal(decision.abandonReason, "no_progress"); +}); + +test("abandon (no_progress): the comparison is a SET, not an ordered array -- reordered-but-identical blockers still count as no progress", () => { + const state = baseState({ + iterationNumber: 2, + maxIterations: 10, + selfReview: { kind: "fail", blockerCodes: ["b_code", "a_code"] }, + previousBlockerCodes: ["a_code", "b_code"], + }); + assert.equal(decideNextAction(state), "abandon"); +}); + +test("continue: a duplicate blocker code does not falsely widen the set and mask real progress", () => { + // ["a","a"] and ["a","b"] must NOT compare equal as sets even though naive array-length comparison alone + // could be fooled by the duplicate. + const state = baseState({ + iterationNumber: 2, + maxIterations: 10, + selfReview: { kind: "fail", blockerCodes: ["a_code", "a_code"] }, + previousBlockerCodes: ["a_code", "b_code"], + }); + assert.equal(decideNextAction(state), "continue"); +}); + +test("continue: a different-length blocker set is never confused with no-progress (the fast-path length check)", () => { + const state = baseState({ + iterationNumber: 2, + maxIterations: 10, + selfReview: { kind: "fail", blockerCodes: ["a_code", "b_code", "c_code"] }, + previousBlockerCodes: ["a_code"], + }); + assert.equal(decideNextAction(state), "continue"); +}); + +test("abandon (no_progress): an empty blocker set unchanged from the prior (also empty) iteration still reads as no progress", () => { + const state = baseState({ + iterationNumber: 2, + maxIterations: 10, + selfReview: { kind: "fail", blockerCodes: [] }, + previousBlockerCodes: [], + }); + const decision = decideNextActionWithReason(state); + assert.equal(decision.action, "abandon"); + assert.equal(decision.abandonReason, "no_progress"); + assert.match(decision.reason, /no blockers listed/); +}); + +test("deriveSelfReviewOutcome: a passing verdict maps to pass with no blocker codes", () => { + const verdict = { passesPredictedGate: true, predictedGateVerdict: { blockers: [] } } as unknown as SelfReviewVerdict; + assert.deepEqual(deriveSelfReviewOutcome(verdict), { kind: "pass" }); +}); + +test("deriveSelfReviewOutcome: a failing verdict maps to fail with the real blocker codes extracted", () => { + const verdict = { + passesPredictedGate: false, + predictedGateVerdict: { + blockers: [ + { code: "duplicate_pr_risk", title: "t1", detail: "d1" }, + { code: "missing_linked_issue", title: "t2", detail: "d2" }, + ], + }, + } as unknown as SelfReviewVerdict; + assert.deepEqual(deriveSelfReviewOutcome(verdict), { kind: "fail", blockerCodes: ["duplicate_pr_risk", "missing_linked_issue"] }); +});