Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions .agent/context/20260913T120000Z-session-details-resume-result.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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.
Expand All @@ -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
Expand Down
32 changes: 13 additions & 19 deletions apps/web/browser/p5.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -47,32 +47,23 @@ async function json(route: Route, status: number, body: unknown): Promise<void>
await route.fulfill({ status, contentType: 'application/json', body: JSON.stringify(body) });
}

async function mockJobApi(
page: Page,
options: { readonly startDelayMs?: number; readonly resumeDelayMs?: number } = {},
): Promise<string[]> {
async function mockJobApi(page: Page): Promise<string[]> {
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') {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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();
Expand Down
22 changes: 0 additions & 22 deletions apps/web/src/api/job-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,6 @@ export class JobApiClient {
return response.ok ? ((await responseJson<JobListResponse>(response))?.jobs ?? []) : [];
}

async get(jobId: string): Promise<JobView> {
const response = await this.#fetch(`${this.#baseUrl}/v1/jobs/${encodeURIComponent(jobId)}`, {
headers: this.#headers(),
});
const body = await responseJson<JobView>(response);
if (!response.ok || !body) throw new Error('Could not load the job status');
return body;
}

async start(request: CreateJobRequest): Promise<JobView> {
const response = await this.#fetch(`${this.#baseUrl}/v1/jobs`, {
method: 'POST',
Expand Down Expand Up @@ -108,19 +99,6 @@ export class JobApiClient {
return body;
}

async resume(jobId: string): Promise<JobView> {
const response = await this.#fetch(
`${this.#baseUrl}/v1/jobs/${encodeURIComponent(jobId)}/resume`,
{
method: 'POST',
headers: this.#headers(),
},
);
const body = await responseJson<JobView>(response);
if (!response.ok || !body) throw new Error('Could not resume supplier delivery');
return body;
}

async result(jobId: string): Promise<SupplierResult | null> {
const response = await this.#fetch(
`${this.#baseUrl}/v1/jobs/${encodeURIComponent(jobId)}/result`,
Expand Down
81 changes: 6 additions & 75 deletions apps/web/src/components/JobWorkspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
return new Promise((resolve) => {
window.setTimeout(resolve, USER_WALLET_PAYMENT_CHECK_DELAY_MS);
});
}

function waitForResultRefresh(): Promise<void> {
return new Promise((resolve) => {
window.setTimeout(resolve, RESULT_REFRESH_DELAY_MS);
});
}

async function waitForSupplierResult(
client: JobApiClient,
jobId: string,
initial: JobView,
): Promise<JobView> {
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,
Expand Down Expand Up @@ -487,53 +461,21 @@ export function JobList(props: {
const [requests, setRequests] = useState<readonly JobView[]>([]);
const [loading, setLoading] = useState(true);
const [error, setError] = useState('');
const [resumingJobId, setResumingJobId] = useState<string | null>(null);
const [checkingPaymentJobId, setCheckingPaymentJobId] = useState<string | null>(null);

async function refresh(): Promise<readonly JobView[] | null> {
async function refresh(): Promise<void> {
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<void> {
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<void> {
const transactionHash = job.user_payment?.transaction_hash;
if (job.payment_mode !== 'USER_WALLET' || !transactionHash) return;
Expand All @@ -557,7 +499,7 @@ export function JobList(props: {
<section
className="panel"
aria-label="Requests and results"
aria-busy={loading || resumingJobId !== null || checkingPaymentJobId !== null}
aria-busy={loading || checkingPaymentJobId !== null}
>
<header className="panel-heading">
<div>
Expand All @@ -567,7 +509,7 @@ export function JobList(props: {
<button
type="button"
className="secondary compact"
disabled={loading || resumingJobId !== null || checkingPaymentJobId !== null}
disabled={loading || checkingPaymentJobId !== null}
onClick={() => void refresh()}
>
{loading ? 'Refreshing…' : 'Refresh requests'}
Expand Down Expand Up @@ -628,30 +570,19 @@ export function JobList(props: {
)}
</p>
)}
{job.result ? (
{job.result && (
<p>
<strong>Result ready:</strong> {job.result.report}
</p>
) : job.payment_state === 'COMMITTED' ? (
<button
type="button"
className="secondary compact"
disabled={resumingJobId !== null}
onClick={() => void resume(job.job_id)}
>
{resumingJobId === job.job_id
? 'Resuming…'
: 'Resume result (no new payment)'}
</button>
) : null}
)}
{job.payment_mode === 'USER_WALLET' &&
job.payment_state === 'UNKNOWN' &&
job.user_payment?.transaction_hash && (
<button
type="button"
className="secondary compact"
disabled={
loading || resumingJobId !== null || checkingPaymentJobId !== null
loading || checkingPaymentJobId !== null
}
onClick={() => void checkRecordedPayment(job)}
>
Expand Down
30 changes: 3 additions & 27 deletions apps/web/test/components.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<JobList client={client as never} onSelectIntent={() => 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 () => {
Expand Down
17 changes: 0 additions & 17 deletions apps/web/test/job-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '';
Expand Down
Loading