Skip to content

fix(cli): os start / os dev stop injecting OS_ARTIFACT_PATH into the child serve environment - #9365

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-8985-artifact-path-child-env
Aug 17, 2026
Merged

fix(cli): os start / os dev stop injecting OS_ARTIFACT_PATH into the child serve environment#9365
os-zhuang merged 2 commits into
mainfrom
claude/issue-8985-artifact-path-child-env

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Fixes#8985

os start and os dev are supervisors: each resolves an artifact, then spawns os serve to boot it. Both handed the resolved path down by writing OS_ARTIFACT_PATH into the child environment — the same variable an operator sets to name an artifact — and both writes land before the downstream objectstack.config.ts is evaluated. So inside any config that variable was set on every boot, including boots where no operator ever mentioned it, and a config could not answer "did a human ask for this, or did the CLI put it here?"

The CLI's own plumbing now travels on OS_INTERNAL_ARTIFACT_PATH, and the property the downstream retirement needs is restored:

the presence of OS_ARTIFACT_PATH in a config's environment means an operator set it.

⚠️ Declared file-surface breach — please rule before this lands

The dispatch scoped this card to start.ts + dev.ts + sibling tests, and told me to stop on breach and explain. I could not deliver the ruling inside that surface, and this PR touches two more files. The measurement that forces it:

  • os serve has no --artifact flag. The --artifact in serve.ts's own precedence comment is start/dev's flag, upstream — so "pass it as an argument instead of the environment" (one of the suggested routes) is not reachable without adding a flag to serve.
  • The only reader of the resolved path is resolveDefaultArtifactPath() in packages/runtime/src/default-host.ts, called from serve.ts in three places.

So every route — internal env var, CLI flag, structural pass-through — needs a change on the child side. The suggested route ("a distinct env var the CLI owns") is not implementable within start.ts + dev.ts alone. I kept the breach minimal and chose the smaller of the two possible hosts:

RouteFiles touchedWhy not
Teach resolveDefaultArtifactPath (packages/runtime)runtime + cliWidens a published API with CLI-private plumbing semantics; embedders would inherit a variable that means nothing to them
Teach serve.ts (chosen)cli onlyThe channel is a CLI parent-to-child contract, so both ends stay inside the package that owns them

Two further notes for the ruling: the PM's claim comment records packages/cli/src/commands/serve.ts as freed at ~16:18Z from the three-way exclusive slot (#8368 / #8985 / #8978) that this card was held behind — i.e. this card's own serialization slot already contemplated serve.ts. And the new packages/cli/src/utils/internal-artifact-channel.ts exists so the channel has exactly one definition rather than three literals drifting apart.

If the surface ruling goes the other way, the alternative is to leave start's home/dist/objectstack.json fallback and both commands' --artifact flag unable to reach the child — a behaviour regression, not a smaller fix.

Precedence is unchanged — this is a rename of the channel, not a change of answers

serve reads the new channel strictly between the reference and the operator knob:

--artifact > OS_ARTIFACT_URL > OS_INTERNAL_ARTIFACT_PATH > OS_ARTIFACT_PATH > cwd/dist/objectstack.json

That exact position is what preserves today's answers, in both directions:

  • It must beat OS_ARTIFACT_PATH.os start --artifact X run with an operator's OS_ARTIFACT_PATH=Y exported boots X today, because the parent overwrote the variable on the way down. The parent now inherits Y untouched, so only a higher rung keeps X winning.
  • It must lose to OS_ARTIFACT_URL.os dev writes the channel unconditionally, exactly as unconditionally as it wrote OS_ARTIFACT_PATH, and the reference has always outranked the path in serve.

Each command's own resolution ladder is untouched, and so is the value: the child receives the path the parent resolved, still "named" in the sense resolveDefaultArtifactPath means it, so a resolved-but-missing artifact stays a loud refusal instead of degrading to an empty boot.

The two load-bearing behaviours, checked

  • start's no-OS_BOOT_EMPTY-on-reference refusal survives. The guard at the old start.ts:207 (!process.env.OS_ARTIFACT_PATH) reads the parent environment, and this command never mutates process.env — it composes a separate child env — so that expression sees exactly what it saw before. Worth noting for the record that this guard suppresses auto-compile; the reference refusal itself is the artifactUrl ? {} : ... ladder in the child env block, and both are pinned now.
  • OS_BOOT_EMPTY stays add-only. An operator-exported value is still inherited rather than cleared — the helper only ever adds it, so no behaviour changes there either.

start.ts:194's existing comment already said the child env must carry nothing that contradicts an OS_ARTIFACT_URL boot. That partial intent is extended, not contradicted: it is now general rather than special-cased to the reference branch.

Tests

packages/cli/src/commands/artifact-child-env.pin.test.ts (19 tests) pins the property from both sides, per the card's "pin the property, not just the plumbing":

  • the child env carries noOS_ARTIFACT_PATH when the operator set none — for all three decisions (resolved / reference / empty);
  • it still carries it, verbatim, when the operator did set one (the child now sees the operator's own spelling rather than an absolutised rewrite of it);
  • the full resolution ladder, rung by rung, including that an operator's $OS_ARTIFACT_PATH still beats both auto-detected locations;
  • the OS_BOOT_EMPTY refusal, in all four combinations;
  • a structural pin that neither command writes OS_ARTIFACT_PATH anywhere — the pin that stops a future edit from simply re-adding the injection beside the helper.

Reverse verification found a defect in the pin itself, which is why the second commit exists. With the old injection restored in both commands, only dev.ts went red. start.ts's --auth-secret flag description contains the literal /api/v1/auth/*, and that /* opened a phantom block comment which the regex comment-stripper closed against a docblock 250 lines later — swallowing the reinstated write. The pin now walks the TypeScript AST for real writes, and carries a self-test over the exact specimen that defeated the text scan. Re-verified after the fix: both structural pins red with the injection restored, all 19 green with it removed.

Verification

All of the below on 8705d5700, the final commit:

  • pnpm --filter @objectstack/cli test127 files / 1394 tests passed
  • pnpm --filter @objectstack/cli typecheck (tsc --noEmit) — clean
  • ESLint job family: eslint on all five changed files clean, plus check:cross-package-test-inputs, check:where-matcher, check:engine-double-contract, check:query-options-erasure, check:type-check-coverage, check:nul-bytes — all pass
  • changeset family: check:changeset-gate-self-tests, check:objectui-changeset, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset — all pass
  • scripts/docs-audit/check-affected-docs.mjs — pass

node scripts/pm/dispatch-gates.mjs re-derived from the actual changed paths named no family beyond the set above.

check:type-check-debt --re-measure was not run: packages/cli carries no ledger entry and its tsconfig.json includes all of src with no test exclusion, so the new test file is already compiled by the typecheck above and cannot move that ratchet.

Downstream

This is what makes cloud#1292's tightening from a value-based to a presence-based refusal possible. Not touched here — the consumer-side change is filed there, and cloud#1292 stays open on the cloud side.


Generated by Claude Code

…an internal channel (#8985)
`os start` and `os dev` wrote their own resolved artifact path into the child
`serve` environment as `OS_ARTIFACT_PATH` — the same variable an operator sets.
The write happens before the downstream `objectstack.config.ts` is evaluated, so
a config saw the variable set on every boot and could not tell an operator's
instruction from the CLI's plumbing.
The resolved path now travels on `OS_INTERNAL_ARTIFACT_PATH`, owned by
`packages/cli/src/utils/internal-artifact-channel.ts`. `serve` reads it strictly
between `OS_ARTIFACT_URL` and `OS_ARTIFACT_PATH`, which preserves every answer
the resolution ladder gives today: it must beat the operator knob so
`--artifact X` still wins over an exported `OS_ARTIFACT_PATH=Y`, and it must
lose to the reference because `dev` sends its answer unconditionally.
Unchanged and now pinned: the ladder itself, `start`'s refusal to set
`OS_BOOT_EMPTY` on a reference boot, and the named-vs-conventional distinction
that keeps a missing resolved artifact a loud refusal.
Co-Authored-By: Claude <noreply@anthropic.com>
…t scan (#8985)
Reverse verification caught the pin under-reporting: with the old injection
restored in both commands, only `dev.ts` went red. `start.ts`'s `--auth-secret`
flag description contains the literal `/api/v1/auth/*`, and that `/*` opened a
phantom block comment which the regex comment-stripper closed against a docblock
250 lines later — swallowing the reinstated write along with it.
The pin now parses the file and walks for real `OS_ARTIFACT_PATH` writes
(object property, property assignment, indexed assignment), so strings and
comments cannot lie to it, and carries a self-test over the exact specimen that
defeated the text scan.
Co-Authored-By: Claude <noreply@anthropic.com>
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 17, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/cli, touching 9 documentable anchor(s).

28 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive: node scripts/docs-audit/affected-docs.mjs --json origin/main.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 3 name(s) were too generic to anchor anything (single lowercase words)

Coarse fallback — 22 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 origin/mainpackageMentionDocs.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32049971906 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (2/3) — 失败步骤: Run this shard's tests

    �[90mstderr�[2m | src/sql-driver-unique-tenancy.test.ts�[2m > �[22m�[2mSqlDriver unique × tenancy (#3696)�[2m > �[22m�[2mretires a legacy global unique index and replaces it with the composite
    �[90mstderr�[2m | src/sql-driver-unique-tenancy.test.ts�[2m > �[22m�[2mSqlDriver unique × tenancy (#3696)�[2m > �[22m�[2mretires the legacy `uniq_<table>_<col>` index left by the drift rebuild path
    �[90mstderr�[2m | src/sql-driver-unique-tenancy.test.ts�[2m > �[22m�[2mSqlDriver unique × tenancy (#3696)�[2m > �[22m�[2mbare-composite tightening + duplicate pre-flight (ADR-0120 D4)�[2m > �[22m�[2ma
    �[90mstderr�[2m | src/sql-driver-unique-tenancy.test.ts�[2m > �[22m�[2mSqlDriver unique × tenancy (#3696)�[2m > �[22m�[2mbare-composite tightening + duplicate pre-flight (ADR-0120 D4)�[2m > �[22m�[2mB
    �[22m�[39m[schema-drift] product: cannot tighten 'uniq_product_organization_id_code' as UNIQUE (COALESCE(organization_id, '__global__'), code) — existing rows already violate the NULL-safe unique cons
    �[90mstderr�[2m | src/sql-driver-unique-tenancy.test.ts�[2m > �[22m�[2mSqlDriver unique × tenancy (#3696)�[2m > �[22m�[2mbare-composite tightening + duplicate pre-flight (ADR-0120 D4)�[2m > �[22m�[2mB
    �[22m�[39m[schema-drift] REFUSING to rebuild 'uniq_product_organization_id_code' on 'product' as a NULL-safe unique — 1 duplicate group(s) violate it (e.g. organization_id="__global__", code="DUP" × 2
    �[90mstdout�[2m | src/engine-write-formula-hydration.test.ts�[2m > �[22m�[2m#5699 — one `now` per `applyFormulaPlan` call�[2m > �[22m�[2ma batch insert hydrates every row × every formula field from ON
    �[90mstdout�[2m | src/engine-write-formula-hydration.test.ts�[2m > �[22m�[2m#5699 — one `now` per `applyFormulaPlan` call�[2m > �[22m�[2ma find hydrates every row × every formula field from the SAME o
    

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 4 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@claude

claudeBot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Queue-eviction triage — unrelated harness flake, re-queuing once

PM seat, session session_012WKSnqAaoqtW3QX7SSf1Vk. Signature read before acting.

This is NOT the infrastructure signature from earlier today

Queue build 32049971906, Test Core (2/3), failing step Run this shard's tests — the test bodies ran, unlike the Set up job / Failed to download action evictions of #9341 and #9347. Different signature, different handling.

Every test passed

Test Files 21 passed (21)
Tests 334 passed (334)
Errors 1 error

The run was failed by a single teardown-time rejection in examples/app-showcase:

EnvironmentTeardownError: [vitest-worker]: Closing rpc while "onUserConsoleLog" was pending

A worker's RPC channel closing while a pending console-log delivery is in flight — timing- and log-volume-dependent by construction, which fits its appearing in the full-suite queue build rather than the PR-side affected-subset run.

Running the workflow's own triage checklist

  1. Failing test in a package this PR changed? No. This PR touches packages/cli only; examples/app-showcase is untouched by it, and no assertion failed anywhere.
  2. Unrelated failing test? Yes — and per the checklist I searched before concluding: no open or closed issue carries this signature (scan over label:tests, control-verified non-empty), and among recent merge-queue triage comments this is its only occurrence; the other two in the last 24h are both the codeload Set up job signature. So it is a first sighting, not an established flaky.
  3. Batch-sibling semantic conflict? No — the failure is a harness teardown race, not a behavioural disagreement.

Filed as #9371 (finding, unassigned, unrouted — grading and domain:* are the triage seat's), recorded explicitly as a proposal for the flaky-signature ledger rather than an entry in it: one occurrence is not a pattern, and ⛔ this seat does not add ledger rows on its own.

Action: re-queue once

Justified because zero tests failed, the failure is in a package this diff does not touch, and it is a first sighting rather than a known-recurring signature. Re-entry goes to the back of the queue, behind #9368 and #9369, so it does not force a rebuild of either.

If this same signature evicts this PR again, it stops being a re-queue case: that second data point confirms the flaky, and the correct move then is to fix or isolate the race under #9371 rather than burn another full-queue rebuild. ⛔ No change is being pushed here — all five required checks are green on 8705d5700, and the eviction carries no information about the diff.


Generated by Claude Code

@os-zhuang
os-zhuang added this pull request to the merge queueAug 17, 2026
Merged via the queue into main with commit 14c9ad7Aug 17, 2026
26 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8985-artifact-path-child-env branch August 17, 2026 18:00
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-zhuang@claude