mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
fix: retain resolved ACP execution timeout metadata (#14986)
## Thinking Path > - Paperclip manages agent work through adapters and heartbeat runs. > - ACP adapters resolve a timeout for the selected execution target. > - An untouched sandbox timeout uses a four-hour default. > - Heartbeat finalization rebuilt the metadata from the stored zero. > - This made a timed-out sandbox run report an effective timeout of zero. > - This change retains the adapter's resolved policy for accurate run diagnostics. ## Linked Issues or Issue Description **What happened?** ACP sandbox runs with `timeoutSec: 0` use the four-hour default. Their terminal metadata reports `effectiveTimeoutSec: 0` and `timeoutSource: config` because heartbeat finalization only reads the stored agent configuration. **Expected behavior** The result must report the policy the adapter used: `14400` and `sandbox_default`. Explicit limits, fractional limits, explicit unlimited overrides, and local defaults must keep their resolved values. **Steps to reproduce** Run an ACP adapter on a sandbox target with `timeoutSec: 0`. Compare the start log's four-hour policy with the terminal result's effective timeout. The new tests exercise the adapter result and the heartbeat metadata merge without waiting four hours. **Paperclip version or commit** Reproduced from `6eaf218924f0a89faf1c02eb0d6877a6c5c8a2cb`. **Deployment mode** Self-hosted server with an ACP sandbox execution target. Searched open timeout and metadata issues and PRs. Related #14804 exposes timeout configuration in forms; #14496 proposes a default policy; #14833 addresses CLI session retention. This PR changes only ACP result metadata. ## What Changed - Retain the resolved timeout in the ACP result after settlement. - Use validated adapter resolution when merging terminal timeout metadata. Preserve config fallbacks for older adapters and the HTTP millisecond policy. - Test sandbox defaults, explicit and fractional limits, explicit unlimited overrides, local defaults, malformed metadata, and unchanged cancellation fields. - Document the result fields and their meaning. ## Verification - `pnpm -r typecheck` passed. - `pnpm build` passed. - Stop metadata tests: 23 passed. - Reporter and diagnostic suites: 84 passed; two real-Sentry-SDK tests skipped because the optional SDK is not installed. - ACP engine suite: all 206 tests passed with a deterministic local `gemini --version` shim. Ambient host CLI probes made the existing Gemini session-resume fixture intermittent (one assertion failure in each of two broad runs); an isolated 15-case rerun and the clean-base 206-test suite also passed. No assertion or timeout was changed. - `pnpm test:run` is in progress. This PR does not claim a complete local suite pass. - Independent review found no blocking issues. The tests cover real adapter emission and the real metadata merge separately. ## Risks Low runtime risk: this changes result diagnostics. It does not change timeout values, cancellation acknowledgement, cleanup, checkpoint safety, retries, or provider operations. It does not fix the cause of a quiet or long-running tool. Older stored results are not rewritten. The new source values apply only when an adapter returns a valid resolution. ## Model Used OpenAI GPT-6 with reasoning, repository inspection, code editing, and test execution. The deployment-specific model ID and context-window size are not exposed in this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
862a5758ba
commit
4abff286c2
5 files changed
+88
-4
No files matched your search
@@ -2974,6 +2974,9 @@ describe("gemini ACP flag selection", () => {
|
||||
expect(explicitZero.runtimeOptions[0]?.timeoutMs).toBe(
|
||||
DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC * 1000,
|
||||
);
|
||||
expect(explicitZero.result.resultJson?.adapterExecutionTimeout).toEqual({
|
||||
timeoutSec: DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC, source: "sandbox_default",
|
||||
});
|
||||
|
||||
// A negative timeoutSec is the documented opt-out from any adapter
|
||||
// wall-clock timeout, sandbox targets included.
|
||||
@@ -2982,6 +2985,7 @@ describe("gemini ACP flag selection", () => {
|
||||
sandboxContext,
|
||||
);
|
||||
expect(negativeOptOut.runtimeOptions[0]?.timeoutMs).toBeUndefined();
|
||||
expect(negativeOptOut.result.resultJson?.adapterExecutionTimeout).toEqual({ timeoutSec: 0, source: "configured" });
|
||||
const startLine = negativeOptOut.logs.find(
|
||||
(entry) => entry.stream === "stderr" && entry.text.includes("Adapter execution timeout:"),
|
||||
);
|
||||
@@ -3047,6 +3051,7 @@ describe("gemini ACP flag selection", () => {
|
||||
"Set adapterConfig.timeoutSec to raise it.";
|
||||
expect(result.timedOut).toBe(true);
|
||||
expect(result.errorCode).toBe("acpx_timeout");
|
||||
expect(result.resultJson?.adapterExecutionTimeout).toEqual({ timeoutSec: 1, source: "configured" });
|
||||
expect(result.errorMessage).toBe(expectedMessage);
|
||||
expect(cancelReasons).toContain(expectedMessage);
|
||||
expect(result.resultJson).toMatchObject({ acpObservedEventCount: 0, acpPendingToolCount: 0, acpToolInventoryComplete: true });
|
||||
|
||||
@@ -5483,6 +5483,15 @@ export function createAcpxEngineExecutor(deps: AcpxEngineExecutorOptions = {}) {
|
||||
} },
|
||||
};
|
||||
}
|
||||
// Preserve the policy resolved for this execution target. The server
|
||||
// cannot reconstruct sandbox defaults from the stored agent config.
|
||||
capturedResult = {
|
||||
...capturedResult,
|
||||
resultJson: {
|
||||
...capturedResult.resultJson,
|
||||
adapterExecutionTimeout: { ...prepared.timeoutResolution },
|
||||
},
|
||||
};
|
||||
// The sync-back settlement step runs before this reproduces the result
|
||||
// (settlement precedes reproduction), so a failed workspace restore is
|
||||
// already recorded by the time we get here. Merge it into `resultJson`
|
||||
|
||||
Reference in new issue
Block a user