Stop publishing THEOREM_GIT_TOKEN in build logs; raise the gulp heap - #181
Stop publishing THEOREM_GIT_TOKEN in build logs; raise the gulp heap#181Travis-Gilbert wants to merge 4 commits into
Conversation
CS-003, reopened. Deploy 4e33d620 built green off 01143ad and /opt/commonplace/studio-server was absent from the running container. Two independent defects, either one fatal alone. No COPY --from=studio-server existed anywhere. The stage wrote /out and the final image never took it, and BuildKit prunes a stage nothing copies from, so the fork was not merely uncopied, it was never compiled. That is why a build carrying the mangler patch finished without ever reaching the mangler. The gate shared a name with the runtime switch. Railway injects every service variable into whatever ARG a stage declares, so IDE_HOST set on the service to keep the runtime host on code-server during cutover would have switched the build off too. The safe-cutover shape this plan rests on, ship both binaries and flip a variable, was unreachable: pinning the variable deleted the binary from the next image. The stage is now gated on BUILD_STUDIO_SERVER, IDE_HOST is runtime only and the dead ARG in the base stage is gone, and the COPY exists. At BUILD_STUDIO_SERVER=0 the stage still writes /out so the COPY stays valid; what it omits is the launcher, which is the file entrypoint.sh probes with -x, so a skipped build falls back to code-server by itself. Proof is the container, not the Dockerfile. The previous done was read off the source, which is how this survived a green build.
The artifact is linux and a mac cannot build it, so the only machine that ever holds one is the container. The way in is railway ssh 'STUDIO_SERVER_DIR=... bash -s' < scripts/smoke-server.sh and piped that way there is no BASH_SOURCE, so under set -u the root derivation aborted at line 12 before a single check ran. A harness that only works when it lives on disk cannot smoke the one place that matters. Fall back to the cwd when BASH_SOURCE is empty. Spelled as an explicit if rather than A && B || C, which is the same SC2015 shape the expect helpers in this file already exist to avoid; shellcheck stays clean. Verified against the live container both ways: it now runs to a real assertion and reports the artifact missing, which is true of the image running today and is the thing the next deploy is meant to change.
Deploy d301c820 was the first time the studio-server stage ever ran, and build.sh server refused in 0.3s on a builder with hundreds of GiB free. `df -g` and `du -sg` are BSD flags. GNU coreutils rejects both, so on Debian df wrote nothing, the empty string read as 0 in the arithmetic, and every floor refused. Written on the mac, never run on linux, which is the same shape as the stage that was never copied. `df -Pk` and `du -sk` are POSIX and identical on both platforms; -P also keeps a long device name from wrapping and shifting the column. The free probe now fails loudly when it cannot read a volume instead of reporting 0, because a probe that cannot answer is not an answer of "full", and conflating the two is what produced a refusal nobody believed. The node_modules credit still degrades quietly, since failing to credit an existing tree only makes the floor stricter. Swept the other studio scripts for the same authoring bias: clean. Both forms verified on macOS so the local path is unchanged.
Deploy 6ddb18cd was SKIPPED for bdd2767, the commit that fixed the Studio build's disk guards. Nothing was wrong with it. The commit touched packaging/commonplace-studio, which was not in watchPatterns, so Railway decided the image could not have changed. It very much could. Dockerfile line 155 copies that entire directory into the studio-server stage: the patch queue, the product overlay and build.sh itself are the IDE door. A SKIPPED deploy reads like a successful no-op, which is how a real fix looked shipped while the running image still had the bug. Audited every COPY from the build context against the list rather than adding back only the one that bit. Three more were missing: LICENSE, which the studio stage copies beside the fork tree; apps/console/src/ styles, which the chat door's web stage copies; and the root package.json, which all three pnpm stages copy while only the lockfile and workspace file were watched. Also corrects the header. `railway up --config` is written in older notes and no longer parses, and uploading without it silently deploys the legacy web service from the root railway.toml. This service builds from its GitHub source on push, so the trigger is a merge.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Warning Review limit reached
Next review available in: 24 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0579fd7a2d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # nothing to select, which is how an image that was supposed to ship both hosts | ||
| # quietly shipped one. The stage asserts its own launcher with `test -x`, so a | ||
| # broken Studio build fails the image here rather than at container start. | ||
| COPY --from=studio-server /out/ /opt/commonplace/studio-server/ |
There was a problem hiding this comment.
With the default BUILD_STUDIO_SERVER=1, this new COPY makes BuildKit execute the Studio stage, but that stage still runs build.sh server without the promised NODE_OPTIONS/STUDIO_NODE_HEAP_MB override. In the inspected Railway build, the newly reachable gulp mangle step exits 134 after exhausting V8's default heap, so every default deployment now fails before producing an image; set the larger heap on the Studio build invocation before forcing this stage into the dependency graph.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Addresses two deploy findings for the Railway “workspace” service: (1) preventing secret leakage during Docker builds, and (2) improving reliability of the Commonplace Studio build pipeline (including the Studio server build gate and operational runbooks).
Changes:
- Updates the workspace image build to make the Studio server stage always participate in the final image via
COPY --from=studio-server, and separates the build gate from the runtime host selector. - Improves Studio build scripts/runbooks for Linux-builder reality (stdin execution, POSIX
df/duflags, clearer operational guidance). - Expands Railway watch patterns to avoid “SKIPPED deploy” false positives when build inputs change.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packaging/workspace/railway.toml | Updates deployment notes and expands watchPatterns to include Studio pipeline inputs and other copied assets. |
| packaging/workspace/Dockerfile | Refactors Studio server build gating and ensures the studio-server stage is included in the final image. |
| packaging/commonplace-studio/scripts/smoke-server.sh | Makes script robust when executed via bash -s over railway ssh (no BASH_SOURCE). |
| packaging/commonplace-studio/scripts/build.sh | Fixes disk-space probing for GNU/Linux builders (df -Pk, du -sk) and makes failures clearer. |
| packaging/commonplace-studio/RUNBOOK.md | Adds an explicit Railway SSH smoke procedure for validating the Linux artifact in-container. |
| docs/plans/ide-substrate-wire/FOLLOW-UP-CODE-SERVE-WEB.md | Updates the follow-up plan/status to reflect the corrected build gate and required image COPY. |
| docs/plans/console/SPEC-COMMONPLACE-OPENWORK-FORK-1.0.md | Aligns the spec with the corrected build-time gate and the required COPY --from=studio-server. |
| .commonplace-canonical | Updates canonical notes to reflect the build gate rename to BUILD_STUDIO_SERVER. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Gated by BUILD_STUDIO_SERVER, deliberately not by IDE_HOST. Railway injects | ||
| # every service variable into whatever ARG a stage declares, so while this stage | ||
| # read `ARG IDE_HOST`, setting IDE_HOST=code-server on the service to choose the | ||
| # runtime host also switched the build off, and the next image shipped no Studio | ||
| # to cut over to. One name cannot be both the runtime switch and the build |
| FROM node:24-bookworm AS studio-server | ||
| ARG IDE_HOST=studio | ||
| RUN if [ "${IDE_HOST}" != "studio" ]; then \ | ||
| ARG BUILD_STUDIO_SERVER=1 |
| echo "studio-server: BUILD_STUDIO_SERVER=${BUILD_STUDIO_SERVER}, skipping the Studio build"; \ | ||
| else \ | ||
| cd /studio/packaging/commonplace-studio \ | ||
| && STUDIO_BUILD_DIR=/studio/build \ |
Two findings from deploy
39ea1ed3, the first build that ever reached the Studio compile.THEOREM_GIT_TOKEN was printed in the build log
BuildKit prints every
RUNinstruction with its build args already substituted, so this linewrote the live PAT verbatim into Railway's persisted log store on every build. The stage carried the comment
# ... Never echo it.directly above the line that echoed it: a correct intention with a mechanism that defeats it.Railway documents no secret mount for Dockerfile builds, so the fix is to keep the value out of the instruction text:
$so BuildKit prints the name and the shell expands the value from the arg's own environment at run time..git/config. This is the same shapeentrypoint.shalready uses.GIT_TERMINAL_PROMPT=0so a helper returning nothing fails immediately instead of hanging on a prompt nobody can answer.Also:
THEOREM_GIT_URLwas declared, documented as the clone source, and then ignored in favour of a hardcoded URL on the next line. It is now used.The token is already burned and needs rotating regardless of this fix, because it is in the logs of every build before this commit.
The Studio compile OOMed
exit 134, seven minutes in, at the mangle step. That is V8 refusing at its own configured ceiling, which is a different failure from the kernel reclaiming the process, and only the first is fixed by raising the limit. (The mac produced the second, which is why it is not the machine for this.)
Ceiling is now 12288MiB via
NODE_OPTIONS, overridable throughSTUDIO_NODE_HEAP_MBbecause the right number is a property of the builder, not of this repository. Set it below the builder's memory: a ceiling above what the host can back converts this readable abort into a kernel kill that loses the stack.Validation
shellcheckclean onbuild.sh(exit 0).${THEOREM_GIT_TOKEN}occurrences remain in the Dockerfile.