Skip to content

Commit c0cbf6b

Browse files
committed
Merge origin/main: co-append validateFlowTemplatePaths + validateAiSurfaceAffinity to the suite
Both branches appended a rule to REFERENCE_INTEGRITY_RULES; resolution keeps main's validateFlowTemplatePaths (#3810) first and this branch's validateAiSurfaceAffinity last, with the suite test's fixture and report-order assertions carrying both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BHjroNkLkajskKbJaidko4
2 parents da774f9 + fd7cfde commit c0cbf6b

237 files changed

Lines changed: 11329 additions & 952 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
"@objectstack/spec": minor
3+
"@objectstack/cli": patch
4+
---
5+
6+
feat(spec,cli): warn the author when a deprecated action alias is discarded (#3743)
7+
8+
#3742 made `target` beat the deprecated `execute` alias everywhere and had the
9+
`ActionSchema` transform **drop** the alias from its output, so "two different
10+
scripts for one button" became unrepresentable. What it left behind: an author
11+
who declares both slots with different values still loses one of the two
12+
handlers they wrote, **silently**. Per Prime Directive #12 that belongs at
13+
authoring time, so it is now reported there.
14+
15+
**New rule — `action-target-execute-conflict` (advisory).** An action declaring
16+
both `target` and `execute` with different values gets a warning naming both
17+
handlers, stating that `target` wins, and giving the one-line fix (delete
18+
`execute`). Identical values in both slots are harmless duplication and stay
19+
quiet. It never fails the build: the resulting stack is well-defined — the cost
20+
is a handler that never runs, not a broken artifact.
21+
22+
The rule must run **pre-parse**, because the parse is what consumes the alias:
23+
once `ObjectStackDefinitionSchema` has run there is no `execute` key left to
24+
report. It therefore lives in `@objectstack/spec`
25+
(`lintDeprecatedAliases`, exported from the package root) and is wired into
26+
both layers that perform the discard:
27+
28+
- **`defineStack`** — the dominant authoring path, and the one that consumes the
29+
alias earliest: it parses inside your own config module, so by the time
30+
`os build` loads that module the alias is already gone. It now warns on the
31+
console before parsing (once per distinct conflict per process).
32+
- **`os build` / `os validate`** — a new pre-parse pass covering stacks that
33+
skip strict `defineStack`: a plain object default-export,
34+
`defineStack(…, { strict: false })`, and inline function handlers (`target` is
35+
`z.string()`, so those cannot pass strict `defineStack` and are lowered by the
36+
CLI instead). Both commands lint the same input, so they agree by construction
37+
(#3782).
38+
39+
Each layer reports only its own discards, so one authored conflict produces
40+
exactly one warning however the stack is compiled.
41+
42+
**Behaviour fix in the same contract.** #3742 fixed compile-time precedence by
43+
probing for a *callable* `target` first, which left one combination still
44+
resolving the alias's way: a **string** `target` beside a **function** `execute`
45+
bound the alias and then overwrote the canonical ref the author wrote. `target`
46+
now wins in every combination of string/function across the two slots, matching
47+
the `ActionSchema` transform — so the new warning states one precedence rule
48+
that is true everywhere. If you relied on an inline `execute` function winning
49+
over a string `target`, move it into `target`; the warning names the action.
50+
51+
Authoring is otherwise unchanged: `execute` alone is still accepted, still
52+
lowered into `target`, and still documented.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/cli": patch
4+
---
5+
6+
fix(spec): `action.undoable` is `live`, not `experimental` — stop warning on a property that works (#3714)
7+
8+
The liveness ledger marked `action.undoable` `experimental` on a #1992-era note:
9+
*"no runtime reader yet — neither service-automation nor objectui consume the
10+
action's `undoable` flag (objectui has an UndoManager but does not key off this
11+
field)."* That was true when written. objectui has since wired **two** readers,
12+
both gating real behaviour:
13+
14+
| Reader | What the flag gates |
15+
|---|---|
16+
| app-shell `useConsoleActionRuntime.tsx:409` | builds the undo operation the success toast's Undo button invokes (`:147`) |
17+
| app-shell `RecordDetailView.tsx:545` | restores the record's prior field values (`:404`) |
18+
19+
`components` `action/action-button.tsx:113` forwards the flag for exactly this
20+
reason, per its own comment: *"without this the flag is dropped and the handler
21+
never builds the undo operation."*
22+
23+
**Why it mattered.** The CLI liveness lint warns on `experimental` as well as
24+
`dead`, so authoring a *working* property produced a
25+
`liveness-experimental-property` warning — "declared but NOT enforced at
26+
runtime". An author (or an AI) reading the ledger or that warning concludes
27+
`undoable` is aspirational and skips it, losing a shipped feature. Authoring
28+
`undoable: true` is now silent, and the protocol reference no longer claims
29+
setting it "currently has no effect".
30+
31+
Nothing to migrate: the schema, the parsed shape, and the runtime are unchanged
32+
— only the classification of what they already do.
33+
34+
This is the *understating* failure direction, the mirror of the preview-renderer
35+
over-claims corrected in #3685/#3711/#3686. Both directions have the same root
36+
cause, now written into `packages/spec/liveness/README.md`: **a ledger entry is a
37+
claim with a timestamp, and code moves under it in both directions** — entries
38+
are worth re-verifying rather than trusting indefinitely.
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
"@objectstack/plugin-approvals": minor
3+
"@objectstack/spec": minor
4+
"@objectstack/lint": minor
5+
---
6+
7+
feat(approvals): cross-organization approver targeting — a plant document can
8+
require a group-side sign-off (ADR-0105 D9)
9+
10+
One organization id used to decide three different things at once in
11+
`openNodeRequest`: where the request row lives, where its inbox index rows
12+
live, and **where its approvers are looked up**. The first two are the
13+
request's own organization by definition. The third is not — a group CFO holds
14+
her `cfo` position in the GROUP organization while the purchase order she signs
15+
off lives in the PLANT organization. `expandPositionUsers('cfo', <plant>)`
16+
matched nobody, the slot fell back to the dead `position:cfo` literal, and a
17+
group escalation could not be expressed at all.
18+
19+
An approver may now declare which organization's directory resolves it:
20+
21+
```yaml
22+
approvers:
23+
- { type: position, value: plant_manager, group: plant }
24+
- { type: position, value: cfo, organization: $root, group: finance }
25+
behavior: per_group
26+
```
27+
28+
- **`$root` / `$parent`** walk D6's `parent_organization_id` tree, so the two
29+
common intents need **no deployment knowledge** — flow metadata is portable
30+
across environments while organization ids are minted per deployment. A slug
31+
covers what the symbols cannot, notably a **sibling** organization (a
32+
shared-services centre approving payables for every plant).
33+
- Declared **per approver**, so one node can require a plant manager and a
34+
group CFO in parallel. A node-level form cannot express that without
35+
splitting into serial nodes, which changes the semantics.
36+
- **Bounded, not free:** the target must share a `parent_organization_id` root
37+
with the request's organization. The rule reads only the organization tree —
38+
never the submitter — so one flow routes identically for everyone.
39+
40+
Everything else fails loudly rather than quietly:
41+
42+
- a non-`group` posture **refuses** the declaration (a `group` → `isolated`
43+
migration must not silently reroute approvals);
44+
- an approver type with no org-scoped directory (`user` / `field` / `manager` /
45+
`team`) refuses it too, and a new `approval-approver-cross-org-unsupported`
46+
lint catches that at author time;
47+
- a targeted approver holding no membership in the request's organization is
48+
dropped with a warning naming them — D2's union wall would otherwise hide the
49+
request from someone already routed to, so the node's existing
50+
`onEmptyApprovers` policy takes over instead of leaving an unopenable task.
51+
52+
Nothing changes for an approver without `organization`: same resolution, same
53+
queries, no extra reads.
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/plugin-approvals": patch
3+
---
4+
5+
fix(approvals): refuse `organization` on directory-less approver types instead
6+
of silently ignoring it (ADR-0105 D9)
7+
8+
`user`, `field` and `manager` return EARLY in `resolveApproverSpec` — they name
9+
a person outright rather than expanding a directory. D9's org resolution was
10+
placed after those returns, so an `organization` declared on one of them never
11+
reached the check: it was silently INERT.
12+
13+
That is the one behaviour ADR-0105 D9 rules out and the authoring docs
14+
explicitly promise against ("`organization` on those is refused at runtime").
15+
The `os lint` rule caught it at author time, but the runtime claim was false —
16+
and a stored flow that predates the lint, or one assembled programmatically,
17+
got no signal at all.
18+
19+
Resolution now happens at the top of `resolveApproverSpec`, above every early
20+
return, so the refusal reaches all three types. The ordinary path is unchanged
21+
and still costs nothing: with no `organization` declared the resolver returns
22+
the request's organization without reading anything.
23+
24+
Found by cloud's group-posture dogfood driving a real `group` boot — the
25+
resolver's own unit tests could not see it, because they call the resolver
26+
directly and never traverse the early return.
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
---
2+
"@objectstack/client": major
3+
"@objectstack/spec": major
4+
---
5+
6+
feat(client,spec)!: the SDK's `ai` namespace now expresses the AI surface that exists (#3718)
7+
8+
`client.ai` and the AI service were **disjoint sets**. The namespace held three
9+
methods — `nlq`, `suggest`, `insights` — whose URLs no repo has ever mounted
10+
(removed in v17), while `service-ai` mounted 12 routes the SDK could not reach
11+
at all. v17 closed the first half by deleting the dead methods. This closes the
12+
second: the SDK now reaches every route that is meant to be tenant API surface.
13+
14+
| SDK | Route |
15+
|---|---|
16+
| `ai.chat(request)` | `POST /api/v1/ai/chat` — forces `stream: false`, so the JSON mode is what you get |
17+
| `ai.chatStream(request)` | `POST /api/v1/ai/chat``AsyncIterable` of UI Message Stream frames |
18+
| `ai.complete(request)` | `POST /api/v1/ai/complete` |
19+
| `ai.models()` | `GET /api/v1/ai/models` — the ADR-0028 plan-filtered picker list |
20+
| `ai.conversations.create/list/get/update/delete/addMessage` | the six `/api/v1/ai/conversations` routes |
21+
22+
`ai.chatStream` returns a promise for an async iterable rather than being an
23+
async generator, so the request is issued — and an HTTP error thrown — when you
24+
call it, not when you first iterate.
25+
26+
**Where the server is.** `service-ai` is a Cloud/EE package in the `cloud`
27+
repo; this repo only proxies `/api/v1/ai/**` and 404s `AI service is not
28+
configured` without it. Check `discovery.services` before calling, exactly as
29+
for any other plugin-provided namespace. For a React chat UI, `useChat()`
30+
(`@ai-sdk/react`) is still the better client — it speaks the same protocol
31+
`ai.chatStream` parses and owns message state; these methods are for callers
32+
that are not components.
33+
34+
**Breaking — the spec's dead AI declarations are retired.** All three had no
35+
implementation anywhere and no runtime consumer:
36+
37+
- `Ai{Nlq,Suggest,Insights}{Request,Response}[Schema]` → replaced by the wire
38+
shapes of the real routes: `AiChat{Request,Response}`, `AiStreamChunk`,
39+
`AiCompleteRequest`, `AiModelsResponse`, `AiConversation`, `AiMessage`,
40+
`{Create,List,Update}AiConversation*`. The six retired JSON Schemas are
41+
dropped from `json-schema.manifest.json` (deliberate retirement, #2978).
42+
- `DEFAULT_AI_ROUTES` → deleted, and `getDefaultRouteRegistrations()` returns 8
43+
groups instead of 9. It declared the three phantom endpoints and had no
44+
runtime consumer; re-declaring the real ones here would recreate the same
45+
illusion, since they are mounted from another repo.
46+
- `AiProtocol` (`aiNlq?` / `aiSuggest?` / `aiInsights?`) → deleted. Nothing
47+
implemented it and nothing dispatched through it. The real server contract is
48+
`IAIService` + `IAIConversationService` in `@objectstack/spec/contracts`.
49+
50+
**The guard.** `/api/v1/ai/` becomes a bounded prefix exemption in the capstone
51+
(#3642) alongside the control plane — bounded from both ends: only `ai.*` may
52+
use it, and the namespace must still be reaching it. That is not a
53+
wave-through. The reachability check lives where the routes are:
54+
`cloud`'s `packages/service-ai/src/ai-route-ledger.conformance.test.ts` reads
55+
the table `buildAIRoutes()` returns and drives this SDK against it, so an
56+
`ai.*` URL that stops resolving fails a test in the repo that mounts it. The
57+
wildcard-only bound stays **0** — these URLs never touch the `* /ai/**` row,
58+
which is what certified three dead methods for years.
59+
60+
The four replaced client tests are worth naming: they mocked `fetch` and
61+
asserted the URL the client *built*, never that anything answered it, and
62+
passed for years against endpoints that did not exist. The new ones assert only
63+
what this repo can honestly know — verb, path, and the body decisions the SDK
64+
makes for you (`stream: false` on `chat`, the 204 on `delete`, SSE frame
65+
parsing) — and leave "does it resolve" to the ledger next to the routes.

.changeset/ci-node-22-pin.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
---
2+
---
3+
4+
ci: validate on the runtime we publish from — pin every workflow to Node 22 (#3825)
5+
6+
CI ran two Node versions at once, and nobody had decided that. All 12 PR gates —
7+
Build Core, Test Core, TypeScript Type Check, Dogfood, ESLint, spec liveness,
8+
dep validation — were on **Node 20**, which reached EOL on **2026-04-30**, while
9+
`release.yml`, `publish-smoke.yml`, `scaffold-e2e.yml` and `showcase-smoke.yml`
10+
were on **22**. So code was verified on one runtime and shipped from another,
11+
and the runtime guarding every merge no longer received security patches.
12+
13+
The split was drift, not policy. `release.yml` carried the receipt in a comment
14+
*"22 (not 20 like the other workflows)"* — because a downstream clone pinned
15+
`engines.node >=22` and pnpm aborted on 20. One workflow got bumped to clear one
16+
error; the other twelve stayed behind, and nothing in CI could see the gap.
17+
18+
It surfaced only by accident in #3812: a test imported `better-sqlite3@13`,
19+
whose `engines` say `>=22`. `engines` is a declaration, not enforcement, so it
20+
loaded on Node 20 and then killed the vitest worker with a **process-level
21+
abort** — no JS error, so the suite reported `Test Files 22 passed (23)` while
22+
**17 cases silently never ran**. A green check that had quietly stopped running
23+
the tests.
24+
25+
All 18 `setup-node` steps now run Node 22, matching what release already used.
26+
27+
**`.nvmrc` is now the single source of truth.** It pins contributors' local
28+
runtime via `nvm use` — previously there was no pin at all, so a contributor on
29+
Node 24 could not reproduce a Node 20 gate failure — and `check:node-version`
30+
(new, wired into the unfiltered, always-required `lint` job) holds every
31+
workflow to it. A version pin is otherwise 18 independent string literals, which
32+
is why this drifted invisibly for so long; bumping Node is now a one-line edit
33+
to `.nvmrc` plus whatever the guard reports. The guard also fails a `setup-node`
34+
step that pins *nothing*, which would silently inherit the runner default.
35+
36+
Nothing about the published packages changes: `engines.node` stays `>=18.0.0`
37+
across all 49 of them. That is a promise to users about what ships, independent
38+
of what CI validates on, and tightening it is a breaking change — left for its
39+
own decision rather than folded in here.

.changeset/ci-node-eol-guard.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
---
2+
---
3+
4+
ci: fail the Node pin guard when the pinned runtime is out of support (#3825)
5+
6+
`check-node-version` proved all 18 workflows agree on a Node version. It would
7+
have said `OK` just as cheerfully when all 18 agreed on **Node 20, three months
8+
after that line went EOL** — which is exactly the state #3825 found the repo in.
9+
Consistency was only half the invariant; the other half is that the thing they
10+
agree on is still receiving patches.
11+
12+
The guard now also checks the pin's lifecycle:
13+
14+
- **past EOL → error.** An unpatched runtime is guarding every merge.
15+
- **within 180 days of EOL → `::warning::`** on the PR, so the bump is scheduled
16+
work rather than an emergency. It stays a warning until support actually ends —
17+
failing months early would block unrelated PRs.
18+
- **a major the guard has no dates for → error.** Adopting Node 26 forces you to
19+
record its dates rather than silently validating on an unknown runtime.
20+
21+
Dates come from `nodejs/Release` `schedule.json`, hardcoded deliberately: a
22+
required gate must not depend on the network, and they move once a year.
23+
24+
The success line now reports the runway, which surfaces something worth knowing:
25+
26+
```
27+
check-node-version: OK (18 setup-node step(s) across 16 workflow(s), all on Node 22).
28+
Node 22 is in maintenance; supported until 2027-04-30 (276 days).
29+
```
30+
31+
**Node 22 entered maintenance on 2025-10-21** — Node 24 has been Active LTS
32+
since 2025-10-28. Moving to 24 is now a one-line `.nvmrc` edit, and this guard
33+
will list the workflows to follow. That is a separate decision; nothing here
34+
forces it.
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
---
3+
4+
ci: assert every test vitest counted actually ran (#3825)
5+
6+
A vitest worker can die at the process level — a native module segfault, OOM, an
7+
abort inside a binding. There is no JS error to catch, so the cases that worker
8+
owned never run and the summary reports only what survived:
9+
10+
```
11+
Test Files 1 passed (40)
12+
Tests 21 passed (401)
13+
```
14+
15+
That leads with "passed". It is **380 tests short**. #3812 hit exactly this shape
16+
(17 cases silently skipped, reported as `22 passed (23)`) and it was found by a
17+
human reading the log closely — which is not a control.
18+
19+
**To be precise about the risk:** the run does exit non-zero, so the gate goes
20+
red. The failure mode is not a false green, it is **a red that reads like a
21+
pass** — someone triaging sees "passed" and a plausible file count and concludes
22+
one file flaked, rather than that a fifth of the suite never executed. This turns
23+
that into a specific, quantified error naming the package and the shortfall. It
24+
also covers the genuinely dangerous variant, where a crash lands somewhere that
25+
does not propagate a non-zero exit at all.
26+
27+
`scripts/check-test-completeness.mjs` reads a saved `turbo run test` log and
28+
asserts, per package, that the tallies (`passed | skipped | failed`) sum to the
29+
declared total. Reading the log rather than wrapping vitest means no change to
30+
the 60+ per-package vitest configs.
31+
32+
Wired into `ci.yml`'s Test Core (both the PR and push steps) and both Dogfood
33+
shards, each as a separate `if: always()` step so it runs **when the suite
34+
failed** — that is when it earns its keep. A red suite plus a green completeness
35+
check means real test failures; a red suite plus a red completeness check means a
36+
worker died.
37+
38+
Two details that are load-bearing rather than incidental:
39+
40+
- The test steps now `tee` their output, and `set -o pipefail` goes with it.
41+
GitHub runs these with `bash -e`, which does **not** set pipefail, so
42+
`turbo … | tee` would report *tee's* exit status and a failing suite would go
43+
green. Verified both ways: with pipefail the step exits 7, without it exits 0.
44+
- Zero summaries in the log is a **pass with an explicit note**, not a silent
45+
one — `turbo run test --affected` legitimately runs nothing when a PR touches
46+
no package.
47+
48+
Validated against the real logs from the #3830 Node 20/22 comparison rather than
49+
synthetic fixtures: the Node 22 log passes (`68 packages, 16678 declared and all
50+
16678 accounted for`), and the Node 20 log fails, naming `@objectstack/driver-sql`
51+
and its 380 missing tests.

0 commit comments

Comments
 (0)