fix(verdict): review/deploy stage 가 LLM 출력을 무시하고 verdict 하드코딩하던 버그
자기야 발견: 다랑이가 코드에 문제가 있다고 답해도 파이프라인이 그대로 deploy
로 넘어가는 현상. 원인은 sister-agent 의 buildSuccessResult 가 review/deploy
stage 의 verdict 를 LLM 출력과 무관하게 "APPROVE" / "DEPLOY_DONE" 로 하드
코딩하고 있었음. FSM 의 review-loop 와 escalation 경로는 정상이었지만
sister 가 한 번도 REQUEST_CHANGES 를 emit 하지 않아 loop 가 죽어 있었음.
수정:
- parseReviewVerdict(text): LLM 출력에서 ABORT / REQUEST_CHANGES / APPROVE
키워드 탐색. 마커가 없으면 conservative 하게 REQUEST_CHANGES 로 분류해
silent approval 방지.
- parseDeployVerdict(text): DEPLOY_FAILED / DEPLOY_DONE 마지막 마커 추출.
마커 없으면 DEPLOY_FAILED 로 bias.
- buildSuccessResult: review/deploy 케이스가 위 파서 결과를 사용. issues
배열에 LLM reason 을 major severity 로 첨부 → runner 가 같은 텍스트를
다음 implement round 에 priorStages 로 전달함.
검증: pipeline 01KNW1MJWGMZHXDE7178SP8FZB
planning → implementing → reviewing
→ REQUEST_CHANGES (round 1) → implementing
→ reviewing → REQUEST_CHANGES (round 2) → implementing
→ reviewing → REQUEST_CHANGES (round 3) → implementing
→ reviewing → REQUEST_CHANGES (max exceeded) → escalated
context: reviewRound=3, lastError="Max review rounds exceeded"
This commit is contained in:
@@ -509,6 +509,85 @@ function derivePreviewUrl(
|
||||
return rawBase.replace("/raw/branch/main", "");
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse the review junior's text output for verdict.
|
||||
*
|
||||
* Prompt asks the LLM to start the response with one of:
|
||||
* APPROVE / REQUEST_CHANGES / ABORT
|
||||
*
|
||||
* We scan the entire text (not just the prefix) because the LLM sometimes
|
||||
* adds a preamble before the verdict keyword. First match wins.
|
||||
*
|
||||
* Default = APPROVE only when the text is empty (LLM failure). Otherwise
|
||||
* if no marker is found we conservatively treat it as REQUEST_CHANGES so
|
||||
* the pipeline doesn't silently approve unparsable output.
|
||||
*/
|
||||
function parseReviewVerdict(text: string): {
|
||||
verdict: "APPROVE" | "REQUEST_CHANGES" | "ABORT";
|
||||
reason: string;
|
||||
} {
|
||||
if (!text || text.trim().length === 0) {
|
||||
return { verdict: "APPROVE", reason: "review junior produced no output" };
|
||||
}
|
||||
const upper = text.toUpperCase();
|
||||
|
||||
// Order matters — REQUEST_CHANGES contains the substring "CHANGES",
|
||||
// ABORT is the strongest signal, so check ABORT first.
|
||||
const abortIdx = upper.search(/\bABORT\b/);
|
||||
const rcIdx = upper.search(/\bREQUEST[_\s-]?CHANGES?\b/);
|
||||
const approveIdx = upper.search(/\bAPPROVE\b/);
|
||||
|
||||
// If both APPROVE and REQUEST_CHANGES appear, the LLM is uncertain —
|
||||
// bias toward REQUEST_CHANGES so problems aren't silently ignored.
|
||||
if (abortIdx >= 0 && (rcIdx < 0 || abortIdx < rcIdx)) {
|
||||
return { verdict: "ABORT", reason: text.slice(0, 1000) };
|
||||
}
|
||||
if (rcIdx >= 0) {
|
||||
return { verdict: "REQUEST_CHANGES", reason: text.slice(0, 1000) };
|
||||
}
|
||||
if (approveIdx >= 0) {
|
||||
return { verdict: "APPROVE", reason: "" };
|
||||
}
|
||||
// No marker found — conservatively request changes rather than auto-approve
|
||||
return {
|
||||
verdict: "REQUEST_CHANGES",
|
||||
reason: "Reviewer did not emit an APPROVE / REQUEST_CHANGES marker. Raw text:\n" + text.slice(0, 800),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse the deploy junior's text output for verdict.
|
||||
* Prompt asks the LLM to end with "DEPLOY_DONE" or "DEPLOY_FAILED".
|
||||
*/
|
||||
function parseDeployVerdict(text: string): {
|
||||
verdict: "DEPLOY_DONE" | "DEPLOY_FAILED";
|
||||
reason: string;
|
||||
} {
|
||||
if (!text || text.trim().length === 0) {
|
||||
return {
|
||||
verdict: "DEPLOY_FAILED",
|
||||
reason: "deploy junior produced no output",
|
||||
};
|
||||
}
|
||||
const upper = text.toUpperCase();
|
||||
const failedIdx = upper.lastIndexOf("DEPLOY_FAILED");
|
||||
const doneIdx = upper.lastIndexOf("DEPLOY_DONE");
|
||||
// Take the LAST marker (the prompt asks for it on the final line)
|
||||
if (failedIdx > doneIdx) {
|
||||
return { verdict: "DEPLOY_FAILED", reason: text.slice(0, 1000) };
|
||||
}
|
||||
if (doneIdx >= 0) {
|
||||
return { verdict: "DEPLOY_DONE", reason: "" };
|
||||
}
|
||||
// No marker — bias toward FAILED so silent passes don't happen
|
||||
return {
|
||||
verdict: "DEPLOY_FAILED",
|
||||
reason:
|
||||
"Deployer did not emit a DEPLOY_DONE / DEPLOY_FAILED marker. Raw text:\n" +
|
||||
text.slice(0, 800),
|
||||
};
|
||||
}
|
||||
|
||||
function buildSuccessResult(
|
||||
stage: InvokeRequest["stage"],
|
||||
task: InvokeRequest["task"],
|
||||
@@ -555,28 +634,74 @@ function buildSuccessResult(
|
||||
},
|
||||
errorReason: "",
|
||||
};
|
||||
case "review":
|
||||
case "review": {
|
||||
const parsed = parseReviewVerdict(summary);
|
||||
if (parsed.verdict === "APPROVE") {
|
||||
return {
|
||||
stage: "review",
|
||||
verdict: "APPROVE",
|
||||
payload: {
|
||||
artifactPath: "",
|
||||
checklistResults: [],
|
||||
issues: [],
|
||||
},
|
||||
abortReason: "",
|
||||
};
|
||||
}
|
||||
if (parsed.verdict === "REQUEST_CHANGES") {
|
||||
return {
|
||||
stage: "review",
|
||||
verdict: "REQUEST_CHANGES",
|
||||
payload: {
|
||||
artifactPath: "",
|
||||
checklistResults: [],
|
||||
issues: [
|
||||
{
|
||||
severity: "major",
|
||||
message: parsed.reason,
|
||||
},
|
||||
],
|
||||
},
|
||||
abortReason: "",
|
||||
};
|
||||
}
|
||||
// ABORT
|
||||
return {
|
||||
stage: "review",
|
||||
verdict: "APPROVE",
|
||||
verdict: "ABORT",
|
||||
payload: {
|
||||
artifactPath: "",
|
||||
checklistResults: [],
|
||||
issues: [],
|
||||
},
|
||||
abortReason: "",
|
||||
abortReason: parsed.reason,
|
||||
};
|
||||
case "deploy":
|
||||
}
|
||||
case "deploy": {
|
||||
const parsed = parseDeployVerdict(summary);
|
||||
if (parsed.verdict === "DEPLOY_DONE") {
|
||||
return {
|
||||
stage: "deploy",
|
||||
verdict: "DEPLOY_DONE",
|
||||
payload: {
|
||||
deployArtifactPath: "",
|
||||
projectType: "llm",
|
||||
verificationResults: { summary },
|
||||
},
|
||||
errorReason: "",
|
||||
};
|
||||
}
|
||||
return {
|
||||
stage: "deploy",
|
||||
verdict: "DEPLOY_DONE",
|
||||
verdict: "DEPLOY_FAILED",
|
||||
payload: {
|
||||
deployArtifactPath: "",
|
||||
projectType: "llm",
|
||||
verificationResults: { summary },
|
||||
verificationResults: { summary, reason: parsed.reason },
|
||||
},
|
||||
errorReason: "",
|
||||
errorReason: parsed.reason,
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user