You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit d5151a7
Browse filesBrowse the repository at this point in the historyBrowse files
fix(rest)!: an async import job whose every row failed ends failed, not succeeded (#27795)
domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F
<!-- claude-projects-attribution: {"github_login":"os-sales"} -->
_Requested by **Sales** · [project
thread](https://claude.ai/code/project/chan_01D5bWFY3rFzgJcytD5gD4Uu?thread=cmsg_01D5bWFY3rFzgJcytD5gD4UuPMZ8BHWtPf6nBTp51qSM64)_Fixes#27676
domain:server#2 · os-sales · session_01L6bhgUCizoYWQVJWjhdR6F
Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: none — no hand-written docs page or published skill states
the async import job's terminal-status rule (`git grep` of
`content/docs` outside `references/`/`releases/` and of `skills/` for
the job status vocabulary: 0 hits; `import-mappings.mdx` documents
per-row outcomes only). The `ImportJobStatus` docblock and the generated
reference are the spec seat's card #27734.
## 维护者速读
- 改了什么:后台导入作业(`POST
/data/OBJ/import/jobs`)在未取消、有行、且没有任何一行被新建/更新/跳过时,终态记为 `failed`(原来是
`succeeded`),`error` 写一句话说明每行都失败、去看 `results`。
- 为什么改:按你的裁决 B(记录 6509036166):一行都没导入的作业不能对只看 `status` 的客户端说「成功」。
- 风险与回滚:只改这一种终态;部分失败、全部跳过、零行作业仍是
`succeeded`,取消仍优先;撤销不受影响(这种作业本来就不可撤销)。回滚即删掉 `data-action-routes.ts`
里的一个判断。
- 先看:`packages/rest/src/data-action-routes.ts` 里 `everyRowFailed`
那段;`packages/rest/src/import-job-integration.test.ts` 末尾新增的
describe;`packages/rest/src/import-row-cell-errors.test.ts` 的轮询期望。
- 两分钟验证:`pnpm --filter @objectstack/rest exec vitest run
src/import-job-integration.test.ts`,预期 21/21 通过。
- 需要你做的:无,席位落地。
## Ruling (binding)
Maintainer ruling B, record 6509036166 on #27676, execution parameters
quoted:
> 1. **Predicate.** At the terminal-status line of the job worker
(`packages/rest/src/data-action-routes.ts`, the `finalStatus`
computation): `!cancelled && total > 0 && created + updated + skipped
=== 0` ⇒ `failed`, else today's rule. Not `ok === 0`: a skipped row is
not counted in `ok`. `error` is set to one sentence saying every row
failed and pointing at `results`, with no tracker number.
> 2. **Kept as `succeeded` on purpose:** a zero-row job, an all-skipped
job, and a partly-failed job. The batch door's "any failure ⇒ false"
rule is not carried over. Cancellation still wins.
> 3. **Dry-run:** the same rule (an all-error dry-run ends `failed`).
> 4. **Server PR** (`domain:server`, after #27501 (a) PR #27711 merges;
same file), with five pins: all rows fail → `failed`, `errors ===
total`, `error` present; 2 ok + 1 fail → `succeeded` (the existing
partial pin is unchanged); all skipped → `succeeded`; cancellation still
wins; all-error dry-run → `failed`.
> 5. **Spec docblock** (`packages/spec/src/api/export.zod.ts`, the
`ImportJobStatus` / `ImportJobProgressSchema.error` sentences, plus
`gen:docs`) goes to the `domain:spec` seat as its own PR, not in the
server PR.
## What changes
- `packages/rest/src/data-action-routes.ts`, the async import worker's
terminal status. The existing cancel resolution runs first and is
unchanged: `summary.cancelled`, then the in-memory `cancelledImportJobs`
flag, then the durable row re-read. Only a status still `succeeded`
after all three is tested by the new predicate, so a cancel always wins,
including a late one.
- The predicate, mapped onto the runner's real summary
(`ImportRunSummary extends ImportProgress`,
`packages/core/src/utils/import-runner.ts`): ruling `cancelled` = the
resolved status is not `cancelled` (`summary.cancelled` plus the two
late re-checks); `total` = `summary.total` (`rows.length`); `created` /
`updated` / `skipped` = `summary.created` / `summary.updated` /
`summary.skipped`. `summary.ok` is not read: it counts created and
updated rows only.
- When the predicate holds, the final patch writes `status: 'failed'`
and `error`: "Every row of this import failed and nothing was imported;
each row's reason is in the job's results." Counts, `results` and
`completed_at` are written exactly as before. A dry run takes the same
path (its valid rows count as created / updated in the runner's
preview).
- No `packages/spec` change: the `ImportJobStatus` / `error` docblocks
are #27734.
- Changeset: `@objectstack/rest` patch, BREAKING note, ADR-0087
`not-required (no-migration-prescription)` marker.
### Readers of `failed` vs `succeeded` in this lane, measured
- Undo (`importJobUndoable`, `rest-server.ts`): admits `succeeded` /
`cancelled` only, and also needs a non-empty undo log. A job that
created and updated nothing has an empty log, so it was not undoable
before and is not now. Pinned: the all-failed job reads `undoable:
false` and undo answers 422 `NOT_UNDOABLE`.
- Cancel route: acts on `pending` / `running` only; unaffected.
- History filter `?status=`: passes the string through; pinned that the
all-failed job lists under `status=failed`.
- Console Import Wizard (`ImportWizard.tsx` `importResultOutcome`):
already reads "nothing imported and something errored" as `failed`, and
`jobStatus === 'failed'` as `failed`; the synthetic row at the
`prog.status === 'failed'` branch fires only when no per-row error
arrived, which this change never produces. `failed` is already in its
closed terminal set. No UI change.
## Tests
`packages/rest/src/import-job-integration.test.ts`, a new describe on
the file's harness (real `RestServer` route, real `ObjectQL` engine +
`ObjectStackProtocolImplementation`, SQLite `:memory:`, real
`sys_import_job`):
- every row fails → `failed`, `errors === total` (3), `error` present
and naming `results`; `results` has 3 failed rows; nothing landed;
`undoable: false`, undo 422 `NOT_UNDOABLE`; listed under
`status=failed`.
- control: every row skipped (`writeMode: 'update'`, no match) →
`succeeded`, no `error`.
- control: a header-only CSV (0 rows) → `succeeded`, no `error`.
- every row of a dry run fails → `failed`, `dryRun: true`.
- control: a valid dry run → `succeeded`.
- control: a cancel that lands after the last row of a job whose every
row fails at the driver → `cancelled`, with `errors: 3` (it waits for
the worker's own final write of `results` before reading).
- 2 ok + 1 fail → `succeeded`: the existing first case of the file,
unchanged.
`packages/rest/src/import-row-cell-errors.test.ts`: its async poll
helper asserted `succeeded` for every job; the two one-row jobs whose
only row fails (`NO_NAME`) now expect `failed`, per the ruling. A stale
fixture, a direct consequence of this change.
## Acceptance notes
- The pins drive the route handlers registered on a real `RestServer`,
not an HTTP transport, the harness the file's existing cases use.
- The first full-suite run showed the late-cancel pin racing a fixed 50
ms wait under load (it read the cancel route's write before the worker's
final patch); the pin now waits on the worker's `results` write instead.
Re-run green in the full suite.
- The sync door (`POST /data/:object/import`) has no status and is
untouched.
## Verification
Readings are at head `2fa11634` unless a line names another sha.
<details><summary>Red before the fix (e536326, tests only)</summary>
```
× every row fails: the job ends failed, errors === total, and error is set
× every row of a dry run fails: the job ends failed
- "status": "failed",
+ "status": "succeeded",
Tests 2 failed | 19 passed (21)
```
</details>
<details><summary>Ablations (scripts/ablation-replace.mjs; each restored
blob == HEAD, `git diff HEAD` empty)</summary>
The suites import `./rest-server` from source, so no dist rebuild was
involved.
1. Delete `if (everyRowFailed) finalStatus = 'failed';` → red: the two
all-failed pins plus the two `import-row-cell-errors` polls (`Tests 4
failed | 23 passed (27)`).
2. Predicate spelled `summary.ok === 0` → red: "every row skipped keeps
succeeded" (`expected "succeeded", received "failed"`).
3. Cancel guard spelled `!summary.cancelled` (ignoring the late
re-checks) → red: the late-cancel control.
4. `&& summary.total > 0` deleted → red: "a job with no rows keeps
succeeded". (A first attempt with an empty `--replacement` was refused
by the tool before running anything; re-run with `--delete`.)
</details>
<details><summary>Package suites and gates</summary>
- `pnpm --filter @objectstack/rest test`: lock VERDICT command-exit 0,
342/342 files, 6701 passed, 326 skipped.
- `pnpm --filter @objectstack/rest typecheck`: exit 0, 0 test-layer
errors (at 138a620; the commit after it touches two test files that the
full suite above compiled and ran).
- `eslint packages/rest --no-inline-config`: exit 0.
- `dispatch-gates --commands`: 59 families derived; results in the
os-dev-report on #27676. `check:dual-build-cjs-loads` exits 3
PREREQUISITE NOT MET (it reads every package's dist): NOT MEASURED
locally, carried by CI `Lint & Repo Gates`.
- No open PR touches `data-action-routes.ts`,
`import-job-integration.test.ts` or `import-row-cell-errors.test.ts`
(REST read of all 35 open PRs' files).
- Dogfood battery (`(narrowing)` arm) at the final head `2c10836a`:
os-verify-lock VERDICT exit 0 (held 31m29s), 355 files passed and 3
skipped (358), 2867 tests passed and 20 skipped (2887), no failures
(os-dev-report 6512268257). An earlier run at `2fa11634` had 2 failures,
both the stale dogfood poll in
`packages/qa/dogfood/test/import-reference-exposure.dogfood.test.ts`,
which `2c10836a` updates (it waited for `succeeded` on an
all-rows-failed job); that file is added to the claim by amendment.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F)_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01L6bhgUCizoYWQVJWjhdR6F
---------
Co-authored-by: Claude <noreply@anthropic.com>
fix(rest)!: an async import job whose every row failed ends `failed`, not `succeeded` (#27676)
6
+
7
+
Clause-②: no (narrowing) — the async import job's `succeeded` status is no longer emitted for a job that imported nothing; it now reads `failed`. No value, key or shape is added.
8
+
Metadata-protocol: none
9
+
10
+
<!-- adr-0087: not-required (no-migration-prescription) No key of any schema is added, removed, renamed or re-shaped, and no export or type changes: one already-declared status value (`failed`) is now emitted for a job that created, updated and skipped no row. There is nothing for a migration to rewrite and no tombstone. -->
11
+
12
+
**BREAKING** a published status value changes meaning at one REST door (`@objectstack/rest`), per maintainer ruling B (record 6509036166).
13
+
14
+
-**Before.** A job of `POST /data/:object/import/jobs` that was not cancelled and did not throw ended `succeeded`, even when every row failed and nothing was written. The only sign was `errors` equal to `total`.
15
+
-**Now.** A job that was not cancelled, had at least one row, and created, updated or skipped none of them ends `failed`. Its `error` says every row failed and points at the job's `results`, which still carry each row's reason. A dry run follows the same rule: a dry run whose every row is invalid ends `failed`.
16
+
-**Unchanged, on purpose.** A job whose rows partly failed still ends `succeeded`. So does a job whose every row was skipped (for example `writeMode: 'update'` with no match), and a job with no rows. A cancel still wins: a cancelled job ends `cancelled` whatever its rows did. Counts and `results` are written as before.
17
+
-**Who sees it.** A client that polls an import job and branches on `status === 'succeeded'` is now told `failed` for a job that imported nothing. Undo is unaffected: such a job wrote nothing, so it was never undoable.
18
+
19
+
**The remedy.** Read `failed` as "this job imported nothing"; read `errors` and `results` for why. A client that wants to treat such a job as finished can treat `succeeded` and `failed` alike as terminal, as before.
0 commit comments