diff --git a/.agent/context/20260913T120000Z-session-details-resume-result.md b/.agent/context/20260913T120000Z-session-details-resume-result.md index 93fb784..3d1d655 100644 --- a/.agent/context/20260913T120000Z-session-details-resume-result.md +++ b/.agent/context/20260913T120000Z-session-details-resume-result.md @@ -17,30 +17,30 @@ Clone `SWOFART/OneShot`, create a branch from `develop`, fix the session-details - The project URL is `https://github.com/SWOFART/OneShot.git`, identified from the user’s already-open GitHub tab. - The requested branch name was not specified; use `fix/session-details-resume-result`. - The resume endpoint is asynchronous (`202 Accepted`), so the UI must observe the durable job state after enqueueing rather than treating the first pending response as final. -- A user report confirmed that a single one-second full-list refresh is too early: the API can legitimately remain `PENDING` while the supplier worker finishes. Follow-up reads must target the same `job_id`, not list the whole workspace. +- A user report confirmed that automatic follow-up reads create unwanted network traffic while the API remains `PENDING`. The UI therefore leaves delivery observation to the existing manual `Refresh requests` action. - The payment proof view remains read-only; result retrieval must never submit a payment. ## Plan 1. Inspect the web components, API client, styles, and existing tests. 2. Add wallet-specific copy behavior and session-details layout styles. -3. Make resume read the same job until the API reports `result`/`AVAILABLE`, with bounded timeout/error handling, and add regression tests. +3. Keep pending supplier delivery explicit, remove the non-working resume button and automatic polling from the web UI, and add regression coverage for manual refresh. 4. Run focused web checks and broader repository checks as feasible. ## Key decisions - Keep copy-session-ID behavior and add a separate wallet-address copy button so both identifiers remain available. -- Send one `POST /v1/jobs/:jobId/resume`, then read only `GET /v1/jobs/:jobId` until the durable job reports a result or the bounded wait expires. Do not add payment or result endpoint behavior that could bypass durable state. +- Do not map settlement proof to supplier-result readiness. `Settlement READY` means authorization/submission ownership; `Result ready` requires the job's durable `result` payload. +- Remove the web resume control and automatic job-status polling. Use the existing explicit `GET /v1/jobs` manual refresh to observe whether the worker has completed delivery. - A response with `payment_state: COMMITTED` and `delivery_state: PENDING` proves payment only; it is not evidence that a supplier result is ready. The UI must not fabricate `Result ready` for that response. ## Files/components touched - `apps/web/src/components/LoginGate.tsx` - separate session-ID and wallet-address copy actions, with a disabled state when no wallet is connected. -- `apps/web/src/api/job-client.ts` - adds a read-only single-job status method for asynchronous delivery observation. -- `apps/web/src/components/JobWorkspace.tsx` - observes the same job until a result is available or retrieval fails/times out, without repeatedly listing all jobs. +- `apps/web/src/components/JobWorkspace.tsx` - removes the non-working resume button and automatic polling; the existing manual refresh remains the only list read. - `apps/web/src/styles.css` - wallet address gets the DID pill treatment and a spaced wallet block. -- `apps/web/test/login-gate.test.tsx`, `apps/web/test/components.test.tsx`, and `apps/web/test/job-client.test.ts` - regression coverage for clipboard behavior, exact-one resume, bounded single-job reads, and async result availability. -- `apps/web/browser/p5.spec.ts` - serves the single-job read endpoint in browser acceptance mocks. +- `apps/web/test/login-gate.test.tsx`, `apps/web/test/components.test.tsx`, and `apps/web/test/job-client.test.ts` - regression coverage for clipboard behavior and explicit pending delivery without a resume action. +- `apps/web/browser/p5.spec.ts` - models `PENDING` on initial load and `AVAILABLE` only after a manual refresh; it asserts no resume request is sent. ## Commands/checks @@ -61,6 +61,12 @@ Clone `SWOFART/OneShot`, create a branch from `develop`, fix the session-details - Follow-up `pnpm --filter @oneshot/web typecheck` - passed. - Follow-up `pnpm --filter @oneshot/web test:browser` - passed, 8 browser tests. - Follow-up `pnpm format:check` - passed after Prettier formatting. +- Latest focused web test after removing resume polling - passed, 22 tests. +- Latest full web test after removing resume polling - passed, 17 files / 95 tests. +- Latest `pnpm --filter @oneshot/web lint` - passed. +- Latest `pnpm --filter @oneshot/web typecheck` - passed. +- Latest `pnpm --filter @oneshot/web test:browser` - passed, 8 browser tests. +- Latest `pnpm format:check` - passed. - `pnpm lint` - passed. - `pnpm build` - passed. - `pnpm test` - passed, 80 files / 1,053 tests. @@ -78,9 +84,9 @@ Clone `SWOFART/OneShot`, create a branch from `develop`, fix the session-details - Branch: `fix/session-details-resume-result` - Base: `develop` at `d671af1da36878b7aaafcfc3049bc958e0daeb34` -- Previous commit: `26dfab5989f39c111d22f626b69a3b38d17a197e` -- Follow-up fix: uncommitted; candidate changes are not yet staged. -- PR: [#135](https://github.com/SWOFART/OneShot/pull/135), open against `develop`, currently at the previous commit. +- Previous commit: `1824728d594a04a797fa1447acd1a3f9edc7fd3e` +- Latest UI change: uncommitted; candidate changes are not yet staged. +- PR: [#136](https://github.com/SWOFART/OneShot/pull/136), open against `develop`, currently at the previous commit. - CI: not run for the follow-up candidate yet. ## Review gates diff --git a/apps/web/browser/p5.spec.ts b/apps/web/browser/p5.spec.ts index a0c7aba..81eac06 100644 --- a/apps/web/browser/p5.spec.ts +++ b/apps/web/browser/p5.spec.ts @@ -5,7 +5,7 @@ import { expect, test, type Page, type Route } from '@playwright/test'; const JOB_ID = `job_${'a'.repeat(64)}`; const INTENT_ID = `intent_${'b'.repeat(64)}`; -function job(deliveryState: 'RETRIEVAL_FAILED' | 'AVAILABLE' = 'RETRIEVAL_FAILED') { +function job(deliveryState: 'PENDING' | 'RETRIEVAL_FAILED' | 'AVAILABLE' = 'RETRIEVAL_FAILED') { return { job_id: JOB_ID, task_key: 'report-browser-acme', @@ -47,32 +47,23 @@ async function json(route: Route, status: number, body: unknown): Promise await route.fulfill({ status, contentType: 'application/json', body: JSON.stringify(body) }); } -async function mockJobApi( - page: Page, - options: { readonly startDelayMs?: number; readonly resumeDelayMs?: number } = {}, -): Promise { +async function mockJobApi(page: Page): Promise { const calls: string[] = []; - let current = job(); + let current = job('PENDING'); + let listReads = 0; await page.route('**/health/ready', (route) => json(route, 200, { status: 'ok' })); await page.route('**/v1/**', async (route) => { const request = route.request(); const pathname = new URL(request.url()).pathname; calls.push(`${request.method()} ${pathname}`); - if (pathname === '/v1/jobs' && request.method() === 'GET') + if (pathname === '/v1/jobs' && request.method() === 'GET') { + listReads += 1; + if (listReads > 1) current = job('AVAILABLE'); return json(route, 200, { jobs: [current] }); - if (pathname === `/v1/jobs/${JOB_ID}` && request.method() === 'GET') - return json(route, 200, current); + } if (pathname === '/v1/jobs/quote' && request.method() === 'POST') return json(route, 200, current.supplier); if (pathname === '/v1/jobs' && request.method() === 'POST') { - if (options.startDelayMs) - await new Promise((resolve) => setTimeout(resolve, options.startDelayMs)); - return json(route, 202, current); - } - if (pathname === `/v1/jobs/${JOB_ID}/resume` && request.method() === 'POST') { - if (options.resumeDelayMs) - await new Promise((resolve) => setTimeout(resolve, options.resumeDelayMs)); - current = job('AVAILABLE'); return json(route, 202, current); } if (pathname === '/v1/activity' && request.method() === 'GET') { @@ -176,7 +167,7 @@ for (const theme of ['light', 'dark'] as const) { test.setTimeout(60_000); await page.setViewportSize({ width, height: 1000 }); await page.addInitScript((value) => localStorage.setItem('oneshot.theme', value), theme); - await mockJobApi(page); + const calls = await mockJobApi(page); const checkContrast = async () => { await page.addScriptTag({ content: axe.source }); const violations = await page.evaluate(async () => { @@ -252,8 +243,11 @@ for (const theme of ['light', 'dark'] as const) { await expect(page.getByRole('heading', { name: 'Review before approval' })).toBeVisible(); } if (label === 'Requests') { - await page.getByRole('button', { name: 'Resume result (no new payment)' }).click(); + await expect(page.getByText('Retrieving result')).toBeVisible(); + expect(calls).not.toContain(`POST /v1/jobs/${JOB_ID}/resume`); + await page.getByRole('button', { name: 'Refresh requests' }).click(); await expect(page.getByText('Recovered original supplier report.')).toBeVisible(); + expect(calls.filter((call) => call === 'GET /v1/jobs')).toHaveLength(2); const results = await page .getByRole('region', { name: 'Requests and results' }) .boundingBox(); diff --git a/apps/web/src/api/job-client.ts b/apps/web/src/api/job-client.ts index f65cd47..edc3aa8 100644 --- a/apps/web/src/api/job-client.ts +++ b/apps/web/src/api/job-client.ts @@ -52,15 +52,6 @@ export class JobApiClient { return response.ok ? ((await responseJson(response))?.jobs ?? []) : []; } - async get(jobId: string): Promise { - const response = await this.#fetch(`${this.#baseUrl}/v1/jobs/${encodeURIComponent(jobId)}`, { - headers: this.#headers(), - }); - const body = await responseJson(response); - if (!response.ok || !body) throw new Error('Could not load the job status'); - return body; - } - async start(request: CreateJobRequest): Promise { const response = await this.#fetch(`${this.#baseUrl}/v1/jobs`, { method: 'POST', @@ -108,19 +99,6 @@ export class JobApiClient { return body; } - async resume(jobId: string): Promise { - const response = await this.#fetch( - `${this.#baseUrl}/v1/jobs/${encodeURIComponent(jobId)}/resume`, - { - method: 'POST', - headers: this.#headers(), - }, - ); - const body = await responseJson(response); - if (!response.ok || !body) throw new Error('Could not resume supplier delivery'); - return body; - } - async result(jobId: string): Promise { const response = await this.#fetch( `${this.#baseUrl}/v1/jobs/${encodeURIComponent(jobId)}/result`, diff --git a/apps/web/src/components/JobWorkspace.tsx b/apps/web/src/components/JobWorkspace.tsx index 11744dc..cd7c950 100644 --- a/apps/web/src/components/JobWorkspace.tsx +++ b/apps/web/src/components/JobWorkspace.tsx @@ -37,38 +37,12 @@ function explorerHref(transactionHash: string | undefined): string | undefined { const USER_WALLET_PAYMENT_CHECK_DELAY_MS = 500; const USER_WALLET_PAYMENT_CHECK_ATTEMPTS = 30; -const RESULT_REFRESH_DELAY_MS = 1000; -const RESULT_REFRESH_ATTEMPTS = 15; - function waitForPaymentCheck(): Promise { return new Promise((resolve) => { window.setTimeout(resolve, USER_WALLET_PAYMENT_CHECK_DELAY_MS); }); } -function waitForResultRefresh(): Promise { - return new Promise((resolve) => { - window.setTimeout(resolve, RESULT_REFRESH_DELAY_MS); - }); -} - -async function waitForSupplierResult( - client: JobApiClient, - jobId: string, - initial: JobView, -): Promise { - let latest = initial; - for ( - let attempt = 0; - attempt < RESULT_REFRESH_ATTEMPTS && latest.delivery_state === 'PENDING' && !latest.result; - attempt += 1 - ) { - await waitForResultRefresh(); - latest = await client.get(jobId); - } - return latest; -} - async function resolveUserWalletPayment( client: JobApiClient, jobId: string, @@ -487,53 +461,21 @@ export function JobList(props: { const [requests, setRequests] = useState([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(''); - const [resumingJobId, setResumingJobId] = useState(null); const [checkingPaymentJobId, setCheckingPaymentJobId] = useState(null); - async function refresh(): Promise { + async function refresh(): Promise { setLoading(true); try { const listed = await props.client.list(); setRequests(listed); setError(''); - return listed; } catch { setError('Requests could not be loaded. Check API readiness and your workspace session.'); - return null; } finally { setLoading(false); } } - async function resume(jobId: string): Promise { - setResumingJobId(jobId); - setError(''); - try { - const resumed = await props.client.resume(jobId); - setRequests((current) => - current.map((job) => (job.job_id === jobId ? resumed : job)), - ); - const latest = - resumed.result || resumed.delivery_state !== 'PENDING' - ? resumed - : await waitForSupplierResult(props.client, jobId, resumed); - setRequests((current) => - current.map((job) => (job.job_id === jobId ? latest : job)), - ); - if (!latest.result) { - setError( - latest.delivery_state === 'RETRIEVAL_FAILED' - ? 'The supplier result could not be retrieved. No new payment was submitted.' - : 'The supplier result is still being retrieved. Refresh requests to check again. No new payment was submitted.', - ); - } - } catch { - setError('The result could not be resumed. No new payment was submitted.'); - } finally { - setResumingJobId(null); - } - } - async function checkRecordedPayment(job: JobView): Promise { const transactionHash = job.user_payment?.transaction_hash; if (job.payment_mode !== 'USER_WALLET' || !transactionHash) return; @@ -557,7 +499,7 @@ export function JobList(props: {
@@ -567,7 +509,7 @@ export function JobList(props: { - ) : null} + )} {job.payment_mode === 'USER_WALLET' && job.payment_state === 'UNKNOWN' && job.user_payment?.transaction_hash && ( @@ -651,7 +582,7 @@ export function JobList(props: { type="button" className="secondary compact" disabled={ - loading || resumingJobId !== null || checkingPaymentJobId !== null + loading || checkingPaymentJobId !== null } onClick={() => void checkRecordedPayment(job)} > diff --git a/apps/web/test/components.test.tsx b/apps/web/test/components.test.tsx index f262e97..137923a 100644 --- a/apps/web/test/components.test.tsx +++ b/apps/web/test/components.test.tsx @@ -181,41 +181,17 @@ describe('IntentStatusView', () => { }); describe('JobWorkspace payment inputs', () => { - it('reads the resumed job until the supplier result is available without listing jobs repeatedly', async () => { - const user = userEvent.setup(); + it('keeps a pending paid delivery explicit without a resume action', async () => { const pendingJob = resumableJob('PENDING'); - const availableJob: JobView = { - ...pendingJob, - delivery_state: 'AVAILABLE', - result: { - order_reference: pendingJob.supplier.order_reference, - result_reference: 'team_report_result_resume', - report: 'Recovered original supplier report.', - }, - }; - let getCalls = 0; const client = { list: vi.fn(async () => [pendingJob]), - get: vi.fn(async () => { - getCalls += 1; - return getCalls === 1 ? pendingJob : availableJob; - }), - resume: vi.fn(async () => pendingJob), }; render( undefined} />); - await user.click(await screen.findByRole('button', { name: 'Resume result (no new payment)' })); - - await waitFor( - () => expect(screen.getByText('Recovered original supplier report.')).toBeTruthy(), - { timeout: 5000 }, - ); - expect(client.resume).toHaveBeenCalledWith(pendingJob.job_id); - expect(client.resume).toHaveBeenCalledTimes(1); + expect(await screen.findByText('Retrieving result')).toBeTruthy(); + expect(screen.queryByRole('button', { name: /Resume result/u })).toBeNull(); expect(client.list).toHaveBeenCalledTimes(1); - expect(getCalls).toBe(2); - expect(screen.getByText('Result ready:')).toBeTruthy(); }); it('sends the entered recipient and integer atomic amount to the quote boundary', async () => { diff --git a/apps/web/test/job-client.test.ts b/apps/web/test/job-client.test.ts index 876b89f..351f64f 100644 --- a/apps/web/test/job-client.test.ts +++ b/apps/web/test/job-client.test.ts @@ -22,23 +22,6 @@ const quote: SupplierQuote = { }; describe('JobApiClient quote flow', () => { - it('reads one job without listing the workspace jobs', async () => { - let calledUrl = ''; - const job = { job_id: 'job-1', delivery_state: 'PENDING' }; - const client = new JobApiClient({ - fetchFn: async (input) => { - calledUrl = String(input); - return new Response(JSON.stringify(job), { - status: 200, - headers: { 'content-type': 'application/json' }, - }); - }, - }); - - await expect(client.get('job-1')).resolves.toEqual(job); - expect(calledUrl).toBe('/v1/jobs/job-1'); - }); - it('requests a non-chargeable quote with the authenticated task payload', async () => { let calledUrl = ''; let calledBody = '';