mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-28 20:21:50 +01:00
Steer the original live ACPX provider turn
Honor the explicit provider binding while retaining durable command identity, reject malformed bindings without fallback, and stop qualification immediately when the real API rejects steering. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
2a6107c049
commit
4237bc369a
7 files changed
+79
-11
No files matched your search
@@ -94,6 +94,20 @@ prices are estimates. Unpriced usage must stay unpriced in the ledger and UI.
|
||||
received one diagnostic shard rerun; it is not counted as passing yet.
|
||||
- Earlier evidence in `doc/architecture/runner-pi-capabilities.md` is historical
|
||||
and must not be counted as qualification of a new source revision.
|
||||
- At `2a6107c04`, normal public Pi setup and closed admission pass on ARM and
|
||||
Intel Mac (7.5 and 29.5 seconds). Intel uses a fresh compiled x64 daemon,
|
||||
packaged before installation through the normal public CLI/server graph.
|
||||
- The fresh steering attempt at `2a6107c04` reaches the real public Steer API
|
||||
but receives `409 steering_stale_turn`. Rust checks the durable command ID
|
||||
against the live provider turn even though the facade supplies a separate
|
||||
`providerTurnId`. The correction uses that explicit provider binding, rejects
|
||||
malformed bindings without fallback and keeps the existing live-turn fence.
|
||||
All 333 core tests and 68 control calibrations pass. The browser fixture now
|
||||
ends immediately on a rejected steering POST before sending any denial.
|
||||
- Linux CI now completes normal Pi setup with bounded larger scratch space.
|
||||
Its offline launch probe still returns an unclassified startup rejection;
|
||||
the verifier gives that launch's private runtime snapshots the same bounded
|
||||
scratch capacity. This is not counted as a passing Linux receipt yet.
|
||||
|
||||
## Rollout and rollback
|
||||
|
||||
|
||||
@@ -773,6 +773,28 @@ pub struct AcpxCommandExecutor {
|
||||
launch_profile: Option<AcpxLaunchProfile>,
|
||||
}
|
||||
|
||||
// The durable command's turn and the live provider callback have separate IDs.
|
||||
// Legacy direct callers use turnId for both; an explicit provider binding must
|
||||
// never fall back to the durable ID if it is malformed.
|
||||
fn turn_control_provider_turn_id(payload: &Value) -> Result<&str, DurableRunnerError> {
|
||||
let durable_turn_id = payload
|
||||
.get("turnId")
|
||||
.and_then(Value::as_str)
|
||||
.ok_or_else(|| DurableRunnerError::invalid("turn.steer payload.turnId is required"))?;
|
||||
let provider_turn_id = match payload.get("providerTurnId") {
|
||||
None => durable_turn_id,
|
||||
Some(value) => value.as_str().ok_or_else(|| {
|
||||
DurableRunnerError::invalid("turn.steer payload.providerTurnId must be a string")
|
||||
})?,
|
||||
};
|
||||
if !is_stable_id(provider_turn_id, DURABLE_STABLE_ID_CHARS) {
|
||||
return Err(DurableRunnerError::invalid(
|
||||
"turn.steer provider turn identity is invalid",
|
||||
));
|
||||
}
|
||||
Ok(provider_turn_id)
|
||||
}
|
||||
|
||||
impl AcpxCommandExecutor {
|
||||
pub fn with_runner_config(state_dir: impl Into<PathBuf>, config: &DurableRunnerConfig) -> Self {
|
||||
Self {
|
||||
@@ -1490,10 +1512,7 @@ impl AcpxCommandExecutor {
|
||||
.get("text")
|
||||
.and_then(Value::as_str)
|
||||
.ok_or_else(|| DurableRunnerError::invalid("turn.steer payload.text is required"))?;
|
||||
let turn_id = payload
|
||||
.get("turnId")
|
||||
.and_then(Value::as_str)
|
||||
.ok_or_else(|| DurableRunnerError::invalid("turn.steer payload.turnId is required"))?;
|
||||
let turn_id = turn_control_provider_turn_id(payload)?;
|
||||
let mode = match payload.get("mode") {
|
||||
None => "steer",
|
||||
Some(Value::String(mode)) => mode.as_str(),
|
||||
@@ -2836,6 +2855,31 @@ mod tests {
|
||||
fs::remove_dir_all(directory).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn turn_control_uses_the_explicit_live_provider_binding() {
|
||||
let command = json!({"turnId":"durable-turn", "providerTurnId":"provider-turn"});
|
||||
assert_eq!(
|
||||
turn_control_provider_turn_id(&command).unwrap(),
|
||||
"provider-turn"
|
||||
);
|
||||
let legacy = json!({"turnId":"provider-turn"});
|
||||
assert_eq!(
|
||||
turn_control_provider_turn_id(&legacy).unwrap(),
|
||||
"provider-turn"
|
||||
);
|
||||
for malformed in [
|
||||
Value::Null,
|
||||
json!(false),
|
||||
json!(1),
|
||||
json!(""),
|
||||
json!("bad\0id"),
|
||||
] {
|
||||
let command = json!({"turnId":"provider-turn", "providerTurnId":malformed});
|
||||
assert!(turn_control_provider_turn_id(&command).is_err());
|
||||
}
|
||||
assert!(turn_control_provider_turn_id(&json!({"providerTurnId":"provider-turn"})).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn retained_events_exposes_terminal_suffix_without_restoring_provider() {
|
||||
let directory = temporary_directory("retained-terminal-suffix");
|
||||
|
||||
@@ -129,7 +129,8 @@ try {
|
||||
assert.equal(receipt.status, 'installed_verified');
|
||||
assert.equal(receipt.target, 'linux-x64');
|
||||
assert.equal(readFileSync(join(consumer, 'package-lock.json'), 'utf8'), consumerLock, 'Pi setup must preserve the consumer dependency graph');
|
||||
console.log(isolated(['node', '/packages/pi-public-install-probe.mjs', '/consumer/node_modules/@paperclipai/server']).toString().trim());
|
||||
// Verified launch leases also materialize the runtime in private scratch.
|
||||
console.log(isolated(['node', '/packages/pi-public-install-probe.mjs', '/consumer/node_modules/@paperclipai/server'], { temporarySizeMiB: 2048 }).toString().trim());
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
@@ -439,8 +439,9 @@ cancelled run and no automatic continuation.
|
||||
Steering submits a random marker only in a browser comment after the permission
|
||||
is pending, binds the queued comment to the exact body submitted by the
|
||||
production Markdown editor, then clicks that comment's production Steer button. It records the
|
||||
exact public POST's queue/revision/run binding and requires the saved run
|
||||
acknowledgment plus the Product facade's same-turn acknowledgment item. The raw
|
||||
exact public POST's queue/revision/run binding. A rejected public POST ends the
|
||||
journey before any denial. Success requires the saved run acknowledgment plus
|
||||
the Product facade's same-turn acknowledgment item. The raw
|
||||
Rust `acpx-control-*` transport echo is suppressed by the facade;
|
||||
`CodexHarnessSession.steer` emits the durable correlated item after the command
|
||||
acknowledges. A deterministic calibration invokes that actual producer. The
|
||||
|
||||
@@ -1090,9 +1090,10 @@ export const runnerSuites: readonly RunnerSuiteFixture[] = [
|
||||
description: "Pending native-write Stop and browser-originated same-turn steering, with exact control receipts and independent retirement/no-effect evidence.",
|
||||
groups: ["native"], profiles: extendedHarnessProfiles.filter(profile => profile.qualificationCandidate === "pi"),
|
||||
environments: runnerEnvironments, tasks: piControlTasks, expectedMatrixSize: 4,
|
||||
definitionMetadata: { version: 3, qualification: "pending", scheduling: "explicit-only", profileVersion: QUALIFIED_ACPX_PROFILES.pi.agentProfileVersion,
|
||||
definitionMetadata: { version: 4, qualification: "pending", scheduling: "explicit-only", profileVersion: QUALIFIED_ACPX_PROFILES.pi.agentProfileVersion,
|
||||
nativeArguments: "streamed-until-exact-target",
|
||||
steeringComment: "exact-browser-submitted-markdown",
|
||||
steeringDispatch: "require-public-api-acceptance",
|
||||
pending: "paperclip.e2e.pi-control-pending.v1", stop: "paperclip.e2e.pi-stop-settlement.v1", steering: "paperclip.e2e.pi-steering-settlement.v1",
|
||||
permissionPolicy: "approve-reads", lifecycle: "per_turn", normalCompletionProvesStop: false, nativeFollowUp: "not-covered", providerDeath: "not-covered",
|
||||
remoteObservationCoverage: "continuous-through-owned-process-retirement", filesystemAfterRemoteRetirementObserved: false },
|
||||
|
||||
@@ -26,7 +26,7 @@ vi.mock("@playwright/test", () => ({ expect: (actual: any, message?: string) =>
|
||||
toHaveCount: async (value: number) => { if (typeof actual.count === "function") expect(actual.count()).toBe(value); },
|
||||
}) }));
|
||||
|
||||
async function exercise(taskId: string, remote: boolean, failure?: "mutation" | "incomplete" | "missing-ack" | "missing-comment" | "foreign-comment" | "foreign-queued-body") {
|
||||
async function exercise(taskId: string, remote: boolean, failure?: "mutation" | "incomplete" | "missing-ack" | "missing-comment" | "foreign-comment" | "foreign-queued-body" | "steer-rejected") {
|
||||
harness.target = ""; harness.prompt = ""; harness.message = ""; harness.mutation = failure === "mutation"; harness.incomplete = failure === "incomplete";
|
||||
const task = piControlTasks.find(t => t.id === taskId)!, stopCase = taskId === "pending-permission-stop";
|
||||
const workspacePath = await mkdtemp(join(tmpdir(), "pi-controls-fixture-"));
|
||||
@@ -77,7 +77,9 @@ async function exercise(taskId: string, remote: boolean, failure?: "mutation" |
|
||||
};
|
||||
const waiters: Array<{ predicate: (request: any) => boolean; resolve: (request: any) => void }> = [];
|
||||
function postBrowser(path: string, body: unknown) {
|
||||
const request = { url: () => `http://fixture${path}`, method: () => "POST", postDataJSON: () => body };
|
||||
const rejected = failure === "steer-rejected" && path.endsWith("/steer");
|
||||
const request = { url: () => `http://fixture${path}`, method: () => "POST", postDataJSON: () => body,
|
||||
response: async () => ({ ok: () => !rejected, status: () => rejected ? 409 : 200 }) };
|
||||
const waiter = waiters.find(w => w.predicate(request)); if (!waiter) throw new Error("Browser request was not awaited");
|
||||
waiter.resolve(request); waiters.splice(waiters.indexOf(waiter), 1);
|
||||
}
|
||||
@@ -134,6 +136,7 @@ async function exercise(taskId: string, remote: boolean, failure?: "mutation" |
|
||||
observe: () => {}, capture: async () => {}, evidence: async (name: string, value: unknown) => saved.set(name, structuredClone(value)), remoteBootstrap,
|
||||
registerCleanupAssertion: (fn: () => Promise<any>) => localCleanup.push(fn), registerBeforeEnvironmentTeardownAssertion: (fn: () => Promise<any>) => remoteCleanup.push(fn) } as any);
|
||||
if (failure === "missing-ack") { await expect(call).rejects.toThrow("acknowledgement"); expect(browserDeclines).toBe(0); }
|
||||
else if (failure === "steer-rejected") { await expect(call).rejects.toThrow("steering rejected: HTTP 409"); expect(browserDeclines).toBe(0); }
|
||||
else if (failure === "foreign-queued-body") { await expect(call).rejects.toThrow("browser comment queued"); expect(browserSteers).toBe(0); expect(browserDeclines).toBe(0); }
|
||||
else if (remote && (failure === "mutation" || failure === "incomplete")) await expect(call).rejects.toThrow();
|
||||
else {
|
||||
@@ -174,4 +177,5 @@ it.each(["mutation", "incomplete"] as const)("fails local cleanup on %s despite
|
||||
it.each(["mutation", "incomplete"] as const)("fails remote lifetime proof on %s despite an absent target", failure => exercise("same-turn-steering", true, failure));
|
||||
it("never denies the native write before the steering acknowledgement exists", () => exercise("same-turn-steering", false, "missing-ack"));
|
||||
it("rejects a queued body that differs from the exact browser submission", () => exercise("same-turn-steering", false, "foreign-queued-body"));
|
||||
it("stops before denial when the real steering API rejects the request", () => exercise("same-turn-steering", false, "steer-rejected"));
|
||||
it.each(["missing-comment", "foreign-comment"] as const)("retains and rejects %s during cleanup without fabricating persisted output", failure => exercise("same-turn-steering", false, failure));
|
||||
@@ -189,8 +189,11 @@ export async function runPiControlsFlow(input: {
|
||||
const route = `/api/issues/${issue.id}/queued-comments/${commentId}/steer`;
|
||||
const posted = page.waitForRequest(request => new URL(request.url()).pathname === route && request.method() === "POST");
|
||||
await page.getByTestId(`task-chat-queued-steer-${commentId}`).click();
|
||||
const body = (await posted).postDataJSON();
|
||||
const steeringRequest = await posted;
|
||||
const body = steeringRequest.postDataJSON();
|
||||
check("exact-browser-steer", body.queueId === queueId && body.revision === queue.revision && body.targetRunId === pending.scope.runId, "Browser steers the exact queued comment into the active run");
|
||||
const response = await steeringRequest.response();
|
||||
if (!response?.ok()) throw new Error(`Pi native steering rejected: HTTP ${response?.status() ?? "missing"}`);
|
||||
steered = { pending, commentId, queueId, marker };
|
||||
await pollUntil({ label: "Pi same-turn steering acknowledgement", deadlineAt: input.deadlineAt, intervalMs: 200, load,
|
||||
accept: state => { assertSamePiPending(pending, observePiControlPending({ ...state, scope: scope() })); readPiSteeringAcknowledgement({ ...state, ...steered! }); return true; } });
|
||||
|
||||
Reference in new issue
Block a user