From 0f8dd9a1f4cdfa091206c053de491d0c5cc18239 Mon Sep 17 00:00:00 2001 From: selezenart Date: Sun, 13 Sep 2026 16:03:16 +0200 Subject: [PATCH] fix(web): re-read a pending delivery until the worker reports the result MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Supplier delivery finishes in the worker, after the browser has already read the request list, so a request whose delivery was still PENDING at read time kept saying "Retrieving result" until someone pressed refresh — long after the result was durably available. Whether it looked right depended only on whether the worker beat the page load, which is why it worked intermittently. The list now re-reads itself while any delivery is PENDING: every four seconds, fifteen attempts, then it stops and leaves the manual refresh as the way to look again. The reads are GET /v1/jobs only. They never call the resume endpoint and never submit a payment, so at-most-once settlement is unaffected, and they do not raise the loading flag, because the spinner, the disabled button, and the tab fade belong to a read the operator asked for. This partially reverses a2903a1, which removed automatic polling after a report of unwanted traffic. The bound and the pending-only condition keep both reports satisfied; the resume control that commit removed stays removed. The browser spec asserted an exact list-read count, which a timed re-read makes timing-dependent; it now asserts a lower bound and still asserts that no resume request is sent. --- ...260913T170000Z-pending-delivery-refresh.md | 101 ++++++++++++++++++ apps/web/browser/p5.spec.ts | 5 +- apps/web/src/components/JobWorkspace.tsx | 48 ++++++++- apps/web/test/components.test.tsx | 58 ++++++++++ 4 files changed, 208 insertions(+), 4 deletions(-) create mode 100644 .agent/context/20260913T170000Z-pending-delivery-refresh.md 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;