Skip to content

fix(cli): read the declared response envelope in the three os datasource commands - #10761

Merged
os-elon merged 1 commit into
mainfrom
claude/issue-10675-datasource-envelope-unwrap
Aug 21, 2026
Merged

fix(cli): read the declared response envelope in the three os datasource commands#10761
os-elon merged 1 commit into
mainfrom
claude/issue-10675-datasource-envelope-unwrap

Conversation

@os-elon

Copy link
Copy Markdown
Collaborator

Fixes#10675

What was broken

All three os datasource subcommands read the pre-#3843 flat response
shape — body.tables, body.draft, body.results, and body.error as a
string — while every REST body the platform emits is the declared envelope
written by sendOk / sendError (packages/types/src/response-envelope.ts):

{ success: true, data: { … } }
{ success: false, error: { code, message } }

Nothing failed loudly. Each payload simply read undefined, and every command
reported that as an ordinary empty result:

commandbehaviour at headserver oracle
list-tablesNo remote tables found. (exit 0)data.tables = 2 tables
introspectError: Failed to generate draft (exit 2)data.draft.source present
validateNo federated objects to validate. (exit 0)missing_column region severity:error
unknown datasourceTypeError: first argument must be a string or instance of Errorerror.message

The severe one is validate: a schema gate answering "fine" about a response
it never read. A crash reports itself; a silent pass is indistinguishable
downstream from a real all-clear, so this PR's reverse verification is centred
on it rather than on the happy path.

The fix

One CLI-side reader for the declared envelope
(packages/cli/src/utils/response-envelope.ts) that all three commands call,
and payload types taken from the service contract
(RemoteTable, ObjectDraft, SchemaValidationResult from
@objectstack/spec/contracts) instead of re-transcribed inline — a copy of a
server shape is what drifted here in the first place, and it drifted in three
files at once.

The reader is total and strict, which is the load-bearing property: a body
that is not the declared envelope produces a loud failure, never
{ ok: true, data: {} }. "Nothing found" is now reachable only from a server
that really said so. The legacy flat shape is deliberately not also accepted
— a consumer-side fallback would re-create the divergence as a second de-facto
contract (Prime Directive #12).

Behaviour change worth calling out:os datasource validate now exits 1
on drift where it previously exited 0. That is the fix, but a pipeline that
treated the command as advisory will start failing on drift that was always
there.

Reverse verification — the silent pass, reproduced and closed

Ran the drift case against pre-fix validate.ts (this branch's tests, the
command file restored from origin/main), then against the fix. Full output in
the issue report; the shape:

pre-fix : logs = [ "No federated objects to validate." ] exit 0 ← the defect
post-fix: logs = [ "✗ missing_column: showcase_customers.region" ]
error = "External schema validation failed." exit 1

Both legs ran from a committed state, and the restored file was proven
byte-identical to HEAD before the green leg. No rebuild leg applies to the
mutated subject: the test imports ./validate.js — a relative specifier inside
the package — so vitest resolves it from src, never through a dist/
artifact. (The fixture builders sendOk/sendError do resolve to
@objectstack/types's dist, and that closure was built before either leg.)

Coverage

  • packages/cli/src/commands/datasource/envelope-unwrap.test.ts — drives all
    three commands in-process against the wrapped envelope, including the induced
    drift, the genuinely-empty case, an unreadable body, and the error path for
    each command.
  • packages/cli/src/utils/response-envelope.test.ts — the reader itself.

Both build their fixtures with the server's own sendOk / sendError rather
than hand-written literals, so a change to the envelope moves these tests with
it instead of leaving them agreeing with a shape the server has left.

Scope

  • ⛔ The draft-quality gap (missing ${namespace}_ prefix / sharingModel) is
    not addressed here — it is server-side in service-datasource, tracked
    separately, and this diff does not change what the server sends.
  • introspect coverage pins only that the CLI emits the source the server
    produced, using an opaque marker string. No draft content is asserted, so the
    cross-lane primary-key fix is not blocked by an assertion written today.
  • One file beyond the three commands the claim named: the shared reader plus
    its tests, all inside packages/cli.

Verification

All at 99f2eac (clean tree, gates run after the final commit):

  • pnpm --filter @objectstack/cli test141 files / 1565 tests passed, the
    package's whole suite, not a narrowed selection.
  • pnpm --filter @objectstack/cli typechecktsc --noEmit, exit 0. The new
    tests sit under src/, which this package's tsconfig.json includes, so they
    are inside the program that gate reads.
  • Gate union re-derived from the real change set with
    node scripts/pm/dispatch-gates.mjs (no hand-written path list) and run green,
    including the convention-triggered set a new test file moves:
    check:nul-bytes · check:cross-package-test-inputs · check:test-source-alias
    · check:type-source-resolution · check:slot-lookup ·
    check:engine-double-contract · check:where-matcher ·
    check:query-options-erasure · check:type-check-coverage ·
    check:type-check-debt · check:changeset-gate-self-tests ·
    check:objectui-changeset · check-empty-changeset ·
    check-changeset-no-major · check-adr-0087-registration ·
    check-affected-docs.
  • check:type-check-debt needed the full workspace closure built first — it
    refuses on an unbuilt tree rather than measuring a different world — so the
    closure was built (turbo run build, 70/70) and then:
    check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured in 284.4s, 1912 raw tsc error(s) total, none above its recorded number.

Generated by Claude Code


Generated by Claude Code

…ds (#10675)
`list-tables`, `introspect` and `validate` read the pre-#3843 flat shape
(`body.tables` / `body.draft` / `body.results`, `error` as a string) while the
server emits `{ success, data }` / `{ success:false, error:{code,message} }`.
Every payload read `undefined`, and each command reported that as an empty
result — most dangerously `validate`, which printed "No federated objects to
validate." and exited 0 against drift the server had flagged
`missing_column … severity:error`.
All three now go through one CLI-side reader for the declared envelope, with
payload types taken from `@objectstack/spec/contracts` rather than
re-transcribed. The reader is strict: a body that is not the envelope is a loud
failure, never an empty payload, so "nothing found" is reachable only from a
server that really said so. The error path passes `error.message` — a string —
instead of handing the error object to oclif's `this.error()`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019bmVFqoQPq63zhKrxdYG1r
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

11 anchor(s) derived from 1 changed package(s); no hand-written page names any of them. ✅

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 23 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 4b84834a320f64efe673d0e07717189f804ae8eepackageMentionDocs.

Which tree this was computed on

This run read content/docs from fef72d3364518c335156957dbf4052cd06b0c47c — the merge of head 99f2eac7d6b22e74ad6fda6e40065d77ad423300 into base 4b84834a320f64efe673d0e07717189f804ae8ee, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin fef72d3364518c335156957dbf4052cd06b0c47c && git checkout fef72d3364518c335156957dbf4052cd06b0c47c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 4b84834a320f64efe673d0e07717189f804ae8ee 99f2eac7d6b22e74ad6fda6e40065d77ad423300 && git checkout -B drift-repro 4b84834a320f64efe673d0e07717189f804ae8ee && git merge --no-ff 99f2eac7d6b22e74ad6fda6e40065d77ad423300
node scripts/docs-audit/affected-docs.mjs --json 4b84834a320f64efe673d0e07717189f804ae8ee

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os datasource list-tables/introspect/validate are non-functional — CLI reads pre-#3843 flat envelope

2 participants

@os-elon@claude