- MONTHS: drop the appended day-of-week field that produced a 6-field cron
(node-cron reads 6-field crons seconds-first, shifting the schedule)
- MONTHS: validate dayOfMonth (1-31) and emit the parsed integer
- MINUTES/HOURS: constrain the schedule to the atTimeStart/atTimeEnd window
instead of ignoring atTimeEnd; require end after start, and an hour-spanning
window for hourly intervals
- WEEKS: reject runEvery > 1 since cron cannot express multi-week intervals
Add POST/GET/DELETE /api/robots/:id/schedule to the Website-to-API
feature so robots can be scheduled through the API, using the same
fields as the dashboard schedule form. Cron building and validation
are pulled into a shared helper so the dashboard and API routes stay
in sync.
Per maintainer review (RohitR311 on #1138): the repo has no existing
test infra or CI to run tests, so introducing jest here is out of
scope. Removes jest configs, test files, tsconfig.test.json, the
test:server script, and jest/ts-jest/@types/jest/jest-environment-jsdom
dependencies from both maxun-core and server.
output-validation.ts stays - it's a real production module (not test
code) extracted to fix the duplicated isPartial bug CodeRabbit flagged;
only its test file is dropped.
computeCrawlSearchPersistDelta advances lastPersistedResultCount as
soon as a delta is computed, before the write is confirmed. If
flushPersistenceBuffer exhausts its retries and drops the batch, that
data was never actually written, but the cursor had already moved
past it - so it was silently lost forever instead of being resent
with the next page's delta.
Roll the cursor back by the dropped delta's size when a batch is
dropped, so the next page recovers the lost items instead of skipping
them. Covered by two new tests exercising the real retry-exhaustion
path via a rejecting sequelize.transaction mock.
If triggerIntegrationUpdates threw, the catch(_) swallowed it and left
partialDataExtracted false even though hasData was true, so isPartial
was persisted incorrectly on the failure path.
Addresses CodeRabbit review on #1138 (both findings confirmed valid
against the actual code, not just the review's claim):
- hasData's scrapeSchema/scrapeList checks used `.length > 0`, but both
are always plain objects keyed by action name, never arrays -
`.length` on a plain object is undefined, so the check could never
be true. isPartial could never reflect a run whose only partial
output was scrapeSchema/scrapeList. Pre-existing bug, not introduced
by this PR, but copied into abortRun's check when extending it here.
- abortRun reloaded the run before checking for partial data but never
forced the interpreter's persistence buffer to flush first -
scrapeList isn't on the immediate-flush allowlist, so a buffered
scrapeList page could be invisible to the reload.
Both fixes now live in one place (hasPartialOutput /
flushReloadAndCheckPartialOutput in output-validation.ts) instead of
duplicated inline in task-runner.ts's two call sites, which is how the
same bug ended up in two places to begin with.
Verified with mutation testing: reintroduced the .length > 0 bug,
confirmed 3 tests fail; reintroduced the reload-before-flush ordering
bug, confirmed the ordering test fails; restored the fix, all 14 pass.
Rebasing onto the latest develop pulled in #1105's per-page screenshot
upload path, which added a BinaryOutputService import to Interpreter.ts
that attempts a real MinIO connection at module load. Mock it so these
tests stay hermetic instead of noisily (and potentially flakily)
failing to connect to a MinIO that isn't there in CI.
Same class of bug already fixed on the maxun-core side: `tsc -p
server/tsconfig.json` (the actual build:server script) was compiling
server/src/**/__tests__ into server/dist, since the base tsconfig's
include (src/**/*) picked them up with nothing excluding them. Found
by actually running the production build rather than trusting
--noEmit type-checks, which don't reveal what gets emitted.
server had no test infra at all (no test script, no jest config). Add
jest/ts-jest scoped to server/, and directly unit test
computeCrawlSearchPersistDelta/mergeCrawlDelta/mergeSearchDelta - the
new delta-slicing and append-merge logic - without needing to mock
the database or drive a full interpreter run.
Verified these tests actually catch the regression they're meant to:
temporarily reverted the slicing to send full arrays again and
confirmed 4 of the 11 tests fail, including the end-to-end one showing
the exact page duplication the fix prevents.
crawl/search deliver the full cumulative array on every page callback,
and persistDataToDatabase was writing that whole array to Postgres on
every single page - meaning page 1's data got re-sent on every write
for the rest of the run, an O(n^2) total-bytes cost that gets worse
the more pages a crawl visits (each carrying full page HTML).
Track how many items have already been persisted per action and slice
off just the new ones before they reach the persistence buffer, then
append (rather than overwrite) that delta into the existing DB state.
The in-memory cache and socket broadcast are untouched - only what
gets written to Postgres changes. Extracted as computeCrawlSearchPersistDelta/
mergeCrawlDelta/mergeSearchDelta so this can be tested directly.
status alone doesn't distinguish a failed/aborted run that still has
usable partial data from one with nothing recovered - that signal only
existed transiently in socket/webhook payloads and was lost on reload.
Persist it as a real column, set from both the crash-catch path and
abortRun, and expose it via formatRunResponse.
Also fix abortRun's partial-data check, which only looked at
scrapeSchema/scrapeList and missed crawl/search entirely, and reload
the run before checking since the interpreter's persistence buffer
writes through a separate Run instance.
persistDataToDatabase only force-flushed the batched write immediately
for scrapeSchema; everything else, including per-page crawl/search
writes, sat in the buffer for up to 3s or 5 items before hitting
Postgres. A hard process kill inside that window would still lose
data. Extend the immediate-flush condition to crawl and search too.
Screenshots were captured per page (single scraping pass) but held in
memory as base64 for the whole run and bulk-uploaded to MinIO only at
the end. Since maxun-core awaits binaryCallback after each page scrape,
the run-mode callback is the natural upload point: page N's screenshot
now lands in MinIO and run.binaryOutput (as a public URL) before page
N+1 is scraped.
- BinaryOutputService.uploadBinaryOutputItem: single-item upload,
extracted from the bulk loop so object-key and public-URL
construction (#832) stay single-source. Returns null on failure
instead of throwing.
- Run-mode binaryCallback uploads first; on failure the item keeps its
base64 payload so the end-of-run bulk upload retries it.
- uploadAndStoreBinaryOutput passes already-uploaded URL entries
through untouched, making the end-of-run call idempotent — required
because it fully replaces run.binaryOutput.
- persistBinaryDataToDatabase stores URL entries as plain strings so
the mid-run DB shape matches the final uploaded shape.
Applies to all run-mode screenshots (crawl/search page captures and
regular screenshot actions share the same callback payload).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- validate formats in PUT /runs/:id before persisting, so a malformed
value can't reach the interpreter and break formats.includes(...)
- re-throw on failed search-result navigation so it records an error
entry instead of scraping/screenshotting the stale page (matches the
crawl loop's behavior)
- correct stale getNavigationWaitStrategy JSDoc (networkidle is driven
by visualRenderRequired, not screenshot formats)
- fix duplicated phrase in processRobotOutputFormats JSDoc
Rohit