Skip to content

Commit cef5389

Browse files
os-zhuangclaude
andcommitted
Merge origin/main — drop this branch's locks_record, keep main's lock_record
main shipped the same feature first. #3815 (`a6c3f3806`, #3814/objectui#2902) landed `ApprovalRequestRow.lock_record`, computed `cfg?.lockRecord !== false` off the `node_config_json` snapshot — byte-for-byte the rule this branch's `locks_record` computes, from the same snapshot, with the same default. Only the test file collided textually. `approval-service.ts` and the spec contract auto-merged *clean*, which is the dangerous part: the result declared both fields, so `GET /api/v1/approvals/requests` would have shipped one policy under two names and every client would have had to guess which to read. Resolved toward main: `lock_record` is already merged and released in a changeset, so dropping it would be a breaking change layered on a duplicate. Removed from this branch: - `locks_record` on `rowFromRequest` (plugin-approvals) - `locks_record?: boolean` on `ApprovalRequestRow` (spec contract) - its 3 tests — main's 3 `lock_record` tests are a superset, covering `openNodeRequest` / `listRequests` / `getRequest` on all three cases - the `locks_record` half of the changeset, which main's `approvals-expose-lock-record.md` already describes The changeset is rewritten to the half that is still this branch's own (`droppedFields` on `POST /batch`) and renamed to match; `plugin-approvals` comes off its bump list, because after this resolution the branch no longer touches that package. #3835 (batch creates through the create ingress) is untouched and keeps its own changeset. Console follow-up, NOT covered here: objectstack-ai/objectui#2914 is still open and reads `locks_record`. It must be repointed at `lock_record` or the band falls back to "assume locked" — the exact bug #3794 filed. Verified after resolution: build 71/71 · spec 6836 · rest 419 (incl. all 5 new batch tests) · plugin-approvals 318 (incl. main's 3 `lock_record` tests) · metadata-protocol 71 · no generated-artifact drift. Co-Authored-By: Claude <noreply@anthropic.com>
2 parents 4dda4e9 + 647ec8b commit cef5389

373 files changed

Lines changed: 20887 additions & 3105 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.

.changeset/adr-0105-d8-delegated-admin-org-role.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ subtree. That gate is implemented, unit-proven and reachable — but no principa
1414
could reach it in a state where it did anything:
1515

1616
- better-auth grants `invitation: ["create"]` to `owner` and `admin` only
17-
(`memberAc` holds `invitation: []`, and roles registered through
18-
`additionalOrgRoles` inherited that empty statement);
17+
(`memberAc` holds `invitation: []`, which every other registered role
18+
inherits);
1919
- under a wall-enforcing posture, owners and admins are auto-elevated to
2020
`organization_admin` (`auto-org-admin-grant.ts`), which carries the wildcard
2121
`modifyAllRecords` that makes `isTenantAdmin()` true — and the gate
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.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/lint": minor
3+
"@objectstack/spec": patch
4+
---
5+
6+
feat(lint): `validate-ai-surface-affinity` — skill ↔ agent surface affinity is now linted (#3820)
7+
8+
An agent binds a product surface (`'ask'` | `'build'`, ADR-0063 §1) and a skill
9+
declares which surface it belongs to (`'ask'` | `'build'` | `'both'`, §3). The
10+
runtime refuses an incompatible binding with a **load error at chat time**
11+
after parse, validate, and deploy all passed cleanly. The new rule reports that
12+
contradiction statically, and joins `REFERENCE_INTEGRITY_RULES`, so
13+
`objectstack validate`, `lint`, and `compile` all pick it up with no CLI
14+
changes.
15+
16+
Scope is deliberately narrow (zero false positives by construction): only
17+
bindings where **both** the agent and the skill are declared in the same stack
18+
are checked. `agent.skills[]` names that don't resolve in-stack (kernel skills
19+
are runtime-registered and statically invisible) are skipped — resolving those
20+
namespaces is #3820 D0/D2, decided by ADR-0109 (Proposed).
21+
22+
The spec side is doc-truth only, no schema shape changes:
23+
24+
- `stack.agents` is documented as **platform-internal** (ADR-0063 §2 — the
25+
kernel ships exactly two agents; third parties extend via skills), replacing
26+
prose that still described the withdrawn ADR-0040 per-app-copilot model.
27+
- `stack.tools` is documented as declaration-only pending the ADR-0109 tool
28+
authoring model.
29+
- `app.defaultAgent` is re-documented as a surface-binding knob (`'ask'`
30+
implicit / `'build'` for authoring surfaces), not a custom-agent slot.
31+
- `SkillSchema` now states that a per-skill `permissions` field deliberately
32+
does not exist (ADR-0049) — authoring one is silently stripped; access is
33+
gated by `agent.access` / `agent.permissions` and per-tool authz.
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
---
2+
"@objectstack/types": minor
3+
"@objectstack/service-analytics": minor
4+
"@objectstack/runtime": minor
5+
"@objectstack/rest": patch
6+
---
7+
8+
fix(analytics,runtime,types): gate cube auto-inference on object existence; stop the dispatcher boundary returning raw SQL (#3867)
9+
10+
Two independent defects on the `/analytics` surface, found while verifying #3770
11+
against a real server. On an authenticated CRM dev server, before this change:
12+
13+
```
14+
POST /api/v1/analytics/query {"cube":"sqlite_master","measures":["count"],"dimensions":["type"]}
15+
→ 200 {"rows":[{"type":"index","count":262},{"type":"table","count":71},{"type":"view","count":1}],
16+
"sql":"SELECT type AS \"type\", COUNT(*) AS \"count\" FROM \"sqlite_master\" GROUP BY type"}
17+
```
18+
19+
That is SQLite's internal schema table — never a registered object — read
20+
successfully through the analytics endpoint. Not merely "the name reaches the
21+
driver and errors": **any table the connection can see was readable.**
22+
23+
**① The cube name reached the driver as a table name.** `AnalyticsService.ensureCube`
24+
auto-infers a minimal Cube when none is registered, with `cube.sql = <the queried
25+
name>`. That is the intended "metric over an object" path — an `object-metric` KPI
26+
widget queries `crm_account` with no authored Cube — but it accepted *any* string,
27+
so the endpoint could aggregate over an arbitrary physical table. The
28+
analytics-side twin of the data-path gap #3770 closed, and it was not covered by
29+
that fix: #3770 gated the protocol's `analyticsQuery`, which is the *degraded
30+
fallback*; a deployment with `@objectstack/service-analytics` installed runs the
31+
real engine instead (`ctx.replaceService`).
32+
33+
Inference is now gated on the same schema registry the data path consults, via a
34+
new optional `AnalyticsServiceConfig.isRegisteredObject` that `plugin.ts` wires
35+
from the `data` engine's `getObject`. Three-way rule: a registered Cube runs
36+
untouched (its `sql` is whatever it declares); an unregistered name that IS an
37+
object still auto-infers exactly as before; neither → `CUBE_NOT_FOUND` / 404
38+
raised before any SQL exists, naming both ways to make the request valid. With no
39+
probe configured the gate stands down and warns once — the same tiering #3770
40+
took for a missing registry. `generateSql` (`/analytics/sql`) is gated too.
41+
42+
**② The dispatcher boundary returned `err.message` verbatim.** `errorResponseBase`
43+
is the single error exit for *every* route the dispatcher plugin mounts —
44+
`/analytics`, `/packages`, `/i18n`, `/storage`, `/automation`, `/auth`,
45+
`/notifications`, `/mcp`. `@objectstack/rest` has guarded its data routes against
46+
driver dumps forever (`mapDataError`); this boundary guarded nothing, so any
47+
driver error on any of those routes shipped its SQL to the client. Unlike ①, this
48+
half is unconditional — it does not depend on the cube being invalid.
49+
50+
The leak heuristic moved out of `rest-server.ts` into `@objectstack/types` as
51+
`looksLikeInternalErrorLeak` (both packages already depend on it) and is now
52+
applied at both boundaries — one predicate, one place to widen when a new
53+
dialect's phrasing shows up. `mapDataError`'s behaviour is unchanged. At the
54+
dispatcher it applies **only to 5xx**: a 4xx message is a deliberate
55+
business/validation answer and must reach the caller intact. Sanitising costs no
56+
diagnostics — the untouched error still reaches `errorReporter` through the
57+
existing `__obsRecordedError` side-channel.
58+
59+
**Also fixed in the same function:** `errorResponseBase` read only
60+
`err.statusCode`, while domain errors across this codebase carry `status` (and
61+
`HttpDispatcher.errorFromThrown` already reads `status` first). Every deliberate
62+
4xx thrown through a dispatcher route — including #3770's `OBJECT_NOT_FOUND` on
63+
the analytics fallback path — was rendered as a **500**. It now reads `status`
64+
then `statusCode`.
65+
66+
**Behaviour change.** `/analytics/query` and `/analytics/sql` return 404
67+
`CUBE_NOT_FOUND` for a cube that is neither registered nor a registered object;
68+
previously the name was passed to the driver. Dashboards and KPI widgets pointed
69+
at real objects or authored cubes are unaffected. A 5xx on a dispatcher route
70+
whose message looks like a driver dump now reads `Internal server error` — check
71+
server logs or your error reporter for the original.

0 commit comments

Comments
 (0)