feat(harness): complete protocol repair stop and archive ISS-016
Add repairable INVALID_PROGRESS_PROTOCOL observations, independent PROGRESS_PROTOCOL_VIOLATED saturation, and controlled release paths. Archive the OpenSpec change after syncing main specs and devflow.
This commit is contained in:
@@ -35,9 +35,13 @@ class HarnessChatConfigurationTest {
|
||||
void diagnosisNoGainThresholdDefaultsAndValidates() {
|
||||
ChatHarnessProperties properties = new ChatHarnessProperties();
|
||||
assertEquals(2, properties.getStopAfterConsecutiveNoGain());
|
||||
assertEquals(2, properties.getStopAfterConsecutiveProgressProtocolViolations());
|
||||
|
||||
properties.setStopAfterConsecutiveNoGain(0);
|
||||
assertThrows(IllegalArgumentException.class, properties::validate);
|
||||
properties.setStopAfterConsecutiveNoGain(2);
|
||||
properties.setStopAfterConsecutiveProgressProtocolViolations(0);
|
||||
assertThrows(IllegalArgumentException.class, properties::validate);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -149,6 +149,41 @@ class DiagnosisAgentUseCaseTest {
|
||||
assertEquals(2, context.budget().snapshot().toolCalls());
|
||||
}
|
||||
|
||||
@Test
|
||||
void consecutiveProgressProtocolViolationsStopWithoutConsumingToolBudget() {
|
||||
DiagnosisHarnessCore core = core(new RunBudgetLimits(8, 8, 8,
|
||||
100, 100, 200, 100_000));
|
||||
RunContext context = core.startRun("session-protocol-stop", "run-protocol-stop");
|
||||
AtomicInteger toolCalls = new AtomicInteger();
|
||||
HarnessEvidenceTools tools = tools((runContext, id, arguments) -> {
|
||||
core.beforeToolCall(runContext, AgentToolContracts.LOOKUP_KNOWLEDGE);
|
||||
toolCalls.incrementAndGet();
|
||||
return ToolBoundaryResult.ready(id, evidence(id), EvidenceStatus.EVIDENCE_FOUND);
|
||||
});
|
||||
// First Tool succeeds and becomes pending evaluation. The next two Tool Calls omit
|
||||
// previous_observation, hit the protocol-violation threshold, and force a controlled stop
|
||||
// when the model still ignores STOP_REQUIRED.
|
||||
ScriptedChatModel model = new ScriptedChatModel(1, 1,
|
||||
toolCall("call-evidence-1", "{\"query\":\"first\"}"),
|
||||
toolCall("call-missing-eval-1", "{\"query\":\"second\"}"),
|
||||
toolCall("call-missing-eval-2", "{\"query\":\"third\"}"),
|
||||
toolCall("call-ignored-protocol-stop", "{\"query\":\"fourth\"}"));
|
||||
|
||||
DiagnosisAgentExecution execution = useCase(core, model, tools, LARGE_LIMITS)
|
||||
.execute(context, new DiagnosisAgentInput("诊断未知故障", null));
|
||||
|
||||
assertNull(execution.draft());
|
||||
assertEquals(DiagnosisStopReason.PROGRESS_PROTOCOL_VIOLATED, execution.stopReason());
|
||||
// Default empty projector is used here; publishable facts are covered by Release tests.
|
||||
// Tracker still retains the completed Tool identity that a real projector would read.
|
||||
assertEquals(1, context.progress().snapshot().completedToolCalls().size());
|
||||
assertEquals("call-evidence-1",
|
||||
context.progress().snapshot().completedToolCalls().get(0).toolCallId());
|
||||
assertEquals(4, model.calls());
|
||||
assertEquals(1, toolCalls.get());
|
||||
assertEquals(1, context.budget().snapshot().toolCalls());
|
||||
}
|
||||
|
||||
@Test
|
||||
void fencedOutputFailsClosedWithoutAgentRetry() {
|
||||
DiagnosisHarnessCore core = core(defaultBudget());
|
||||
|
||||
@@ -10,6 +10,7 @@ import com.superbiz.agent.harness.contract.EvidenceStatus;
|
||||
import com.superbiz.agent.harness.core.DiagnosisHarnessCore;
|
||||
import com.superbiz.agent.harness.core.RunBudgetLimits;
|
||||
import com.superbiz.agent.harness.core.RunContext;
|
||||
import com.superbiz.agent.harness.progress.DiagnosisStopReason;
|
||||
import com.superbiz.agent.harness.retry.HarnessRetryPolicies;
|
||||
import com.superbiz.agent.harness.tool.adapter.MysqlToolAdapter;
|
||||
import com.superbiz.agent.harness.tool.adapter.QueryLogsToolAdapter;
|
||||
@@ -182,8 +183,16 @@ class HarnessToolInterceptorTest {
|
||||
"""), ignored -> null);
|
||||
|
||||
assertTrue(missing.isError());
|
||||
assertEquals("INVALID_PROGRESS_PROTOCOL",
|
||||
objectMapper.readTree(missing.getResult()).path("error_code").asText());
|
||||
JsonNode missingObservation = objectMapper.readTree(missing.getResult());
|
||||
assertEquals("INVALID_PROGRESS_PROTOCOL", missingObservation.path("error_code").asText());
|
||||
assertTrue(missingObservation.path("repair_required").asBoolean());
|
||||
assertEquals("MISSING_PREVIOUS_OBSERVATION",
|
||||
missingObservation.path("violation_type").asText());
|
||||
assertEquals("previous_observation", missingObservation.path("missing_field").asText());
|
||||
assertEquals("call-1", missingObservation.path("expected_previous_tool_call_id").asText());
|
||||
assertEquals(List.of("GAINED", "NO_GAIN"), objectMapper.convertValue(
|
||||
missingObservation.path("allowed_information_gain"), List.class));
|
||||
assertFalse(missing.getResult().contains("two"));
|
||||
assertFalse(accepted.isError());
|
||||
assertEquals(2, invocations.get());
|
||||
DiagnosisTraceAuditEvent rejected = trace.stream()
|
||||
@@ -191,9 +200,124 @@ class HarnessToolInterceptorTest {
|
||||
.findFirst().orElseThrow();
|
||||
assertEquals("call-2", rejected.details().get("tool_call_id"));
|
||||
assertEquals("INVALID_PROGRESS_PROTOCOL", rejected.details().get("error_code"));
|
||||
assertEquals("MISSING_PREVIOUS_OBSERVATION", rejected.details().get("violation_type"));
|
||||
assertEquals(true, rejected.details().get("repair_prompt_delivered"));
|
||||
assertEquals(1, rejected.details().get("consecutive_protocol_violations"));
|
||||
String details = rejected.details().toString();
|
||||
assertFalse(details.contains("two"));
|
||||
assertFalse(details.contains("previous_observation"));
|
||||
assertFalse(details.contains("secret"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void outOfOrderPreviousObservationIsRepairableWithoutExecutingTool() throws Exception {
|
||||
AtomicInteger invocations = new AtomicInteger();
|
||||
HarnessEvidenceTools tools = fakeTools((context, id, arguments) -> {
|
||||
invocations.incrementAndGet();
|
||||
return ready(id);
|
||||
});
|
||||
HarnessToolInterceptor interceptor = new HarnessToolInterceptor(
|
||||
context(3, 3, 3), tools, objectMapper);
|
||||
|
||||
interceptor.interceptToolCall(
|
||||
request(AgentToolContracts.LOOKUP_KNOWLEDGE, "call-1",
|
||||
"{\"input\":{\"query\":\"one\"}}"), ignored -> null);
|
||||
ToolCallResponse response = interceptor.interceptToolCall(
|
||||
request(AgentToolContracts.LOOKUP_KNOWLEDGE, "call-2", """
|
||||
{"previous_observation":{"tool_call_id":"call-other","information_gain":"GAINED"},
|
||||
"input":{"query":"two-secret"}}
|
||||
"""), ignored -> null);
|
||||
|
||||
JsonNode observation = objectMapper.readTree(response.getResult());
|
||||
assertTrue(response.isError());
|
||||
assertEquals(1, invocations.get());
|
||||
assertEquals("OUT_OF_ORDER_PREVIOUS_OBSERVATION",
|
||||
observation.path("violation_type").asText());
|
||||
assertEquals("call-1", observation.path("expected_previous_tool_call_id").asText());
|
||||
assertFalse(response.getResult().contains("two-secret"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void consecutiveProtocolViolationsDeliverStopRequiredOnce() throws Exception {
|
||||
AtomicInteger invocations = new AtomicInteger();
|
||||
List<DiagnosisTraceAuditEvent> trace = new ArrayList<>();
|
||||
HarnessEvidenceTools tools = fakeTools((context, id, arguments) -> {
|
||||
invocations.incrementAndGet();
|
||||
return ready(id);
|
||||
});
|
||||
RunContext run = context(6, 6, 6);
|
||||
HarnessToolInterceptor interceptor = new HarnessToolInterceptor(
|
||||
run, tools, objectMapper, trace::add);
|
||||
|
||||
interceptor.interceptToolCall(
|
||||
request(AgentToolContracts.LOOKUP_KNOWLEDGE, "call-1",
|
||||
"{\"input\":{\"query\":\"one\"}}"), ignored -> null);
|
||||
ToolCallResponse first = interceptor.interceptToolCall(
|
||||
request(AgentToolContracts.LOOKUP_KNOWLEDGE, "call-2",
|
||||
"{\"input\":{\"query\":\"two\"}}"), ignored -> null);
|
||||
ToolCallResponse stop = interceptor.interceptToolCall(
|
||||
request(AgentToolContracts.LOOKUP_KNOWLEDGE, "call-3",
|
||||
"{\"input\":{\"query\":\"three\"}}"), ignored -> null);
|
||||
|
||||
JsonNode firstObservation = objectMapper.readTree(first.getResult());
|
||||
JsonNode stopObservation = objectMapper.readTree(stop.getResult());
|
||||
assertTrue(firstObservation.path("repair_required").asBoolean());
|
||||
assertTrue(stopObservation.path("stop_required").asBoolean());
|
||||
assertEquals("PROGRESS_PROTOCOL_VIOLATED", stopObservation.path("reason").asText());
|
||||
assertEquals(1, invocations.get());
|
||||
assertEquals(DiagnosisStopReason.PROGRESS_PROTOCOL_VIOLATED,
|
||||
run.progress().snapshot().stopReason());
|
||||
|
||||
DiagnosisCollectionStoppedException ignoredStop = assertThrows(
|
||||
DiagnosisCollectionStoppedException.class,
|
||||
() -> interceptor.interceptToolCall(
|
||||
request(AgentToolContracts.LOOKUP_KNOWLEDGE, "call-4",
|
||||
"{\"input\":{\"query\":\"four\"}}"), ignored -> null));
|
||||
assertEquals(DiagnosisStopReason.PROGRESS_PROTOCOL_VIOLATED, ignoredStop.stopReason());
|
||||
assertEquals(1, invocations.get());
|
||||
|
||||
DiagnosisTraceAuditEvent stopRejection = trace.stream()
|
||||
.filter(event -> event.eventType() == TraceEventType.TOOL_REQUEST_REJECTED)
|
||||
.filter(event -> "call-3".equals(event.details().get("tool_call_id")))
|
||||
.findFirst().orElseThrow();
|
||||
assertEquals("INVALID_PROGRESS_PROTOCOL", stopRejection.details().get("error_code"));
|
||||
assertEquals("MISSING_PREVIOUS_OBSERVATION", stopRejection.details().get("violation_type"));
|
||||
assertEquals(false, stopRejection.details().get("repair_prompt_delivered"));
|
||||
assertEquals(2, stopRejection.details().get("consecutive_protocol_violations"));
|
||||
assertEquals("PROGRESS_PROTOCOL_VIOLATED", stopRejection.details().get("stop_reason"));
|
||||
String details = stopRejection.details().toString();
|
||||
assertFalse(details.contains("three"));
|
||||
assertFalse(details.contains("exception"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void missingInputAndInvalidEnvelopeProduceTypedProtocolErrors() throws Exception {
|
||||
AtomicInteger invocations = new AtomicInteger();
|
||||
HarnessEvidenceTools tools = fakeTools((context, id, arguments) -> {
|
||||
invocations.incrementAndGet();
|
||||
return ready(id);
|
||||
});
|
||||
HarnessToolInterceptor missingInterceptor = new HarnessToolInterceptor(
|
||||
context(3, 3, 3), tools, objectMapper);
|
||||
HarnessToolInterceptor invalidInterceptor = new HarnessToolInterceptor(
|
||||
context(3, 3, 3), tools, objectMapper);
|
||||
|
||||
ToolCallResponse missingInput = missingInterceptor.interceptToolCall(
|
||||
request(AgentToolContracts.LOOKUP_KNOWLEDGE, "call-missing-input",
|
||||
"{\"previous_observation\":null}"), ignored -> null);
|
||||
ToolCallResponse invalidEnvelope = invalidInterceptor.interceptToolCall(
|
||||
request(AgentToolContracts.LOOKUP_KNOWLEDGE, "call-invalid",
|
||||
"{not-json"), ignored -> null);
|
||||
|
||||
JsonNode missing = objectMapper.readTree(missingInput.getResult());
|
||||
JsonNode invalid = objectMapper.readTree(invalidEnvelope.getResult());
|
||||
assertEquals("MISSING_INPUT", missing.path("violation_type").asText());
|
||||
assertEquals("input", missing.path("missing_field").asText());
|
||||
assertEquals("INVALID_ENVELOPE", invalid.path("violation_type").asText());
|
||||
assertTrue(missing.path("repair_required").asBoolean());
|
||||
assertTrue(invalid.path("repair_required").asBoolean());
|
||||
assertEquals(0, invocations.get());
|
||||
assertFalse(missingInput.getResult().contains("not-json"));
|
||||
assertFalse(invalidEnvelope.getResult().contains("not-json"));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -61,6 +61,53 @@ class DiagnosisProgressTrackerTest {
|
||||
assertFalse(tracker.isDuplicate("lookup_knowledge", "scope-2"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void progressProtocolViolationsAccumulateIndependentlyFromNoGain() {
|
||||
DiagnosisProgressTracker tracker = new DiagnosisProgressTracker(3, 2);
|
||||
tracker.recordDuplicateScope();
|
||||
|
||||
DiagnosisProgressSnapshotState first = tracker.recordProgressProtocolViolation();
|
||||
assertEquals(1, first.consecutiveProgressProtocolViolations());
|
||||
assertEquals(1, first.consecutiveNoGain());
|
||||
assertEquals(DiagnosisCollectionState.COLLECTING, first.collectionState());
|
||||
|
||||
DiagnosisProgressSnapshotState second = tracker.recordProgressProtocolViolation();
|
||||
assertEquals(2, second.consecutiveProgressProtocolViolations());
|
||||
assertEquals(1, second.consecutiveNoGain());
|
||||
assertEquals(DiagnosisCollectionState.SATURATED, second.collectionState());
|
||||
assertEquals(DiagnosisStopReason.PROGRESS_PROTOCOL_VIOLATED, second.stopReason());
|
||||
assertTrue(tracker.claimStopInstruction());
|
||||
assertFalse(tracker.claimStopInstruction());
|
||||
}
|
||||
|
||||
@Test
|
||||
void validEvaluationClearsProtocolViolationCount() {
|
||||
DiagnosisProgressTracker tracker = new DiagnosisProgressTracker(2, 3);
|
||||
tracker.recordCompleted(call("call-1", "scope-1"), EvidenceStatus.EVIDENCE_FOUND);
|
||||
tracker.recordProgressProtocolViolation();
|
||||
|
||||
tracker.applyPreviousObservation(new PreviousObservation("call-1", InformationGain.GAINED));
|
||||
|
||||
DiagnosisProgressSnapshotState state = tracker.snapshot();
|
||||
assertEquals(0, state.consecutiveProgressProtocolViolations());
|
||||
assertEquals(0, state.consecutiveNoGain());
|
||||
assertEquals(DiagnosisCollectionState.COLLECTING, state.collectionState());
|
||||
}
|
||||
|
||||
@Test
|
||||
void rejectsUnexpectedPreviousObservationWithTypedViolation() {
|
||||
DiagnosisProgressTracker tracker = new DiagnosisProgressTracker(2, 2);
|
||||
|
||||
ProgressProtocolViolationException failure = assertThrows(
|
||||
ProgressProtocolViolationException.class,
|
||||
() -> tracker.applyPreviousObservation(
|
||||
new PreviousObservation("call-x", InformationGain.GAINED)));
|
||||
|
||||
assertEquals(ProgressProtocolViolationType.UNEXPECTED_PREVIOUS_OBSERVATION,
|
||||
failure.violationType());
|
||||
assertEquals("previous_observation", failure.missingField());
|
||||
}
|
||||
|
||||
private CompletedToolCall call(String id, String scope) {
|
||||
return new CompletedToolCall(id, "lookup_knowledge", scope);
|
||||
}
|
||||
|
||||
@@ -53,6 +53,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||
import static org.junit.jupiter.api.Assertions.assertSame;
|
||||
import static org.junit.jupiter.api.Assertions.assertThrows;
|
||||
import static org.junit.jupiter.api.Assertions.assertTrue;
|
||||
|
||||
class DiagnosisReleaseUseCaseTest {
|
||||
@@ -175,6 +176,37 @@ class DiagnosisReleaseUseCaseTest {
|
||||
assertEquals(0, fixture.model.calls.get());
|
||||
}
|
||||
|
||||
@Test
|
||||
void controlledProtocolViolationPublishesProgressWithoutAnotherModelCall() {
|
||||
Fixture fixture = fixture();
|
||||
|
||||
DiagnosisReleaseResult result = fixture.useCase.execute(
|
||||
fixture.context,
|
||||
"诊断未知故障",
|
||||
DiagnosisAgentExecution.stopped(
|
||||
progress(DiagnosisStopReason.PROGRESS_PROTOCOL_VIOLATED),
|
||||
DiagnosisStopReason.PROGRESS_PROTOCOL_VIOLATED));
|
||||
|
||||
assertFallback(result, FallbackType.INSUFFICIENT_EVIDENCE, 1);
|
||||
assertEquals(0, fixture.model.calls.get());
|
||||
}
|
||||
|
||||
@Test
|
||||
void controlledProtocolViolationWithoutProgressFailsClosed() {
|
||||
Fixture fixture = fixture();
|
||||
|
||||
IllegalStateException failure = assertThrows(IllegalStateException.class, () ->
|
||||
fixture.useCase.execute(
|
||||
fixture.context,
|
||||
"诊断未知故障",
|
||||
DiagnosisAgentExecution.stopped(
|
||||
DiagnosisProgressSnapshot.empty(),
|
||||
DiagnosisStopReason.PROGRESS_PROTOCOL_VIOLATED)));
|
||||
|
||||
assertTrue(failure.getMessage().contains("no verified publishable progress"));
|
||||
assertEquals(0, fixture.model.calls.get());
|
||||
}
|
||||
|
||||
@Test
|
||||
void invalidDraftWithVerifiedProgressPublishesOnlyProgressFallback() {
|
||||
Fixture fixture = fixture();
|
||||
|
||||
Reference in New Issue
Block a user