Skip to content

Commit 4842762

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-15944-completed-run-phantom-strand
2 parents 165037c + 6a1e382 commit 4842762

19 files changed

Lines changed: 2046 additions & 166 deletions
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
`flow-update-readonly-field` and `hook-api-update-readonly-field` now report a non-system **create** of a static-`readonly` field — a new **error**-severity finding that fails `os lint` / `os validate` / `os build` on a shape they used to accept.
6+
7+
Both rules scanned only the update verb (`update_record`; `ctx.api…update()` / `.updateById()`) and justified the omission with the same sentence: INSERT is engine-exempt from the author-declared `readonly` strip, so a create that seeds a `readonly` column is not a no-op. The maintainer ruling of 2026-09-03 (option C, #14147) made that false — `engine.insert` now runs the same `isSystem`-gated `stripReadonlyFields` the update path runs — so a flow `create_record` without `runAs: 'system'`, or a hook body's `ctx.api.object('…').insert()` under a non-system trigger, that writes a `readonly` field became a **silent no-op**: the row lands without the column (which falls back to its `defaultValue`), the step reports `success`, and only a run-time warning names the dropped field (measured end to end in `@objectstack/service-automation`'s `create-record-readonly-drop.test.ts`). Nothing reported it at build time. This closes that scan gap (#15394).
8+
9+
**What now fails that passed before.** Exactly one new shape per rule, at `error`:
10+
11+
- a flow `create_record` node whose literal `fields` map writes a field the target object declares `readonly: true`, on a flow that does not declare `runAs: 'system'`;
12+
- an L2 hook body's literal `ctx.api.object('<name>').insert({ … })` writing such a field, on a hook that does not declare `runAs: 'system'`.
13+
14+
The rule ids and severities are the update ones — one id per shape, not per verb — and each finding's message names the verb it was judged on and what actually happens to a create. Everything the rules already skipped is still skipped: a templated object name, a non-literal payload, an object outside the stack or declaring no fields, an unknown field (the unknown-field rules' question), and any `runAs: 'system'` flow or hook, because seeding a `readonly` column at create time is a system act and that write lands.
15+
16+
**Deliberately not reported.**
17+
18+
- No `readonlyWhen` (conditional) finding on a create, on either surface: a conditional lock is evaluated against the record being written over, which a create does not have, and the engine runs no conditional strip on INSERT ("INSERT stays exempt"). A warning there would state something false about a write that lands.
19+
- The hook rule judges `.insert()` only, not `.create()`. The host `ObjectRepository` aliases `create()` to `insert()`, but L2 bodies run in QuickJS and the VM-side `ctx.api.object()` installs no `create` leaf — a body calling `.create()` throws `TypeError: not a function` on its first run, a loud failure rather than the silent drop this rule reports. The silence is recorded as a reasoned method exclusion (`READONLY_HOOK_METHOD_EXCLUSIONS`) and pinned.
20+
- No create finding on a **platform object** — one declaring `managedBy`, or in the reserved `sys_` namespace. The engine's create-side strip does not judge those at all (`staticReadonlyInsertSubject`: their own ADR-0086 write guard governs them), so a finding there would describe a strip that never runs. The update verb keeps judging them, exactly as the engine's update path does.
21+
- `validate-readonly-action-writes` is unchanged: an action body runs system-elevated by design, so its create genuinely lands.
22+
23+
**Migration.** If your build reds on the new finding, the fix is one of: declare `runAs: 'system'` on the flow or hook when seeding the `readonly` column is the intent (the intended channel — `readonly` governs the end-user/API surface, not trusted system writers); remove the key from the `create_record` `fields` / `insert()` payload when it is not; or stamp it in a `beforeInsert` hook on the target object (`ctx.input.<field> = …`), which is a server value the strip does not touch. Measured over this repository's shipped examples (`app-crm`, `app-showcase`, `app-todo`): zero in-repo flows or hooks go red — the two `create_record` nodes that target an object carrying a `readonly` field write none of its `readonly` fields, and the one flow that creates unauthenticated already declares `runAs: 'system'`; no shipped hook body inserts through `ctx.api`.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
---
4+
5+
`findReferencesToMeta`'s unanswerable-target refusal now opens with prose instead of a machine-shaped `[unanswerable_target]` tag that nothing read.
6+
7+
```
8+
before 501 {"error":{"code":"NOT_IMPLEMENTED","message":"[unanswerable_target] References to a 'field' item cannot be computed. … Ask the owning object instead: GET /api/v1/meta/object/account/references."}}
9+
after 501 {"error":{"code":"NOT_IMPLEMENTED","message":"References to a 'field' item cannot be computed. … Ask the owning object instead: GET /api/v1/meta/object/account/references."}}
10+
```
11+
12+
Nothing else moves: same `501`, same `NOT_IMPLEMENTED`, same envelope position, and the prescriptive sentence ADR-0110 D3 requires is untouched. Callers branch on `code`, which is unchanged; only the human-facing sentence is shorter.
13+
14+
Why the tag was wrong here specifically. This producer writes a bracketed tag on many refusals, and every other one is the lowercase restatement of that throw's own declared `code``[item_locked]` with `ITEM_LOCKED`, `[no_draft]` with `NO_DRAFT`, `[invalid_request]` with `INVALID_REQUEST`. Measured across the two producer files, 30 of the 31 tagged throw sites that declare a code restate it that way. This refusal declares `NOT_IMPLEMENTED`, so its tag was the sole exception: it named a token the envelope carries on no axis, and a repo-wide search finds no parser, no switch, no assertion and no doc that reads it. Per the ruling behind the `/data` door's `FORBIDDEN:` prefix removal, `error` is human language and `code` is the machine token.
15+
16+
It became worth fixing when the `/meta/:type/:name/references` door started relaying the producer's prose verbatim: before that the whole sentence was replaced by `Internal server error` and the tag reached nobody, and after it the tag was the first thing an operator read on the screen where they decide whether to delete something. The `@objectstack/rest` entry in this release quotes the pre-removal sentence in its example; this entry is the later word on that wire text.
17+
18+
The absence is now pinned in `protocol.reference-target-unanswerable.test.ts` — nothing pinned the tag, so without a pin nothing would have pinned its removal either.

.github/workflows/ci.yml

Lines changed: 88 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ jobs:
539539
node scripts/partition-test-shards.mjs "$RUNNER_TEMP/turbo-ls.json" \
540540
--shard ${{ matrix.shard }}/6 --exclude @objectstack/dogfood \
541541
> "$RUNNER_TEMP/shard-packages.txt"
542-
echo "Packages on this shard:"
542+
echo 'Items on this shard (a package name, or a package plus a k/n file-level slice):'
543543
cat "$RUNNER_TEMP/shard-packages.txt"
544544
545545
# --concurrency=4: turbo's default (10) oversubscribes the 4-vCPU
@@ -587,11 +587,69 @@ jobs:
587587
# vitest's own "use the default" signal. Needs turbo.json's
588588
# globalPassThroughEnv entry or turbo strips it — see the script header.
589589
export VITEST_MAX_WORKERS="$(node scripts/vitest-worker-cap.mjs)"
590-
FILTERS=$(sed 's/^/--filter=/' "$RUNNER_TEMP/shard-packages.txt" | tr '\n' ' ')
591590
mkdir -p "$RUNNER_TEMP/stall-reports"
592-
node scripts/run-with-stall-guard.mjs --log "$RUNNER_TEMP/test-core.log" --stall-minutes 10 \
593-
--report-dir "$RUNNER_TEMP/stall-reports" -- \
594-
pnpm turbo run test $FILTERS --concurrency=4 --summarize --log-order=stream
591+
592+
# Split the shard's ITEMS into the whole packages, which share one
593+
# turbo run as they always have, and the file-level slices, which
594+
# cannot: `--shard=k/n` is passed through to vitest by turbo as a
595+
# RUN-level argument, so it would reach every package in the run —
596+
# and on any package with fewer test files than n that is a hard
597+
# vitest failure (or, with --passWithNoTests, silently no tests at
598+
# all). A slice therefore gets its own invocation, filtered to the one
599+
# package the partitioner sliced.
600+
FILTERS=""
601+
SLICES=""
602+
while read -r PKG SLICE; do
603+
[ -n "$PKG" ] || continue
604+
if [ -n "$SLICE" ]; then
605+
SLICES="$SLICES $PKG=$SLICE"
606+
else
607+
FILTERS="$FILTERS --filter=$PKG"
608+
fi
609+
done < "$RUNNER_TEMP/shard-packages.txt"
610+
611+
# Each leg tees to its OWN log — run-with-stall-guard opens the log
612+
# with 'w', so a second leg pointed at one path would truncate the
613+
# first leg's output and the completeness guard below would grade half
614+
# a shard. They are concatenated afterwards, and that concatenation
615+
# happens even when a leg failed, because a red suite is exactly when
616+
# the completeness guard earns its keep.
617+
#
618+
# ⛔ A failing leg STOPS the remaining ones, the same way turbo stops
619+
# scheduling on the first failure inside one run. Carrying on would add
620+
# a second full suite to a job that is already red and already inside a
621+
# 30-minute wall — turning an informative red into a killed job with no
622+
# attestation at all, which is the #16173 failure mode itself.
623+
STATUS=0
624+
LOGS=""
625+
for LEG in __whole__ $SLICES; do
626+
if [ "$LEG" = __whole__ ]; then
627+
[ -n "$FILTERS" ] || continue
628+
LOG="$RUNNER_TEMP/test-core-packages.log"
629+
set -- pnpm turbo run test $FILTERS --concurrency=4 --summarize --log-order=stream
630+
else
631+
PKG="${LEG%%=*}"
632+
SLICE="${LEG#*=}"
633+
LOG="$RUNNER_TEMP/test-core-slice-$(printf '%s' "$PKG" | tr -c 'A-Za-z0-9' '-').log"
634+
set -- pnpm turbo run test "--filter=$PKG" --concurrency=4 --summarize --log-order=stream -- "--shard=$SLICE"
635+
fi
636+
LOGS="$LOGS $LOG"
637+
node scripts/run-with-stall-guard.mjs --log "$LOG" --stall-minutes 10 \
638+
--report-dir "$RUNNER_TEMP/stall-reports" -- "$@" || { STATUS=$?; break; }
639+
done
640+
641+
# Only over logs that EXIST, and via an explicit `if` rather than a
642+
# `&&` chain: a leg whose guard never got far enough to open its log
643+
# would otherwise make `cat` non-zero, and under `set -e` that replaces
644+
# the SUITE's exit status with cat's — the step would report the wrong
645+
# reason for its own red.
646+
: > "$RUNNER_TEMP/test-core.log"
647+
for LOG in $LOGS; do
648+
if [ -f "$LOG" ]; then
649+
cat "$LOG" >> "$RUNNER_TEMP/test-core.log"
650+
fi
651+
done
652+
exit $STATUS
595653
596654
# --summarize above costs nothing at runtime and writes
597655
# `.turbo/runs/<id>.json`: one per-task record with the execution window
@@ -635,6 +693,31 @@ jobs:
635693
if-no-files-found: ignore
636694
retention-days: 1
637695

696+
# ⛔ THE DRIFT STEP IS DELIBERATELY NOT WIRED HERE YET (#16173).
697+
#
698+
# partition-test-shards.mjs carries a fully-tested `--check-drift` mode —
699+
# it reads the summary uploaded above back and reds when a shard's MEASURED
700+
# test total outruns its PREDICTED one past MAX_MEASURED_OVER_PREDICTED. The
701+
# code, its self-tests and its ablation are all on this branch; only this
702+
# invocation waits, and the wait is a SEQUENCING decision, not an oversight.
703+
#
704+
# Why: scripts/test-shard-timings.json is still stale for @objectstack/cli
705+
# (458.15s recorded, 1231.52s measured), so wiring the step today would red
706+
# the shard carrying a CLI slice on every single PR — a true reading, but one
707+
# that blocks everything until the dataset is refreshed. The file-level split
708+
# above already removes the urgent hazard on its own, taking the worst shard
709+
# from ~1445s (80% of this job's 30-minute wall) to ~1059s (59%) with the
710+
# dataset untouched, because the CLI is halved across two runners instead of
711+
# falling on one.
712+
#
713+
# ⇒ The refresh and this step land TOGETHER in the follow-up, in that order.
714+
# The refresh recipe is in PR #16220's body. When it lands, restore a step
715+
# here that runs `--check-drift` over `.turbo/runs/*.json` with
716+
# `--label "Test Core (${{ matrix.shard }}/6)"`, with NO `if:` and NO
717+
# `continue-on-error` (the point is the red), placed ABOVE the attestation
718+
# pair for the #6082 reason documented on the upload above — so a drift red
719+
# also withholds the attestation, which is the fail-closed direction.
720+
638721
# Runs even when the suite failed — that is when it earns its keep. It
639722
# answers TWO questions about a red suite, and needs both to be able to
640723
# say anything at all about a green one.

content/docs/automation/hook-bodies.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,10 +265,10 @@ There is an asymmetry here that costs data if you learn it the hard way, so lear
265265

266266
The dropped case is the dangerous one: nothing fails, the step reports success, and the column is simply always null. Because both halves of that judgement are declared in your own stack, it is checked at author time and **gates the build**:
267267

268-
- `hook-api-update-readonly-field`**error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` writes a field the named object declares `readonly: true`.
268+
- `hook-api-update-readonly-field`**error**. A body's literal `ctx.api.object('…').update()` / `.updateById()` / `.insert()` writes a field the named object declares `readonly: true`. Since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) the `insert` row of the table above is reported at build time exactly like the `update` row — same id, same severity, a message naming the verb — unless the hook declares `runAs: 'system'`. Only the static shape is judged on an insert: a `readonlyWhen` field has no prior record to lock on and the engine runs no conditional strip on INSERT, so no warning is produced there.
269269
- `hook-api-update-readonly-when-field`**warning**. The same write against a `readonlyWhen` field, which strips per record *state*. The own-hook stamp **is** the workaround here, exactly as it is for static `readonly`: since [#9107](https://github.com/objectstack-ai/objectstack/issues/9107) the conditional strip judges the *caller's* entry payload, so a value a `beforeUpdate` hook **derives** is not caller-supplied and lands even on a locked record. (Deriving is the operative word — a hook that merely echoes the caller's own value back has written nothing the strip can tell from the caller's, and it still goes.) What does **not** help is elevation: unlike the static strip, the conditional lock is **not** waived by a system context, so neither `runAs: 'system'` nor the `sudo()` a body cannot reach makes a caller-supplied value survive. On this shape, confirm the write only targets records whose predicate is `false`, or derive the field in a `beforeUpdate` hook on the target object.
270270

271-
Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name and an object this stack does not declare are all skipped, so the rule has no opinion on them. `insert`/`create` are skipped too — but since the 2026-09-03 ruling that is a **scan gap**, not an exemption: the write is dropped exactly as the table says, and nothing reports it at build time yet ([#15394](https://github.com/objectstack-ai/objectstack/issues/15394)). The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425), with the same gap on `create_record`.
271+
Only literal object names and literal payload keys are seen; a `sudo()` chain, a dynamic object name and an object this stack does not declare are all skipped, so the rule has no opinion on them. `.create()` is skipped too, for a reason about the **sandbox** rather than the engine: the VM-side `ctx.api.object()` installs `insert` / `update` / `delete` / `updateMany` / `deleteMany` / `upsert` and no `create` leaf, so a body calling `.create()` throws `TypeError: not a function` on its first run — a loud failure, not a silent drop — and the same payload spelled `.insert()` is what the rule judges. The flow surface has carried the same gate as `flow-update-readonly-field` since [#3425](https://github.com/objectstack-ai/objectstack/issues/3425), and since [#15394](https://github.com/objectstack-ai/objectstack/issues/15394) it reports a non-`runAs: 'system'` `create_record` node's static-`readonly` write at the same **error**, again with no conditional finding on a create.
272272

273273
The table above is about a **hook** body. An **action** body is the one surface where the answer changes, so read this before you move a body from one to the other: an action body runs **elevated** — its `ctx.api` is built over the caller's envelope with `isSystem` set, which is the same trusted posture that lets an action bypass row and field permissions — and the static strip applies only to non-system callers. So `ctx.api.object('x').update({ someReadonlyField })` **lands** in an action, and there is no finding for it. Elevation does not waive the *conditional* lock, though, so that half does carry across: `action-api-update-readonly-when-field` — a **warning** — on an action body's literal `ctx.api` update to a `readonlyWhen` field ([#13770](https://github.com/objectstack-ai/objectstack/issues/13770)). Net effect when you move a body: a `readonly` write changes behaviour, a `readonlyWhen` write does not.
274274

0 commit comments

Comments
 (0)