[chien1120yc]

Summary

Add a Skip report and exit button to the ending overlay so candidates can leave without waiting for report generation or recovery to finish.

• Require confirmation before skipping the report.
• Cancel pending report generation, publication, delivery, and recovery when Skip is confirmed.
• Stop recovery timers before awaiting control publication.
• Delete the provisional report for the current interview while preserving reports from earlier interviews.
• Skip unnecessary report deletion for guests without a saved account copy.
• Track successful account saves so cleanup cannot silently succeed with a local-only deletion after the candidate signs out.
• Keep the candidate on the page with Retry cleanup and exit when required cleanup fails.
• Preserve normal report generation and recovery when Skip is not selected.

Testing

• make hooks (passed; four Git hooks installed).
• ./scripts/test.sh (exit code 0).
• cargo test --lib (1,163 passed).
• Browser tests (1,028 passed, 0 failed, 320 skipped in the latest local gate).
• GitHub Actions (all checks passed, including mutation testing).

The local test gate reported skipped lanes because some optional tools and browser dependencies were unavailable, including Playwright Chromium, ruff, shfmt, shellcheck, actionlint, and cargo-audit.

Regression tests cover cancellation, report delivery and recovery, events received during pending publication, candidate departure, guest cleanup, pending history saves, and account-copy deletion failures.

Manual UI Testing

The Skip Report flow was manually verified during report generation and recovery.

• Canceling the confirmation leaves the existing report workflow unchanged.
• Confirming Skip returns the candidate to the lobby.
• The Skip option remains accessible during report recovery.
• In guest mode, Skip returns to the lobby without issuing a report deletion request.

Skip Report During Evaluation Generation

The Skip report and exit button is available while the evaluation report is being generated.

Skip Report During Recovery

The Skip report and exit button remains available alongside the recovery options when report generation is delayed.

Closes #100

[jserv]

CI fails on the indent gate: interview.html has formatting drift. Run make indent, then amend and force-push.

Check CONTRIBUTING.md carefully.

[jserv]

The new guard replaced state.phase = "report" instead of extending it, so the offline summary now renders with the phase still ending. A second showReport call (the 300 ms timer plus a click on #force-report) rebuilds and saves the summary again, and a late agent report passes the state.phase === "report" check in receiveReport and replaces the summary the candidate is reading.

Suggested change

	if (
	state.phase === "report" ||
	state.phase === "skipped" ||
	state.reportReceiving
	)
	return;
	if (
	state.phase === "report" ||
	state.phase === "skipped" ||
	state.reportReceiving
	)
	return;
	state.phase = "report";

[chien1120yc]

Fixed. showReport() now sets state.phase = "report" immediately after the early-return guard, before saving or rendering the offline summary.

This prevents the offline summary from remaining in the ending phase and being overwritten by a late agent report. I also added regression coverage for the transition.

[chien1120yc]

Fixed. I ran make indent and updated the PR accordingly.

The latest GitHub Actions run now passes all checks, including the formatting and browser gates. Thank you for pointing this out.

[chien1120yc]

I rechecked the PR against CONTRIBUTING.md and corrected the PR title and description.

make hooks succeeded, and ./scripts/test.sh exited with code 0. The gate reported skipped lanes for unavailable local dependencies. All GitHub Actions checks are passing.

The review fixes are included in b929494. The PR is ready for re-review. Thank you!

[jserv]

Putting report_owner_matches and mod owner_checks here moves the existing /// doc comment ("200 with a count of 0 ...") off delete_report_handler and onto this helper, so rustdoc now documents the wrong function. Move the helper and the test module above that doc comment so it stays attached to the handler.

[chien1120yc]

Fixed as suggested. Moved report_owner_matches and the external owner_checks test module above the existing /// documentation, so the comment is once again attached to delete_report_handler.

This is a placement-only change; the handler logic and tests remain unchanged. cargo fmt --all -- --check, ./scripts/indent.sh --check, and git diff --check all pass.

[jserv]

tests/fixtures/README.md names every producer and consumer of control.json in one row, and this adds one of each: skipReportPayload in web/lib.js, and the Skip handling in apply_control and is_skip_report_with_presence on the agent side. Add them to that row so the table still says what crosses the boundary.

[chien1120yc]

Thanks for pointing this out. I've updated the control.json entry in tests/fixtures/README.md to document skipReportPayload as the producer and the actual Rust consumer is_skip_report_with_presence, alongside apply_control.

The fixture table now accurately describes the Skip report payload's producer and consumers.

[jserv]

The two server behaviors this PR adds, ownerId in the save response and the 409 from delete_report_handler when x-codetrial-report-owner names another account, are only exercised by fetch doubles in the browser tests and by report_owner_matches in isolation. Nothing in tests/web/accounts.rs drives the real routes, so dropping ownerId from this body or the check from the delete handler keeps both suites green while Skip's cleanup quietly falls back to the weaker unknown-owner path. Add a case there that saves a report, checks ownerId, then deletes with a mismatching header and expects 409 with the row still present.

[chien1120yc]

Thanks for pointing this out. This is now covered by the real HTTP routes in tests/web/accounts.rs.

The test saves a report through POST /api/reports and verifies that the response contains the expected ownerId. It then attempts to delete that report using a mismatched x-codetrial-report-owner header, asserts that the response is 409, and fetches the reports again to verify that the original report is still present.

This exercises the actual routes rather than relying only on mocked fetch responses.

[jserv]

Confirming Skip leaves every word on the overlay as it was: the title still reads "Jim is writing up your evaluation..." and the spinner keeps turning while the control packet and the report cleanup run, and the button stays enabled with its old label. The failure paths then rewrite only endingDetail and the button text, so the heading contradicts them ("Could not send Skip" under "writing up your evaluation", or under "Evaluation temporarily unavailable" in recovery). Set the title and hide the spinner here, and mark the button busy until the retry state replaces it, so the candidate can see that Skip took effect.

[ColtenOuO]

If Skip is already queued in recovery_events before generation starts (during settle_phase_judge / settle_for_report / the transcript flush), the biased select in until_skip returns Skipped without ever polling work. No request was sent, but this still logs outcome=cancelled cost=unknown. The same applies to the Disconnected arm. Could until_skip report whether work was polled, the way generation_polled does in recovery?

[chien1120yc]

Thanks for catching this. Fixed in 4d0d852.

The generation future now sets generation_polled when it is first polled. Both the Skipped and Disconnected branches check this flag before logging outcome=cancelled cost=unknown.

This prevents an early Skip or disconnect from being logged as a cancelled generation when the generation future was never polled.

[ColtenOuO]

In "skipped" (for example, the Skip publish is still waiting for Reconnected), an arriving report is still claimed and receiveReportDelivery sends report_received before receiveReport returns early. The agent then logs report_delivery outcome=acknowledged for a report the page discarded, and state.reportReceiving stays true. Checking state.phase === "skipped" before claimReport would keep the receipt from going out.

[ColtenOuO]

Once in "skipped", the only way out is the retry button. If cleanup keeps failing (the report was saved and the user signed out in another tab, so deleteReport fails every time; or the data channel stays down and every Skip retry times out), the candidate is stuck on the ending overlay. Could this show a "Leave anyway" after a failed retry, like the owner-conflict path does?

[jserv]

Before asking someone to review, you MUST resolve any regressions flagged by GitHub Actions.

[jserv]

Rebase latest main branch.

The gates (browser) failure here is the lobby fits on one screen instead of a wall of problems (tests/browser/lobby.test.js). This branch did not cause it: the lobby overflowed the 800px viewport by 5px on CI fonts on main itself. #478 fixed that (trimmed the credits spacing in web/styles.css and hardened the test), so rebasing picks up the fix.

After the rebase, confirm gates (browser) is green. If it still fails on a different test, explain that failure in a comment.

[jserv]

dismissSkipConfirmation() hides the confirm and cancel buttons but does not restore #skip-report, which requestSkipReport hid when the confirmation opened. If the provisional recovery report arrives while the candidate is looking at "Skip this report?", recovery takes over the overlay with only retry and "Save incomplete report and leave", and nothing sets nodes.skipReport.hidden = false again (only cancelSkipReport and the failure paths do). The candidate who was about to skip has no way to. Show the Skip button again here when the phase is still ending or report_recovery, and restore the detail text the confirmation replaced.

[jserv]

When possiblySavedToAccount is set but no ownerId was ever recorded, cleanup demands deleted === 1. Two cases break it. A POST that is definitively rejected (for example 507 on account capacity) saved nothing, so the DELETE returns deleted: 0 and every "Retry cleanup and exit" fails, leaving the provisional local report behind. A POST that committed with a lost response, followed by a DELETE that committed with a lost response, also returns deleted: 0 on every retry. Distinguish a definitive rejection from an uncertain write: clear possiblySavedToAccount on a non-ok POST, and treat a zero-row delete as complete when the request was authenticated and the save outcome was uncertain.

[jserv]

sendData resolving only means the packet was queued locally, and during a reconnect it can still be buffered. room.disconnect() follows immediately, so the skip_report packet can be dropped while cleanup succeeds and the page navigates to /. The agent then keeps generating and delivering a report the candidate was told was skipped. Have the agent acknowledge the skip on the control topic and await that acknowledgement (bounded, falling back to the existing "Retry Skip and exit" path) before disconnecting.