diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs b/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs index 6b04cdd71e5..cf7f5468dbf 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/provider_backend.rs @@ -4834,7 +4834,7 @@ mod tests { } #[test] - fn invalid_tool_results_expose_only_reserved_static_schema_guidance() { + fn invalid_tool_results_expose_only_reserved_schema_guidance() { let finish_result = schema_rejection( "paperclip_finish", json!({"secretSubmittedValue": "must-not-appear"}), @@ -4844,6 +4844,9 @@ mod tests { let finish_message = finish_result.result["error"]["message"].as_str().unwrap(); assert!(finish_message .contains("continuation must include kind=response_wake, summary, and idempotencyKey")); + assert!(finish_message.contains("/required (missing \"requiredField\")")); + assert!(finish_message.contains("/additionalProperties")); + assert!(!finish_message.contains("secretSubmittedValue")); assert!(finish_message.len() <= 512); assert!(!finish_message.chars().any(char::is_control)); assert!(!finish_result.result.to_string().contains("must-not-appear")); diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/provider_bridge.rs b/packages/paperclip-runner/runner/crates/runner-core/src/provider_bridge.rs index 81569330f30..77307873814 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/provider_bridge.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/provider_bridge.rs @@ -223,7 +223,7 @@ pub struct ProviderToolBridge { #[derive(Clone, Debug, PartialEq, Eq)] pub struct ProviderBridgeError { message: String, - safe_provider_message: Option<&'static str>, + safe_provider_message: Option, } impl ProviderBridgeError { @@ -242,10 +242,48 @@ impl ProviderBridgeError { }; Self { message: format!("provider arguments for {operation_id} failed JSON Schema validation"), - safe_provider_message, + safe_provider_message: safe_provider_message.map(str::to_owned), } } + fn with_schema_locations(mut self, validator: &jsonschema::Validator, input: &Value) -> Self { + // Only completion tools have a provider-safe hint. Report bounded paths + // into the authorized schema, never the submitted values or unknown keys. + if let Some(message) = self.safe_provider_message.as_mut() { + let locations = validator + .iter_errors(input) + .take(3) + .map(|error| { + let location = error + .schema_path() + .to_string() + .chars() + .filter(|c| c.is_ascii_alphanumeric() || "/_~.-".contains(*c)) + .take(128) + .collect::(); + match error.kind() { + jsonschema::error::ValidationErrorKind::Required { property } => { + // Required property names come from the authorized schema. + let name = property + .to_string() + .chars() + .filter(|c| c.is_ascii_graphic() || *c == ' ') + .take(64) + .collect::(); + format!("{location} (missing {name})") + } + _ => location, + } + }) + .collect::>(); + message.push_str(" Check these input schema locations: "); + message.push_str(&locations.join(", ")); + message.push('.'); + message.truncate(message.len().min(512)); + } + self + } + fn active_turn_receipt_limit() -> Self { Self::invalid(ACTIVE_TURN_RECEIPT_LIMIT_MESSAGE) } @@ -254,8 +292,8 @@ impl ProviderBridgeError { self.message == ACTIVE_TURN_RECEIPT_LIMIT_MESSAGE } - pub fn safe_provider_message(&self) -> Option<&'static str> { - self.safe_provider_message + pub fn safe_provider_message(&self) -> Option<&str> { + self.safe_provider_message.as_deref() } } @@ -612,7 +650,8 @@ impl ProviderToolBridge { )) })?; if !validator.is_valid(&input) { - return Err(ProviderBridgeError::input_schema_validation(&operation_id)); + return Err(ProviderBridgeError::input_schema_validation(&operation_id) + .with_schema_locations(&validator, &input)); } if matches!( operation_id.as_str(), @@ -1522,6 +1561,65 @@ mod tests { assert!(!over_limit.has_call_receipt("call-over-limit")); } + #[test] + fn completion_validation_identifies_schema_location_without_echoing_input() { + let mut bridge = completion_bridge(); + let error = bridge + .begin_call( + "bad-summary".to_owned(), + "paperclip_finish".to_owned(), + json!({"summary": {"PRIVATE_FIELD": "PRIVATE_VALUE"}}), + ) + .unwrap_err(); + let message = error.safe_provider_message().unwrap(); + assert!(message.contains("/properties/summary/type"), "{message}"); + assert!(!message.contains("PRIVATE_FIELD")); + assert!(!message.contains("PRIVATE_VALUE")); + assert!(!bridge.has_call_receipt("bad-summary")); + bridge + .begin_call( + "corrected-summary".to_owned(), + "paperclip_finish".to_owned(), + json!({"summary": "Saved the requested output."}), + ) + .unwrap(); + } + + #[test] + fn completion_validation_names_missing_fields_and_bounds_feedback() { + let mut bridge = completion_bridge(); + let error = bridge + .begin_call( + "missing-summary".to_owned(), + "paperclip_finish".to_owned(), + json!({}), + ) + .unwrap_err(); + assert!(error + .safe_provider_message() + .unwrap() + .contains("/required (missing \"summary\")")); + + let schema = json!({"type": "object", "properties": { + "a": {"type": "string"}, "b": {"type": "string"}, + "c": {"type": "string"}, "d": {"type": "string"}, + }}); + let validator = jsonschema::validator_for(&schema).unwrap(); + let error = ProviderBridgeError::input_schema_validation("paperclip_finish") + .with_schema_locations(&validator, &json!({"a": 1, "b": 2, "c": 3, "d": 4})); + assert_eq!( + error + .safe_provider_message() + .unwrap() + .matches("/properties/") + .count(), + 3 + ); + let other = ProviderBridgeError::input_schema_validation("some_other_tool") + .with_schema_locations(&validator, &json!({"a": 1})); + assert_eq!(other.safe_provider_message(), None); + } + #[test] fn canonical_number_uses_decimal_notation_at_javascript_lower_boundary() { for encoded in ["1e-6", "0.000001"] { diff --git a/tests/runner-e2e/README.md b/tests/runner-e2e/README.md index 7d54c59de8b..ef33a335a50 100644 --- a/tests/runner-e2e/README.md +++ b/tests/runner-e2e/README.md @@ -784,3 +784,17 @@ with the test flow. Native provider runs do not require an unrelated host Changes to privileged worker setup must reach the default branch before a branch-targeted paid campaign can exercise them. The report job resolves its lockfile from its own trusted checkout, never from the tested branch. + +### Injected interruption diagnostics + +Everyday restart and Stop scenarios exempt only their recorded cancellation, +graceful-shutdown interruption, or process-loss outcome. A later adapter error +on that same run still fails immediately and fails the lifecycle grader. The +run ID alone is not an exemption from recovery failures. + +The review-handoff case also requires proof that the parent was blocked before +the review wake. When all tasks finish and persisted timestamps prove that the +accepted review started before any parent run finished, the harness fails +promptly with an unexercised-boundary diagnostic. Missing evidence in separately +fetched snapshots does not trigger this rejection. Successful work alone does +not prove that this recovery path was tested. diff --git a/tests/runner-e2e/everyday-flow.ts b/tests/runner-e2e/everyday-flow.ts index 6885868d8d2..a0c4fc5d2c7 100644 --- a/tests/runner-e2e/everyday-flow.ts +++ b/tests/runner-e2e/everyday-flow.ts @@ -22,6 +22,9 @@ import { LATE_REQUIREMENT, SLUGIFY_REVISION, requiresEverydayArtifactOracle } fr import { isActiveStoryRun, isStoryWorkspaceDeferral, + isExpectedStoryInterruption, + storyUnexpectedRunFailure, + storyUnexercisedReviewBoundary, storyLifecycleChecks, storyRepliesConsumed, storyHasAgentReply, @@ -304,11 +307,7 @@ export async function runEverydayFlow(input: Input) { )), reject: (state) => { if (state.runs.length > 12) return "bounded execution count exceeded"; - const bad = state.runs.find( - (r) => - ["failed", "timed_out"].includes(r.status) && - !ev.allowedInterruptedRuns.includes(r.id), - ); + const bad = storyUnexpectedRunFailure(state.runs, ev.allowedInterruptedRuns); if (bad) return `native execution failed ${bad.errorCode ?? ""}: ${bad.error ?? bad.status}`; if ( @@ -695,7 +694,7 @@ export async function runEverydayFlow(input: Input) { ); return failed ? `Review handoff prerequisite failed: ${failed.errorCode}: ${failed.error}` - : undefined; + : storyUnexercisedReviewBoundary(state.issues, state.runs, parent!.id, fixtures.agent.id); }, }); const child = boundary.issues.find((issue) => issue.parentId === parent!.id)!; @@ -1387,7 +1386,7 @@ export async function runEverydayFlow(input: Input) { .filter( (r) => !isStoryWorkspaceDeferral(r) && - !ev.allowedInterruptedRuns.includes(r.id), + !isExpectedStoryInterruption(r, ev.allowedInterruptedRuns), ) .every( (r) => diff --git a/tests/runner-e2e/everyday-observations.ts b/tests/runner-e2e/everyday-observations.ts index 9fa05c1e7b5..adbaaeed81d 100644 --- a/tests/runner-e2e/everyday-observations.ts +++ b/tests/runner-e2e/everyday-observations.ts @@ -208,6 +208,61 @@ export function isStoryWorkspaceDeferral(run: StoryRun) { !run.processPid ); } +/** Exempt the injected stop itself, never a later recovery/provider failure on that run. */ +export function isExpectedStoryInterruption( + run: StoryRun, + allowedRunIds: readonly string[], +): boolean { + if (!allowedRunIds.includes(run.id)) return false; + return ( + (run.status === "cancelled" && (!run.errorCode || run.errorCode === "cancelled")) || + (run.status === "interrupted" && run.errorCode === "server_shutdown_interrupted") || + (run.status === "failed" && run.errorCode === "process_lost") + ); +} + +export function storyUnexpectedRunFailure(runs: StoryRun[], allowedRunIds: readonly string[]) { + return runs.find((run) => + ["failed", "timed_out", "cancelled", "interrupted"].includes(run.status) && + !isStoryWorkspaceDeferral(run) && + !isExpectedStoryInterruption(run, allowedRunIds), + ); +} + +/** Called after the boundary matcher; reject only with durable proof of the wrong ordering. */ +export function storyUnexercisedReviewBoundary( + issues: StoryIssue[], + runs: StoryRun[], + parentId: string, + leadId: string, +): string | undefined { + if (issues.length === 0 || runs.length === 0) return; + if (!issues.every((issue) => + issue.status === "done" && !issue.scheduledRetry && !issue.activeRecoveryAction, + )) return; + if (!runs.every((run) => run.status === "succeeded")) return; + const child = issues.find((issue) => issue.parentId === parentId); + const accepted = child?.interactions?.flatMap((interaction) => { + const review = storyAcceptedAgentReview(child, interaction.id, leadId, runs); + return review ? [review] : []; + })[0]; + const reviewRun = accepted && runs.find((run) => run.id === accepted.resolvedByRunId); + const reviewStartedAt = Date.parse(reviewRun?.startedAt ?? ""); + const parentRuns = runs.filter((run) => + run.nativeIssueId === parentId || + run.contextSnapshot?.issueId === parentId || + run.contextSnapshot?.taskId === parentId, + ); + // Snapshots are fetched separately. Missing review evidence is not proof that it + // never happened; wait unless persisted timestamps rule out the required order. + if (!Number.isFinite(reviewStartedAt) || parentRuns.length === 0) return; + if (!parentRuns.every((run) => { + const finishedAt = Date.parse(run.finishedAt ?? ""); + return Number.isFinite(finishedAt) && finishedAt > reviewStartedAt; + })) return; + return "Review handoff boundary not exercised: the accepted review started before any parent run finished, so the blocked-parent-before-review ordering was not tested."; +} + export function storyLifecycleChecks(input: { issues: StoryIssue[]; runs: StoryRun[]; @@ -216,7 +271,7 @@ export function storyLifecycleChecks(input: { allowedInterruptedRuns?: string[]; }): StoryCheck[] { const executed = input.runs.filter((r) => !isStoryWorkspaceDeferral(r)); - const allowed = new Set(input.allowedInterruptedRuns ?? []); + const allowed = input.allowedInterruptedRuns ?? []; const check = (id: string, passed: boolean, detail: string): StoryCheck => ({ id, passed, @@ -244,8 +299,8 @@ export function storyLifecycleChecks(input: { ), check( "successful-runs", - executed.every((r) => r.status === "succeeded" || allowed.has(r.id)), - "Only explicitly interrupted runs may have a non-success terminal state.", + executed.every((r) => r.status === "succeeded" || isExpectedStoryInterruption(r, allowed)), + "Only the expected stop or process-loss outcome of an injected interruption is exempt.", ), check( "bounded-work", diff --git a/tests/runner-e2e/everyday.test.ts b/tests/runner-e2e/everyday.test.ts index bc86c2ab3f7..b4bbdfba429 100644 --- a/tests/runner-e2e/everyday.test.ts +++ b/tests/runner-e2e/everyday.test.ts @@ -4,6 +4,8 @@ import { parseRunnerSelectors, selectRunnerExecutions } from "./selectors.js"; import { everydayTasks, productionStoryProfile } from "./everyday-cases.js"; import { isStoryWorkspaceDeferral, + storyUnexpectedRunFailure, + storyUnexercisedReviewBoundary, storyLifecycleChecks, storyRepliesConsumed, storyHasAgentReply, @@ -22,6 +24,7 @@ import { storyParentCompletionPrecedesReview, storyAcceptedAgentReview, type StoryRun, + type StoryIssue, } from "./everyday-observations.js"; describe("everyday workflow grader and review timing", () => { @@ -53,6 +56,49 @@ describe("everyday workflow grader and review timing", () => { }); }); +describe("unexercised review boundary diagnostics", () => { + const done = { id: "parent", companyId: "company", title: "task", status: "done" }; + const completed = { + id: "run", companyId: "company", agentId: "lead", status: "succeeded", + nativeIssueId: "parent", finishedAt: "2026-09-19T14:47:48.134Z", + }; + const child = { + ...done, id: "child", parentId: "parent", interactions: [{ + id: "card", issueId: "child", kind: "request_confirmation", status: "accepted", + addresseeAgentId: "lead", resolvedByAgentId: "lead", resolvedByRunId: "review", + createdAt: "2026-09-19T14:47:04.000Z", resolvedAt: "2026-09-19T14:47:21.876Z", + payload: { target: { type: "custom", key: "native_completion_review", revisionId: "decision" } }, + result: { version: 1, outcome: "accepted" }, + }], + }; + const review = { + ...completed, id: "review", nativeIssueId: "child", startedAt: "2026-09-19T14:47:05.802Z", + contextSnapshot: { nativeReviewInteractionId: "card", nativeReviewDecisionId: "decision" }, + }; + const diagnose = (issues: StoryIssue[] = [done, child], runs: StoryRun[] = [completed, review]) => + storyUnexercisedReviewBoundary(issues, runs, "parent", "lead"); + it("fails promptly with persisted proof that review preceded parent completion", () => { + expect(diagnose()).toContain("not exercised"); + }); + it("does not preempt in-flight finalization, recovery, or unfinished work", () => { + expect(diagnose([done, child], [{ ...completed, status: "running" }, review])).toBeUndefined(); + expect(storyUnexercisedReviewBoundary([{ ...done, scheduledRetry: {} }, child], [completed, review], "parent", "lead")).toBeUndefined(); + expect(storyUnexercisedReviewBoundary([{ ...done, activeRecoveryAction: {} }, child], [completed, review], "parent", "lead")).toBeUndefined(); + expect(diagnose([{ ...done, status: "blocked" }, child])).toBeUndefined(); + expect(diagnose([], [])).toBeUndefined(); + }); + it("waits when separately fetched snapshots lack review evidence or timestamps", () => { + expect(diagnose([done, { ...child, interactions: [] }])).toBeUndefined(); + expect(diagnose([done, child], [completed])).toBeUndefined(); + expect(diagnose([done, child], [{ ...completed, finishedAt: "" }, review])).toBeUndefined(); + expect(diagnose([done, child], [completed, { ...review, startedAt: "" }])).toBeUndefined(); + expect(diagnose([done, child], [review])).toBeUndefined(); + }); + it("does not reject a completed workflow with the required timing", () => { + expect(diagnose([done, child], [{ ...completed, finishedAt: review.startedAt }, review])).toBeUndefined(); + }); +}); + describe("multi-round agent review handoff", () => { const child = { id: "child", @@ -274,6 +320,28 @@ describe("lifecycle oracle calibrated failures", () => { ); expect(checks.find((c) => c.id === "successful-runs")?.passed).toBe(false); }); + it.each(["adapter_failed", "tool_validation_error", "timed_out"])( + "does not excuse a later %s on the same deliberately interrupted run", + (errorCode) => { + const checks = score( + [{ ...run, status: "failed", errorCode }], + [parent], + [run.id], + ); + expect(checks.find((c) => c.id === "successful-runs")?.passed).toBe(false); + const failed = { ...run, status: "failed", errorCode }; + expect(storyUnexpectedRunFailure([failed], [run.id])).toBe(failed); + }, + ); + it.each([ + ["cancelled", "cancelled"], + ["interrupted", "server_shutdown_interrupted"], + ["failed", "process_lost"], + ])("accepts an injected interruption with %s / %s", (status, errorCode) => { + const checks = score([{ ...run, status, errorCode }], [parent], [run.id]); + expect(checks.find((c) => c.id === "successful-runs")?.passed).toBe(true); + expect(storyUnexpectedRunFailure([{ ...run, status, errorCode }], [run.id])).toBeUndefined(); + }); it("excludes a proven pre-dispatch workspace deferral without hiding executed failures", () => { const deferred: StoryRun = { id: "deferred", diff --git a/tests/runner-e2e/web-server-shutdown.test.ts b/tests/runner-e2e/web-server-shutdown.test.ts index ceea1ee701d..6fc14b5aedb 100644 --- a/tests/runner-e2e/web-server-shutdown.test.ts +++ b/tests/runner-e2e/web-server-shutdown.test.ts @@ -19,7 +19,7 @@ it("lets Playwright reap a restarted server through the production bounded shutd const pidsPath = path.join(root, "pids.json"); const configPath = path.join(root, "playwright.config.cjs"); const testModule = require.resolve("@playwright/test"); - const cli = path.join(path.dirname(require.resolve("playwright/package.json")), "cli.js"); + const cli = require.resolve("@playwright/test/cli"); let child: ReturnType | undefined; let timer: ReturnType | undefined; let reservationReleased = false;