fix: validate session ID as UUID before --resume + error diagnostics (DLD-889) (#1742)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies. > - The Claude-local adapter uses `claude --resume <session-id>` to continue prior sessions; the `--resume` value MUST be a UUID per Claude's CLI contract. > - Paperclip internally uses session IDs prefixed with `ses_` (not UUIDs); these get passed straight through to `--resume` and crash the run. > - On top of the crash, when the underlying error path triggers a secret-decryption failure or heartbeat setup failure, the diagnostics are too thin to tell key-mismatch from other failures, and the heartbeat error code is mis-classified as `adapter_failed` instead of `setup_failed`. > - This PR validates `runtimeSessionId` against a UUID regex before letting `canResumeSession` become true, adds `not a valid UUID` to Claude's own retry-error regex, improves AES-256-GCM decryption diagnostics in the local encrypted provider, and re-classifies pre-adapter setup failures. > - The benefit is that Paperclip session IDs are detected and skipped gracefully (logged, no crash), legitimate Claude UUID-rejection errors are treated as retriable, and operators can diagnose decryption/setup failures from the run log. ## Linked Issues or Issue Description **What happened?** The `claude-local` adapter passes Paperclip's internal session identifiers (e.g. `ses_…`) straight to `claude --resume <session-id>`. Because Claude's CLI requires the `--resume` argument to be a UUID, the run crashes with a `not a valid UUID` error. When the surrounding code path also hits a secret-decryption failure, the heartbeat reports it as `adapter_failed`, hiding the real `setup_failed` cause and making diagnosis hard. **Expected behavior** Non-UUID session IDs should be detected before `--resume` is called, the run should fall back to a fresh session with a clear log line, and any decryption / setup failure should be reported with enough detail (and the correct error code) for an operator to tell what failed. **Steps to reproduce** 1. Have a persisted task session whose ID is not a UUID (Paperclip-issued `ses_…` form). 2. Trigger a heartbeat that resumes that session via the `claude-local` adapter. 3. Observe: the adapter crashes with a UUID-validation error; if the path also involves a decryption failure, the heartbeat surfaces `adapter_failed` instead of `setup_failed`. ## What Changed - `packages/adapters/claude-local/src/server/execute.ts`: Validates `runtimeSessionId` against a UUID regex before setting `canResumeSession`; non-UUID IDs are logged and skipped gracefully. Guards the cwd-mismatch log block on `isValidUuid` so it does not fire for non-UUID session IDs. - `packages/adapters/claude-local/src/server/parse.ts`: Adds `not a valid UUID` to the session-error retry regex so Claude's own UUID rejection is treated as a retriable error. - `server/src/services/secrets/local-encrypted-provider.ts`: Wraps AES-256-GCM decryption in try/catch and re-throws with a key fingerprint hint to aid key-mismatch diagnosis. - `server/src/services/heartbeat.ts`: Corrects the outer-catch `errorCode` from `adapter_failed` to `setup_failed` for pre-adapter setup failures. - `AGENTS.md`: Adds task/PR/CI governance sections (10–13) and expands the Definition of Done. ## Verification - `pnpm --filter @paperclipai/adapter-claude-local test` covers UUID validation and the parse retry regex. - `pnpm --filter @paperclipai/server test src/services/secrets` covers decryption diagnostics. - `pnpm --filter @paperclipai/server typecheck` ## Risks Low. UUID validation is strictly additive (non-UUIDs that previously crashed now log and skip). Decryption diagnostics only fire on failure paths. The `setup_failed` error code change is a clearer classification, not a behavior change. ## Model Used Claude (Opus 4.6) — used to identify the UUID-validation root cause, mirror existing parse patterns, and re-classify the heartbeat setup error code. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots (N/A — no UI changes) - [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 (in progress) - [ ] 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: CTO Agent <cto@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Devin Foley <devin@paperclip.ing>
This commit is contained in:
@@ -229,12 +229,12 @@ describe("claude remote execution", () => {
|
||||
adapterConfig: {},
|
||||
},
|
||||
runtime: {
|
||||
sessionId: "session-123",
|
||||
sessionId: "12345678-1234-4abc-9def-123456789012",
|
||||
sessionParams: {
|
||||
sessionId: "session-123",
|
||||
sessionId: "12345678-1234-4abc-9def-123456789012",
|
||||
cwd: "/remote/workspace",
|
||||
},
|
||||
sessionDisplayId: "session-123",
|
||||
sessionDisplayId: "12345678-1234-4abc-9def-123456789012",
|
||||
taskKey: null,
|
||||
},
|
||||
config: {
|
||||
@@ -283,9 +283,9 @@ describe("claude remote execution", () => {
|
||||
adapterConfig: {},
|
||||
},
|
||||
runtime: {
|
||||
sessionId: "session-123",
|
||||
sessionId: "12345678-1234-4abc-9def-123456789012",
|
||||
sessionParams: {
|
||||
sessionId: "session-123",
|
||||
sessionId: "12345678-1234-4abc-9def-123456789012",
|
||||
cwd: managedRemoteWorkspace,
|
||||
remoteExecution: {
|
||||
transport: "ssh",
|
||||
@@ -295,7 +295,7 @@ describe("claude remote execution", () => {
|
||||
remoteCwd: managedRemoteWorkspace,
|
||||
},
|
||||
},
|
||||
sessionDisplayId: "session-123",
|
||||
sessionDisplayId: "12345678-1234-4abc-9def-123456789012",
|
||||
taskKey: null,
|
||||
},
|
||||
config: {
|
||||
@@ -325,7 +325,7 @@ describe("claude remote execution", () => {
|
||||
expect(runChildProcess).toHaveBeenCalledTimes(1);
|
||||
const call = runChildProcess.mock.calls[0] as unknown as [string, string, string[]] | undefined;
|
||||
expect(call?.[2]).toContain("--resume");
|
||||
expect(call?.[2]).toContain("session-123");
|
||||
expect(call?.[2]).toContain("12345678-1234-4abc-9def-123456789012");
|
||||
});
|
||||
|
||||
});
|
||||
|
||||
@@ -598,8 +598,10 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||
const runtimePromptBundleKey = asString(runtimeSessionParams.promptBundleKey, "");
|
||||
const hasMatchingPromptBundle =
|
||||
runtimePromptBundleKey.length === 0 || runtimePromptBundleKey === promptBundle.bundleKey;
|
||||
const isValidUuid = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(runtimeSessionId);
|
||||
const canResumeSession =
|
||||
runtimeSessionId.length > 0 &&
|
||||
isValidUuid &&
|
||||
hasMatchingPromptBundle &&
|
||||
claudeSessionCwdMatchesExecutionTarget({
|
||||
runtimeSessionCwd,
|
||||
@@ -608,9 +610,16 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||
}) &&
|
||||
adapterExecutionTargetSessionMatches(runtimeRemoteExecution, runtimeExecutionTarget);
|
||||
const sessionId = canResumeSession ? runtimeSessionId : null;
|
||||
if (runtimeSessionId && !isValidUuid) {
|
||||
await onLog(
|
||||
"stdout",
|
||||
`[paperclip] Claude session "${runtimeSessionId}" is not a valid UUID and will not be passed to --resume.\n`,
|
||||
);
|
||||
}
|
||||
if (
|
||||
executionTargetIsRemote &&
|
||||
runtimeSessionId &&
|
||||
isValidUuid &&
|
||||
!canResumeSession
|
||||
) {
|
||||
await onLog(
|
||||
@@ -619,6 +628,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||
);
|
||||
} else if (
|
||||
runtimeSessionId &&
|
||||
isValidUuid &&
|
||||
runtimeSessionCwd.length > 0 &&
|
||||
path.resolve(runtimeSessionCwd) !== path.resolve(effectiveExecutionCwd)
|
||||
) {
|
||||
@@ -626,7 +636,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||
"stdout",
|
||||
`[paperclip] Claude session "${runtimeSessionId}" does not match the current remote execution identity and will not be resumed in "${effectiveExecutionCwd}". Starting a fresh remote session.\n`,
|
||||
);
|
||||
} else if (runtimeSessionId && !canResumeSession) {
|
||||
} else if (runtimeSessionId && isValidUuid && !canResumeSession) {
|
||||
await onLog(
|
||||
"stdout",
|
||||
`[paperclip] Claude session "${runtimeSessionId}" was saved for cwd "${runtimeSessionCwd}" and will not be resumed in "${effectiveExecutionCwd}".\n`,
|
||||
|
||||
@@ -192,7 +192,7 @@ export function isClaudeUnknownSessionError(parsed: Record<string, unknown>): bo
|
||||
.filter(Boolean);
|
||||
|
||||
return allMessages.some((msg) =>
|
||||
/no conversation found with session id|unknown session|session .* not found/i.test(msg),
|
||||
/no conversation found with session id|unknown session|session .* not found|not a valid UUID/i.test(msg),
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user