Repository navigation
fix(report): keep the delete credential when the record cannot be written - #633
Conversation
|
@claude /review |
|
Claude finished @theCodeDrift's task in 1m 19s —— View job Review complete ✅Review mode: incremental — read 0 prior review thread(s) before reviewing.
SummaryThis is a clean, well-scoped fix. The collision handling in
Tests are thorough: unit coverage for both the suffix-assignment and read-only-directory paths, plus end-to-end coverage through the built CLI for both One minor/informational point (inline)
No correctness, security, or test-adequacy issues found beyond that nit. Docs ( |
Thanks. The one inline point, about suffixes past — AI Coding Agent |
…tten Two publishes in one second collided on the record name, and an unwritable reports directory failed the same way, after the report was already live. A taken name now moves to a _N suffix, and a record that still cannot be written exits with REPORT_RECORD_FAILED carrying the URL and delete URL in the message and the --json envelope.
942420a to
a9c30dd
Compare
taskless report --frompublishes a report, then writes.taskless/reports/<stamp>.json, the only place the report's delete URL is kept. The stamp has one-second resolution and the write refuses to overwrite, so two publishes in one second made the second write throwEEXISTafter its report was already live. An unwritable reports directory failed the same way. Either way the raw fs error reached the top-level handler, which printed only its message: the report stayed live for 30 days with no way to delete it, and the user never saw its URL.What changed
report/records.ts). A taken name moves to<stamp>_2.json,<stamp>_3.json, … (up to 100 tries), still withwx, so nothing is overwritten._sorts after., so a suffixed record sorts after the one it collided with.--deletealready accepts any.jsondirectly in.taskless/reports/, so suffixed names resolve unchanged.REPORT_RECORD_FAILED. When the record still cannot be written,writeRecordthrows aCLIErrorwhose message says the report is live, gives its URL, expiry and delete URL, and shows how to delete it. Printing the credential is the lesser harm compared with a page nobody can take down.--jsonenvelope carries the credential.CLIErrorgains optionaldetails, andCLIErrorEnvelopegains optionalurl,deleteUrlandexpiresAt(CLIErrorDetails), present only for this code.report'sfail()passeserror.detailstowriteJsonError. That one-line change is the only edit outside the publish path incommands/report.ts, so this merges cleanly with fix(onboard): refuse a behind .taskless/ in onboard and report instead of migrating #630 in either order.reportlists the new code (topic v8).cli-report's "A published report is recorded and summarized" restated with the suffix rule and the failure case, plus two scenarios. Archived in this PR, and all 53 prior scenarios are still present.patch.Testing
writeRecordunit tests: a taken stamp yields_2and then_3, the existing record is unchanged,readRecordresolves the suffixed name, and it sorts after the original. A read-only reports directory throwsREPORT_RECORD_FAILEDwith the URL and delete URL in the message and indetails._2with the right delete URL. With.taskless/reports/read-only, the publish exits non-zero, stderr carries the code, URL and delete URL, and the--jsonenvelope carriesurl,deleteUrlandexpiresAt.pnpm typecheck,pnpm lintandpnpm --filter @taskless/cli test(2970 passed) all pass.Fixes #622