feat(recovery): route failed write-less stages to recovery instead of futile retry
Adds the failure-ticket + recovery-routing mechanism: a deterministic gate->capability table gates whether a stage has agency to fix its own failure, opens a FailureTicketOpenedEvent when it doesn't, and routes to a metadata role=recovery stage (per-stage budget, cap 2, not reset by TransitionExecuted) instead of retrying in place. Extends salvage decisions with a RECOVER option so the review-gate judge can also route to recovery, unifies deterministic-gate and review-gate routing through routeToRecovery(), and has ExecutionPlanCompiler synthesize a write-capable recovery stage + edge for freestyle plans. Read-only tools (file_read, list_dir) are now always available on any tool-granting stage so a recovery stage can inspect the write-less stage's failure without flooding context via shell ls -R.
This commit is contained in:
@@ -2,6 +2,7 @@ package com.correx.apps.server.bridge
|
||||
|
||||
import com.correx.apps.server.protocol.AssessedIssueDto
|
||||
import com.correx.apps.server.protocol.PauseReason
|
||||
import com.correx.apps.server.protocol.ReviewFindingDto
|
||||
import com.correx.apps.server.protocol.RiskSummaryDto
|
||||
import com.correx.apps.server.protocol.ServerMessage
|
||||
import com.correx.apps.server.protocol.toDto
|
||||
@@ -22,6 +23,7 @@ import com.correx.core.events.events.WorkflowStartedEvent
|
||||
import com.correx.core.events.events.InferenceFailedEvent
|
||||
import com.correx.core.events.events.InferenceStartedEvent
|
||||
import com.correx.core.events.events.InferenceTimeoutEvent
|
||||
import com.correx.core.events.events.ReviewFindingsRaisedEvent
|
||||
import com.correx.core.events.events.RetryAttemptedEvent
|
||||
import com.correx.core.events.events.ModelLoadedEvent
|
||||
import com.correx.core.events.events.ModelUnloadedEvent
|
||||
@@ -190,6 +192,7 @@ suspend fun domainEventToServerMessage(
|
||||
outputSummary = p.receipt.outputSummary,
|
||||
occurredAt = event.metadata.timestamp.toEpochMilliseconds(),
|
||||
diff = p.receipt.diff,
|
||||
affectedEntities = p.receipt.affectedEntities,
|
||||
sequence = seq,
|
||||
sessionSequence = sessionSequence,
|
||||
)
|
||||
@@ -222,6 +225,26 @@ suspend fun domainEventToServerMessage(
|
||||
sessionSequence = sessionSequence,
|
||||
)
|
||||
|
||||
is ReviewFindingsRaisedEvent -> ServerMessage.ReviewFindings(
|
||||
sessionId = p.sessionId,
|
||||
stageId = p.stageId,
|
||||
verdict = p.verdict.name,
|
||||
findings = p.findings.map {
|
||||
ReviewFindingDto(
|
||||
severity = it.severity.name,
|
||||
confidence = it.confidence,
|
||||
category = it.category,
|
||||
target = it.target,
|
||||
message = it.message,
|
||||
suggestedFix = it.suggestedFix,
|
||||
correctness = it.correctness,
|
||||
)
|
||||
},
|
||||
blocked = p.blocked,
|
||||
sequence = seq,
|
||||
sessionSequence = sessionSequence,
|
||||
)
|
||||
|
||||
is ApprovalRequestedEvent -> mapApprovalRequested(p, seq, sessionSequence)
|
||||
is ClarificationRequestedEvent -> ServerMessage.ClarificationRequired(
|
||||
sessionId = p.sessionId,
|
||||
|
||||
@@ -19,6 +19,17 @@ data class AssessedIssueDto(
|
||||
val severity: String,
|
||||
)
|
||||
|
||||
@Serializable
|
||||
data class ReviewFindingDto(
|
||||
val severity: String,
|
||||
val confidence: Double,
|
||||
val category: String,
|
||||
val target: String,
|
||||
val message: String,
|
||||
val suggestedFix: String? = null,
|
||||
val correctness: Boolean = false,
|
||||
)
|
||||
|
||||
@Serializable
|
||||
data class ProviderHealthDto(
|
||||
val providerId: String,
|
||||
|
||||
@@ -246,6 +246,7 @@ sealed interface ServerMessage {
|
||||
val outputSummary: String,
|
||||
val occurredAt: Long,
|
||||
val diff: String? = null,
|
||||
val affectedEntities: List<String> = emptyList(),
|
||||
override val sequence: Long,
|
||||
override val sessionSequence: Long,
|
||||
) : ServerMessage, SessionMessage
|
||||
@@ -284,6 +285,18 @@ sealed interface ServerMessage {
|
||||
override val sessionSequence: Long,
|
||||
) : ServerMessage, SessionMessage
|
||||
|
||||
@Serializable
|
||||
@SerialName("review.findings")
|
||||
data class ReviewFindings(
|
||||
val sessionId: SessionId,
|
||||
val stageId: StageId,
|
||||
val verdict: String,
|
||||
val findings: List<ReviewFindingDto>,
|
||||
val blocked: Boolean,
|
||||
override val sequence: Long,
|
||||
override val sessionSequence: Long,
|
||||
) : ServerMessage, SessionMessage
|
||||
|
||||
// -- Approval --
|
||||
|
||||
@Serializable
|
||||
|
||||
@@ -349,7 +349,10 @@ class DomainEventMapperTest {
|
||||
)
|
||||
val result = domainEventToServerMessage(event, noopStore, sessionSequence = 0L)
|
||||
assertEquals(
|
||||
ServerMessage.ToolCompleted(sessionId, "file_write", "wrote 3 lines", occurredAt, diff = null, event.sequence, 0L),
|
||||
ServerMessage.ToolCompleted(
|
||||
sessionId, "file_write", "wrote 3 lines", occurredAt,
|
||||
diff = null, affectedEntities = emptyList(), event.sequence, 0L,
|
||||
),
|
||||
result,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -211,6 +211,29 @@ type Session struct {
|
||||
Clar *Clarification // open questions awaiting answers (clarification view)
|
||||
Propose *Proposal // router workflow suggestion awaiting a pick (propose view)
|
||||
Active bool // an inference/tool is in flight (drives the spinner)
|
||||
|
||||
// LastReview is the most recent semantic reviewer (Gate 3) verdict for this
|
||||
// session, from review.findings. Nil until the first review lands.
|
||||
LastReview *ReviewResult
|
||||
}
|
||||
|
||||
// ReviewResult is the semantic reviewer's verdict + findings over a stage's
|
||||
// produced files (mirrors ReviewFindingsRaisedEvent).
|
||||
type ReviewResult struct {
|
||||
StageID string
|
||||
Verdict string // PASS | WARN | FAIL
|
||||
Findings []ReviewFinding
|
||||
Blocked bool
|
||||
}
|
||||
|
||||
type ReviewFinding struct {
|
||||
Severity string
|
||||
Confidence float64
|
||||
Category string
|
||||
Target string
|
||||
Message string
|
||||
SuggestedFix string
|
||||
Correctness bool
|
||||
}
|
||||
|
||||
// Workflow is a launchable workflow advertised by the server.
|
||||
|
||||
@@ -296,6 +296,21 @@ func matrixCases() []matrixCase {
|
||||
},
|
||||
want: []string{"ToolAssessed", "BLOCK", "PATH_ESCAPE"},
|
||||
},
|
||||
{
|
||||
name: "review.findings",
|
||||
kinds: []string{protocol.TypeReviewFindings},
|
||||
surface: surfaceEvents,
|
||||
build: func() protocol.ServerMessage {
|
||||
return msg(protocol.TypeReviewFindings, withStage("write_script"), func(m *protocol.ServerMessage) {
|
||||
m.ReviewVerdict = "FAIL"
|
||||
m.ReviewBlocked = true
|
||||
m.ReviewFindings = []protocol.ReviewFindingDto{
|
||||
{Severity: "CRITICAL", Confidence: 0.9, Category: "correctness", Target: "main.go:12", Message: "nil deref"},
|
||||
}
|
||||
})
|
||||
},
|
||||
want: []string{"ReviewFindings", "write_script", "FAIL"},
|
||||
},
|
||||
|
||||
// --- artifacts / plan (event stream rows) ---
|
||||
{
|
||||
|
||||
@@ -221,7 +221,52 @@ func (m *Model) applyServer(msg protocol.ServerMessage) {
|
||||
m.appendAction(msg.SessionID, "✎", "wrote "+path+countSuffix(add, del))
|
||||
m.appendRouter(msg.SessionID, RouterEntry{Role: "tool", Content: *msg.Diff})
|
||||
} else {
|
||||
m.appendAction(msg.SessionID, "✓", actionToolText(msg.ToolName, msg.Summary))
|
||||
label := msg.ToolName
|
||||
if len(msg.AffectedEntities) > 0 {
|
||||
label += " " + strings.Join(msg.AffectedEntities, ", ")
|
||||
}
|
||||
m.appendAction(msg.SessionID, "✓", actionToolText(label, msg.Summary))
|
||||
}
|
||||
case protocol.TypeReviewFindings:
|
||||
if s := m.session(msg.SessionID); s != nil {
|
||||
findings := make([]ReviewFinding, 0, len(msg.ReviewFindings))
|
||||
for _, f := range msg.ReviewFindings {
|
||||
fix := ""
|
||||
if f.SuggestedFix != nil {
|
||||
fix = *f.SuggestedFix
|
||||
}
|
||||
findings = append(findings, ReviewFinding{
|
||||
Severity: f.Severity,
|
||||
Confidence: f.Confidence,
|
||||
Category: f.Category,
|
||||
Target: f.Target,
|
||||
Message: f.Message,
|
||||
SuggestedFix: fix,
|
||||
Correctness: f.Correctness,
|
||||
})
|
||||
}
|
||||
s.LastReview = &ReviewResult{
|
||||
StageID: msg.StageID,
|
||||
Verdict: msg.ReviewVerdict,
|
||||
Findings: findings,
|
||||
Blocked: msg.ReviewBlocked,
|
||||
}
|
||||
detail := msg.ReviewVerdict
|
||||
if len(findings) > 0 {
|
||||
detail += " (" + plural(len(findings), "finding") + ")"
|
||||
}
|
||||
if msg.ReviewBlocked {
|
||||
detail += " · blocked"
|
||||
}
|
||||
s.LastOutput = "review: " + detail
|
||||
s.addEvent(nowMillis(), "ReviewFindings", msg.StageID+": "+detail)
|
||||
icon := "✓"
|
||||
if msg.ReviewVerdict == "FAIL" {
|
||||
icon = "✕"
|
||||
} else if msg.ReviewVerdict == "WARN" {
|
||||
icon = "▲"
|
||||
}
|
||||
m.appendAction(msg.SessionID, icon, "review "+detail)
|
||||
}
|
||||
case protocol.TypeToolAssessed:
|
||||
if s := m.session(msg.SessionID); s != nil {
|
||||
|
||||
@@ -225,7 +225,11 @@ func (m Model) handleKey(k keyMsg) (tea.Model, tea.Cmd) {
|
||||
// On the idle launcher, Tab cycles the launch target (chat → workflows → chat) in any
|
||||
// edit mode, so it works whether or not the input is focused.
|
||||
if m.displayState() == StateIdle && k.Type == keyTab {
|
||||
if k.Shift {
|
||||
m.cycleLauncherWfBack()
|
||||
} else {
|
||||
m.cycleLauncherWf()
|
||||
}
|
||||
return m, nil
|
||||
}
|
||||
if m.editMode == ModeInsert {
|
||||
@@ -1290,6 +1294,12 @@ func (m *Model) cycleLauncherWf() {
|
||||
m.launcherWf = (m.launcherWf + 1) % (len(m.workflows) + 1)
|
||||
}
|
||||
|
||||
// cycleLauncherWfBack is cycleLauncherWf in reverse, for Shift+Tab.
|
||||
func (m *Model) cycleLauncherWfBack() {
|
||||
n := len(m.workflows) + 1
|
||||
m.launcherWf = (m.launcherWf - 1 + n) % n
|
||||
}
|
||||
|
||||
// cycleRightPanel advances the in-session right panel: events → changes → off → events.
|
||||
func (m *Model) cycleRightPanel() {
|
||||
m.rightPanel = (m.rightPanel + 1) % 3
|
||||
|
||||
@@ -908,7 +908,7 @@ func shortPath(p string) string {
|
||||
|
||||
// fill left-justifies a styled line and pads with bg to width w.
|
||||
func (m Model) fill(s string, w int, bg color.Color) string {
|
||||
return " " + padTo(s, w-1, bg)
|
||||
return lipgloss.NewStyle().Background(bg).Render(" ") + padTo(s, w-1, bg)
|
||||
}
|
||||
|
||||
// justify places left and right segments on a bg-filled line of width w, truncating
|
||||
@@ -936,7 +936,8 @@ func (m Model) justify(left, right string, w int, bg color.Color) string {
|
||||
mid = 1
|
||||
}
|
||||
pad := lipgloss.NewStyle().Background(bg).Render(strings.Repeat(" ", mid))
|
||||
return " " + left + pad + right + lipgloss.NewStyle().Background(bg).Render(" ")
|
||||
edge := lipgloss.NewStyle().Background(bg).Render(" ")
|
||||
return edge + left + pad + right + edge
|
||||
}
|
||||
|
||||
// padTo pads (or truncates) a possibly-styled string to visible width w, filling
|
||||
|
||||
@@ -60,6 +60,7 @@ const (
|
||||
TypeHealthChecks = "health.checks"
|
||||
TypeFileList = "file.list"
|
||||
TypeGrantList = "grant.list"
|
||||
TypeReviewFindings = "review.findings"
|
||||
)
|
||||
|
||||
// ServerMessage is a flat decode of every server->client variant. Field names
|
||||
@@ -82,6 +83,8 @@ type ServerMessage struct {
|
||||
LastResponse string `json:"lastResponse"` // session_snapshot: last stage response (reopen)
|
||||
Content string `json:"content"`
|
||||
Diff *string `json:"diff"`
|
||||
// tool.completed: file/dir path(s) the tool read, wrote, or listed.
|
||||
AffectedEntities []string `json:"affectedEntities"`
|
||||
Tier string `json:"tier"`
|
||||
Message string `json:"message"`
|
||||
RequestID string `json:"requestId"`
|
||||
@@ -136,6 +139,11 @@ type ServerMessage struct {
|
||||
AssessedIssues []AssessedIssueDto `json:"issues"`
|
||||
Artifacts []ArtifactDto `json:"artifacts"`
|
||||
|
||||
// review.findings
|
||||
ReviewVerdict string `json:"verdict"`
|
||||
ReviewFindings []ReviewFindingDto `json:"findings"`
|
||||
ReviewBlocked bool `json:"blocked"`
|
||||
|
||||
// config.snapshot
|
||||
ConfigFields []ConfigFieldDto `json:"fields"`
|
||||
ConfigRestartRequired []string `json:"restartRequired"`
|
||||
@@ -331,6 +339,17 @@ type AssessedIssueDto struct {
|
||||
Severity string `json:"severity"`
|
||||
}
|
||||
|
||||
// ReviewFindingDto is a PR-comment-style finding from the semantic reviewer (Gate 3).
|
||||
type ReviewFindingDto struct {
|
||||
Severity string `json:"severity"`
|
||||
Confidence float64 `json:"confidence"`
|
||||
Category string `json:"category"`
|
||||
Target string `json:"target"`
|
||||
Message string `json:"message"`
|
||||
SuggestedFix *string `json:"suggestedFix"`
|
||||
Correctness bool `json:"correctness"`
|
||||
}
|
||||
|
||||
type ProviderHealthDto struct {
|
||||
ProviderID string `json:"providerId"`
|
||||
Status string `json:"status"`
|
||||
@@ -396,7 +415,7 @@ func (m ServerMessage) IsEventBearing() bool {
|
||||
TypeWorkflowProposed,
|
||||
TypeWorkspaceBound, TypeArtifactCreated,
|
||||
TypeArtifactValid, TypePlanLocked,
|
||||
TypeRouterNarration:
|
||||
TypeRouterNarration, TypeReviewFindings:
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
|
||||
@@ -145,6 +145,13 @@ object KindContractTable {
|
||||
val overrides = projectOverrides[kindId].orEmpty()
|
||||
.filter { ov -> base.none { it.id == ov.id } }
|
||||
.map { it.copy(target = path) }
|
||||
return base + overrides
|
||||
// A dotfile (basename begins with '.') may legitimately be empty: .gitkeep/.keep directory
|
||||
// placeholders, .nojekyll markers, an empty .gitignore. Asserting file_nonempty on one makes
|
||||
// the contract unsatisfiable by construction — a real .gitkeep IS empty — which live-locks the
|
||||
// producing stage rewriting the same placeholder every retry (observed: scaffold_frontend
|
||||
// looping on four src/*/.gitkeep files). file_exists still applies, so the file must exist.
|
||||
return (base + overrides).filterNot { it.id == "file_nonempty" && isDotfile(path) }
|
||||
}
|
||||
|
||||
private fun isDotfile(path: String): Boolean = path.substringAfterLast('/').startsWith(".")
|
||||
}
|
||||
|
||||
+26
@@ -61,6 +61,32 @@ class KindContractTableTest {
|
||||
assertTrue("registered_in_serialization" in ids)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `dotfiles are exempt from file_nonempty but still must exist`() {
|
||||
// Regression: a .gitkeep placeholder is empty by definition, so a file_nonempty floor made
|
||||
// its contract unsatisfiable and live-locked scaffold_frontend rewriting src/*/.gitkeep every
|
||||
// retry. Any dotfile (basename begins with '.') may legitimately be empty.
|
||||
val gitkeep = KindContractTable.assertionsFor(
|
||||
KindInference.kindFor("frontend/src/hooks/.gitkeep") ?: "",
|
||||
"frontend/src/hooks/.gitkeep",
|
||||
).map { it.id }
|
||||
assertTrue("file_exists" in gitkeep, "dotfile must still be required to exist")
|
||||
assertTrue("file_nonempty" !in gitkeep, "an empty .gitkeep must not fail file_nonempty")
|
||||
|
||||
// A non-dotfile with the same unknown kind keeps the full floor.
|
||||
val regular = KindContractTable.assertionsFor("", "frontend/src/hooks/placeholder.ts").map { it.id }
|
||||
assertTrue("file_nonempty" in regular, "non-dotfiles keep the nonempty floor")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `dotfile exemption also strips a would-be file_nonempty project override`() {
|
||||
KindContractTable.projectOverrides = mapOf(
|
||||
"" to listOf(ContractAssertion("file_nonempty", "", AssertionEvaluator.FS)),
|
||||
)
|
||||
val ids = KindContractTable.assertionsFor("", "config/.nojekyll").map { it.id }
|
||||
assertTrue("file_nonempty" !in ids, "dotfile exemption is final, even against overrides")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `project override is additive and cannot remove checked-in assertions`() {
|
||||
KindContractTable.projectOverrides = mapOf(
|
||||
|
||||
@@ -105,8 +105,16 @@ data class FailureTicketOpenedEvent(
|
||||
val routeTo: StageId,
|
||||
// The gate's captured failure output (command/exit/stderr/findings) that recovery must resolve.
|
||||
val evidence: String,
|
||||
// 1-based count of times this stage has been routed to recovery (own, small route budget).
|
||||
// Count of NO-PROGRESS routes charged against this stage's recovery budget so far. A route whose
|
||||
// failure fingerprint changed from the previous route (recovery made progress — a different error
|
||||
// now) is FREE and leaves this flat; only a route that reproduces the same failure charges it. So
|
||||
// a multi-cause failure repaired one layer at a time is not penalised for progressing. Folded
|
||||
// directly by the reducer (idempotent under replay). Defaulted for backward-compatible replay of
|
||||
// pre-progress-aware tickets.
|
||||
val routeAttempt: Int,
|
||||
// Fingerprint of [evidence] (whitespace/digit-normalised, see FailureFingerprint) at route time.
|
||||
// The next route compares against this to decide progress vs. no-progress. Empty on old events.
|
||||
val fingerprint: String = "",
|
||||
) : EventPayload
|
||||
|
||||
/**
|
||||
|
||||
@@ -31,4 +31,11 @@ data class OrchestrationState(
|
||||
// verification→recovery→verification loop must stay bounded across those transitions, exactly as
|
||||
// refinementIterations does for back-edge loops.
|
||||
val recoveryRoutes: Map<String, Int> = emptyMap(),
|
||||
// Last-seen failure fingerprint per recovering stage id — the recovery-routing analogue of
|
||||
// gateFailureFingerprints. Lets routeToRecovery tell a no-progress route (same fingerprint,
|
||||
// charged against recoveryRoutes) from a genuine-progress one (changed fingerprint, free), so a
|
||||
// recovery that fixes one cause and surfaces the next is not killed by the small route budget.
|
||||
// Like recoveryRoutes, NOT reset by TransitionExecutedEvent — it must survive the
|
||||
// verification→recovery→verification round-trip.
|
||||
val recoveryFailureFingerprints: Map<String, String> = emptyMap(),
|
||||
)
|
||||
|
||||
+4
-2
@@ -95,10 +95,12 @@ class DefaultOrchestrationReducer : OrchestrationReducer {
|
||||
refinementIterations = state.refinementIterations + (p.cycleKey to p.iteration),
|
||||
)
|
||||
|
||||
// Charge the failing stage's recovery-route budget. routeAttempt is the 1-based count the
|
||||
// orchestrator computed, so fold it directly (idempotent under replay).
|
||||
// Charge the failing stage's recovery-route budget. routeAttempt is the no-progress count the
|
||||
// orchestrator computed (progress-aware), so fold it directly (idempotent under replay), and
|
||||
// record the failure fingerprint the next route compares against.
|
||||
is FailureTicketOpenedEvent -> state.copy(
|
||||
recoveryRoutes = state.recoveryRoutes + (p.stageId.value to p.routeAttempt),
|
||||
recoveryFailureFingerprints = state.recoveryFailureFingerprints + (p.stageId.value to p.fingerprint),
|
||||
)
|
||||
|
||||
else -> state
|
||||
|
||||
+19
-4
@@ -36,6 +36,7 @@ import com.correx.core.kernel.execution.WorkflowResult
|
||||
import com.correx.core.kernel.orchestration.subagent.InSessionSubagentRunner
|
||||
import com.correx.core.kernel.orchestration.subagent.SubagentRunRequest
|
||||
import com.correx.core.kernel.orchestration.subagent.SubagentRunner
|
||||
import com.correx.core.kernel.retry.FailureFingerprint
|
||||
import com.correx.core.kernel.retry.RetryCoordinator
|
||||
import com.correx.core.kernel.retry.RetryDecision
|
||||
import com.correx.core.sessions.Session
|
||||
@@ -298,8 +299,19 @@ class DefaultSessionOrchestrator(
|
||||
state: OrchestrationState,
|
||||
): StepResult? {
|
||||
val recoveryId = findRecoveryStage(ctx.graph, stageId) ?: return null // no recovery stage: legacy path
|
||||
// Progress-aware charging (mirrors the per-gate retry budget, design §5): compare this
|
||||
// failure's fingerprint to the previous route's. A changed fingerprint means the last
|
||||
// recovery round moved the needle — fixed one cause and surfaced a different one — so the
|
||||
// route is FREE (budget unchanged) and only the recorded fingerprint advances. Same
|
||||
// fingerprint = the round changed nothing, so it charges the small route budget. Terminal
|
||||
// only once the no-progress count is spent; the recovery→origin back-edge refinement guard
|
||||
// (executeMove, cap = origin stage maxRetries) remains the ultimate loop bound for the
|
||||
// pathological all-progress case.
|
||||
val fingerprint = FailureFingerprint.of(reason)
|
||||
val prev = state.recoveryFailureFingerprints[stageId.value]
|
||||
val progressed = prev != null && prev != fingerprint
|
||||
val used = state.recoveryRoutes[stageId.value] ?: 0
|
||||
if (used >= RECOVERY_ROUTE_BUDGET) {
|
||||
if (!progressed && used >= RECOVERY_ROUTE_BUDGET) {
|
||||
return StepResult.Terminal(
|
||||
failWorkflow(
|
||||
ctx.sessionId,
|
||||
@@ -309,6 +321,7 @@ class DefaultSessionOrchestrator(
|
||||
),
|
||||
)
|
||||
}
|
||||
val routeAttempt = if (progressed) used else used + 1
|
||||
emit(
|
||||
ctx.sessionId,
|
||||
FailureTicketOpenedEvent(
|
||||
@@ -319,13 +332,15 @@ class DefaultSessionOrchestrator(
|
||||
requiredCapability = requiredCapability,
|
||||
routeTo = recoveryId,
|
||||
evidence = reason,
|
||||
routeAttempt = used + 1,
|
||||
routeAttempt = routeAttempt,
|
||||
fingerprint = fingerprint,
|
||||
),
|
||||
)
|
||||
log.info(
|
||||
"[Orchestrator] failure ticket session={} stage={} gate={} lacks={} -> route to recovery={} ({}/{})",
|
||||
"[Orchestrator] failure ticket session={} stage={} gate={} lacks={} -> recovery={} " +
|
||||
"(charged={}/{}, progressed={})",
|
||||
ctx.sessionId.value, stageId.value, gate, requiredCapability,
|
||||
recoveryId.value, used + 1, RECOVERY_ROUTE_BUDGET,
|
||||
recoveryId.value, routeAttempt, RECOVERY_ROUTE_BUDGET, progressed,
|
||||
)
|
||||
val advancedTo = advanceStage(
|
||||
ctx.sessionId,
|
||||
|
||||
+6
-1
@@ -1915,6 +1915,11 @@ abstract class SessionOrchestrator(
|
||||
is ToolResult.Success -> {
|
||||
val diff = result.metadata["diff"]?.takeIf { it.isNotBlank() }
|
||||
val affected = fileTool?.affectedPaths(request).orEmpty()
|
||||
// Read-only tools (file_read, list_dir) have no FileAffectingTool, so `affected`
|
||||
// is empty — fall back to the raw `path` argument so the receipt still names
|
||||
// the file/dir the tool looked at (surfaced in the TUI's output row).
|
||||
val affectedNames = affected.map { it.toString() }
|
||||
.ifEmpty { listOfNotNull(request.parameters["path"] as? String) }
|
||||
emit(
|
||||
sessionId,
|
||||
ToolExecutionCompletedEvent(
|
||||
@@ -1929,7 +1934,7 @@ abstract class SessionOrchestrator(
|
||||
// Carry the tool's structured metadata (e.g. file_read's contentHash) so
|
||||
// session projections can read it.
|
||||
structuredOutput = result.metadata,
|
||||
affectedEntities = affected.map { it.toString() },
|
||||
affectedEntities = affectedNames,
|
||||
durationMs = 0,
|
||||
tier = tier,
|
||||
timestamp = Clock.System.now(),
|
||||
|
||||
+11
-7
@@ -27,11 +27,15 @@ import kotlin.io.path.name
|
||||
import kotlin.io.path.readText
|
||||
|
||||
/**
|
||||
* Recursive, `.gitignore`-aware directory listing — the affordance weak local models should reach
|
||||
* for instead of shell `ls -R`, which dumps `node_modules`/`build`/`dist` and drowns a small model
|
||||
* in noise. Read-only (Tier T1, FILE_READ): it shares [FileReadTool]'s path jail and workspace
|
||||
* anchor. `.gitignore` is honoured for *enumeration only* — the mutation tools deliberately do NOT
|
||||
* respect it (you legitimately write to ignored paths like `.env`/`dist`).
|
||||
* `.gitignore`-aware directory listing — the affordance weak local models should reach for instead
|
||||
* of shell `ls -R`, which dumps `node_modules`/`build`/`dist` and drowns a small model in noise.
|
||||
* Shallow by default (`recursive=false`): a bare `list_dir .` returns the top-level entries, which
|
||||
* is what the common question ("what's at the root / does `frontend/` exist?") actually wants — a
|
||||
* recursive default buried that answer under an alphabetical 400-entry `docs/` subtree flood and
|
||||
* drove models to re-issue the same call 2-3x (observed: discovery looping on `list_dir .`). Pass
|
||||
* `recursive:true` to descend. Read-only (Tier T1, FILE_READ): it shares [FileReadTool]'s path jail
|
||||
* and workspace anchor. `.gitignore` is honoured for *enumeration only* — the mutation tools
|
||||
* deliberately do NOT respect it (you legitimately write to ignored paths like `.env`/`dist`).
|
||||
*/
|
||||
class ListDirTool(
|
||||
private val allowedPaths: Set<Path> = emptySet(),
|
||||
@@ -60,7 +64,7 @@ class ListDirTool(
|
||||
}
|
||||
putJsonObject("recursive") {
|
||||
put("type", "boolean")
|
||||
put("description", "Descend into sub-directories (gitignored subtrees are pruned). Default true.")
|
||||
put("description", "Descend into sub-directories (gitignored subtrees are pruned). Default false.")
|
||||
}
|
||||
}
|
||||
put("required", buildJsonArray {})
|
||||
@@ -90,7 +94,7 @@ class ListDirTool(
|
||||
return@withContext ToolResult.Failure(request.invocationId, it.reason, recoverable = false)
|
||||
}
|
||||
val pathString = (request.parameters["path"] as? String) ?: "."
|
||||
val recursive = request.parameters["recursive"]?.toString()?.toBoolean() ?: true
|
||||
val recursive = request.parameters["recursive"]?.toString()?.toBoolean() ?: false
|
||||
val root = resolvePath(pathString)
|
||||
when {
|
||||
!Files.exists(root) ->
|
||||
|
||||
+14
-1
@@ -36,7 +36,7 @@ class ListDirToolTest {
|
||||
Files.writeString(root.resolve("package.json"), "{}")
|
||||
|
||||
val tool = ListDirTool(allowedPaths = setOf(root), workingDir = root)
|
||||
val out = (tool.execute(request()) as ToolResult.Success).output
|
||||
val out = (tool.execute(request(recursive = true)) as ToolResult.Success).output
|
||||
|
||||
assertTrue(out.contains("src/main.ts"), out)
|
||||
assertTrue(out.contains("package.json"), out)
|
||||
@@ -45,6 +45,19 @@ class ListDirToolTest {
|
||||
assertFalse(out.contains("debug.log"), out)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `default is shallow — a bare list_dir does not descend`(): Unit = runBlocking {
|
||||
// Regression: a recursive default buried top-level answers ("does frontend/ exist?") under a
|
||||
// 400-entry alphabetical flood and drove models to re-issue the same list_dir 2-3x. A bare
|
||||
// call (no recursive param) must now list only immediate children.
|
||||
val root = Files.createTempDirectory("listdir")
|
||||
Files.createDirectories(root.resolve("src/deep")).also { Files.writeString(it.resolve("f.ts"), "f") }
|
||||
val tool = ListDirTool(allowedPaths = setOf(root), workingDir = root)
|
||||
val out = (tool.execute(request()) as ToolResult.Success).output
|
||||
assertTrue(out.contains("src/"), out)
|
||||
assertFalse(out.contains("deep"), out)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `non-recursive lists only immediate children`(): Unit = runBlocking {
|
||||
val root = Files.createTempDirectory("listdir")
|
||||
|
||||
+13
@@ -5,6 +5,7 @@ import com.correx.core.artifacts.kind.TypedArtifactSlot
|
||||
import com.correx.core.events.types.ArtifactId
|
||||
import com.correx.core.events.types.StageId
|
||||
import com.correx.core.events.types.TransitionId
|
||||
import com.correx.core.inference.GenerationConfig
|
||||
import com.correx.core.transitions.graph.BuildExpectation
|
||||
import com.correx.core.transitions.graph.StageConfig
|
||||
import com.correx.core.transitions.graph.TransitionEdge
|
||||
@@ -38,6 +39,16 @@ private const val RECOVERY_PROMPT =
|
||||
// leaves ample headroom for completion.)
|
||||
private const val DEFAULT_STAGE_TOKEN_BUDGET = 16384
|
||||
|
||||
// A compiled freestyle stage must also lift its inference completion cap off StageConfig's 2048
|
||||
// default, or the model is truncated (finishReason=length) mid-artifact — and a degenerating local
|
||||
// model burns the whole 2048 on garbage (e.g. a `<|channel>thought` repetition loop) before it can
|
||||
// stop. Mirror the static TomlWorkflowLoader path: pin the completion cap to the stage token budget.
|
||||
private val DEFAULT_STAGE_GENERATION = GenerationConfig(
|
||||
temperature = 0.7,
|
||||
topP = 1.0,
|
||||
maxTokens = DEFAULT_STAGE_TOKEN_BUDGET,
|
||||
)
|
||||
|
||||
class ExecutionPlanCompiler(
|
||||
private val registry: ArtifactKindRegistry,
|
||||
// Names of every registered tool. A stage that references a tool the runtime can't resolve
|
||||
@@ -117,6 +128,7 @@ class ExecutionPlanCompiler(
|
||||
autoBuildGate = s.id == autoGateStageId,
|
||||
semanticReview = s.semanticReview,
|
||||
tokenBudget = DEFAULT_STAGE_TOKEN_BUDGET,
|
||||
generationConfig = DEFAULT_STAGE_GENERATION,
|
||||
metadata = mapOf("promptInline" to s.prompt),
|
||||
)
|
||||
}
|
||||
@@ -134,6 +146,7 @@ class ExecutionPlanCompiler(
|
||||
StageId(RECOVERY_STAGE) to StageConfig(
|
||||
allowedTools = setOf("file_write", "shell"),
|
||||
tokenBudget = DEFAULT_STAGE_TOKEN_BUDGET,
|
||||
generationConfig = DEFAULT_STAGE_GENERATION,
|
||||
metadata = mapOf("role" to "recovery", "promptInline" to RECOVERY_PROMPT),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -126,7 +126,10 @@ class RecoveryRoutingTest {
|
||||
override fun all(): List<Tool> = listOf(tool)
|
||||
}
|
||||
|
||||
private fun buildOrchestrator(): Pair<DefaultSessionOrchestrator, InMemoryEventStore> {
|
||||
private fun buildOrchestrator(
|
||||
staticAnalysis: StaticAnalysisRunner =
|
||||
StaticAnalysisRunner { _, _ -> StaticAnalysisRunResult(exitCode = 1, output = "TS5023: bad option") },
|
||||
): Pair<DefaultSessionOrchestrator, InMemoryEventStore> {
|
||||
val eventStore = InMemoryEventStore()
|
||||
val shell = ShellTool(allowedExecutables = setOf("true"), workingDir = workspace)
|
||||
val toolRegistry = SingleToolRegistry(shell)
|
||||
@@ -141,9 +144,6 @@ class RecoveryRoutingTest {
|
||||
val inferenceRouter = object : InferenceRouter {
|
||||
override suspend fun route(stageId: StageId, requiredCapabilities: Set<ModelCapability>) = provider
|
||||
}
|
||||
val failingStaticAnalysis =
|
||||
StaticAnalysisRunner { _, _ -> StaticAnalysisRunResult(exitCode = 1, output = "TS5023: bad option") }
|
||||
|
||||
val repositories = OrchestratorRepositories(
|
||||
eventStore = eventStore,
|
||||
inferenceRepository = InferenceRepository(object : EventReplayer<InferenceState> {
|
||||
@@ -172,7 +172,7 @@ class RecoveryRoutingTest {
|
||||
toolExecutor = executor,
|
||||
toolRegistry = toolRegistry,
|
||||
workspacePolicy = WorkspacePolicy(workspace),
|
||||
staticAnalysisRunner = failingStaticAnalysis,
|
||||
staticAnalysisRunner = staticAnalysis,
|
||||
approvalEngine = object : ApprovalEngine {
|
||||
override fun evaluate(
|
||||
request: DomainApprovalRequest,
|
||||
@@ -254,4 +254,37 @@ class RecoveryRoutingTest {
|
||||
assertEquals(listOf(1, 2), tickets.map { it.routeAttempt })
|
||||
assertTrue(events.any { it.payload is WorkflowFailedEvent })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `recovery whose failure keeps changing is not charged the route budget (progress-aware)`(): Unit = runBlocking {
|
||||
// Each verify run surfaces a DIFFERENT failure (distinct non-digit words so FailureFingerprint,
|
||||
// which collapses digit runs, actually sees a changed fingerprint). This models a genuine
|
||||
// multi-cause repair: recovery fixes one cause, the gate now fails on the next. Progress must
|
||||
// NOT charge the small route budget — so the run routes to recovery MORE than
|
||||
// RECOVERY_ROUTE_BUDGET times and is instead bounded by the recovery->verify refinement guard.
|
||||
val variants = listOf("cannot resolve module alpha", "cannot resolve module beta", "cannot resolve module gamma", "cannot resolve module delta", "cannot resolve module epsilon")
|
||||
var call = 0
|
||||
val changingStaticAnalysis = StaticAnalysisRunner { _, _ ->
|
||||
StaticAnalysisRunResult(exitCode = 1, output = variants[minOf(call++, variants.lastIndex)])
|
||||
}
|
||||
val (orchestrator, eventStore) = buildOrchestrator(changingStaticAnalysis)
|
||||
val sessionId = SessionId("recovery-progress")
|
||||
val config = OrchestrationConfig(retryPolicy = RetryPolicy(maxAttempts = 10, backoffMs = 0))
|
||||
|
||||
val result = orchestrator.run(sessionId, recoveryGraph(), config)
|
||||
|
||||
assertTrue(result is WorkflowResult.Failed, "still terminates — via the refinement guard, not the route budget")
|
||||
val tickets = eventStore.read(sessionId).mapNotNull { it.payload as? FailureTicketOpenedEvent }
|
||||
assertTrue(tickets.size > RECOVERY_ROUTE_BUDGET_TEST, "progress routes are free, so more than the budget of routes happen: got ${tickets.size}")
|
||||
// Every route progressed (changed fingerprint) except the first, so the charged count never
|
||||
// climbs past its initial 1 — the old unconditional charging would have read [1, 2] then died.
|
||||
assertEquals(1, tickets.map { it.routeAttempt }.max(), "no-progress charge count must never exceed 1 when every round progresses")
|
||||
assertTrue(tickets.map { it.fingerprint }.toSet().size == tickets.size, "each route recorded a distinct fingerprint")
|
||||
}
|
||||
|
||||
private companion object {
|
||||
// Mirror of DefaultSessionOrchestrator.RECOVERY_ROUTE_BUDGET (private there); kept in sync by the
|
||||
// `[1, 2]` assertion in the exhaustion test above.
|
||||
const val RECOVERY_ROUTE_BUDGET_TEST = 2
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user