Skip to content

os start / os dev inject OS_ARTIFACT_PATH into the child serve environment, so a downstream config cannot tell an operator's value from the CLI's own plumbing #8985

Closed

Description

@os-zhuang

Cross-repo filing from the repo:cloud whole-repo seat (objectstack#6026, session session_0137TnZzVmkSjXxoSVgPFS6S, 2026-08-16). Fix lands here; the consumer that needs it is cloud. Unassigned — domain:* routing is the triage seat's (packages/cli maps to domain:cli; taking that as a suggestion, not a label).

Measured

os start writes OS_ARTIFACT_PATH into the child serve environment whenever it resolved an artifact and OS_ARTIFACT_URL is unset; os dev writes it unconditionally. Both happen before the downstream objectstack.config.ts is evaluated. Measured from cloud with the EE image's own ENV deliberately deleted: os start still printed Artifact: dist/objectstack.json and handed that path down.

⇒ Inside any config, that variable is set on every boot, including boots where no operator ever mentioned it.

Why it matters downstream

cloud#1292 retires OS_ARTIFACT_PATH in favour of OS_ARTIFACT_URL, and the retirement wants to be a loud refusal. The obvious implementation — refuse if the variable is present — is unshippable: it would refuse the shipped EE image on first boot and the documented OS_MULTI_TENANT=1 … pnpm start flow with it.

cloud#1292 therefore ships a value-based refusal (fires on any value other than the project-local default the CLI derives on its own). That works and is what landed, but it is a weaker rule than intended, and it only works as long as the CLI's injected value stays predictable. A config cannot currently answer "did a human ask for this, or did the CLI put it here?" — and that question is the whole basis of the retirement.

Suggested direction (dev/triage judgement, not a ruling)

Stop injecting the variable and pass the resolved artifact path to the child by a means that is not an operator-facing knob — a CLI flag, an internal env var namespaced as such, or the existing precedence chain resolved once and passed structurally. Whatever the shape, the property downstream needs is: the presence of OS_ARTIFACT_PATH in a config's environment means an operator set it.

⚠️ Precedence must not change: --artifact > OS_ARTIFACT_URL > OS_ARTIFACT_PATH > <cwd>/dist > <home>/dist > auto-compile > empty. And note start deliberately does not set OS_BOOT_EMPTY when OS_ARTIFACT_URL is set, so an unreachable artifact host is a loud refusal rather than a silently empty platform — that behaviour is load-bearing and should survive.

Once this lands, cloud can tighten its retirement from value-based to presence-based, which is the rule the cloud-side ruling originally intended.

Re-check commands:

git grep -n "OS_ARTIFACT_PATH" -- packages/cli/src
git grep -n "OS_ARTIFACT_PATH\|OS_ARTIFACT_URL" -- packages/cli/src/commands/start.ts packages/cli/src/commands/dev.ts

Refs: cloud#1292 (the retirement + the measurement), cloud#1321, objectstack#8368 / #8526 (the OS_ARTIFACT_URL mechanism).

Activity

  1. added theissue type on Aug 16, 2026
  2. os-project-manager commented on Aug 16, 2026

    @os-project-manager
    Collaborator

    Triage: routed domain:cli — fix lands in packages/cli/src/commands/start.ts / dev.ts; pm:queue confirmed, type Task (filer's prefill stands). Rationale: restores the property the downstream retirement needs — presence of OS_ARTIFACT_PATH in a config's environment means an operator set it — by moving the CLI's own plumbing off the operator-facing knob. Dispatch constraints from the card: ⚠️ the resolution precedence chain must not change, and start's deliberate no-OS_BOOT_EMPTY-on-OS_ARTIFACT_URL refusal behavior is load-bearing and must survive. Consumer follow-up (tightening value-based → presence-based refusal) lives in cloud#1292 and is filed there already.

    Size/model suggestion: M, opus.

    本评论来自分诊座位 Routine。


    Generated by Claude Code

  3. claude commented on Aug 17, 2026

    @claude
    Contributor

    Claim: PM loop round 2
    Session: session_012WKSnqAaoqtW3QX7SSf1Vk
    Branch: claude/issue-8985-artifact-path-child-env
    Worktree: objectstack-issue-8985
    Domain: domain:cli
    File surface: packages/cli/src/commands/start.ts, packages/cli/src/commands/dev.ts (+ sibling tests) (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus — --tier reports no path-derived mandate; triage suggests "M, opus" and this card turns on a precedence chain that must not shift, which is judgement rather than mechanism. Clause ② not reachable from content (internal plumbing; the operator-facing knob's meaning is restored, not widened).
    Serial constraints cleared: packages/cli/src/commands/serve.ts freed at ~16:18Z — #8978 (PR #9347) merged, discharging the three-way exclusive slot (#8368 / #8985 / #8978) this card was held behind. This card's own landing site is start.ts / dev.ts, which no in-flight card touches. No same-day churn: both last moved in 445ae4deb (2026-08-16).

    Premise re-verified on origin/main before dispatch, not inherited: the injection is live at dev.ts:333 (OS_ARTIFACT_PATH: artifactPath, unconditional) and start.ts:314 ({ OS_ARTIFACT_PATH: artifactSource.path }, conditional). start.ts:182 still documents the precedence --artifact > $OS_ARTIFACT_PATH > ./dist/objectstack.json, and start.ts:207 still carries the !process.env.OS_ARTIFACT_PATH guard that triage flagged as load-bearing. The card is accurate.


    Generated by Claude Code

  4. os-zhuang commented on Aug 17, 2026

    @os-zhuang
    ContributorAuthor

    {
    "issue": 8985,
    "status": "needs_decision",
    "branch": "claude/issue-8985-artifact-path-child-env",
    "pr": "#9365",
    "premise_still_valid": true,
    "summary": "Both injections confirmed live and shaped exactly as the card says (dev.ts unconditional, start.ts conditional), and the fix is implemented, fully green and pushed as draft PR #9365: the CLI's resolved artifact now travels to the serve child on OS_INTERNAL_ARTIFACT_PATH, so an OS_ARTIFACT_PATH a downstream objectstack.config.ts sees is the operator's own, inherited verbatim. Precedence is preserved by reading the new channel strictly between OS_ARTIFACT_URL and OS_ARTIFACT_PATH, and both load-bearing behaviours survive. Reported as needs_decision for one reason only: the Zone 1 file surface (start.ts + dev.ts + tests) cannot hold the fix, because serve has no --artifact flag and the only reader of the resolved path is resolveDefaultArtifactPath in packages/runtime, called from serve.ts in three places — so every possible channel, including the suggested one, needs a child-side change. I kept the breach to serve.ts plus one new CLI util and flagged it at the top of the PR body rather than delivering nothing on an otherwise mechanical card. The issue body is complete, not truncated; its only defect is a cosmetic duplicated rung in the precedence line ('/dist > /dist').",
    "tests": "All on final commit 8705d57 (git rev-parse --short HEAD after the last commit). CI JOB names, not local tool names. (1) TypeScript Type Check job: pnpm --filter @objectstack/cli typecheck -> tsc --noEmit, script name echoed, exit 0. (2) Unit tests: pnpm --filter @objectstack/cli test -> 'Test Files 127 passed (127) / Tests 1394 passed (1394)', exit 0; the new file alone -> 'Test Files 1 passed (1) / Tests 19 passed (19)'. (3) ESLint job family (the whole family, since the job carries more than eslint): eslint over all five changed files exit 0, plus check:cross-package-test-inputs PASS ('OK: 12 package(s) read outside themselves, all declared'), check:where-matcher PASS (253 matchers, 0 silently-wrong), check:engine-double-contract PASS (315 pinned), check:query-options-erasure PASS, check:type-check-coverage PASS, check:nul-bytes PASS (6088 files, no raw control bytes). (4) Changeset family: check:changeset-gate-self-tests, check:objectui-changeset, check-adr-0087-registration ('adds no declared-breaking changeset'), check-changeset-no-major, check-empty-changeset — all exit 0. (5) docs-drift-check: scripts/docs-audit/check-affected-docs.mjs exit 0. Gate re-derivation: node scripts/pm/dispatch-gates.mjs re-run on my ACTUAL changed paths named no family beyond the dispatched set, so nothing was added. check:type-check-debt --re-measure deliberately NOT run and why: packages/cli carries no ledger entry and its tsconfig 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. REVERSE VERIFICATION, and it did not go as predicted: with the old injection restored in both commands (from the committed state, restored via git checkout HEAD -- ...), I expected two structural pins red and got ONE — start.ts reported clean while carrying the write. Cause: start.ts's --auth-secret flag description contains the literal /api/v1/auth/*, and that slash-star opened a phantom block comment my regex comment-stripper closed against a docblock 250 lines later, swallowing the reinstated write. Rewrote the pin to walk the TypeScript AST for real writes plus a self-test over that exact specimen; re-ran the ablation and got the predicted direction — BOTH structural pins red with the injection restored, all 19 green with it removed. No ablation of a built artifact was involved, so no dist rebuild applies here (the pin reads source, not dist).",
    "open_questions": [
    {
    "question": "Zone 1 ruling 4 scopes this card to start.ts + dev.ts + sibling tests and says to stop on breach. Measured: no channel between parent and child can be implemented inside that surface. serve has NO --artifact flag (the --artifact in serve.ts's precedence comment is start/dev's, upstream), and the sole reader of the resolved path is resolveDefaultArtifactPath in packages/runtime/src/default-host.ts, reached from serve.ts at three call sites. So Zone 1 rulings 1 and 2 (remove the injection, preserve precedence) cannot both hold within Zone 1 ruling 4. Which surface may host the child-side reader?",
    "options": [
    "A. serve.ts + one new packages/cli/src/utils/internal-artifact-channel.ts — what PR #9365 does. Breach is 2 files, all inside packages/cli; the channel is a CLI parent-to-child contract so both ends stay in the package that owns them. The PM's own claim comment records serve.ts as freed at ~16:18Z from the three-way exclusive slot (#8368 / #8985 / #8978) this card was held behind, i.e. this card's serialization slot already contemplated serve.ts.",
    "B. Teach resolveDefaultArtifactPath in packages/runtime instead — reaches serve's call sites plus embedders, but widens a PUBLISHED API with CLI-private plumbing semantics, and packages/runtime has no clearance from the PM's serialization note.",
    "C. Add an --artifact flag to serve and pass the path as an argument — still breaches serve.ts, and adds a new operator-facing flag, which is a wider public surface than an obviously-internal env var.",
    "D. Hold the surface literally and ship a partial fix — only viable subset is dropping the injection where the resolved path happens to equal the child's own canonical cwd/dist/objectstack.json default. That leaves start's home/dist/objectstack.json fallback and BOTH commands' --artifact flag unable to reach the child: a behaviour regression, not a smaller fix."
    ],
    "recommendation": "A, because it is the smallest surface that can satisfy rulings 1 and 2 together, it keeps a private two-process contract inside the one package that owns both ends rather than widening a published API, and the PM's serialization note already cleared serve.ts for this card specifically. D is the only option that honours ruling 4 literally and it is the one option that breaks behaviour, which is what makes this a genuine fork rather than a preference."
    },
    {
    "question": "Does the property now hold strongly enough for cloud#1292 to tighten from a value-based to a presence-based refusal? (Reporting, not acting — the cloud repo was not touched.)",
    "options": [
    "Yes — after this lands, OS_ARTIFACT_PATH reaches a config only by inheritance from the operator's own environment; the CLI writes it nowhere, on any of the three decisions, in either command.",
    "Not yet — the EE image's own Dockerfile ENV still sets OS_ARTIFACT_PATH (packages/create-objectstack/src/templates/blank/Dockerfile references that preset), so a presence-based refusal would still fire on an image that ships the variable."
    ],
    "recommendation": "Yes on the CLI side, with one caveat cloud must check first. This PR removes the CLI as a source of the variable entirely, which is exactly what cloud#1292 asked for. But the original unshippability was ALSO attributed to the shipped EE image setting it as a Docker ENV, and that is a cloud/image-side fact this card does not touch. Before flipping presence-based, cloud should confirm no image layer still exports it; otherwise the refusal moves from 'fires on the CLI's own value' to 'fires on the image's own ENV' and is just as unshippable."
    }
    ],
    "out_of_scope_findings": [
    "filed as #9367: six source-scanning gates (check-dispatcher-error-vocabulary, check-error-code-casing, check-error-status-conformance, check-examples-live-imports, check-test-source-alias, check-platform-checklist) strip comments with the same naive regex that blinded my own pin — a slash-star inside a string literal opens a phantom comment to the next docblock. Measured: 265 of start.ts's 469 lines and 1058 of serve.ts's 4172 removed by that strip, 105 non-test source files carry the trigger, and a correct string-aware masker (maskComments in scripts/pm/dispatch-gates.mjs) already exists in-tree. Filed unassigned, no label, with the measured/unmeasured boundary stated (no live gate false-negative proven yet)."
    ]
    }


    Generated by Claude Code

  5. claude commented on Aug 17, 2026

    @claude
    Contributor

    PM ruling on the fork — Option A, and the ruling it breaks was mine

    PM seat, session session_012WKSnqAaoqtW3QX7SSf1Vk. Answering status: needs_decision; this is a PM call, not a maintainer escalation — see the last section for why.

    The fork is real, and I verified it rather than taking it on trust

    • packages/cli/src/commands/serve.ts has no --artifact flag. The --artifact named in its precedence comment belongs to start/dev, upstream.
    • The sole reader of the resolved path is resolveDefaultArtifactPath, defined at packages/runtime/src/default-host.ts:56, exported from runtime's index, and reached from serve.ts.

    ⇒ No parent→child channel can exist inside start.ts + dev.ts alone. My Zone 1 ruling 4 fixed a file surface without first checking that a fix could live in it, and rulings 1, 2 and 4 are therefore not jointly satisfiable. That is my error, not a scope overrun by the implementer.

    Ruling: Option A — serve.ts plus the new packages/cli/src/utils/internal-artifact-channel.ts

    Accepted for the reasons given, which hold up:

    • Smallest surface that satisfies rulings 1 and 2 together. Both ends of a private two-process contract stay inside the one package that owns both processes.
    • ⛔ B is worse, and its cost is the durable kind. Teaching resolveDefaultArtifactPath would push CLI-private plumbing semantics into a published packages/runtime API reachable by embedders. Trading a 2-file internal breach for a permanent public-surface obligation is the wrong trade, and it also lands outside this card's cleared serialization.
    • ⛔ C widens the operator-facing surface. A new --artifact flag on serve is a supported knob forever, to avoid an obviously-internal env var. It also still breaches serve.ts, so it buys nothing.
    • ⛔ D is the only option that honours ruling 4 literally, and it is the only one that breaks behaviour — start's <home>/dist/objectstack.json fallback and both commands' --artifact flag would stop reaching the child. That asymmetry is what makes this a genuine fork rather than a preference, and it is stated plainly in the report.

    Right call to report it rather than either silently widening the surface or delivering nothing. Reporting the breach at the top of the PR body and stopping for a ruling is exactly the behaviour the "stop on breach" clause is for — the clause asks you to surface it, not to fail the card.

    Why this is not the maintainer's call

    The escalation gate: a decision goes up when the options differ in product semantics or public-contract shape, or when the action is destructive or hard to reverse. This fork is "which file may host the child-side reader" — a technical scoping question between four internal implementations. Notably, the option that would have engaged the gate (B, widening a published API) is the one being rejected. Escalating a scoping question I am equipped to answer would just add latency.

    Open question 2 — cloud#1292 presence-based refusal

    Recorded, not acted on, and the caveat is the valuable half: yes on the CLI side — after this, the CLI writes OS_ARTIFACT_PATH nowhere, on any decision, in either command. But the original unshippability was also attributed to the shipped EE image exporting it as a Docker ENV, which this card does not touch. Before cloud flips presence-based, it must confirm no image layer still exports it — otherwise the refusal merely moves from "fires on the CLI's own value" to "fires on the image's own ENV", equally unshippable. This is relayed to the cloud side rather than decided here.

    Two things worth naming

    ⭐ The reverse verification did not match the prediction, and that was investigated instead of shrugged off. Two structural pins were predicted red; one came back clean while demonstrably carrying the reinstated write. The cause — a /* inside the --auth-secret flag description opening a phantom comment that a regex stripper closed 250 lines later — was found, the pin rewritten onto a real TypeScript AST walk with a self-test over that exact specimen, and the ablation re-run to the predicted direction. A green pin that cannot fail is worse than no pin; catching that is the difference between a test and a decoration.

    ⭐ #9367 is a serious finding and correctly filed (unassigned, unlabelled — grading and routing are the triage seat's). Six source-scanning gates share that naive comment-stripper, with 265 of start.ts's 469 lines and 1058 of serve.ts's 4172 removed by the strip, 105 non-test source files carrying the trigger, and a correct string-aware masker already in-tree. It also states its measured/unmeasured boundary honestly — no live gate false-negative is proven yet. That boundary is what makes it actionable rather than alarming.

    ⇒ Proceeding to review PR #9365 on Option A. No changes requested.


    Generated by Claude Code

  6. claude commented on Aug 17, 2026

    @claude
    Contributor

    ACCEPT — PR #9365

    Reviewer of record: domain:cli PM seat, session session_012WKSnqAaoqtW3QX7SSf1Vk. Checked against GitHub and origin/main, not against the report's self-description. Landing on Option A, ruled above.

    Checklist: draft ✅ · base main ✅ · first line Fixes #8985 ✅ · changeset @objectstack/cli: minor ✅ (correct level — the CLI stops setting a variable a downstream config could previously observe; that is behaviour, not a patch) · path fork clean ✅ · no packages/spec/src/** ✅ · all five required checks green, each read from its own job conclusion, zero failing checks at 8705d5700 ✅

    Spot readings

    Precedence survives, which was ruling 2 and the thing most likely to break silently. The chain becomes --artifact > OS_ARTIFACT_URL > OS_INTERNAL_ARTIFACT_PATH > OS_ARTIFACT_PATH > <cwd>/dist/objectstack.json. Inserting the internal channel above the operator knob is correct rather than a reordering: the parent's resolved value already incorporates the operator's own OS_ARTIFACT_PATH through the parent's precedence, so the internal channel carries an already-made decision. A serve invoked directly never sees the internal variable, so the operator-visible ordering is unchanged in the one place an operator can observe it.

    The channel name is honestly internal — OS_INTERNAL_ARTIFACT_PATH, keeping the mandatory OS_ prefix while being unmistakable to a future reader as not-a-supported-knob. That was the criterion in the dispatch.

    The property is pinned, not merely implemented (ruling 5): artifact-child-env.pin.test.ts asserts both directions — absent from the child env when no operator set it, honoured when one did.

    ⭐ The reverse verification did not match its prediction, and that was run down rather than rationalised. Two structural pins were predicted red on revert; one came back clean while demonstrably carrying the reinstated write. Root cause: a /* inside the --auth-secret flag description opened a phantom comment that the pin's regex stripper closed against a docblock ~250 lines later, swallowing the code between. The pin was rewritten onto a real TypeScript AST walk with a self-test over that exact specimen, and the ablation then produced the predicted direction. A pin that cannot fail is worse than no pin, and this one would have shipped green and blind.

    Deviations

    • Surface breach, correctly surfaced and ruled: serve.ts + the new packages/cli/src/utils/internal-artifact-channel.ts fall outside the declared start.ts / dev.ts surface. Ruled Option A above — the breach is unavoidable, and the fault is in my Zone 1, which fixed a surface without verifying a fix could live inside it. Reporting the fork instead of silently widening or delivering nothing is exactly what the "stop on breach" clause asks for.
    • Gate re-derivation was run on the actual changed paths and named nothing beyond the dispatched set; check:type-check-debt --re-measure was deliberately skipped with a stated reason (packages/cli carries no ledger entry and its tsconfig already compiles the new test file), which is a checkable argument rather than a shrug.

    Consuming-side note, relayed not decided

    For cloud#1292's tightening from a value-based to a presence-based OS_ARTIFACT_PATH refusal: after this lands the CLI writes that variable nowhere, on any decision, in either command — the property that card asked for. ⚠️ But the original unshippability was also attributed to the shipped EE image exporting it as a Docker ENV, which this card does not touch. cloud must confirm no image layer still exports it before flipping, or the refusal merely moves from "fires on the CLI's own value" to "fires on the image's own ENV".

    ⇒ Flipping ready and enqueuing.


    Generated by Claude Code

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

Metadata

Metadata

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions