diff --git a/.agent/context/20260913T170000Z-pending-delivery-refresh.md b/.agent/context/20260913T170000Z-pending-delivery-refresh.md new file mode 100644 index 0000000..7d76869 --- /dev/null +++ b/.agent/context/20260913T170000Z-pending-delivery-refresh.md @@ -0,0 +1,101 @@ +# Session Context: pending delivery refresh + +## Date/time + +- UTC: 2026-09-13T17:00:00Z + +## User goal + +A request whose payment has settled kept showing "Retrieving result" even +though the supplier result was already available, intermittently and without a +clear trigger. Make the list reflect the delivery that has actually completed. + +## Original prompt/request + +"we need another fix. After transaction is done it still showing retrieving +result (picture 1), when results logically should be ready, like in picture 2. +Sometimes it works, sometimes it doesnt, i dont know scenarios, but right now on +our latest operation we dont need retrieving results, bc results ARE ready." +Follow-up: skip Gate A and Gate B, open the pull request as a draft. + +## Assumptions + +- The reported screenshots show one list read: the newest request is `PENDING` + while an older one is `AVAILABLE`, which is a stale snapshot of a delivery + still in flight rather than a rendering fault. +- Bounded read-only re-reads are an acceptable middle ground against the + earlier report that automatic follow-up reads created unwanted traffic. +- No live payment or deployment is involved. + +## Plan + +1. Commit, push, and open a draft pull request against `develop`. + +## Key decisions + +- `JobList` re-reads `GET /v1/jobs` while any delivery is `PENDING`: every four + seconds, fifteen attempts, then it stops. It never calls the resume endpoint + and never submits a payment, so the at-most-once settlement invariant is + untouched. +- The re-reads are quiet: they do not raise the loading flag, because the + spinner, the disabled refresh button, and the `.tab-fade` remount all belong + to a read the operator asked for. +- This partially reverses commit `a2903a1`, which removed automatic polling + after a report of unwanted traffic. The bound and the pending-only condition + are what keep both reports satisfied; the resume control it removed stays + removed. +- A delivery stranded in `PENDING` server-side is out of scope here and is + recorded below as an unresolved risk. + +## Files/components touched + +- `apps/web/src/components/JobWorkspace.tsx` - bounded, quiet re-reads of the + request list while a delivery is pending. +- `apps/web/test/components.test.tsx` - the re-read reaches `Result ready`, + stops once nothing is pending, and gives up on a delivery that stays pending. +- `apps/web/browser/p5.spec.ts` - the read count is now a lower bound, because + an exact count would flake once a pending delivery is re-read on a timer. + +## Commands/checks + +- `pnpm format:check` - PASS +- `pnpm lint` - PASS +- `pnpm typecheck` - PASS +- `pnpm test` - PASS, 81 files / 1057 tests, includes build. The root vitest + config covers no `.tsx` file, so the command below is the one that exercises + these components. +- `pnpm --filter @oneshot/web test` - PASS, 17 files / 97 tests +- `pnpm test:browser` - PASS, 8 tests +- Local Node is 24.20.0 against the pinned 24.19.0; CI must validate the + pinned runtime. + +## External-doc findings + +- None. No version-sensitive integration changed. + +## Unresolved questions + +- A delivery that is stranded in `PENDING` cannot be recovered: `resumeDelivery` + re-queues only `NOT_REQUESTED` or `RETRIEVAL_FAILED`, and the web resume + control was removed, so a `PENDING` job whose outbox row was already consumed + has no path forward. The bounded re-reads give up on such a job rather than + fixing it. This needs a separate backend change. + +## Git and PR state + +- Branch: fix/pending-delivery-refresh +- Base: develop (68626d26bd0ddb7a39dda2aa02f53979d493a5f9) +- Commit: this record plus the implementation commit +- PR: draft, opened after push +- CI: runs on the pushed head + +## Review gates + +- Gate A: SKIPPED at the user's explicit instruction. This is a deliberate + deviation from `.agent/IMPLEMENTATION_LOOP.md` §4-5, not a pass. +- Gate B: SKIPPED at the user's explicit instruction. Same deviation, §7. + +## Handoff/next steps + +1. Run Gate A, and Gate B on the PR head, before this leaves draft. +2. A human owner reviews and merges. diff --git a/apps/web/browser/p5.spec.ts b/apps/web/browser/p5.spec.ts index 81eac06..7e7377c 100644 --- a/apps/web/browser/p5.spec.ts +++ b/apps/web/browser/p5.spec.ts @@ -247,7 +247,10 @@ for (const theme of ['light', 'dark'] as const) { 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); + // At least the mount read and the manual refresh. It is not an exact + // count: a delivery reported as PENDING is re-read on a timer, so a + // slower run legitimately reads more times. + expect(calls.filter((call) => call === 'GET /v1/jobs').length).toBeGreaterThanOrEqual(2); const results = await page .getByRole('region', { name: 'Requests and results' }) .boundingBox(); diff --git a/apps/web/src/components/JobWorkspace.tsx b/apps/web/src/components/JobWorkspace.tsx index 549a54a..88b11fd 100644 --- a/apps/web/src/components/JobWorkspace.tsx +++ b/apps/web/src/components/JobWorkspace.tsx @@ -41,6 +41,21 @@ function mcpPaymentStillSignable(job: JobView): boolean { const USER_WALLET_PAYMENT_CHECK_DELAY_MS = 500; const USER_WALLET_PAYMENT_CHECK_ATTEMPTS = 30; + +/** + * Supplier delivery finishes in the worker, after the browser has already read + * the list, so a request opened while its delivery is `PENDING` kept saying + * "Retrieving result" until someone pressed refresh — even once the result was + * durably available. These bounded re-reads close that window. + * + * They are `GET /v1/jobs` only: never the resume endpoint, and never anything + * that could pay. They run only while a delivery is actually `PENDING`, stop as + * soon as none is, and give up after the attempts below (about a minute) so a + * delivery that is genuinely stuck does not read forever. Past that, the manual + * refresh stays the way to look again. + */ +const DELIVERY_READ_DELAY_MS = 4000; +const DELIVERY_READ_ATTEMPTS = 15; function waitForPaymentCheck(): Promise { return new Promise((resolve) => { window.setTimeout(resolve, USER_WALLET_PAYMENT_CHECK_DELAY_MS); @@ -559,9 +574,23 @@ export function JobList(props: { const [loading, setLoading] = useState(true); const [error, setError] = useState(''); const [checkingPaymentJobId, setCheckingPaymentJobId] = useState(null); + const [deliveryReadsLeft, setDeliveryReadsLeft] = useState(DELIVERY_READ_ATTEMPTS); + // Which deliveries are pending, not how many reads have happened: a new + // pending delivery is a new wait and gets the full budget, while the same one + // staying pending keeps spending the budget it already started. + const pendingDeliveryIds = requests + .filter((request) => request.delivery_state === 'PENDING') + .map((request) => request.job_id) + .join(' '); - async function refresh(): Promise { - setLoading(true); + /** + * `quiet` reads keep the panel as it is while they run: the spinner and the + * disabled refresh button belong to a read the operator asked for, and the + * list body is keyed on `loading`, so flipping it would replay the tab fade + * every few seconds. + */ + async function refresh(options: { readonly quiet?: boolean } = {}): Promise { + if (!options.quiet) setLoading(true); try { const listed = await props.client.list(); setRequests(listed); @@ -569,7 +598,7 @@ export function JobList(props: { } catch { setError('Requests could not be loaded. Check API readiness and your workspace session.'); } finally { - setLoading(false); + if (!options.quiet) setLoading(false); } } @@ -592,6 +621,19 @@ export function JobList(props: { void refresh(); }, []); + useEffect(() => { + setDeliveryReadsLeft(DELIVERY_READ_ATTEMPTS); + }, [pendingDeliveryIds]); + + useEffect(() => { + if (pendingDeliveryIds === '' || deliveryReadsLeft <= 0) return; + const timer = window.setTimeout(() => { + setDeliveryReadsLeft((left) => left - 1); + void refresh({ quiet: true }); + }, DELIVERY_READ_DELAY_MS); + return () => window.clearTimeout(timer); + }, [pendingDeliveryIds, deliveryReadsLeft]); + return (
{ render( undefined} />); expect(await screen.findByText('Retrieving result')).toBeTruthy(); + // The list never offers to resume: only the worker may retrieve a result, + // and nothing here may lead to a second payment. expect(screen.queryByRole('button', { name: /Resume result/u })).toBeNull(); expect(client.list).toHaveBeenCalledTimes(1); }); + /** + * Delivery completes in the worker after the browser has read the list, so a + * request whose delivery was still `PENDING` at read time kept claiming + * "Retrieving result" long after the result was durably available. The reads + * below are the fix; they are read-only and they stop on their own. + */ + it('re-reads a pending delivery until the worker reports the result', async () => { + vi.useFakeTimers(); + try { + const list = vi + .fn<[], Promise>() + .mockResolvedValueOnce([resumableJob('PENDING')]) + .mockResolvedValue([resumableJob('AVAILABLE')]); + + render( undefined} />); + await vi.waitFor(() => expect(screen.getByText('Retrieving result')).toBeTruthy()); + + await vi.advanceTimersByTimeAsync(4000); + await vi.waitFor(() => expect(screen.getByText('Result ready')).toBeTruthy()); + expect(list).toHaveBeenCalledTimes(2); + + // Nothing is pending any more, so the reads stop rather than continuing + // to poll a settled list. + await vi.advanceTimersByTimeAsync(60_000); + expect(list).toHaveBeenCalledTimes(2); + } finally { + vi.useRealTimers(); + } + }); + + it('gives up re-reading a delivery that stays pending', async () => { + vi.useFakeTimers(); + try { + const list = vi.fn(async () => [resumableJob('PENDING')]); + + render( undefined} />); + await vi.waitFor(() => expect(screen.getByText('Retrieving result')).toBeTruthy()); + + // Far beyond the attempt budget: a stuck delivery must not read forever. + // Stepped, so each re-read's effect can schedule the next one. + for (let tick = 0; tick < 30; tick += 1) await vi.advanceTimersByTimeAsync(4000); + const spent = list.mock.calls.length; + // It kept looking while the delivery was pending, but never past the + // mount read plus the fifteen-attempt budget. + expect(spent).toBeGreaterThan(2); + expect(spent).toBeLessThanOrEqual(1 + 15); + + // The budget is spent, so a stuck delivery stops being read. + for (let tick = 0; tick < 30; tick += 1) await vi.advanceTimersByTimeAsync(4000); + expect(list).toHaveBeenCalledTimes(spent); + expect(screen.getByText('Retrieving result')).toBeTruthy(); + } finally { + vi.useRealTimers(); + } + }); + it('sends the entered recipient and integer atomic amount to the quote boundary', async () => { const user = userEvent.setup(); let quotedRequest: unknown;