diff --git a/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-stall-watchdogs.test.ts b/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-stall-watchdogs.test.ts index 7c6f047fd..84549dfe8 100644 --- a/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-stall-watchdogs.test.ts +++ b/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-stall-watchdogs.test.ts @@ -1,3 +1,5 @@ +import fs from 'node:fs/promises'; + import { ACP_ENVELOPE_EVENT_TYPES, TaskEventName, @@ -16,6 +18,7 @@ import type { const TEST_OPENCODE_MODEL = 'test-provider/main-model'; const TURN_STALL_TIMEOUT_MS = 60_000; const STEER_TEXT = 'Use this newer instruction instead.'; +let harnessSequence = 0; class FakeOpenCodeServerClient { private eventHandler: @@ -70,6 +73,7 @@ function createLogger() { function createHarness(client = new FakeOpenCodeServerClient()) { const logger = createLogger(); + const visualProofAttemptStatePath = `/tmp/roomote-visual-proof-stall-test-${process.pid}-${harnessSequence++}.json`; const harness = new OpenCodeServerHarness({ client: client as unknown as OpenCodeServerClient, workspacePath: '/tmp/workspace', @@ -77,9 +81,10 @@ function createHarness(client = new FakeOpenCodeServerClient()) { model: TEST_OPENCODE_MODEL, eventStreamReadyTimeoutMs: 100, turnStallTimeoutMs: TURN_STALL_TIMEOUT_MS, + visualProofAttemptStatePath, }); - return { client, harness, logger }; + return { client, harness, logger, visualProofAttemptStatePath }; } async function connectHarness( @@ -347,7 +352,7 @@ describe('OpenCode turn stall watchdog', () => { }); it('aborts a wedged turn, surfaces a retryable error, and ends the task', async () => { - const { client, harness } = createHarness(); + const { client, harness, visualProofAttemptStatePath } = createHarness(); const taskEvents: TaskEvent[] = []; const persistedEnvelopes: AcpPersistedEnvelope[] = []; harness.subscribe((event) => taskEvents.push(event)); @@ -359,6 +364,10 @@ describe('OpenCode turn stall watchdog', () => { await connectHarness(harness, client); vi.useFakeTimers(); await startTask(client, harness); + await fs.writeFile( + visualProofAttemptStatePath, + JSON.stringify({ attemptId: 'wedged-attempt' }), + ); // Re-baseline the activity clock (startTask's waitFor advances fake // time a little) so the window boundary below is exact. await client.emit(assistantTextPartEvent()); @@ -393,6 +402,9 @@ describe('OpenCode turn stall watchdog', () => { (event) => event.eventName === TaskEventName.TaskAborted, ), ).toHaveLength(1); + await expect( + fs.readFile(visualProofAttemptStatePath, 'utf8'), + ).rejects.toMatchObject({ code: 'ENOENT' }); // The MessageAbortedError provoked by the recovery abort stays // suppressed instead of surfacing a second error or terminal abort. diff --git a/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-subagent-watchdog.test.ts b/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-subagent-watchdog.test.ts index 1d9107781..3d9795e8b 100644 --- a/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-subagent-watchdog.test.ts +++ b/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-subagent-watchdog.test.ts @@ -1,3 +1,5 @@ +import fs from 'node:fs/promises'; + import { ACP_ENVELOPE_EVENT_TYPES, TaskEventName, @@ -75,12 +77,14 @@ function createLogger() { const SETTLEMENT_GRACE_MS = 10_000; const VISUAL_PROOF_TIMEOUT_MS = 5_000; +let harnessSequence = 0; function createHarness( client = new FakeOpenCodeServerClient(), options: { visualProofTimeoutMs?: number } = {}, ) { const logger = createLogger(); + const visualProofAttemptStatePath = `/tmp/roomote-visual-proof-attempt-test-${process.pid}-${harnessSequence++}.json`; const harness = new OpenCodeServerHarness({ client: client as unknown as OpenCodeServerClient, workspacePath: '/tmp/workspace', @@ -89,9 +93,10 @@ function createHarness( eventStreamReadyTimeoutMs: 100, subagentSettlementGraceMs: SETTLEMENT_GRACE_MS, visualProofTimeoutMs: options.visualProofTimeoutMs, + visualProofAttemptStatePath, }); - return { client, harness, logger }; + return { client, harness, logger, visualProofAttemptStatePath }; } function createSkillToolPart(name: string) { @@ -938,9 +943,12 @@ describe('OpenCode visual proof deadline', () => { it('bounds the entire visual proof workflow and resumes with a timeout handoff', async () => { const client = new FakeOpenCodeServerClient(); - const { harness, logger } = createHarness(client, { - visualProofTimeoutMs: VISUAL_PROOF_TIMEOUT_MS, - }); + const { harness, logger, visualProofAttemptStatePath } = createHarness( + client, + { + visualProofTimeoutMs: VISUAL_PROOF_TIMEOUT_MS, + }, + ); try { await connectHarness(harness, client); @@ -971,9 +979,32 @@ describe('OpenCode visual proof deadline', () => { ]), }, }); + expect(client.promptAsync.mock.calls[1]?.[0]).toMatchObject({ + request: { + parts: expect.arrayContaining([ + expect.objectContaining({ + text: expect.stringContaining( + "list this task's `visual-proof` artifacts once", + ), + }), + ]), + }, + }); + const attemptState = JSON.parse( + await fs.readFile(visualProofAttemptStatePath, 'utf8'), + ) as { attemptId: string; startedAt: string }; + expect(attemptState.attemptId).toMatch(/^[0-9a-f-]{36}$/); + expect(attemptState.startedAt).toBeTruthy(); + expect(JSON.stringify(client.promptAsync.mock.calls[1]?.[0])).toContain( + `tmp/capture-visual-proof/${attemptState.attemptId}/`, + ); expect(logger.warn).toHaveBeenCalledWith( expect.stringContaining('shared 5000ms deadline'), ); + harness.dispose(); + await expect( + fs.readFile(visualProofAttemptStatePath, 'utf8'), + ).rejects.toMatchObject({ code: 'ENOENT' }); } finally { harness.dispose(); } @@ -1014,6 +1045,77 @@ describe('OpenCode visual proof deadline', () => { } }); + it('clears proof attempt state when the task is cancelled', async () => { + const client = new FakeOpenCodeServerClient(); + const { harness, visualProofAttemptStatePath } = createHarness(client, { + visualProofTimeoutMs: VISUAL_PROOF_TIMEOUT_MS, + }); + + try { + await connectHarness(harness, client); + await armSpawn(client, harness); + await client.emit({ + type: 'message.part.updated', + properties: { part: createSkillToolPart('capture-visual-proof') }, + }); + + await expect( + fs.readFile(visualProofAttemptStatePath, 'utf8'), + ).resolves.toContain('attemptId'); + + harness.sendCommand({ commandName: TaskCommandName.CancelTask }); + + await vi.waitFor(async () => { + await expect( + fs.readFile(visualProofAttemptStatePath, 'utf8'), + ).rejects.toMatchObject({ code: 'ENOENT' }); + }); + } finally { + harness.dispose(); + } + }); + + it('clears proof attempt state after a terminal provider error', async () => { + const client = new FakeOpenCodeServerClient(); + const { harness, visualProofAttemptStatePath } = createHarness(client, { + visualProofTimeoutMs: VISUAL_PROOF_TIMEOUT_MS, + }); + + try { + await connectHarness(harness, client); + await armSpawn(client, harness); + await client.emit({ + type: 'message.part.updated', + properties: { part: createSkillToolPart('capture-visual-proof') }, + }); + + await expect( + fs.readFile(visualProofAttemptStatePath, 'utf8'), + ).resolves.toContain('attemptId'); + + await client.emit({ + type: 'session.error', + properties: { + sessionID: 'ses_1', + error: { + name: 'APIError', + data: { + message: 'Provider rejected the request.', + statusCode: 403, + isRetryable: false, + }, + }, + }, + }); + + await expect( + fs.readFile(visualProofAttemptStatePath, 'utf8'), + ).rejects.toMatchObject({ code: 'ENOENT' }); + } finally { + harness.dispose(); + } + }); + it('does not reset the deadline for a delegated capture retry', async () => { const client = new FakeOpenCodeServerClient(); const { harness } = createHarness(client, { diff --git a/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts b/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts index 70aa08519..2c5751000 100644 --- a/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts +++ b/apps/worker/src/sandbox-server/lib/harnesses/opencode-server/harness.ts @@ -1,4 +1,6 @@ import EventEmitter from 'node:events'; +import { randomUUID } from 'node:crypto'; +import { rmSync, writeFileSync } from 'node:fs'; import fs from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; @@ -130,6 +132,7 @@ interface OpenCodeServerHarnessOptions { turnStallTimeoutMs?: number; subagentSettlementGraceMs?: number; visualProofTimeoutMs?: number; + visualProofAttemptStatePath?: string; queuedPromptRetryDelayMs?: number; /** * Max automatic continue attempts after a provider rate-limit session.error @@ -334,8 +337,15 @@ const FALLBACK_OPENCODE_STOP_HOOK_REMINDER = 'Before finalizing, post a terminal chat-visible reply for the current turn.'; const ROOMOTE_OPENCODE_VISUAL_AGENT_NAME = 'visual'; const CAPTURE_VISUAL_PROOF_SKILL = 'capture-visual-proof'; -const VISUAL_PROOF_TIMEOUT_RECOVERY_PROMPT = - 'The visual proof step exceeded its shared five-minute deadline. Do not retry capture or run further proof recovery. Return a blocked proof handoff with blocker type `proof capture timed out`, then continue the active parent workflow without visual proof.'; +const VISUAL_PROOF_ATTEMPT_STATE_PATH = + '/tmp/roomote-visual-proof-attempt.json'; +const formatVisualProofTimeoutRecoveryPrompt = (attemptId: string | null) => { + const attemptPath = attemptId + ? `tmp/capture-visual-proof/${attemptId}/` + : 'unavailable'; + + return `The visual proof step exceeded its shared five-minute deadline. Do not retry capture or run further proof recovery. Before reporting the outcome, list this task's \`visual-proof\` artifacts once. The interrupted proof attempt ID is ${attemptId ?? 'unavailable'} and its exact artifact path prefix is \`${attemptPath}\`. Carry only matching artifacts into delivery instead of reporting a timeout. If the attempt ID is unavailable or no matching uploads exist, return a blocked proof handoff with blocker type \`proof capture timed out\`, then continue the active parent workflow without visual proof. Never reuse artifacts from another proof attempt.`; +}; // OpenCode's built-in tool for loading skills into the session. const OPENCODE_SKILL_TOOL = 'skill'; // Hidden continuation submitted automatically after a turn that exited plan @@ -1605,6 +1615,7 @@ export class OpenCodeServerHarness private readonly stopHookReminderStallTimeoutMs: number; private readonly subagentSettlementGraceMs: number; private readonly visualProofTimeoutMs: number; + private readonly visualProofAttemptStatePath: string; private readonly queuedPromptRetryDelayMs: number; private readonly providerRateLimitMaxRetries: number; private readonly providerRateLimitBaseDelayMs: number; @@ -1690,6 +1701,7 @@ export class OpenCodeServerHarness // turns can run on the built-in read-only `plan` agent. private activeWorkflowSkill: string | null = null; private visualProofTimeoutTimer: ReturnType | null = null; + private visualProofAttemptId: string | null = null; private commandEnv: Record | undefined; private stopHookReminderCount = 0; private terminalChatReplyDeliveryFailed = false; @@ -1757,6 +1769,8 @@ export class OpenCodeServerHarness options.subagentSettlementGraceMs ?? DEFAULT_SUBAGENT_SETTLEMENT_GRACE_MS; this.visualProofTimeoutMs = options.visualProofTimeoutMs ?? DEFAULT_VISUAL_PROOF_TIMEOUT_MS; + this.visualProofAttemptStatePath = + options.visualProofAttemptStatePath ?? VISUAL_PROOF_ATTEMPT_STATE_PATH; this.queuedPromptRetryDelayMs = options.queuedPromptRetryDelayMs ?? DEFAULT_QUEUED_PROMPT_RETRY_DELAY_MS; this.providerRateLimitMaxRetries = @@ -1958,6 +1972,7 @@ export class OpenCodeServerHarness this.clearQueuedPromptRetryTimer(); this.clearProviderErrorRecoveryState(); this.clearVisualProofTimeout(); + this.clearVisualProofAttemptState(); this.clearAllExecuteToolProgress(); void this.cleanupVisualAttachmentDirectories(); this.rejectEventStreamReady?.( @@ -2155,6 +2170,7 @@ export class OpenCodeServerHarness this.currentWorkflowPhase = null; this.activeWorkflowSkill = null; this.clearVisualProofTimeout(); + this.clearVisualProofAttemptState(); this.inFlight = false; this.prompts.clear(); this.clearQueuedPromptRetryTimer(); @@ -2216,6 +2232,7 @@ export class OpenCodeServerHarness this.currentWorkflowPhase = command.data.workflowPhase ?? null; this.activeWorkflowSkill = null; this.clearVisualProofTimeout(); + this.clearVisualProofAttemptState(); this.cancelRequestedBeforeSession = false; this.resetSessionCreateAbortController(); @@ -2379,6 +2396,7 @@ export class OpenCodeServerHarness private async handleCancelTask(command?: CancelTaskCommand): Promise { this.clearVisualProofTimeout(); + this.clearVisualProofAttemptState(); const sessionId = this.sessionId; if (!sessionId) { @@ -3712,6 +3730,25 @@ export class OpenCodeServerHarness return; } + this.visualProofAttemptId = randomUUID(); + try { + writeFileSync( + this.visualProofAttemptStatePath, + `${JSON.stringify({ + attemptId: this.visualProofAttemptId, + startedAt: new Date().toISOString(), + })}\n`, + { encoding: 'utf8', mode: 0o600 }, + ); + } catch (error) { + this.logger.warn( + `Failed to persist visual proof attempt state: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + this.visualProofAttemptId = null; + } + const timer = setTimeout(() => { this.visualProofTimeoutTimer = null; void this.recoverVisualProofTimeout(); @@ -3729,6 +3766,11 @@ export class OpenCodeServerHarness this.visualProofTimeoutTimer = null; } + private clearVisualProofAttemptState(): void { + this.visualProofAttemptId = null; + rmSync(this.visualProofAttemptStatePath, { force: true }); + } + private async recoverVisualProofTimeout(): Promise { if ( this.disposed || @@ -3744,7 +3786,7 @@ export class OpenCodeServerHarness // The replay is a parent-workflow continuation, not a new proof attempt. this.activeWorkflowSkill = null; const queuedId = this.prompts.enqueue({ - text: VISUAL_PROOF_TIMEOUT_RECOVERY_PROMPT, + text: formatVisualProofTimeoutRecoveryPrompt(this.visualProofAttemptId), visibleInTranscript: false, }); this.prompts.prioritize(queuedId); @@ -4248,6 +4290,7 @@ export class OpenCodeServerHarness this.pendingUserInputRequests.clear(); this.nativeQuestionRequestIds.clear(); this.clearAllExecuteToolProgress(); + this.clearVisualProofAttemptState(); this.runtimeEvents.taskAborted(sessionId); } @@ -5282,6 +5325,7 @@ export class OpenCodeServerHarness this.inFlight = false; this.finalizedAssistantTurn = null; this.clearAllExecuteToolProgress(); + this.clearVisualProofAttemptState(); if (this.prompts.hasQueuedMessages()) { await this.drainQueuedPrompts(); diff --git a/packages/cloud-agents/src/server/workflows/__tests__/captureVisualProofSkill.test.ts b/packages/cloud-agents/src/server/workflows/__tests__/captureVisualProofSkill.test.ts index a668b953d..bc4af7c12 100644 --- a/packages/cloud-agents/src/server/workflows/__tests__/captureVisualProofSkill.test.ts +++ b/packages/cloud-agents/src/server/workflows/__tests__/captureVisualProofSkill.test.ts @@ -42,10 +42,22 @@ describe('Capture visual proof skill', () => { it('keeps browser output out of the transcript and leaves image review to the judge', () => { expect(skillContent).toContain( - 'Write screenshots, recordings, and keyframes to files under `/tmp/capture-visual-proof/`, never print image bytes', + 'The capture directory is `/tmp/capture-visual-proof/` plus that value and a trailing slash', + ); + expect(skillContent).toContain('Never print or read image bytes.'); + }); + + it('uses attempt-scoped artifact paths for timeout recovery', () => { + expect(skillContent).toContain('/tmp/roomote-visual-proof-attempt.json'); + expect(skillContent).not.toMatch( + /(?:&(?:amp;)?lt;|�*60;|�*3c;)attemptId(?:&(?:amp;)?gt;|�*62;|�*3e;)/i, + ); + expect(skillContent).not.toContain(''); + expect(skillContent).toContain( + 'save all captures there for timeout recovery', ); expect(skillContent).toContain( - 'Do not read the captured images back yourself; the judge does that.', + 'Invalid state means `proof runtime unavailable`; do not upload.', ); }); diff --git a/packages/cloud-agents/src/server/workflows/__tests__/prDescriptionPromptScope.test.ts b/packages/cloud-agents/src/server/workflows/__tests__/prDescriptionPromptScope.test.ts index ad3c9f24b..d4876f0e1 100644 --- a/packages/cloud-agents/src/server/workflows/__tests__/prDescriptionPromptScope.test.ts +++ b/packages/cloud-agents/src/server/workflows/__tests__/prDescriptionPromptScope.test.ts @@ -183,6 +183,29 @@ describe('PR description prompt scope', () => { ); } + for (const skillContent of [ + createPrSkill, + createDraftPrSkill, + fixPrSkill, + ]) { + expect(skillContent).toContain( + 'read the actual `attemptId` value from `/tmp/roomote-visual-proof-attempt.json`, then call the Roomote MCP tool `mcp__roomote__manage_artifacts` with `action: "list"` and `artifactType: "visual-proof"`', + ); + expect(skillContent).toContain( + 'Build the expected artifact path prefix by appending that value and a trailing slash to `tmp/capture-visual-proof/`.', + ); + expect(skillContent).toContain( + 'use those records to recover uploads after a lost child result and refresh their signed URLs', + ); + expect(skillContent).not.toMatch( + /(?:&(?:amp;)?lt;|�*60;|�*3c;)attemptId(?:&(?:amp;)?gt;|�*62;|�*3e;)/i, + ); + expect(skillContent).not.toContain(''); + expect(skillContent).toContain( + 'delete `/tmp/roomote-visual-proof-attempt.json`', + ); + } + expect(createDraftPrSkill).toContain( 'HEAD` to capture the full PR diff for the branch. Use this local git diff for every provider.', ); diff --git a/packages/cloud-agents/src/server/workflows/skills/standard/capture-visual-proof/SKILL.md b/packages/cloud-agents/src/server/workflows/skills/standard/capture-visual-proof/SKILL.md index d2d88b080..7434d013f 100644 --- a/packages/cloud-agents/src/server/workflows/skills/standard/capture-visual-proof/SKILL.md +++ b/packages/cloud-agents/src/server/workflows/skills/standard/capture-visual-proof/SKILL.md @@ -17,7 +17,7 @@ You capture visual proof of a shipped change yourself. Decide whether browser pr Derive the browser target from the current environment instructions and use the exact sandbox-local browser URL they specify. Preserve the configured hostname literally: if the environment says `localhost`, use `localhost`; if it says `127.0.0.1`, use `127.0.0.1`. Do not capture proof against external preview URLs unless the public proxy or hostname itself is part of the claim. Capture proof in the real product surface where the behavior lives. Use Storybook only when the user asked for it, the change itself is Storybook-scoped, or the product surface is unreachable for infrastructure reasons and a checked-in story proves the same rendered claim. Label any such evidence `Storybook fallback proof`. Never use Storybook when the product surface is reachable and the shipped UI is failing the claim. `agent-browser` is a command-line executable invoked from the shell, and it is the only allowed browser automation path. Do not use Playwright, browser DevTools, curl-only screenshot substitutes, or any other browser automation path. Before the first browser command, load the `agent-browser` skill once with the Skill tool, or run `agent-browser skills get core --full` when that skill is unavailable, and follow that guidance for session, wait, snapshot, screenshot, and recording commands. -Keep browser output out of the transcript. Write screenshots, recordings, and keyframes to files under `/tmp/capture-visual-proof/`, never print image bytes, and use `agent-browser snapshot -i` only when you need to verify page state or find an element. Do not read the captured images back yourself; the judge does that. +Read the actual `attemptId` from `/tmp/roomote-visual-proof-attempt.json`. The capture directory is `/tmp/capture-visual-proof/` plus that value and a trailing slash; save all captures there for timeout recovery. Invalid state means `proof runtime unavailable`; do not upload. Never print or read image bytes. Before the first browser command, snapshot the complete shipped diff, committed and uncommitted, against the branch base. Initialize the snapshot file once: `mkdir -p /tmp/capture-visual-proof && : > /tmp/capture-visual-proof/diff-at-start.patch`. Then, from inside each repository (every repository in a shared-root workspace), append its diff: `git diff "$(git merge-base HEAD origin/HEAD 2>/dev/null || git rev-parse --verify -q HEAD~1 || git hash-object -t tree /dev/null)" >> /tmp/capture-visual-proof/diff-at-start.patch; git ls-files --others --exclude-standard -z | xargs -0 -I{} git diff --no-index -- /dev/null {} >> /tmp/capture-visual-proof/diff-at-start.patch; true`. Always append with `>>`; a second repository must never truncate the first repository's snapshot. The fallbacks cover a missing `origin/HEAD` and a single-commit history, and the second command records every untracked file as a full addition so a new source file is compared by content rather than listed by name. Do not snapshot only `git diff HEAD`: workflows such as `fix-pr` commit and push before this step, and an empty snapshot would make the judge flag every shipped file as drift. The judge compares that snapshot with the delivery diff. Any source change you make after the snapshot, whether for simulation or for a fix, must be listed in the `Simulation disclosure` section or reverted before this skill returns. Judge proof truthfulness by the claim being proved. Prefer genuine application, database, authentication, feature-flag, fixture, test-record, or form-submission state when it is practical to establish. When genuine state is difficult or impractical to reproduce, transparent simulation may modify application source, hardcode a condition, role, feature state, or network response, mock UI or network responses, or arrange DOM or rendered component state so the actual UI can be inspected. Inability to establish genuine state is not a proof blocker when such a simulation can exercise the relevant rendered UI. Every simulation, mock, source modification, or hardcoded state must be disclosed explicitly in each affected artifact's proof metadata and in the final proof report. State separately that the artifact proves only visual appearance, layout, or interaction under the disclosed simulated state and does not prove the real data flow, authorization, backend behavior, network integration, or end-to-end correctness. Remove temporary simulation changes before returning, and report any cleanup you could not complete under `Cleanup`. @@ -50,7 +50,7 @@ You capture visual proof of a shipped change yourself. Decide whether browser pr Snapshot the diff, load the `agent-browser` guidance, then open the configured browser target and confirm it is up with `agent-browser get url` or `agent-browser snapshot -i`. If the target is unreachable or the app is not ready, inspect the port or current HTTP response once, then return blocked with blocker type `browser surface unavailable` and the observed port state, HTTP response, or visible browser error. Do not loop on retries or improvise a different surface. Reach each checklist state through genuine setup or disclosed simulation. Make at most two focused attempts per state; then record that item as unproved. - Capture one artifact per checklist item, or one artifact that clearly shows several items together. For screencasts, start recording before the interaction that matters, stop as soon as the proof is visible, validate the clip with `ffprobe`, and extract 3 to 5 keyframes under `/tmp/capture-visual-proof/`. + Capture one artifact per checklist item, or one artifact that clearly shows several items together. For screencasts, start recording before the interaction that matters, stop as soon as the proof is visible, validate the clip with `ffprobe`, and extract 3 to 5 keyframes into the same attempt-specific capture directory. Recapture an artifact once when the first honest capture is obviously blank, clipped, or misses the required visible state. That is the only retry this skill allows. If you capture only partial supporting evidence and the remaining checklist items cannot be shown honestly, return the result as blocked with the covered and missing items instead of reporting a narrowed success. diff --git a/packages/cloud-agents/src/server/workflows/skills/standard/create-draft-pr/SKILL.md b/packages/cloud-agents/src/server/workflows/skills/standard/create-draft-pr/SKILL.md index 315d31762..216a016a6 100644 --- a/packages/cloud-agents/src/server/workflows/skills/standard/create-draft-pr/SKILL.md +++ b/packages/cloud-agents/src/server/workflows/skills/standard/create-draft-pr/SKILL.md @@ -159,11 +159,13 @@ You are executing a command to create a draft pull request with the current chan Collect the pull request number and URL returned by each successful `mcp__roomote__manage_source_control` result, and treat that tool result as the live pull request reference instead of treating the final message as proof that the pull request exists. After the first creation or refresh pass, if this run produced or refreshed more than one pull request for the same task, rebuild each PR body so it includes a `## Related PRs` section linking the sibling pull requests by repository or surface label, then call `mcp__roomote__manage_source_control` again for each sibling pull request to backfill those cross-links. When maintaining the `## Related PRs` section, omit self-links, keep only sibling pull requests from the current task split, and remove stale links to superseded or unrelated PRs. + After every repository's final PR metadata call has completed, delete `/tmp/roomote-visual-proof-attempt.json` so a later delivery cycle cannot reuse the consumed attempt marker. Do not delete it between repositories in a multi-repository delivery. Run `git diff $(git merge-base HEAD origin/ 2>/dev/null || echo "HEAD~1") HEAD` to capture the full PR diff for the branch. Use this local git diff for every provider. When an earlier delivery pass in this task produced a `/tmp/pr-body.md`, read it before overwriting it so still-applicable metadata can be recovered, including current `## Related PRs` links, `## Linked work items`, and proof sections; this workflow does not read the remote pull request body. Call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "get_messages"` for the current task using `limit: 20`. Reverse the returned newest-first message list before extracting the original problem statement, motivation, and key decisions from the conversation history. + Immediately before writing PR metadata, read the actual `attemptId` value from `/tmp/roomote-visual-proof-attempt.json`, then call the Roomote MCP tool `mcp__roomote__manage_artifacts` with `action: "list"` and `artifactType: "visual-proof"` for the current task. Build the expected artifact path prefix by appending that value and a trailing slash to `tmp/capture-visual-proof/`. Only listed artifacts whose paths start with that exact prefix belong to the attempt; use those records to recover uploads after a lost child result and refresh their signed URLs. When a timeout summary conflicts with matching current-attempt artifacts, include those artifacts and do not claim proof timed out. If the state file is missing or invalid, or no listed path matches the expected prefix, keep the latest honest blocked or no-op outcome instead of attaching artifacts from another proof attempt. Use the recovered conversation to choose which participant the pull request is opened on behalf of. Prefer the person who requested or explicitly authorized the implementation or PR creation; do not infer ownership from thread ownership, task initiation, or the latest comment alone. When one participant is clear, pass their conversation display name or source-control login as `prAttribution`. If the conversation is genuinely ambiguous, or the task has no human participants at all (for example an automation-started task with no conversation), omit `prAttribution` so the platform retains current acting-user attribution. Never invent a name from repository history, issue trackers, or prior tasks; only participants recorded in this task's conversation, the current acting user, or the task owner are eligible. Before writing `/tmp/pr-body.md`, check for a checked-in repository pull request or merge request template in the locations the repository's source-control provider supports. On GitHub, inspect `.github/pull_request_template.md`, `.github/PULL_REQUEST_TEMPLATE.md`, any `.md` files inside `.github/PULL_REQUEST_TEMPLATE/`, `docs/pull_request_template.md`, `docs/PULL_REQUEST_TEMPLATE.md`, `pull_request_template.md`, and `PULL_REQUEST_TEMPLATE.md`. On GitLab, inspect the `.md` files inside `.gitlab/merge_request_templates/`, preferring `Default.md` when present. On Gitea, inspect `.gitea/pull_request_template.md`, `.gitea/PULL_REQUEST_TEMPLATE.md`, any `.md` files inside `.gitea/PULL_REQUEST_TEMPLATE/`, and the same root-level and `docs/` fallbacks GitHub supports. On Azure DevOps, inspect `.azuredevops/pull_request_template.md`, any branch-specific `.md` files inside `.azuredevops/pull_request_template/branches/`, `docs/pull_request_template.md`, and root-level `pull_request_template.md`. Match the repo's actual filename casing when present. When multiple template files exist in the directory path, choose the single template that best matches the current PR scope and treat it as the selected repo template for this run. Write `/tmp/pr-body.md` using the PR diff, the recovered conversation, and any still-applicable metadata recovered from the previous `/tmp/pr-body.md`. When a selected repo template exists, use it as the starting scaffold for `/tmp/pr-body.md`: preserve its reviewer-facing headings, checklist items, and other required structure, replace placeholder guidance with final content, and merge the `pr-writing-guide` substance into that scaffold instead of replacing the template. When no repo template exists, structure the body per the `pr-writing-guide` section below. If the caller supplied a PR provenance block, make it the opening blockquote. Preserve or refresh `## Related PRs` when the previous `/tmp/pr-body.md` or current task context identifies sibling PRs. Preserve or refresh `## Linked work items` when the previous `/tmp/pr-body.md` or current workflow instructions identify linked work items. When the current workflow instructions include a pre-rendered linked-work-item block for this run, include that block verbatim and do not rewrite provider-specific closing or reference syntax. When the latest `capture-visual-proof` handoff reports an uploaded artifact list from `manage_artifacts` upload results, treat it as the authoritative proof-section input: render `## Screenshots` from its reported screenshots only when present, embedding each screenshot as `![]()` so the image renders inline in the PR body; do not create `## Visual proof` for screenshots and do not render screenshot artifact viewer links when `rawUrl` exists; render `## Screencasts` from its reported screencasts only when present using `[![]()]()`, plus a caption line below each embed, where `` is the clip's uploaded `viewUrl`, and explicitly remove any existing `## Screenshots` or `## Screencasts` section whose latest reported set is empty so stale evidence is not preserved. When that uploaded artifact list does not exist and the latest proof handoff is an honest no-op result because this cycle did not run `capture-visual-proof`, preserve any existing `## Screenshots` and `## Screencasts` sections from the previous `/tmp/pr-body.md` when they already contain valid artifact URLs or screencast embeds. When that uploaded artifact list does not exist and the latest proof handoff reports that browser proof is not applicable, that screenshots and screencasts are unnecessary, or that capture is blocked, explicitly remove any existing `## Screenshots` and `## Screencasts` sections instead of preserving stale proof from an earlier cycle. Only when that uploaded artifact list does not exist and screenshot `rawUrl` values are still available from the latest proof handoff should the screenshot-only fallback include `## Screenshots`, embedding each screenshot as `![]()` so the image renders inline in the PR body; do not create `## Visual proof` for screenshots and do not render screenshot artifact viewer links when `rawUrl` exists. When no previous `/tmp/pr-body.md` exists, there is no prior body to preserve, so include proof sections only when current-cycle proof links are available and include `## Linked work items` only when the current workflow instructions provide one. diff --git a/packages/cloud-agents/src/server/workflows/skills/standard/create-pr/SKILL.md b/packages/cloud-agents/src/server/workflows/skills/standard/create-pr/SKILL.md index d534cc75a..ef101266a 100644 --- a/packages/cloud-agents/src/server/workflows/skills/standard/create-pr/SKILL.md +++ b/packages/cloud-agents/src/server/workflows/skills/standard/create-pr/SKILL.md @@ -159,11 +159,13 @@ You are executing a command to create a pull request with the current changes. Collect the pull request number and URL returned by each successful `mcp__roomote__manage_source_control` result, and treat that tool result as the live pull request reference instead of treating the final message as proof that the pull request exists. After the first creation or refresh pass, if this run produced or refreshed more than one pull request for the same task, rebuild each PR body so it includes a `## Related PRs` section linking the sibling pull requests by repository or surface label, then call `mcp__roomote__manage_source_control` again for each sibling pull request to backfill those cross-links. When maintaining the `## Related PRs` section, omit self-links, keep only sibling pull requests from the current task split, and remove stale links to superseded or unrelated PRs. + After every repository's final PR metadata call has completed, delete `/tmp/roomote-visual-proof-attempt.json` so a later delivery cycle cannot reuse the consumed attempt marker. Do not delete it between repositories in a multi-repository delivery. Run `git diff $(git merge-base HEAD origin/ 2>/dev/null || echo "HEAD~1") HEAD` to capture the full shipped diff for the branch. Use this local git diff for every provider. When an earlier delivery pass in this task produced a `/tmp/pr-body.md`, read it before overwriting it so still-applicable metadata can be recovered, including current `## Related PRs` links, `## Linked work items`, and proof sections; this workflow does not read the remote pull request body. Call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "get_messages"` for the current task using `limit: 20`. Reverse the returned newest-first message list before extracting the original problem statement, motivation, and key decisions from the conversation history. + Immediately before writing PR metadata, read the actual `attemptId` value from `/tmp/roomote-visual-proof-attempt.json`, then call the Roomote MCP tool `mcp__roomote__manage_artifacts` with `action: "list"` and `artifactType: "visual-proof"` for the current task. Build the expected artifact path prefix by appending that value and a trailing slash to `tmp/capture-visual-proof/`. Only listed artifacts whose paths start with that exact prefix belong to the attempt; use those records to recover uploads after a lost child result and refresh their signed URLs. When a timeout summary conflicts with matching current-attempt artifacts, include those artifacts and do not claim proof timed out. If the state file is missing or invalid, or no listed path matches the expected prefix, keep the latest honest blocked or no-op outcome instead of attaching artifacts from another proof attempt. Use the recovered conversation to choose which participant the pull request is opened on behalf of. Prefer the person who requested or explicitly authorized the implementation or PR creation; do not infer ownership from thread ownership, task initiation, or the latest comment alone. When one participant is clear, pass their conversation display name or source-control login as `prAttribution`. If the conversation is genuinely ambiguous, or the task has no human participants at all (for example an automation-started task with no conversation), omit `prAttribution` so the platform retains current acting-user attribution. Never invent a name from repository history, issue trackers, or prior tasks; only participants recorded in this task's conversation, the current acting user, or the task owner are eligible. Before writing `/tmp/pr-body.md`, check for a checked-in repository pull request or merge request template in the locations the repository's source-control provider supports. On GitHub, inspect `.github/pull_request_template.md`, `.github/PULL_REQUEST_TEMPLATE.md`, any `.md` files inside `.github/PULL_REQUEST_TEMPLATE/`, `docs/pull_request_template.md`, `docs/PULL_REQUEST_TEMPLATE.md`, `pull_request_template.md`, and `PULL_REQUEST_TEMPLATE.md`. On GitLab, inspect the `.md` files inside `.gitlab/merge_request_templates/`, preferring `Default.md` when present. On Gitea, inspect `.gitea/pull_request_template.md`, `.gitea/PULL_REQUEST_TEMPLATE.md`, any `.md` files inside `.gitea/PULL_REQUEST_TEMPLATE/`, and the same root-level and `docs/` fallbacks GitHub supports. On Azure DevOps, inspect `.azuredevops/pull_request_template.md`, any branch-specific `.md` files inside `.azuredevops/pull_request_template/branches/`, `docs/pull_request_template.md`, and root-level `pull_request_template.md`. Match the repo's actual filename casing when present. When multiple template files exist in the directory path, choose the single template that best matches the current PR scope and treat it as the selected repo template for this run. Write `/tmp/pr-body.md` using the shipped diff, the recovered conversation, and any still-applicable metadata recovered from the previous `/tmp/pr-body.md`. When a selected repo template exists, use it as the starting scaffold for `/tmp/pr-body.md`: preserve its reviewer-facing headings, checklist items, and other required structure, replace placeholder guidance with final content, and merge the `pr-writing-guide` substance into that scaffold instead of replacing the template. When no repo template exists, structure the body per the `pr-writing-guide` section below. If the caller supplied a PR provenance block, make it the opening blockquote. Preserve or refresh `## Related PRs` when the previous `/tmp/pr-body.md` or current task context identifies sibling PRs. Preserve or refresh `## Linked work items` when the previous `/tmp/pr-body.md` or current workflow instructions identify linked work items. When the current workflow instructions include a pre-rendered linked-work-item block for this run, include that block verbatim and do not rewrite provider-specific closing or reference syntax. When the latest `capture-visual-proof` handoff reports an uploaded artifact list from `manage_artifacts` upload results, treat it as the authoritative proof-section input: render `## Screenshots` from its reported screenshots only when present, embedding each screenshot as `![]()` so the image renders inline in the PR body; do not create `## Visual proof` for screenshots and do not render screenshot artifact viewer links when `rawUrl` exists; render `## Screencasts` from its reported screencasts only when present using `[![]()]()`, plus a caption line below each embed, where `` is the clip's uploaded `viewUrl`, and explicitly remove any existing `## Screenshots` or `## Screencasts` section whose latest reported set is empty so stale evidence is not preserved. When that uploaded artifact list does not exist and the latest proof handoff is an honest no-op result because this cycle did not run `capture-visual-proof`, preserve any existing `## Screenshots` and `## Screencasts` sections from the previous `/tmp/pr-body.md` when they already contain valid artifact URLs or screencast embeds. When that uploaded artifact list does not exist and the latest proof handoff reports that browser proof is not applicable, that screenshots and screencasts are unnecessary, or that capture is blocked, explicitly remove any existing `## Screenshots` and `## Screencasts` sections instead of preserving stale proof from an earlier cycle. Only when that uploaded artifact list does not exist and screenshot `rawUrl` values are still available from the latest proof handoff should the screenshot-only fallback include `## Screenshots`, embedding each screenshot as `![]()` so the image renders inline in the PR body; do not create `## Visual proof` for screenshots and do not render screenshot artifact viewer links when `rawUrl` exists. When no previous `/tmp/pr-body.md` exists, there is no prior body to preserve, so include proof sections only when current-cycle proof links are available and include `## Linked work items` only when the current workflow instructions provide one. diff --git a/packages/cloud-agents/src/server/workflows/skills/standard/fix-pr/SKILL.md b/packages/cloud-agents/src/server/workflows/skills/standard/fix-pr/SKILL.md index 793ab9436..250b3e776 100644 --- a/packages/cloud-agents/src/server/workflows/skills/standard/fix-pr/SKILL.md +++ b/packages/cloud-agents/src/server/workflows/skills/standard/fix-pr/SKILL.md @@ -167,6 +167,7 @@ You are a pull-request fixer. Resolve requested PR feedback in code, keep the re When an open pull request exists, capture the full shipped diff for the branch locally with `git fetch origin ''` and `git diff ...HEAD`, using the base SHA and branches from the latest `get_pull_request` result. Use this local git diff for every provider. When an open pull request exists, recover existing PR-body metadata that still applies, including current `## Related PRs` links, from the `body` field of the latest `get_pull_request` result. Call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "get_messages"` for the current task using `limit: 20`. Reverse the returned newest-first message list before extracting the original problem statement, motivation, and key decisions from the conversation history. + Immediately before writing PR metadata, read the actual `attemptId` value from `/tmp/roomote-visual-proof-attempt.json`, then call the Roomote MCP tool `mcp__roomote__manage_artifacts` with `action: "list"` and `artifactType: "visual-proof"` for the current task. Build the expected artifact path prefix by appending that value and a trailing slash to `tmp/capture-visual-proof/`. Only listed artifacts whose paths start with that exact prefix belong to the attempt; use those records to recover uploads after a lost child result and refresh their signed URLs. When a timeout summary conflicts with matching current-attempt artifacts, include those artifacts and do not claim proof timed out. If the state file is missing or invalid, or no listed path matches the expected prefix, keep the latest honest blocked or no-op outcome instead of attaching artifacts from another proof attempt. Use the recovered conversation to choose which participant the pull request is opened on behalf of. Prefer the person who requested or explicitly authorized the implementation or PR creation; do not infer ownership from thread ownership, task initiation, or the latest comment alone. When one participant is clear, pass their conversation display name or source-control login as `prAttribution`. If the conversation is genuinely ambiguous, or the task has no human participants at all (for example an automation-started task with no conversation), omit `prAttribution` so the platform retains current acting-user attribution. Never invent a name from repository history, issue trackers, or prior tasks; only participants recorded in this task's conversation, the current acting user, or the task owner are eligible. Before writing `/tmp/pr-body.md`, check for a checked-in repository pull request or merge request template in the locations the repository's source-control provider supports. On GitHub, inspect `.github/pull_request_template.md`, `.github/PULL_REQUEST_TEMPLATE.md`, any `.md` files inside `.github/PULL_REQUEST_TEMPLATE/`, `docs/pull_request_template.md`, `docs/PULL_REQUEST_TEMPLATE.md`, `pull_request_template.md`, and `PULL_REQUEST_TEMPLATE.md`. On GitLab, inspect the `.md` files inside `.gitlab/merge_request_templates/`, preferring `Default.md` when present. On Gitea, inspect `.gitea/pull_request_template.md`, `.gitea/PULL_REQUEST_TEMPLATE.md`, any `.md` files inside `.gitea/PULL_REQUEST_TEMPLATE/`, and the same root-level and `docs/` fallbacks GitHub supports. On Azure DevOps, inspect `.azuredevops/pull_request_template.md`, any branch-specific `.md` files inside `.azuredevops/pull_request_template/branches/`, `docs/pull_request_template.md`, and root-level `pull_request_template.md`. Match the repo's actual filename casing when present. When multiple template files exist in the directory path, choose the single template that best matches the current PR scope and treat it as the selected repo template for this run. Write `/tmp/pr-body.md` using the shipped diff, the recovered conversation, and the `pr-writing-guide` section below. When a selected repo template exists, use it as the starting scaffold for `/tmp/pr-body.md`: preserve its reviewer-facing headings, checklist items, and other required structure, replace placeholder guidance with final content, and merge the `pr-writing-guide` substance into that scaffold instead of replacing the template. When no repo template exists, structure the body per the `pr-writing-guide` section below. If the current PR body is non-contract relative to the selected repo template or the fallback Roomote contract, rebuild it from the final shipped diff instead of preserving unrelated old sections from that body. @@ -177,6 +178,7 @@ You are a pull-request fixer. Resolve requested PR feedback in code, keep the re Derive a refreshed PR title per the `pr-writing-guide` section below. Before the refresh call, validate the exact title and `/tmp/pr-body.md` against the PR writing guide. The title must begin with exactly one approved bracketed type tag. When a selected repo template exists, the body must preserve the template's reviewer-facing headings, checklist items, and required structure, replace placeholder guidance with final content, and cover the same reviewer substance the `pr-writing-guide` requires without forcing Roomote-only headings that the template does not use. When no repo template exists, the body must include `## What changed`, `## Why this change was made`, and `## Impact`. Do not add legacy top-level sections such as `## Summary`, `## Changes`, `## Validation`, `## Checks`, or `## Status` for routine successful runs unless the selected repo template explicitly requires them. Treat this as a hard gate: if the metadata fails, rewrite it and re-check before running the source-control mutation. When the latest `get_pull_request` result confirms the pull request is open, refresh it with `mcp__roomote__manage_source_control` `action: "create_or_update_pull_request"`, passing `repositoryFullName`, `sourceBranch` and `targetBranch` from that result, the refreshed `title`, `body` set to the exact `/tmp/pr-body.md` contents, and `prAttribution` when the conversation establishes a clear participant. The refresh never flips draft status: the platform preserves the pull request's existing draft state on update. + After the PR metadata refresh completes, delete `/tmp/roomote-visual-proof-attempt.json` so a later fixer cycle cannot reuse the consumed attempt marker. When no open pull request exists for the branch, report a blocker instead of refreshing: `create_or_update_pull_request` would open a new pull request, and `fix-pr` must never create one. Verify the open state from `get_pull_request` before calling the tool.