Skip to content

fix(service-datasource): an unset env credential is a note after the driver's error, and a failed connect holds no claim on its variable (#28283) - #28325

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-t43yct
Oct 31, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-t43yct

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #28283

Clause-②: no — measured: the connect verdict's reason text changes shape and an internal owner map releases on failure. No schema, export, type, error code or status moves, nothing is newly refused or accepted, and no consumer outside this package's own pins reads the sentence (git grep "not set on this host" outside service-datasource: 0 hits).
Metadata-protocol: none

维护者速读

  • 改了什么:代码里声明的数据源在环境变量 OS_DATASOURCE_NAME_PASSWORD 没设、连接又失败时,失败原因改为「驱动自己的错误 + 结尾的 (note: …变量没设…)」,不再用破折号把变量说成失败的原因;另外,一次失败的连接会释放它登记的变量,之后另一个推导出同一变量的数据源不会再被误判为冲突。
  • 为什么改:feat(service-datasource): a code-defined datasource reads its credential from OS_DATASOURCE_<NAME>_PASSWORD #28208 的契约评审 S1/S2:主机不可达、驱动模块缺失、TLS 失败时,运维被引去先设密码;失败的连接还一直占着冲突登记,直到 disconnect。
  • 风险与回滚:低。只改 datasource-connection-service.ts 里 env 凭据槽的两处:原因文本的拼接,和失败路径上释放本次登记。两个都活着的同变量数据源仍被拒绝。回滚即 revert。
  • 先看:packages/services/service-datasource/src/datasource-connection-service.ts(claimedVariable 的登记与 catch 里的释放、(note: …) 那一句);src/__tests__/datasource-env-credential.test.ts(两条新 pin)。
  • 两分钟验证:pnpm --filter @objectstack/service-datasource exec vitest run --maxWorkers=2 src/__tests__/datasource-env-credential.test.ts,预期 26/26 通过。
  • 需要你做的:无,席位落地。

What changes

Both items are in DatasourceConnectionService.connect()'s env-credential slot (#27687):

  • S1, the reason. With the slot applied and the variable unset, a failed build used to answer DRIVER_ERROR — datasource 'x' is defined in code and reads its credential from OS_DATASOURCE_X_PASSWORD at connect, which is not set on this host. The base already led with the driver's error, so the order's hypothesis that the reason led with the variable did not hold as written. The dash joined the sentence on as the explanation, though, for every cause: an unreachable host, a missing driver module, a TLS failure. It now reads DRIVER_ERROR (note: datasource 'x' is defined in code and reads its credential from OS_DATASOURCE_X_PASSWORD at connect, which is not set on this host). The note stays for a login refusal, so that pin still names the variable to set. I did not take the card's other route, matching the driver's error for "reads as an authentication refusal": that needs a per-driver table of auth codes (pg 28P01, mysql 1045, mssql ELOGIN…) that would drift silently, and the note is true for every cause.
  • S2, the owner. The collision check and the claim stay one step, before the build, with no await between them. claimedVariable records that THIS attempt made the claim, and the failed-degraded catch releases it before handleFailure, which may throw the fail-fast error. A connect of a name that already owned the variable claims nothing new and releases nothing. I kept the claim before the build, rather than recording it after the build succeeds, so that two overlapping connects of colliding names cannot both pass the check across the build's awaits.
  • A patch changeset for @objectstack/service-datasource. The docs line in external-datasources.mdx ("its reason names the variable to set") stays true and is not edited.

Tests

src/__tests__/datasource-env-credential.test.ts, through the real DatasourceConnectionService:

  • S1 pin (new): a code datasource, slot applied, variable unset, whose connect throws connect ECONNREFUSED 127.0.0.1:1 (code: 'ECONNREFUSED'). The reason starts with the driver's error, the rest is exactly (note: … OS_DATASOURCE_WAREHOUSE_PASSWORD …), and the retained connection state carries the same reason.
  • S1 control (unchanged): "the database refusing the login answers the variable to set" passes.
  • S2 pin (new): eu-sales fails failed-degraded (getaddrinfo ENOTFOUND), then eu_sales, which derives the same OS_DATASOURCE_EU_SALES_PASSWORD, connects.
  • S2 control (unchanged): "the second name is refused with both names and the variable" (both live) passes.
Red first, green, and one ablation per pin
  • Base, the fix not yet applied: Tests 2 failed | 24 passed (26). S1: expected ' — datasource \'warehouse\' is define…' to match /^ \(note: …/. S2: expected 'failed-credentials' to be 'connected'.
  • Head: Tests 26 passed (26).
  • Ablation 1, through scripts/ablation-replace.mjs, restores } — datasource '${name}' is defined. On disk: note-count 0, dash-count 1. Result: only the S1 pin fails (1 failed | 25 passed). Restored: blob == HEAD 2753417323b5, git diff HEAD empty.
  • Ablation 2 deletes if (claimedVariable !== undefined) this.envCredentialOwners.delete(claimedVariable);. On disk: release-count 0. Result: only the S2 pin fails (1 failed | 25 passed). Restored: blob == HEAD, git diff HEAD empty.
  • The suite imports ../datasource-connection-service.js from src/, so no dist/ rebuild sits in either leg.
Package suite, typecheck, lint
  • pnpm --filter @objectstack/service-datasource test: Test Files 66 passed (66), Tests 1100 passed (1100), lock VERDICT command-exit 0.
  • pnpm --filter @objectstack/service-datasource typecheck: tsc --noEmit, VERDICT command-exit 0. --listFiles includes the test file.
  • eslint src --no-inline-config in the package: exit 0.

Acceptance notes

  • Security: no credential value appears in a test, log or this body. The pins use variable NAMES and a fixed fake value, and the existing no-leak pin passes.
  • The reason is operator-facing free text: the retained connect verdict (Setup → Datasources statusReason, the saved-datasource Test answer) and the fail-fast boot message. It is pinned only by this package's tests, and only by toContain on the variable name, which still holds.
  • packages/runtime's datasource-env-credential.test.ts reads this package from dist/ and asserts no reason text. No export changes, so per the local-verification scope it is left to CI's Test Core.

Verification

Gates from node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack on this diff (60 commands), run on head d3ceca42 with each exit code captured before any pipe: 59 exit 0. pnpm check:dual-build-cjs-loads exits 3, PREREQUISITE NOT MET (a fresh worktree has not built every package's dist), so it is NOT MEASURED locally and left to CI. check-plugin-teardown-shape.mjs --self-test first exited 3 on the shallow clone, and exits 0 after fetching its fixture commit. dispatch-gates --ran: "60 derived famil(ies) accounted for — 59 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)".

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic. (packages/spec untouched.)
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes (scoped to the touched package).
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: n/a.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English (the 维护者速读 block is the seat's required Chinese summary).
  • The Before Submitting list in CONTRIBUTING.md is done.

Generated by Claude Code

…driver's error, and a failed connect holds no claim on its variable

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5m1ywz2SChxvNGPb5U
@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 list — not 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
  • the SDK route bridge reached 59 of 211 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 3 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 b33946512c9f968432cb8b094961424220da4bc6 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 79ebeac547ffc2b13a203b46f4baa9cbd16b687a — the merge of head d3ceca42a2a5e84a77b3329e5a0ee6e800329d4d into base b33946512c9f968432cb8b094961424220da4bc6, 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 79ebeac547ffc2b13a203b46f4baa9cbd16b687a && git checkout 79ebeac547ffc2b13a203b46f4baa9cbd16b687a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b33946512c9f968432cb8b094961424220da4bc6 d3ceca42a2a5e84a77b3329e5a0ee6e800329d4d && git checkout -B drift-repro b33946512c9f968432cb8b094961424220da4bc6 && git merge --no-ff d3ceca42a2a5e84a77b3329e5a0ee6e800329d4d

node scripts/docs-audit/affected-docs.mjs --json b33946512c9f968432cb8b094961424220da4bc6

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo

席位核对

ACCEPT · domain:server seat 2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-10-31T05:05Z · head d3ceca42 · #28283

The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」.

  1. Shape:
    • A draft against main with no assignees and a patch changeset for @objectstack/service-datasource.
    • It says Fixes #28283.
    • check-governed-merges --pr 28325 reads NOT governed, 83 changed lines.
    • The claim (6538910253) names this PR's branch, claude/project-thread-t43yct.
  2. Against the card's Wanted:
    • S1. The unset-variable sentence is now a trailing "(note: …)" after the driver's own error, so the driver's error stays the explanation. The premise needed one correction: the reason already led with the driver's error on base, so the defect was the em-dash framing that read as its cause. The new pin reds on base on exactly that. The login-refusal pin ("the database refusing the login answers the variable to set") passes unchanged.
    • S2. The owner is still claimed at the check, before the build, so the check and the claim stay one step with no await between them. claimedVariable records that this attempt made the claim, and the failed-degraded catch releases it. A later name deriving the same variable now connects. The control, two live datasources refused as a collision, passes unchanged.
    • Red first, then ablation. Base: 2 failed, 24 passed. Head: 26 of 26 passed. Each ablation reds only its own pin, and every restore matches HEAD.
  3. Contract review: not owed. There is no spec path and no governed path, and the PR declares Clause-②: no. The change is operator-facing reason text and an internal map; no code, status or export moves, and git grep finds no reader of the sentence outside the package. It narrows nothing.
  4. Gates:
    • Locally, the dev ran 60 derived families: 59 exit 0, and 1 NOT-MEASURED, check:dual-build-cjs-loads (it needs a full build; CI ran the build). The package suite passed 1100 of 1100, and typecheck and lint pass.
    • CI on head d3ceca42: 35 success, 13 skipped, 0 red. The state is clean.
  5. Overlap: no other open PR touches packages/services/service-datasource/**. git merge-tree reports no conflict against main b1e1dcf2.
  6. Security discipline: no credential value, env value or connection string appears in the tests, the PR or the changeset. Variable names only.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 31, 2026 05:06
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 31, 2026 05:06
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 31, 2026
Merged via the queue into main with commit a0f8654 Oct 31, 2026
49 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-t43yct branch October 31, 2026 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/s tests tooling

Projects

None yet

1 participant