fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235) - #14480

Merged
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape
Sep 2, 2026
Merged

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)#14480
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Fixes#14235

RemoteTransport.aggregate emits a caller-supplied output NAME in exactly two positions. #13714 routed BOTH of driver-sql's through SqlDriver.aliasIdentifierSql; #14113 moved this transport's aggregation alias to the same escaping and deliberately left the groupBy alias gated, because that position carried a landed #6401 pin asserting the refusal. This card is the recorded, non-silent reversal of that pin.

An output-column key is a NAME — quoted and escaped. A column REFERENCE is grammar — validated. So field keeps assertSafeIdentifier and outKey no longer has it:

// beforethis.assertSafeIdentifier(outKey);selectParts.push(outKey===field ? `"${field}"` : `"${field}" AS "${outKey}"`);// afterselectParts.push(outKey===field ? `"${field}"` : `"${field}" AS ${this.aliasIdentifierSql(outKey)}`,);

GroupByNodeSchema.alias is the same class of key as AggregationNodeSchema.alias; the in-memory face projects g.alias ?? g.field verbatim and the MongoDB and SQL faces agree, so this face was the last one refusing names the contract admits — 'Region Name', 'deal.stage_bucket' — with an opaque 500 out of mapDataError before #14287 and a 400 since. outKey === field still emits the alias-less "field", byte-identical.

The file surface was amended mid-round, and why

The dispatch bound this card to two files and predicted that no test outside remote-transport-groupby-node.test.ts pinned the groupBy alias refusal. That prediction was wrong, and the round stopped on it rather than widening its own scope: two more tests pinned it, each left behind by a sibling card as a hand-off addressed to this one.

The PM ruled option A on the card and amended the surface to include both. Each file has exactly one case rewritten, on the same input, from a refusal assertion to the quoted emission; everything else in both files is byte-identical.

⛔ What did NOT move: unsafeIdentifierError and UNSAFE_IDENTIFIER_CODE are untouched, and every other refusal position — object, the aggregation field, the groupBy FIELD, the DDL table / column / index names — still asserts INVALID_REQUEST / 400. The envelope file's describe('the accept set is UNTOUCHED …') block drives the groupBy FIELD position and is byte-identical; its header paragraph gains one sentence recording that the OUT KEY position moved here. #14287's envelope ruling is not re-decided by this PR — this card decided which POSITIONS the gate governs, not what the gate answers when it fires.

Clause-② self-reading: no

No contract move. Nothing in packages/spec changes; GroupByNodeSchema.alias already declares this key and already admits these names. What widens is the driver's accept set, toward the contract the other three faces already implement — declared = enforced, restored. #14287's landed header says of this position "it MOVES the accept set", which is that same driver-level reading, not a spec one.

Evidence — every figure below measured at ffabb9991

Package suite green.pnpm --filter @objectstack/driver-turso testTest Files 42 passed (42) · Tests 1146 passed (1146). Both previously-red controls pass on the new truth.

Typecheck.pnpm --filter @objectstack/driver-turso typecheck exit 0, and the tests are really in the program rather than assumed to be: tsc --noEmit --listFiles names all three edited test files exactly once each.

Ablation — direction predicted in the suite header BEFORE the run. Mutation: restore the two pre-change lines at the groupBy select site. Proved on disk by grep counts on both the injected and the removed text plus git hash-object (mutated e3ee37e6d vs HEAD blob 7de7f70ff). Result: Tests 4 failed | 14 passed (18) — the two capture cases and both executing cases fail by THROWING inside the call (unsafe identifier rejected: "bucket"; DROP TABLE deal; --", "Region Name", "deal.stage_bucket"), never on a comparison, while the field-position control, the two byte-identical-emission cases, the string-form control, all five date-bucket refusals and both parity cases stay GREEN. Restore proved by the blob hash matching HEAD again plus an empty git diff HEAD; the script restores under a trap on EXIT INT TERM with an absolute path. No dist/ leg applies, and that is measured: the suite imports the mutated unit through a relative in-package specifier vitest resolves to src/, and the package's vitest.config.ts declares no alias.

Escaped, not merely emitted. The security-relevant claim is pinned against a real SQLite-backed libsql stub, not only on a captured string: an alias of bucket"; DROP TABLE deal; -- comes back as a COLUMN NAME and the table still holds all three rows.

Driver conformance ledger, before the first edit and after the last commit — identical:

check-driver-conformance: OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt.

Gate union. The 36 families node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derives for this changeset, re-derived after the last commit and run at ffabb9991: 34 exit 0. The other two exit 3 = NOT MEASURED by their own text, both environmental and neither a finding: check-test-completeness ("There is no local log to hand it, so the local reading for this gate is NOT MEASURED. It is not a red, and there is nothing here to fix.") and pm/check-half-states ("it is no reading at all"). Exit codes captured after redirect, never through a pipe. Plus pnpm lint over the whole repo — exit 0 — and pnpm check:error-status-conformance — exit 0, "every derivable runtime status is documented, and every documented status is reachable."

Changeset: @objectstack/driver-turso patch. Branch merged origin/main (never rebased, never force-pushed).

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…nstead of gating it (#14235)
The second output-name position of `RemoteTransport.aggregate`. #14113 moved
the aggregation alias to `aliasIdentifierSql` and left this one gated because
it carried a landed #6401 pin asserting the refusal; #13714 had already routed
both of driver-sql's output-name positions through escaping. An output-column
key is a NAME — quoted and escaped; a column reference is grammar — validated.
So `field` keeps `assertSafeIdentifier` and `outKey` no longer has it.
The #6401 pin is rewritten in place on the same input to assert what the
transport now emits, plus dotted/spaced round-trip cases and two executing
cases against the SQLite-backed libsql stub.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
… alias (#14235)
The input the rewritten #6401 pin carries has no aggregation `field`, so the
emission is `count(*)`, not `count("stage")`; and the one-statement assertion
now counts SELECTs rather than probing for a quote that the escape never
produces.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…14235)
4 failed / 14 passed of 18, case for case as predicted; every failure throws
inside the call rather than failing a comparison, and the `field`-position
control stays green. Notes why no `dist/` leg applies and how the mutation and
the restore were each proved on disk.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…#14287 left (#14235)
Both controls were hand-offs addressed to this card in their own comments —
#14113's "this control records the state it was left in", #14287's "the day
that card lands, this is the test that has to be rewritten deliberately rather
than found red by surprise". PM ruling on the stop condition (comment
5506404777) amended this card's file surface to include them.
Each file has exactly one case rewritten, on the same input, to assert the
quoted emission instead of the refusal; the envelope file also records in its
`## The accept set is UNTOUCHED` paragraph that the groupBy OUT KEY position
moved. Every other refusal position — object, field, groupBy FIELD, aggregation
alias, DDL — still asserts INVALID_REQUEST / 400, the accept-set describe that
drives the groupBy FIELD is byte-identical, and `unsafeIdentifierError` /
`UNSAFE_IDENTIFIER_CODE` are untouched.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 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 9e286e248866c40d2db662f69aa0ba6e71b4b096packageMentionDocs.

Which tree this was computed on

This run read content/docs from 22ea82c33374c3c9ba99719c1df84f2c600b60a3 — the merge of head ffabb9991c5eefbb6020632a829dcc39725debc2 into base 9e286e248866c40d2db662f69aa0ba6e71b4b096, 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 22ea82c33374c3c9ba99719c1df84f2c600b60a3 && git checkout 22ea82c33374c3c9ba99719c1df84f2c600b60a3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e286e248866c40d2db662f69aa0ba6e71b4b096 ffabb9991c5eefbb6020632a829dcc39725debc2 && git checkout -B drift-repro 9e286e248866c40d2db662f69aa0ba6e71b4b096 && git merge --no-ff ffabb9991c5eefbb6020632a829dcc39725debc2
node scripts/docs-audit/affected-docs.mjs --json 9e286e248866c40d2db662f69aa0ba6e71b4b096

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

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance (engine execution seat, session_0112hMx9hjJ9BgB28X97DS68) — flipped to ready and auto-merge (squash) armed at 09:18Z on head ffabb9991.

  • ACCEPT on the card: drivers(turso): RemoteTransport still refuses a dotted groupBy alias while driver-sql escapes it — the second output-name position #14113 deliberately left alone #14235 (comment 5506982748); the mid-round surface amendment and the option-A ruling on the two hand-off controls: comment 5506404777.
  • Pre-landing checks at flip time: every check run on ffabb9991 completed with conclusion success or skipped (33 runs, perPage: 50; Lint & Repo Gates and all Type Check jobs success); node scripts/pm/check-governed-merges.mjs --test on the PR's 5 changed paths: 0 of 5 hit the register — not governed, ordinary queue landing applies.
  • Clause-② self-reading no, concurred at ACCEPT: driver accept set widened toward the declared GroupByNodeSchema.alias contract, no packages/spec change, no public-entry export change ⇒ @objectstack/driver-turso: patch is the right level. No needs:contract-review carrier on card or PR.
  • Closing keyword: Fixes #14235 only; the card closes at MERGED and its pm:dispatched is stripped in the landing record.

Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit ed44512Sep 2, 2026
35 checks passed
@os-musk
os-musk deleted the claude/issue-14235-turso-groupby-alias-escape branch September 2, 2026 09:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235) - #14480

Merged
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape
Sep 2, 2026
Merged

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)#14480
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Fixes#14235

RemoteTransport.aggregate emits a caller-supplied output NAME in exactly two positions. #13714 routed BOTH of driver-sql's through SqlDriver.aliasIdentifierSql; #14113 moved this transport's aggregation alias to the same escaping and deliberately left the groupBy alias gated, because that position carried a landed #6401 pin asserting the refusal. This card is the recorded, non-silent reversal of that pin.

An output-column key is a NAME — quoted and escaped. A column REFERENCE is grammar — validated. So field keeps assertSafeIdentifier and outKey no longer has it:

// beforethis.assertSafeIdentifier(outKey);selectParts.push(outKey===field ? `"${field}"` : `"${field}" AS "${outKey}"`);// afterselectParts.push(outKey===field ? `"${field}"` : `"${field}" AS ${this.aliasIdentifierSql(outKey)}`,);

GroupByNodeSchema.alias is the same class of key as AggregationNodeSchema.alias; the in-memory face projects g.alias ?? g.field verbatim and the MongoDB and SQL faces agree, so this face was the last one refusing names the contract admits — 'Region Name', 'deal.stage_bucket' — with an opaque 500 out of mapDataError before #14287 and a 400 since. outKey === field still emits the alias-less "field", byte-identical.

The file surface was amended mid-round, and why

The dispatch bound this card to two files and predicted that no test outside remote-transport-groupby-node.test.ts pinned the groupBy alias refusal. That prediction was wrong, and the round stopped on it rather than widening its own scope: two more tests pinned it, each left behind by a sibling card as a hand-off addressed to this one.

The PM ruled option A on the card and amended the surface to include both. Each file has exactly one case rewritten, on the same input, from a refusal assertion to the quoted emission; everything else in both files is byte-identical.

⛔ What did NOT move: unsafeIdentifierError and UNSAFE_IDENTIFIER_CODE are untouched, and every other refusal position — object, the aggregation field, the groupBy FIELD, the DDL table / column / index names — still asserts INVALID_REQUEST / 400. The envelope file's describe('the accept set is UNTOUCHED …') block drives the groupBy FIELD position and is byte-identical; its header paragraph gains one sentence recording that the OUT KEY position moved here. #14287's envelope ruling is not re-decided by this PR — this card decided which POSITIONS the gate governs, not what the gate answers when it fires.

Clause-② self-reading: no

No contract move. Nothing in packages/spec changes; GroupByNodeSchema.alias already declares this key and already admits these names. What widens is the driver's accept set, toward the contract the other three faces already implement — declared = enforced, restored. #14287's landed header says of this position "it MOVES the accept set", which is that same driver-level reading, not a spec one.

Evidence — every figure below measured at ffabb9991

Package suite green.pnpm --filter @objectstack/driver-turso testTest Files 42 passed (42) · Tests 1146 passed (1146). Both previously-red controls pass on the new truth.

Typecheck.pnpm --filter @objectstack/driver-turso typecheck exit 0, and the tests are really in the program rather than assumed to be: tsc --noEmit --listFiles names all three edited test files exactly once each.

Ablation — direction predicted in the suite header BEFORE the run. Mutation: restore the two pre-change lines at the groupBy select site. Proved on disk by grep counts on both the injected and the removed text plus git hash-object (mutated e3ee37e6d vs HEAD blob 7de7f70ff). Result: Tests 4 failed | 14 passed (18) — the two capture cases and both executing cases fail by THROWING inside the call (unsafe identifier rejected: "bucket"; DROP TABLE deal; --", "Region Name", "deal.stage_bucket"), never on a comparison, while the field-position control, the two byte-identical-emission cases, the string-form control, all five date-bucket refusals and both parity cases stay GREEN. Restore proved by the blob hash matching HEAD again plus an empty git diff HEAD; the script restores under a trap on EXIT INT TERM with an absolute path. No dist/ leg applies, and that is measured: the suite imports the mutated unit through a relative in-package specifier vitest resolves to src/, and the package's vitest.config.ts declares no alias.

Escaped, not merely emitted. The security-relevant claim is pinned against a real SQLite-backed libsql stub, not only on a captured string: an alias of bucket"; DROP TABLE deal; -- comes back as a COLUMN NAME and the table still holds all three rows.

Driver conformance ledger, before the first edit and after the last commit — identical:

check-driver-conformance: OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt.

Gate union. The 36 families node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derives for this changeset, re-derived after the last commit and run at ffabb9991: 34 exit 0. The other two exit 3 = NOT MEASURED by their own text, both environmental and neither a finding: check-test-completeness ("There is no local log to hand it, so the local reading for this gate is NOT MEASURED. It is not a red, and there is nothing here to fix.") and pm/check-half-states ("it is no reading at all"). Exit codes captured after redirect, never through a pipe. Plus pnpm lint over the whole repo — exit 0 — and pnpm check:error-status-conformance — exit 0, "every derivable runtime status is documented, and every documented status is reachable."

Changeset: @objectstack/driver-turso patch. Branch merged origin/main (never rebased, never force-pushed).

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…nstead of gating it (#14235)
The second output-name position of `RemoteTransport.aggregate`. #14113 moved
the aggregation alias to `aliasIdentifierSql` and left this one gated because
it carried a landed #6401 pin asserting the refusal; #13714 had already routed
both of driver-sql's output-name positions through escaping. An output-column
key is a NAME — quoted and escaped; a column reference is grammar — validated.
So `field` keeps `assertSafeIdentifier` and `outKey` no longer has it.
The #6401 pin is rewritten in place on the same input to assert what the
transport now emits, plus dotted/spaced round-trip cases and two executing
cases against the SQLite-backed libsql stub.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
… alias (#14235)
The input the rewritten #6401 pin carries has no aggregation `field`, so the
emission is `count(*)`, not `count("stage")`; and the one-statement assertion
now counts SELECTs rather than probing for a quote that the escape never
produces.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…14235)
4 failed / 14 passed of 18, case for case as predicted; every failure throws
inside the call rather than failing a comparison, and the `field`-position
control stays green. Notes why no `dist/` leg applies and how the mutation and
the restore were each proved on disk.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…#14287 left (#14235)
Both controls were hand-offs addressed to this card in their own comments —
#14113's "this control records the state it was left in", #14287's "the day
that card lands, this is the test that has to be rewritten deliberately rather
than found red by surprise". PM ruling on the stop condition (comment
5506404777) amended this card's file surface to include them.
Each file has exactly one case rewritten, on the same input, to assert the
quoted emission instead of the refusal; the envelope file also records in its
`## The accept set is UNTOUCHED` paragraph that the groupBy OUT KEY position
moved. Every other refusal position — object, field, groupBy FIELD, aggregation
alias, DDL — still asserts INVALID_REQUEST / 400, the accept-set describe that
drives the groupBy FIELD is byte-identical, and `unsafeIdentifierError` /
`UNSAFE_IDENTIFIER_CODE` are untouched.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 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 9e286e248866c40d2db662f69aa0ba6e71b4b096packageMentionDocs.

Which tree this was computed on

This run read content/docs from 22ea82c33374c3c9ba99719c1df84f2c600b60a3 — the merge of head ffabb9991c5eefbb6020632a829dcc39725debc2 into base 9e286e248866c40d2db662f69aa0ba6e71b4b096, 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 22ea82c33374c3c9ba99719c1df84f2c600b60a3 && git checkout 22ea82c33374c3c9ba99719c1df84f2c600b60a3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e286e248866c40d2db662f69aa0ba6e71b4b096 ffabb9991c5eefbb6020632a829dcc39725debc2 && git checkout -B drift-repro 9e286e248866c40d2db662f69aa0ba6e71b4b096 && git merge --no-ff ffabb9991c5eefbb6020632a829dcc39725debc2
node scripts/docs-audit/affected-docs.mjs --json 9e286e248866c40d2db662f69aa0ba6e71b4b096

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

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance (engine execution seat, session_0112hMx9hjJ9BgB28X97DS68) — flipped to ready and auto-merge (squash) armed at 09:18Z on head ffabb9991.

  • ACCEPT on the card: drivers(turso): RemoteTransport still refuses a dotted groupBy alias while driver-sql escapes it — the second output-name position #14113 deliberately left alone #14235 (comment 5506982748); the mid-round surface amendment and the option-A ruling on the two hand-off controls: comment 5506404777.
  • Pre-landing checks at flip time: every check run on ffabb9991 completed with conclusion success or skipped (33 runs, perPage: 50; Lint & Repo Gates and all Type Check jobs success); node scripts/pm/check-governed-merges.mjs --test on the PR's 5 changed paths: 0 of 5 hit the register — not governed, ordinary queue landing applies.
  • Clause-② self-reading no, concurred at ACCEPT: driver accept set widened toward the declared GroupByNodeSchema.alias contract, no packages/spec change, no public-entry export change ⇒ @objectstack/driver-turso: patch is the right level. No needs:contract-review carrier on card or PR.
  • Closing keyword: Fixes #14235 only; the card closes at MERGED and its pm:dispatched is stripped in the landing record.

Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit ed44512Sep 2, 2026
35 checks passed
@os-musk
os-musk deleted the claude/issue-14235-turso-groupby-alias-escape branch September 2, 2026 09:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235) - #14480

Merged
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape
Sep 2, 2026
Merged

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)#14480
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Fixes#14235

RemoteTransport.aggregate emits a caller-supplied output NAME in exactly two positions. #13714 routed BOTH of driver-sql's through SqlDriver.aliasIdentifierSql; #14113 moved this transport's aggregation alias to the same escaping and deliberately left the groupBy alias gated, because that position carried a landed #6401 pin asserting the refusal. This card is the recorded, non-silent reversal of that pin.

An output-column key is a NAME — quoted and escaped. A column REFERENCE is grammar — validated. So field keeps assertSafeIdentifier and outKey no longer has it:

// beforethis.assertSafeIdentifier(outKey);selectParts.push(outKey===field ? `"${field}"` : `"${field}" AS "${outKey}"`);// afterselectParts.push(outKey===field ? `"${field}"` : `"${field}" AS ${this.aliasIdentifierSql(outKey)}`,);

GroupByNodeSchema.alias is the same class of key as AggregationNodeSchema.alias; the in-memory face projects g.alias ?? g.field verbatim and the MongoDB and SQL faces agree, so this face was the last one refusing names the contract admits — 'Region Name', 'deal.stage_bucket' — with an opaque 500 out of mapDataError before #14287 and a 400 since. outKey === field still emits the alias-less "field", byte-identical.

The file surface was amended mid-round, and why

The dispatch bound this card to two files and predicted that no test outside remote-transport-groupby-node.test.ts pinned the groupBy alias refusal. That prediction was wrong, and the round stopped on it rather than widening its own scope: two more tests pinned it, each left behind by a sibling card as a hand-off addressed to this one.

The PM ruled option A on the card and amended the surface to include both. Each file has exactly one case rewritten, on the same input, from a refusal assertion to the quoted emission; everything else in both files is byte-identical.

⛔ What did NOT move: unsafeIdentifierError and UNSAFE_IDENTIFIER_CODE are untouched, and every other refusal position — object, the aggregation field, the groupBy FIELD, the DDL table / column / index names — still asserts INVALID_REQUEST / 400. The envelope file's describe('the accept set is UNTOUCHED …') block drives the groupBy FIELD position and is byte-identical; its header paragraph gains one sentence recording that the OUT KEY position moved here. #14287's envelope ruling is not re-decided by this PR — this card decided which POSITIONS the gate governs, not what the gate answers when it fires.

Clause-② self-reading: no

No contract move. Nothing in packages/spec changes; GroupByNodeSchema.alias already declares this key and already admits these names. What widens is the driver's accept set, toward the contract the other three faces already implement — declared = enforced, restored. #14287's landed header says of this position "it MOVES the accept set", which is that same driver-level reading, not a spec one.

Evidence — every figure below measured at ffabb9991

Package suite green.pnpm --filter @objectstack/driver-turso testTest Files 42 passed (42) · Tests 1146 passed (1146). Both previously-red controls pass on the new truth.

Typecheck.pnpm --filter @objectstack/driver-turso typecheck exit 0, and the tests are really in the program rather than assumed to be: tsc --noEmit --listFiles names all three edited test files exactly once each.

Ablation — direction predicted in the suite header BEFORE the run. Mutation: restore the two pre-change lines at the groupBy select site. Proved on disk by grep counts on both the injected and the removed text plus git hash-object (mutated e3ee37e6d vs HEAD blob 7de7f70ff). Result: Tests 4 failed | 14 passed (18) — the two capture cases and both executing cases fail by THROWING inside the call (unsafe identifier rejected: "bucket"; DROP TABLE deal; --", "Region Name", "deal.stage_bucket"), never on a comparison, while the field-position control, the two byte-identical-emission cases, the string-form control, all five date-bucket refusals and both parity cases stay GREEN. Restore proved by the blob hash matching HEAD again plus an empty git diff HEAD; the script restores under a trap on EXIT INT TERM with an absolute path. No dist/ leg applies, and that is measured: the suite imports the mutated unit through a relative in-package specifier vitest resolves to src/, and the package's vitest.config.ts declares no alias.

Escaped, not merely emitted. The security-relevant claim is pinned against a real SQLite-backed libsql stub, not only on a captured string: an alias of bucket"; DROP TABLE deal; -- comes back as a COLUMN NAME and the table still holds all three rows.

Driver conformance ledger, before the first edit and after the last commit — identical:

check-driver-conformance: OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt.

Gate union. The 36 families node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derives for this changeset, re-derived after the last commit and run at ffabb9991: 34 exit 0. The other two exit 3 = NOT MEASURED by their own text, both environmental and neither a finding: check-test-completeness ("There is no local log to hand it, so the local reading for this gate is NOT MEASURED. It is not a red, and there is nothing here to fix.") and pm/check-half-states ("it is no reading at all"). Exit codes captured after redirect, never through a pipe. Plus pnpm lint over the whole repo — exit 0 — and pnpm check:error-status-conformance — exit 0, "every derivable runtime status is documented, and every documented status is reachable."

Changeset: @objectstack/driver-turso patch. Branch merged origin/main (never rebased, never force-pushed).

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…nstead of gating it (#14235)
The second output-name position of `RemoteTransport.aggregate`. #14113 moved
the aggregation alias to `aliasIdentifierSql` and left this one gated because
it carried a landed #6401 pin asserting the refusal; #13714 had already routed
both of driver-sql's output-name positions through escaping. An output-column
key is a NAME — quoted and escaped; a column reference is grammar — validated.
So `field` keeps `assertSafeIdentifier` and `outKey` no longer has it.
The #6401 pin is rewritten in place on the same input to assert what the
transport now emits, plus dotted/spaced round-trip cases and two executing
cases against the SQLite-backed libsql stub.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
… alias (#14235)
The input the rewritten #6401 pin carries has no aggregation `field`, so the
emission is `count(*)`, not `count("stage")`; and the one-statement assertion
now counts SELECTs rather than probing for a quote that the escape never
produces.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…14235)
4 failed / 14 passed of 18, case for case as predicted; every failure throws
inside the call rather than failing a comparison, and the `field`-position
control stays green. Notes why no `dist/` leg applies and how the mutation and
the restore were each proved on disk.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…#14287 left (#14235)
Both controls were hand-offs addressed to this card in their own comments —
#14113's "this control records the state it was left in", #14287's "the day
that card lands, this is the test that has to be rewritten deliberately rather
than found red by surprise". PM ruling on the stop condition (comment
5506404777) amended this card's file surface to include them.
Each file has exactly one case rewritten, on the same input, to assert the
quoted emission instead of the refusal; the envelope file also records in its
`## The accept set is UNTOUCHED` paragraph that the groupBy OUT KEY position
moved. Every other refusal position — object, field, groupBy FIELD, aggregation
alias, DDL — still asserts INVALID_REQUEST / 400, the accept-set describe that
drives the groupBy FIELD is byte-identical, and `unsafeIdentifierError` /
`UNSAFE_IDENTIFIER_CODE` are untouched.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 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 9e286e248866c40d2db662f69aa0ba6e71b4b096packageMentionDocs.

Which tree this was computed on

This run read content/docs from 22ea82c33374c3c9ba99719c1df84f2c600b60a3 — the merge of head ffabb9991c5eefbb6020632a829dcc39725debc2 into base 9e286e248866c40d2db662f69aa0ba6e71b4b096, 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 22ea82c33374c3c9ba99719c1df84f2c600b60a3 && git checkout 22ea82c33374c3c9ba99719c1df84f2c600b60a3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e286e248866c40d2db662f69aa0ba6e71b4b096 ffabb9991c5eefbb6020632a829dcc39725debc2 && git checkout -B drift-repro 9e286e248866c40d2db662f69aa0ba6e71b4b096 && git merge --no-ff ffabb9991c5eefbb6020632a829dcc39725debc2
node scripts/docs-audit/affected-docs.mjs --json 9e286e248866c40d2db662f69aa0ba6e71b4b096

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

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance (engine execution seat, session_0112hMx9hjJ9BgB28X97DS68) — flipped to ready and auto-merge (squash) armed at 09:18Z on head ffabb9991.

  • ACCEPT on the card: drivers(turso): RemoteTransport still refuses a dotted groupBy alias while driver-sql escapes it — the second output-name position #14113 deliberately left alone #14235 (comment 5506982748); the mid-round surface amendment and the option-A ruling on the two hand-off controls: comment 5506404777.
  • Pre-landing checks at flip time: every check run on ffabb9991 completed with conclusion success or skipped (33 runs, perPage: 50; Lint & Repo Gates and all Type Check jobs success); node scripts/pm/check-governed-merges.mjs --test on the PR's 5 changed paths: 0 of 5 hit the register — not governed, ordinary queue landing applies.
  • Clause-② self-reading no, concurred at ACCEPT: driver accept set widened toward the declared GroupByNodeSchema.alias contract, no packages/spec change, no public-entry export change ⇒ @objectstack/driver-turso: patch is the right level. No needs:contract-review carrier on card or PR.
  • Closing keyword: Fixes #14235 only; the card closes at MERGED and its pm:dispatched is stripped in the landing record.

Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit ed44512Sep 2, 2026
35 checks passed
@os-musk
os-musk deleted the claude/issue-14235-turso-groupby-alias-escape branch September 2, 2026 09:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235) - #14480

Merged
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape
Sep 2, 2026
Merged

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)#14480
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Fixes#14235

RemoteTransport.aggregate emits a caller-supplied output NAME in exactly two positions. #13714 routed BOTH of driver-sql's through SqlDriver.aliasIdentifierSql; #14113 moved this transport's aggregation alias to the same escaping and deliberately left the groupBy alias gated, because that position carried a landed #6401 pin asserting the refusal. This card is the recorded, non-silent reversal of that pin.

An output-column key is a NAME — quoted and escaped. A column REFERENCE is grammar — validated. So field keeps assertSafeIdentifier and outKey no longer has it:

// beforethis.assertSafeIdentifier(outKey);selectParts.push(outKey===field ? `"${field}"` : `"${field}" AS "${outKey}"`);// afterselectParts.push(outKey===field ? `"${field}"` : `"${field}" AS ${this.aliasIdentifierSql(outKey)}`,);

GroupByNodeSchema.alias is the same class of key as AggregationNodeSchema.alias; the in-memory face projects g.alias ?? g.field verbatim and the MongoDB and SQL faces agree, so this face was the last one refusing names the contract admits — 'Region Name', 'deal.stage_bucket' — with an opaque 500 out of mapDataError before #14287 and a 400 since. outKey === field still emits the alias-less "field", byte-identical.

The file surface was amended mid-round, and why

The dispatch bound this card to two files and predicted that no test outside remote-transport-groupby-node.test.ts pinned the groupBy alias refusal. That prediction was wrong, and the round stopped on it rather than widening its own scope: two more tests pinned it, each left behind by a sibling card as a hand-off addressed to this one.

The PM ruled option A on the card and amended the surface to include both. Each file has exactly one case rewritten, on the same input, from a refusal assertion to the quoted emission; everything else in both files is byte-identical.

⛔ What did NOT move: unsafeIdentifierError and UNSAFE_IDENTIFIER_CODE are untouched, and every other refusal position — object, the aggregation field, the groupBy FIELD, the DDL table / column / index names — still asserts INVALID_REQUEST / 400. The envelope file's describe('the accept set is UNTOUCHED …') block drives the groupBy FIELD position and is byte-identical; its header paragraph gains one sentence recording that the OUT KEY position moved here. #14287's envelope ruling is not re-decided by this PR — this card decided which POSITIONS the gate governs, not what the gate answers when it fires.

Clause-② self-reading: no

No contract move. Nothing in packages/spec changes; GroupByNodeSchema.alias already declares this key and already admits these names. What widens is the driver's accept set, toward the contract the other three faces already implement — declared = enforced, restored. #14287's landed header says of this position "it MOVES the accept set", which is that same driver-level reading, not a spec one.

Evidence — every figure below measured at ffabb9991

Package suite green.pnpm --filter @objectstack/driver-turso testTest Files 42 passed (42) · Tests 1146 passed (1146). Both previously-red controls pass on the new truth.

Typecheck.pnpm --filter @objectstack/driver-turso typecheck exit 0, and the tests are really in the program rather than assumed to be: tsc --noEmit --listFiles names all three edited test files exactly once each.

Ablation — direction predicted in the suite header BEFORE the run. Mutation: restore the two pre-change lines at the groupBy select site. Proved on disk by grep counts on both the injected and the removed text plus git hash-object (mutated e3ee37e6d vs HEAD blob 7de7f70ff). Result: Tests 4 failed | 14 passed (18) — the two capture cases and both executing cases fail by THROWING inside the call (unsafe identifier rejected: "bucket"; DROP TABLE deal; --", "Region Name", "deal.stage_bucket"), never on a comparison, while the field-position control, the two byte-identical-emission cases, the string-form control, all five date-bucket refusals and both parity cases stay GREEN. Restore proved by the blob hash matching HEAD again plus an empty git diff HEAD; the script restores under a trap on EXIT INT TERM with an absolute path. No dist/ leg applies, and that is measured: the suite imports the mutated unit through a relative in-package specifier vitest resolves to src/, and the package's vitest.config.ts declares no alias.

Escaped, not merely emitted. The security-relevant claim is pinned against a real SQLite-backed libsql stub, not only on a captured string: an alias of bucket"; DROP TABLE deal; -- comes back as a COLUMN NAME and the table still holds all three rows.

Driver conformance ledger, before the first edit and after the last commit — identical:

check-driver-conformance: OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt.

Gate union. The 36 families node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derives for this changeset, re-derived after the last commit and run at ffabb9991: 34 exit 0. The other two exit 3 = NOT MEASURED by their own text, both environmental and neither a finding: check-test-completeness ("There is no local log to hand it, so the local reading for this gate is NOT MEASURED. It is not a red, and there is nothing here to fix.") and pm/check-half-states ("it is no reading at all"). Exit codes captured after redirect, never through a pipe. Plus pnpm lint over the whole repo — exit 0 — and pnpm check:error-status-conformance — exit 0, "every derivable runtime status is documented, and every documented status is reachable."

Changeset: @objectstack/driver-turso patch. Branch merged origin/main (never rebased, never force-pushed).

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…nstead of gating it (#14235)
The second output-name position of `RemoteTransport.aggregate`. #14113 moved
the aggregation alias to `aliasIdentifierSql` and left this one gated because
it carried a landed #6401 pin asserting the refusal; #13714 had already routed
both of driver-sql's output-name positions through escaping. An output-column
key is a NAME — quoted and escaped; a column reference is grammar — validated.
So `field` keeps `assertSafeIdentifier` and `outKey` no longer has it.
The #6401 pin is rewritten in place on the same input to assert what the
transport now emits, plus dotted/spaced round-trip cases and two executing
cases against the SQLite-backed libsql stub.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
… alias (#14235)
The input the rewritten #6401 pin carries has no aggregation `field`, so the
emission is `count(*)`, not `count("stage")`; and the one-statement assertion
now counts SELECTs rather than probing for a quote that the escape never
produces.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…14235)
4 failed / 14 passed of 18, case for case as predicted; every failure throws
inside the call rather than failing a comparison, and the `field`-position
control stays green. Notes why no `dist/` leg applies and how the mutation and
the restore were each proved on disk.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…#14287 left (#14235)
Both controls were hand-offs addressed to this card in their own comments —
#14113's "this control records the state it was left in", #14287's "the day
that card lands, this is the test that has to be rewritten deliberately rather
than found red by surprise". PM ruling on the stop condition (comment
5506404777) amended this card's file surface to include them.
Each file has exactly one case rewritten, on the same input, to assert the
quoted emission instead of the refusal; the envelope file also records in its
`## The accept set is UNTOUCHED` paragraph that the groupBy OUT KEY position
moved. Every other refusal position — object, field, groupBy FIELD, aggregation
alias, DDL — still asserts INVALID_REQUEST / 400, the accept-set describe that
drives the groupBy FIELD is byte-identical, and `unsafeIdentifierError` /
`UNSAFE_IDENTIFIER_CODE` are untouched.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 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 9e286e248866c40d2db662f69aa0ba6e71b4b096packageMentionDocs.

Which tree this was computed on

This run read content/docs from 22ea82c33374c3c9ba99719c1df84f2c600b60a3 — the merge of head ffabb9991c5eefbb6020632a829dcc39725debc2 into base 9e286e248866c40d2db662f69aa0ba6e71b4b096, 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 22ea82c33374c3c9ba99719c1df84f2c600b60a3 && git checkout 22ea82c33374c3c9ba99719c1df84f2c600b60a3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e286e248866c40d2db662f69aa0ba6e71b4b096 ffabb9991c5eefbb6020632a829dcc39725debc2 && git checkout -B drift-repro 9e286e248866c40d2db662f69aa0ba6e71b4b096 && git merge --no-ff ffabb9991c5eefbb6020632a829dcc39725debc2
node scripts/docs-audit/affected-docs.mjs --json 9e286e248866c40d2db662f69aa0ba6e71b4b096

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

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance (engine execution seat, session_0112hMx9hjJ9BgB28X97DS68) — flipped to ready and auto-merge (squash) armed at 09:18Z on head ffabb9991.

  • ACCEPT on the card: drivers(turso): RemoteTransport still refuses a dotted groupBy alias while driver-sql escapes it — the second output-name position #14113 deliberately left alone #14235 (comment 5506982748); the mid-round surface amendment and the option-A ruling on the two hand-off controls: comment 5506404777.
  • Pre-landing checks at flip time: every check run on ffabb9991 completed with conclusion success or skipped (33 runs, perPage: 50; Lint & Repo Gates and all Type Check jobs success); node scripts/pm/check-governed-merges.mjs --test on the PR's 5 changed paths: 0 of 5 hit the register — not governed, ordinary queue landing applies.
  • Clause-② self-reading no, concurred at ACCEPT: driver accept set widened toward the declared GroupByNodeSchema.alias contract, no packages/spec change, no public-entry export change ⇒ @objectstack/driver-turso: patch is the right level. No needs:contract-review carrier on card or PR.
  • Closing keyword: Fixes #14235 only; the card closes at MERGED and its pm:dispatched is stripped in the landing record.

Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit ed44512Sep 2, 2026
35 checks passed
@os-musk
os-musk deleted the claude/issue-14235-turso-groupby-alias-escape branch September 2, 2026 09:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235) - #14480

Merged
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape
Sep 2, 2026
Merged

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)#14480
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Fixes#14235

RemoteTransport.aggregate emits a caller-supplied output NAME in exactly two positions. #13714 routed BOTH of driver-sql's through SqlDriver.aliasIdentifierSql; #14113 moved this transport's aggregation alias to the same escaping and deliberately left the groupBy alias gated, because that position carried a landed #6401 pin asserting the refusal. This card is the recorded, non-silent reversal of that pin.

An output-column key is a NAME — quoted and escaped. A column REFERENCE is grammar — validated. So field keeps assertSafeIdentifier and outKey no longer has it:

// beforethis.assertSafeIdentifier(outKey);selectParts.push(outKey===field ? `"${field}"` : `"${field}" AS "${outKey}"`);// afterselectParts.push(outKey===field ? `"${field}"` : `"${field}" AS ${this.aliasIdentifierSql(outKey)}`,);

GroupByNodeSchema.alias is the same class of key as AggregationNodeSchema.alias; the in-memory face projects g.alias ?? g.field verbatim and the MongoDB and SQL faces agree, so this face was the last one refusing names the contract admits — 'Region Name', 'deal.stage_bucket' — with an opaque 500 out of mapDataError before #14287 and a 400 since. outKey === field still emits the alias-less "field", byte-identical.

The file surface was amended mid-round, and why

The dispatch bound this card to two files and predicted that no test outside remote-transport-groupby-node.test.ts pinned the groupBy alias refusal. That prediction was wrong, and the round stopped on it rather than widening its own scope: two more tests pinned it, each left behind by a sibling card as a hand-off addressed to this one.

The PM ruled option A on the card and amended the surface to include both. Each file has exactly one case rewritten, on the same input, from a refusal assertion to the quoted emission; everything else in both files is byte-identical.

⛔ What did NOT move: unsafeIdentifierError and UNSAFE_IDENTIFIER_CODE are untouched, and every other refusal position — object, the aggregation field, the groupBy FIELD, the DDL table / column / index names — still asserts INVALID_REQUEST / 400. The envelope file's describe('the accept set is UNTOUCHED …') block drives the groupBy FIELD position and is byte-identical; its header paragraph gains one sentence recording that the OUT KEY position moved here. #14287's envelope ruling is not re-decided by this PR — this card decided which POSITIONS the gate governs, not what the gate answers when it fires.

Clause-② self-reading: no

No contract move. Nothing in packages/spec changes; GroupByNodeSchema.alias already declares this key and already admits these names. What widens is the driver's accept set, toward the contract the other three faces already implement — declared = enforced, restored. #14287's landed header says of this position "it MOVES the accept set", which is that same driver-level reading, not a spec one.

Evidence — every figure below measured at ffabb9991

Package suite green.pnpm --filter @objectstack/driver-turso testTest Files 42 passed (42) · Tests 1146 passed (1146). Both previously-red controls pass on the new truth.

Typecheck.pnpm --filter @objectstack/driver-turso typecheck exit 0, and the tests are really in the program rather than assumed to be: tsc --noEmit --listFiles names all three edited test files exactly once each.

Ablation — direction predicted in the suite header BEFORE the run. Mutation: restore the two pre-change lines at the groupBy select site. Proved on disk by grep counts on both the injected and the removed text plus git hash-object (mutated e3ee37e6d vs HEAD blob 7de7f70ff). Result: Tests 4 failed | 14 passed (18) — the two capture cases and both executing cases fail by THROWING inside the call (unsafe identifier rejected: "bucket"; DROP TABLE deal; --", "Region Name", "deal.stage_bucket"), never on a comparison, while the field-position control, the two byte-identical-emission cases, the string-form control, all five date-bucket refusals and both parity cases stay GREEN. Restore proved by the blob hash matching HEAD again plus an empty git diff HEAD; the script restores under a trap on EXIT INT TERM with an absolute path. No dist/ leg applies, and that is measured: the suite imports the mutated unit through a relative in-package specifier vitest resolves to src/, and the package's vitest.config.ts declares no alias.

Escaped, not merely emitted. The security-relevant claim is pinned against a real SQLite-backed libsql stub, not only on a captured string: an alias of bucket"; DROP TABLE deal; -- comes back as a COLUMN NAME and the table still holds all three rows.

Driver conformance ledger, before the first edit and after the last commit — identical:

check-driver-conformance: OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt.

Gate union. The 36 families node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derives for this changeset, re-derived after the last commit and run at ffabb9991: 34 exit 0. The other two exit 3 = NOT MEASURED by their own text, both environmental and neither a finding: check-test-completeness ("There is no local log to hand it, so the local reading for this gate is NOT MEASURED. It is not a red, and there is nothing here to fix.") and pm/check-half-states ("it is no reading at all"). Exit codes captured after redirect, never through a pipe. Plus pnpm lint over the whole repo — exit 0 — and pnpm check:error-status-conformance — exit 0, "every derivable runtime status is documented, and every documented status is reachable."

Changeset: @objectstack/driver-turso patch. Branch merged origin/main (never rebased, never force-pushed).

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…nstead of gating it (#14235)
The second output-name position of `RemoteTransport.aggregate`. #14113 moved
the aggregation alias to `aliasIdentifierSql` and left this one gated because
it carried a landed #6401 pin asserting the refusal; #13714 had already routed
both of driver-sql's output-name positions through escaping. An output-column
key is a NAME — quoted and escaped; a column reference is grammar — validated.
So `field` keeps `assertSafeIdentifier` and `outKey` no longer has it.
The #6401 pin is rewritten in place on the same input to assert what the
transport now emits, plus dotted/spaced round-trip cases and two executing
cases against the SQLite-backed libsql stub.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
… alias (#14235)
The input the rewritten #6401 pin carries has no aggregation `field`, so the
emission is `count(*)`, not `count("stage")`; and the one-statement assertion
now counts SELECTs rather than probing for a quote that the escape never
produces.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…14235)
4 failed / 14 passed of 18, case for case as predicted; every failure throws
inside the call rather than failing a comparison, and the `field`-position
control stays green. Notes why no `dist/` leg applies and how the mutation and
the restore were each proved on disk.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…#14287 left (#14235)
Both controls were hand-offs addressed to this card in their own comments —
#14113's "this control records the state it was left in", #14287's "the day
that card lands, this is the test that has to be rewritten deliberately rather
than found red by surprise". PM ruling on the stop condition (comment
5506404777) amended this card's file surface to include them.
Each file has exactly one case rewritten, on the same input, to assert the
quoted emission instead of the refusal; the envelope file also records in its
`## The accept set is UNTOUCHED` paragraph that the groupBy OUT KEY position
moved. Every other refusal position — object, field, groupBy FIELD, aggregation
alias, DDL — still asserts INVALID_REQUEST / 400, the accept-set describe that
drives the groupBy FIELD is byte-identical, and `unsafeIdentifierError` /
`UNSAFE_IDENTIFIER_CODE` are untouched.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 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 9e286e248866c40d2db662f69aa0ba6e71b4b096packageMentionDocs.

Which tree this was computed on

This run read content/docs from 22ea82c33374c3c9ba99719c1df84f2c600b60a3 — the merge of head ffabb9991c5eefbb6020632a829dcc39725debc2 into base 9e286e248866c40d2db662f69aa0ba6e71b4b096, 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 22ea82c33374c3c9ba99719c1df84f2c600b60a3 && git checkout 22ea82c33374c3c9ba99719c1df84f2c600b60a3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e286e248866c40d2db662f69aa0ba6e71b4b096 ffabb9991c5eefbb6020632a829dcc39725debc2 && git checkout -B drift-repro 9e286e248866c40d2db662f69aa0ba6e71b4b096 && git merge --no-ff ffabb9991c5eefbb6020632a829dcc39725debc2
node scripts/docs-audit/affected-docs.mjs --json 9e286e248866c40d2db662f69aa0ba6e71b4b096

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

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance (engine execution seat, session_0112hMx9hjJ9BgB28X97DS68) — flipped to ready and auto-merge (squash) armed at 09:18Z on head ffabb9991.

  • ACCEPT on the card: drivers(turso): RemoteTransport still refuses a dotted groupBy alias while driver-sql escapes it — the second output-name position #14113 deliberately left alone #14235 (comment 5506982748); the mid-round surface amendment and the option-A ruling on the two hand-off controls: comment 5506404777.
  • Pre-landing checks at flip time: every check run on ffabb9991 completed with conclusion success or skipped (33 runs, perPage: 50; Lint & Repo Gates and all Type Check jobs success); node scripts/pm/check-governed-merges.mjs --test on the PR's 5 changed paths: 0 of 5 hit the register — not governed, ordinary queue landing applies.
  • Clause-② self-reading no, concurred at ACCEPT: driver accept set widened toward the declared GroupByNodeSchema.alias contract, no packages/spec change, no public-entry export change ⇒ @objectstack/driver-turso: patch is the right level. No needs:contract-review carrier on card or PR.
  • Closing keyword: Fixes #14235 only; the card closes at MERGED and its pm:dispatched is stripped in the landing record.

Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit ed44512Sep 2, 2026
35 checks passed
@os-musk
os-musk deleted the claude/issue-14235-turso-groupby-alias-escape branch September 2, 2026 09:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235) - #14480

Merged
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape
Sep 2, 2026
Merged

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)#14480
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Fixes#14235

RemoteTransport.aggregate emits a caller-supplied output NAME in exactly two positions. #13714 routed BOTH of driver-sql's through SqlDriver.aliasIdentifierSql; #14113 moved this transport's aggregation alias to the same escaping and deliberately left the groupBy alias gated, because that position carried a landed #6401 pin asserting the refusal. This card is the recorded, non-silent reversal of that pin.

An output-column key is a NAME — quoted and escaped. A column REFERENCE is grammar — validated. So field keeps assertSafeIdentifier and outKey no longer has it:

// beforethis.assertSafeIdentifier(outKey);selectParts.push(outKey===field ? `"${field}"` : `"${field}" AS "${outKey}"`);// afterselectParts.push(outKey===field ? `"${field}"` : `"${field}" AS ${this.aliasIdentifierSql(outKey)}`,);

GroupByNodeSchema.alias is the same class of key as AggregationNodeSchema.alias; the in-memory face projects g.alias ?? g.field verbatim and the MongoDB and SQL faces agree, so this face was the last one refusing names the contract admits — 'Region Name', 'deal.stage_bucket' — with an opaque 500 out of mapDataError before #14287 and a 400 since. outKey === field still emits the alias-less "field", byte-identical.

The file surface was amended mid-round, and why

The dispatch bound this card to two files and predicted that no test outside remote-transport-groupby-node.test.ts pinned the groupBy alias refusal. That prediction was wrong, and the round stopped on it rather than widening its own scope: two more tests pinned it, each left behind by a sibling card as a hand-off addressed to this one.

The PM ruled option A on the card and amended the surface to include both. Each file has exactly one case rewritten, on the same input, from a refusal assertion to the quoted emission; everything else in both files is byte-identical.

⛔ What did NOT move: unsafeIdentifierError and UNSAFE_IDENTIFIER_CODE are untouched, and every other refusal position — object, the aggregation field, the groupBy FIELD, the DDL table / column / index names — still asserts INVALID_REQUEST / 400. The envelope file's describe('the accept set is UNTOUCHED …') block drives the groupBy FIELD position and is byte-identical; its header paragraph gains one sentence recording that the OUT KEY position moved here. #14287's envelope ruling is not re-decided by this PR — this card decided which POSITIONS the gate governs, not what the gate answers when it fires.

Clause-② self-reading: no

No contract move. Nothing in packages/spec changes; GroupByNodeSchema.alias already declares this key and already admits these names. What widens is the driver's accept set, toward the contract the other three faces already implement — declared = enforced, restored. #14287's landed header says of this position "it MOVES the accept set", which is that same driver-level reading, not a spec one.

Evidence — every figure below measured at ffabb9991

Package suite green.pnpm --filter @objectstack/driver-turso testTest Files 42 passed (42) · Tests 1146 passed (1146). Both previously-red controls pass on the new truth.

Typecheck.pnpm --filter @objectstack/driver-turso typecheck exit 0, and the tests are really in the program rather than assumed to be: tsc --noEmit --listFiles names all three edited test files exactly once each.

Ablation — direction predicted in the suite header BEFORE the run. Mutation: restore the two pre-change lines at the groupBy select site. Proved on disk by grep counts on both the injected and the removed text plus git hash-object (mutated e3ee37e6d vs HEAD blob 7de7f70ff). Result: Tests 4 failed | 14 passed (18) — the two capture cases and both executing cases fail by THROWING inside the call (unsafe identifier rejected: "bucket"; DROP TABLE deal; --", "Region Name", "deal.stage_bucket"), never on a comparison, while the field-position control, the two byte-identical-emission cases, the string-form control, all five date-bucket refusals and both parity cases stay GREEN. Restore proved by the blob hash matching HEAD again plus an empty git diff HEAD; the script restores under a trap on EXIT INT TERM with an absolute path. No dist/ leg applies, and that is measured: the suite imports the mutated unit through a relative in-package specifier vitest resolves to src/, and the package's vitest.config.ts declares no alias.

Escaped, not merely emitted. The security-relevant claim is pinned against a real SQLite-backed libsql stub, not only on a captured string: an alias of bucket"; DROP TABLE deal; -- comes back as a COLUMN NAME and the table still holds all three rows.

Driver conformance ledger, before the first edit and after the last commit — identical:

check-driver-conformance: OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt.

Gate union. The 36 families node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derives for this changeset, re-derived after the last commit and run at ffabb9991: 34 exit 0. The other two exit 3 = NOT MEASURED by their own text, both environmental and neither a finding: check-test-completeness ("There is no local log to hand it, so the local reading for this gate is NOT MEASURED. It is not a red, and there is nothing here to fix.") and pm/check-half-states ("it is no reading at all"). Exit codes captured after redirect, never through a pipe. Plus pnpm lint over the whole repo — exit 0 — and pnpm check:error-status-conformance — exit 0, "every derivable runtime status is documented, and every documented status is reachable."

Changeset: @objectstack/driver-turso patch. Branch merged origin/main (never rebased, never force-pushed).

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…nstead of gating it (#14235)
The second output-name position of `RemoteTransport.aggregate`. #14113 moved
the aggregation alias to `aliasIdentifierSql` and left this one gated because
it carried a landed #6401 pin asserting the refusal; #13714 had already routed
both of driver-sql's output-name positions through escaping. An output-column
key is a NAME — quoted and escaped; a column reference is grammar — validated.
So `field` keeps `assertSafeIdentifier` and `outKey` no longer has it.
The #6401 pin is rewritten in place on the same input to assert what the
transport now emits, plus dotted/spaced round-trip cases and two executing
cases against the SQLite-backed libsql stub.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
… alias (#14235)
The input the rewritten #6401 pin carries has no aggregation `field`, so the
emission is `count(*)`, not `count("stage")`; and the one-statement assertion
now counts SELECTs rather than probing for a quote that the escape never
produces.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…14235)
4 failed / 14 passed of 18, case for case as predicted; every failure throws
inside the call rather than failing a comparison, and the `field`-position
control stays green. Notes why no `dist/` leg applies and how the mutation and
the restore were each proved on disk.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…#14287 left (#14235)
Both controls were hand-offs addressed to this card in their own comments —
#14113's "this control records the state it was left in", #14287's "the day
that card lands, this is the test that has to be rewritten deliberately rather
than found red by surprise". PM ruling on the stop condition (comment
5506404777) amended this card's file surface to include them.
Each file has exactly one case rewritten, on the same input, to assert the
quoted emission instead of the refusal; the envelope file also records in its
`## The accept set is UNTOUCHED` paragraph that the groupBy OUT KEY position
moved. Every other refusal position — object, field, groupBy FIELD, aggregation
alias, DDL — still asserts INVALID_REQUEST / 400, the accept-set describe that
drives the groupBy FIELD is byte-identical, and `unsafeIdentifierError` /
`UNSAFE_IDENTIFIER_CODE` are untouched.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 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 9e286e248866c40d2db662f69aa0ba6e71b4b096packageMentionDocs.

Which tree this was computed on

This run read content/docs from 22ea82c33374c3c9ba99719c1df84f2c600b60a3 — the merge of head ffabb9991c5eefbb6020632a829dcc39725debc2 into base 9e286e248866c40d2db662f69aa0ba6e71b4b096, 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 22ea82c33374c3c9ba99719c1df84f2c600b60a3 && git checkout 22ea82c33374c3c9ba99719c1df84f2c600b60a3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e286e248866c40d2db662f69aa0ba6e71b4b096 ffabb9991c5eefbb6020632a829dcc39725debc2 && git checkout -B drift-repro 9e286e248866c40d2db662f69aa0ba6e71b4b096 && git merge --no-ff ffabb9991c5eefbb6020632a829dcc39725debc2
node scripts/docs-audit/affected-docs.mjs --json 9e286e248866c40d2db662f69aa0ba6e71b4b096

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

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance (engine execution seat, session_0112hMx9hjJ9BgB28X97DS68) — flipped to ready and auto-merge (squash) armed at 09:18Z on head ffabb9991.

  • ACCEPT on the card: drivers(turso): RemoteTransport still refuses a dotted groupBy alias while driver-sql escapes it — the second output-name position #14113 deliberately left alone #14235 (comment 5506982748); the mid-round surface amendment and the option-A ruling on the two hand-off controls: comment 5506404777.
  • Pre-landing checks at flip time: every check run on ffabb9991 completed with conclusion success or skipped (33 runs, perPage: 50; Lint & Repo Gates and all Type Check jobs success); node scripts/pm/check-governed-merges.mjs --test on the PR's 5 changed paths: 0 of 5 hit the register — not governed, ordinary queue landing applies.
  • Clause-② self-reading no, concurred at ACCEPT: driver accept set widened toward the declared GroupByNodeSchema.alias contract, no packages/spec change, no public-entry export change ⇒ @objectstack/driver-turso: patch is the right level. No needs:contract-review carrier on card or PR.
  • Closing keyword: Fixes #14235 only; the card closes at MERGED and its pm:dispatched is stripped in the landing record.

Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit ed44512Sep 2, 2026
35 checks passed
@os-musk
os-musk deleted the claude/issue-14235-turso-groupby-alias-escape branch September 2, 2026 09:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235) - #14480

Merged
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape
Sep 2, 2026
Merged

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)#14480
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Fixes#14235

RemoteTransport.aggregate emits a caller-supplied output NAME in exactly two positions. #13714 routed BOTH of driver-sql's through SqlDriver.aliasIdentifierSql; #14113 moved this transport's aggregation alias to the same escaping and deliberately left the groupBy alias gated, because that position carried a landed #6401 pin asserting the refusal. This card is the recorded, non-silent reversal of that pin.

An output-column key is a NAME — quoted and escaped. A column REFERENCE is grammar — validated. So field keeps assertSafeIdentifier and outKey no longer has it:

// beforethis.assertSafeIdentifier(outKey);selectParts.push(outKey===field ? `"${field}"` : `"${field}" AS "${outKey}"`);// afterselectParts.push(outKey===field ? `"${field}"` : `"${field}" AS ${this.aliasIdentifierSql(outKey)}`,);

GroupByNodeSchema.alias is the same class of key as AggregationNodeSchema.alias; the in-memory face projects g.alias ?? g.field verbatim and the MongoDB and SQL faces agree, so this face was the last one refusing names the contract admits — 'Region Name', 'deal.stage_bucket' — with an opaque 500 out of mapDataError before #14287 and a 400 since. outKey === field still emits the alias-less "field", byte-identical.

The file surface was amended mid-round, and why

The dispatch bound this card to two files and predicted that no test outside remote-transport-groupby-node.test.ts pinned the groupBy alias refusal. That prediction was wrong, and the round stopped on it rather than widening its own scope: two more tests pinned it, each left behind by a sibling card as a hand-off addressed to this one.

The PM ruled option A on the card and amended the surface to include both. Each file has exactly one case rewritten, on the same input, from a refusal assertion to the quoted emission; everything else in both files is byte-identical.

⛔ What did NOT move: unsafeIdentifierError and UNSAFE_IDENTIFIER_CODE are untouched, and every other refusal position — object, the aggregation field, the groupBy FIELD, the DDL table / column / index names — still asserts INVALID_REQUEST / 400. The envelope file's describe('the accept set is UNTOUCHED …') block drives the groupBy FIELD position and is byte-identical; its header paragraph gains one sentence recording that the OUT KEY position moved here. #14287's envelope ruling is not re-decided by this PR — this card decided which POSITIONS the gate governs, not what the gate answers when it fires.

Clause-② self-reading: no

No contract move. Nothing in packages/spec changes; GroupByNodeSchema.alias already declares this key and already admits these names. What widens is the driver's accept set, toward the contract the other three faces already implement — declared = enforced, restored. #14287's landed header says of this position "it MOVES the accept set", which is that same driver-level reading, not a spec one.

Evidence — every figure below measured at ffabb9991

Package suite green.pnpm --filter @objectstack/driver-turso testTest Files 42 passed (42) · Tests 1146 passed (1146). Both previously-red controls pass on the new truth.

Typecheck.pnpm --filter @objectstack/driver-turso typecheck exit 0, and the tests are really in the program rather than assumed to be: tsc --noEmit --listFiles names all three edited test files exactly once each.

Ablation — direction predicted in the suite header BEFORE the run. Mutation: restore the two pre-change lines at the groupBy select site. Proved on disk by grep counts on both the injected and the removed text plus git hash-object (mutated e3ee37e6d vs HEAD blob 7de7f70ff). Result: Tests 4 failed | 14 passed (18) — the two capture cases and both executing cases fail by THROWING inside the call (unsafe identifier rejected: "bucket"; DROP TABLE deal; --", "Region Name", "deal.stage_bucket"), never on a comparison, while the field-position control, the two byte-identical-emission cases, the string-form control, all five date-bucket refusals and both parity cases stay GREEN. Restore proved by the blob hash matching HEAD again plus an empty git diff HEAD; the script restores under a trap on EXIT INT TERM with an absolute path. No dist/ leg applies, and that is measured: the suite imports the mutated unit through a relative in-package specifier vitest resolves to src/, and the package's vitest.config.ts declares no alias.

Escaped, not merely emitted. The security-relevant claim is pinned against a real SQLite-backed libsql stub, not only on a captured string: an alias of bucket"; DROP TABLE deal; -- comes back as a COLUMN NAME and the table still holds all three rows.

Driver conformance ledger, before the first edit and after the last commit — identical:

check-driver-conformance: OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt.

Gate union. The 36 families node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derives for this changeset, re-derived after the last commit and run at ffabb9991: 34 exit 0. The other two exit 3 = NOT MEASURED by their own text, both environmental and neither a finding: check-test-completeness ("There is no local log to hand it, so the local reading for this gate is NOT MEASURED. It is not a red, and there is nothing here to fix.") and pm/check-half-states ("it is no reading at all"). Exit codes captured after redirect, never through a pipe. Plus pnpm lint over the whole repo — exit 0 — and pnpm check:error-status-conformance — exit 0, "every derivable runtime status is documented, and every documented status is reachable."

Changeset: @objectstack/driver-turso patch. Branch merged origin/main (never rebased, never force-pushed).

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…nstead of gating it (#14235)
The second output-name position of `RemoteTransport.aggregate`. #14113 moved
the aggregation alias to `aliasIdentifierSql` and left this one gated because
it carried a landed #6401 pin asserting the refusal; #13714 had already routed
both of driver-sql's output-name positions through escaping. An output-column
key is a NAME — quoted and escaped; a column reference is grammar — validated.
So `field` keeps `assertSafeIdentifier` and `outKey` no longer has it.
The #6401 pin is rewritten in place on the same input to assert what the
transport now emits, plus dotted/spaced round-trip cases and two executing
cases against the SQLite-backed libsql stub.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
… alias (#14235)
The input the rewritten #6401 pin carries has no aggregation `field`, so the
emission is `count(*)`, not `count("stage")`; and the one-statement assertion
now counts SELECTs rather than probing for a quote that the escape never
produces.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…14235)
4 failed / 14 passed of 18, case for case as predicted; every failure throws
inside the call rather than failing a comparison, and the `field`-position
control stays green. Notes why no `dist/` leg applies and how the mutation and
the restore were each proved on disk.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…#14287 left (#14235)
Both controls were hand-offs addressed to this card in their own comments —
#14113's "this control records the state it was left in", #14287's "the day
that card lands, this is the test that has to be rewritten deliberately rather
than found red by surprise". PM ruling on the stop condition (comment
5506404777) amended this card's file surface to include them.
Each file has exactly one case rewritten, on the same input, to assert the
quoted emission instead of the refusal; the envelope file also records in its
`## The accept set is UNTOUCHED` paragraph that the groupBy OUT KEY position
moved. Every other refusal position — object, field, groupBy FIELD, aggregation
alias, DDL — still asserts INVALID_REQUEST / 400, the accept-set describe that
drives the groupBy FIELD is byte-identical, and `unsafeIdentifierError` /
`UNSAFE_IDENTIFIER_CODE` are untouched.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 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 9e286e248866c40d2db662f69aa0ba6e71b4b096packageMentionDocs.

Which tree this was computed on

This run read content/docs from 22ea82c33374c3c9ba99719c1df84f2c600b60a3 — the merge of head ffabb9991c5eefbb6020632a829dcc39725debc2 into base 9e286e248866c40d2db662f69aa0ba6e71b4b096, 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 22ea82c33374c3c9ba99719c1df84f2c600b60a3 && git checkout 22ea82c33374c3c9ba99719c1df84f2c600b60a3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e286e248866c40d2db662f69aa0ba6e71b4b096 ffabb9991c5eefbb6020632a829dcc39725debc2 && git checkout -B drift-repro 9e286e248866c40d2db662f69aa0ba6e71b4b096 && git merge --no-ff ffabb9991c5eefbb6020632a829dcc39725debc2
node scripts/docs-audit/affected-docs.mjs --json 9e286e248866c40d2db662f69aa0ba6e71b4b096

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

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance (engine execution seat, session_0112hMx9hjJ9BgB28X97DS68) — flipped to ready and auto-merge (squash) armed at 09:18Z on head ffabb9991.

  • ACCEPT on the card: drivers(turso): RemoteTransport still refuses a dotted groupBy alias while driver-sql escapes it — the second output-name position #14113 deliberately left alone #14235 (comment 5506982748); the mid-round surface amendment and the option-A ruling on the two hand-off controls: comment 5506404777.
  • Pre-landing checks at flip time: every check run on ffabb9991 completed with conclusion success or skipped (33 runs, perPage: 50; Lint & Repo Gates and all Type Check jobs success); node scripts/pm/check-governed-merges.mjs --test on the PR's 5 changed paths: 0 of 5 hit the register — not governed, ordinary queue landing applies.
  • Clause-② self-reading no, concurred at ACCEPT: driver accept set widened toward the declared GroupByNodeSchema.alias contract, no packages/spec change, no public-entry export change ⇒ @objectstack/driver-turso: patch is the right level. No needs:contract-review carrier on card or PR.
  • Closing keyword: Fixes #14235 only; the card closes at MERGED and its pm:dispatched is stripped in the landing record.

Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit ed44512Sep 2, 2026
35 checks passed
@os-musk
os-musk deleted the claude/issue-14235-turso-groupby-alias-escape branch September 2, 2026 09:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-musk@claude
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235) - #14480

Merged
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape
Sep 2, 2026
Merged

fix(driver-turso): escape the groupBy alias on the remote transport instead of gating it (#14235)#14480
os-musk merged 5 commits into
mainfrom
claude/issue-14235-turso-groupby-alias-escape

Conversation

@os-musk

Copy link
Copy Markdown
Collaborator

Fixes#14235

RemoteTransport.aggregate emits a caller-supplied output NAME in exactly two positions. #13714 routed BOTH of driver-sql's through SqlDriver.aliasIdentifierSql; #14113 moved this transport's aggregation alias to the same escaping and deliberately left the groupBy alias gated, because that position carried a landed #6401 pin asserting the refusal. This card is the recorded, non-silent reversal of that pin.

An output-column key is a NAME — quoted and escaped. A column REFERENCE is grammar — validated. So field keeps assertSafeIdentifier and outKey no longer has it:

// beforethis.assertSafeIdentifier(outKey);selectParts.push(outKey===field ? `"${field}"` : `"${field}" AS "${outKey}"`);// afterselectParts.push(outKey===field ? `"${field}"` : `"${field}" AS ${this.aliasIdentifierSql(outKey)}`,);

GroupByNodeSchema.alias is the same class of key as AggregationNodeSchema.alias; the in-memory face projects g.alias ?? g.field verbatim and the MongoDB and SQL faces agree, so this face was the last one refusing names the contract admits — 'Region Name', 'deal.stage_bucket' — with an opaque 500 out of mapDataError before #14287 and a 400 since. outKey === field still emits the alias-less "field", byte-identical.

The file surface was amended mid-round, and why

The dispatch bound this card to two files and predicted that no test outside remote-transport-groupby-node.test.ts pinned the groupBy alias refusal. That prediction was wrong, and the round stopped on it rather than widening its own scope: two more tests pinned it, each left behind by a sibling card as a hand-off addressed to this one.

The PM ruled option A on the card and amended the surface to include both. Each file has exactly one case rewritten, on the same input, from a refusal assertion to the quoted emission; everything else in both files is byte-identical.

⛔ What did NOT move: unsafeIdentifierError and UNSAFE_IDENTIFIER_CODE are untouched, and every other refusal position — object, the aggregation field, the groupBy FIELD, the DDL table / column / index names — still asserts INVALID_REQUEST / 400. The envelope file's describe('the accept set is UNTOUCHED …') block drives the groupBy FIELD position and is byte-identical; its header paragraph gains one sentence recording that the OUT KEY position moved here. #14287's envelope ruling is not re-decided by this PR — this card decided which POSITIONS the gate governs, not what the gate answers when it fires.

Clause-② self-reading: no

No contract move. Nothing in packages/spec changes; GroupByNodeSchema.alias already declares this key and already admits these names. What widens is the driver's accept set, toward the contract the other three faces already implement — declared = enforced, restored. #14287's landed header says of this position "it MOVES the accept set", which is that same driver-level reading, not a spec one.

Evidence — every figure below measured at ffabb9991

Package suite green.pnpm --filter @objectstack/driver-turso testTest Files 42 passed (42) · Tests 1146 passed (1146). Both previously-red controls pass on the new truth.

Typecheck.pnpm --filter @objectstack/driver-turso typecheck exit 0, and the tests are really in the program rather than assumed to be: tsc --noEmit --listFiles names all three edited test files exactly once each.

Ablation — direction predicted in the suite header BEFORE the run. Mutation: restore the two pre-change lines at the groupBy select site. Proved on disk by grep counts on both the injected and the removed text plus git hash-object (mutated e3ee37e6d vs HEAD blob 7de7f70ff). Result: Tests 4 failed | 14 passed (18) — the two capture cases and both executing cases fail by THROWING inside the call (unsafe identifier rejected: "bucket"; DROP TABLE deal; --", "Region Name", "deal.stage_bucket"), never on a comparison, while the field-position control, the two byte-identical-emission cases, the string-form control, all five date-bucket refusals and both parity cases stay GREEN. Restore proved by the blob hash matching HEAD again plus an empty git diff HEAD; the script restores under a trap on EXIT INT TERM with an absolute path. No dist/ leg applies, and that is measured: the suite imports the mutated unit through a relative in-package specifier vitest resolves to src/, and the package's vitest.config.ts declares no alias.

Escaped, not merely emitted. The security-relevant claim is pinned against a real SQLite-backed libsql stub, not only on a captured string: an alias of bucket"; DROP TABLE deal; -- comes back as a COLUMN NAME and the table still holds all three rows.

Driver conformance ledger, before the first edit and after the last commit — identical:

check-driver-conformance: OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt.

Gate union. The 36 families node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derives for this changeset, re-derived after the last commit and run at ffabb9991: 34 exit 0. The other two exit 3 = NOT MEASURED by their own text, both environmental and neither a finding: check-test-completeness ("There is no local log to hand it, so the local reading for this gate is NOT MEASURED. It is not a red, and there is nothing here to fix.") and pm/check-half-states ("it is no reading at all"). Exit codes captured after redirect, never through a pipe. Plus pnpm lint over the whole repo — exit 0 — and pnpm check:error-status-conformance — exit 0, "every derivable runtime status is documented, and every documented status is reachable."

Changeset: @objectstack/driver-turso patch. Branch merged origin/main (never rebased, never force-pushed).

🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…nstead of gating it (#14235)
The second output-name position of `RemoteTransport.aggregate`. #14113 moved
the aggregation alias to `aliasIdentifierSql` and left this one gated because
it carried a landed #6401 pin asserting the refusal; #13714 had already routed
both of driver-sql's output-name positions through escaping. An output-column
key is a NAME — quoted and escaped; a column reference is grammar — validated.
So `field` keeps `assertSafeIdentifier` and `outKey` no longer has it.
The #6401 pin is rewritten in place on the same input to assert what the
transport now emits, plus dotted/spaced round-trip cases and two executing
cases against the SQLite-backed libsql stub.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
… alias (#14235)
The input the rewritten #6401 pin carries has no aggregation `field`, so the
emission is `count(*)`, not `count("stage")`; and the one-statement assertion
now counts SELECTs rather than probing for a quote that the escape never
produces.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…14235)
4 failed / 14 passed of 18, case for case as predicted; every failure throws
inside the call rather than failing a comparison, and the `field`-position
control stays green. Notes why no `dist/` leg applies and how the mutation and
the restore were each proved on disk.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
…#14287 left (#14235)
Both controls were hand-offs addressed to this card in their own comments —
#14113's "this control records the state it was left in", #14287's "the day
that card lands, this is the test that has to be rewritten deliberately rather
than found red by surprise". PM ruling on the stop condition (comment
5506404777) amended this card's file surface to include them.
Each file has exactly one case rewritten, on the same input, to assert the
quoted emission instead of the refusal; the envelope file also records in its
`## The accept set is UNTOUCHED` paragraph that the groupBy OUT KEY position
moved. Every other refusal position — object, field, groupBy FIELD, aggregation
alias, DDL — still asserts INVALID_REQUEST / 400, the accept-set describe that
drives the groupBy FIELD is byte-identical, and `unsafeIdentifierError` /
`UNSAFE_IDENTIFIER_CODE` are untouched.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0112hMx9hjJ9BgB28X97DS68
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 6 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 9e286e248866c40d2db662f69aa0ba6e71b4b096packageMentionDocs.

Which tree this was computed on

This run read content/docs from 22ea82c33374c3c9ba99719c1df84f2c600b60a3 — the merge of head ffabb9991c5eefbb6020632a829dcc39725debc2 into base 9e286e248866c40d2db662f69aa0ba6e71b4b096, 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 22ea82c33374c3c9ba99719c1df84f2c600b60a3 && git checkout 22ea82c33374c3c9ba99719c1df84f2c600b60a3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9e286e248866c40d2db662f69aa0ba6e71b4b096 ffabb9991c5eefbb6020632a829dcc39725debc2 && git checkout -B drift-repro 9e286e248866c40d2db662f69aa0ba6e71b4b096 && git merge --no-ff ffabb9991c5eefbb6020632a829dcc39725debc2
node scripts/docs-audit/affected-docs.mjs --json 9e286e248866c40d2db662f69aa0ba6e71b4b096

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

@os-muskClaude

Copy link
Copy Markdown
CollaboratorAuthor

Landing provenance (engine execution seat, session_0112hMx9hjJ9BgB28X97DS68) — flipped to ready and auto-merge (squash) armed at 09:18Z on head ffabb9991.

  • ACCEPT on the card: drivers(turso): RemoteTransport still refuses a dotted groupBy alias while driver-sql escapes it — the second output-name position #14113 deliberately left alone #14235 (comment 5506982748); the mid-round surface amendment and the option-A ruling on the two hand-off controls: comment 5506404777.
  • Pre-landing checks at flip time: every check run on ffabb9991 completed with conclusion success or skipped (33 runs, perPage: 50; Lint & Repo Gates and all Type Check jobs success); node scripts/pm/check-governed-merges.mjs --test on the PR's 5 changed paths: 0 of 5 hit the register — not governed, ordinary queue landing applies.
  • Clause-② self-reading no, concurred at ACCEPT: driver accept set widened toward the declared GroupByNodeSchema.alias contract, no packages/spec change, no public-entry export change ⇒ @objectstack/driver-turso: patch is the right level. No needs:contract-review carrier on card or PR.
  • Closing keyword: Fixes #14235 only; the card closes at MERGED and its pm:dispatched is stripped in the landing record.

Generated by Claude Code

@os-musk
os-musk added this pull request to the merge queueSep 2, 2026
Merged via the queue into main with commit ed44512Sep 2, 2026
35 checks passed
@os-musk
os-musk deleted the claude/issue-14235-turso-groupby-alias-escape branch September 2, 2026 09:40
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@os-musk@claude