fix(build-c4): declare /output volume mount so main-push deploy step succeeds #553

Merged

bosun merged 1 commit from i/396-build-c4-output-mount into main 2026-07-27 00:11:32 +02:00

Conversation 1 Commits 1 Files changed 1 +15

quartermaster commented 2026-07-26 23:55:36 +02:00

Owner

Copy link

Closes alcatraz-infra#396 (Path B).

Summary

build-c4.yml's deploy step (rm -rf /output/* && cp -r dist/* /output/) referenced /output as the bind-mount to /srv/docker/likec4-dist/ per the header comment + alcatraz-infra#128 §2 memo, but the workflow never declared container.volumes on the build job — so /output did not exist in the container.

Latent from the original #483 workflow: build failed on graphviz + docs/architecture/c4/ was empty on main, so the deploy step never fired. The post-#550 first main-push exposed both halves at once — the build succeeded via in-workflow graphviz install (1952d7d), and the deploy step then ran for the first time and failed with:

cp: target '/output/' is not a directory

Same "never-ran path activated by new content" shape as the graphviz layer. Sibling: alcatraz-infra#395 (durable graphviz fix in the ci-playwright image itself — infra-territory, tracked separately).

Fix

Path B per alcatraz-infra#396's decision tree: workflow declares the mount.

jobs:
  build:
    runs-on: playwright
    container:
      volumes:
        - '/srv/docker/likec4-dist:/output'
    steps:
      ...

The runner's container.valid_volumes: ['**'] (in /srv/docker/forgejo-runner/config.yml) already authorizes any host path; this workflow requests the specific one it deploys to. No image: under container: — the playwright label already binds the ci-playwright image; container: composes additional per-job options (here: volumes) onto that.

Path A (runner-side mount) was declined per the tracker's framing: build-c4 is toolkit-self infra (workflow is alcatraz-coupled by design — deploys the arch site), so an alcatraz host path in the workflow is acceptable.

Post-fix expected shape

On the next c4-file main-push (workflow trigger: paths: ['docs/architecture/c4/**/*.likec4']):

1. Build job spawns from ci-playwright image with /srv/docker/likec4-dist mounted at /output.
2. Build step (likec4 build) unaffected — doesn't touch /output.
3. Deploy step (rm -rf /output/* && cp -r dist/* /output/) succeeds.
4. arch.saratow.net serves the current LikeC4 model.

Verification

• Local: python3 -c "import yaml; yaml.safe_load(open('.forgejo/workflows/build-c4.yml'))" parses OK; jobs.build.container.volumes = ['/srv/docker/likec4-dist:/output'].
• Empirical: unavailable pre-merge — the workflow triggers only on paths: ['docs/architecture/c4/**/*.likec4'] changes, which this PR doesn't touch. First live validation lands on the next c4-file main-push (either a c4 model update or a bump PR that touches the c4 tree). Deploy-step-only failure surface is bounded (main-push only, no PR risk).

Deploy-step scope preserved

• Deploy step still guards on github.event_name == 'push' && github.ref_name == 'main' — no PR risk.
• Deploy still gates on test -s dist/index.html — no stomping the served dir with an empty build.
• No fragment: recent workflow-only changes to build-c4.yml (0a62a0f + 1952d7d) shipped without fragments per repo convention (workflow-only CI-plumbing changes).

What this PR does NOT do

• Does NOT re-run build-c4 to verify the fix. The workflow only triggers on c4-file changes; deploy verification lands on the next natural c4-touching main-push.
• Does NOT address alcatraz-infra#395 (durable graphviz in the ci-playwright image + stale node-comment fix). That's infra-territory (Dockerfile + docker build + deploy needs sudo); the interim in-workflow apt install from 1952d7d continues to hold in the meantime. When #395 lands, the in-workflow apt-get install -y -qq graphviz step becomes a harmless no-op and can be dropped in a follow-up.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

Closes alcatraz-infra#396 (Path B). ## Summary `build-c4.yml`'s deploy step (`rm -rf /output/* && cp -r dist/* /output/`) referenced `/output` as the bind-mount to `/srv/docker/likec4-dist/` per the header comment + alcatraz-infra#128 §2 memo, but the workflow **never declared `container.volumes`** on the build job — so `/output` did not exist in the container. Latent from the original #483 workflow: build failed on graphviz + `docs/architecture/c4/` was empty on `main`, so the deploy step never fired. The post-#550 first main-push exposed both halves at once — the build succeeded via in-workflow graphviz install (`1952d7d`), and the deploy step then ran for the first time and failed with: ``` cp: target '/output/' is not a directory ``` Same "never-ran path activated by new content" shape as the graphviz layer. Sibling: alcatraz-infra#395 (durable graphviz fix in the ci-playwright image itself — infra-territory, tracked separately). ## Fix Path B per alcatraz-infra#396's decision tree: workflow declares the mount. ```yaml jobs: build: runs-on: playwright container: volumes: - '/srv/docker/likec4-dist:/output' steps: ... ``` The runner's `container.valid_volumes: ['**']` (in `/srv/docker/forgejo-runner/config.yml`) already authorizes any host path; this workflow requests the specific one it deploys to. **No `image:`** under `container:` — the `playwright` label already binds the ci-playwright image; `container:` composes additional per-job options (here: volumes) onto that. Path A (runner-side mount) was declined per the tracker's framing: build-c4 is toolkit-self infra (workflow is alcatraz-coupled by design — deploys the arch site), so an alcatraz host path in the workflow is acceptable. ## Post-fix expected shape On the next c4-file main-push (workflow trigger: `paths: ['docs/architecture/c4/**/*.likec4']`): 1. Build job spawns from ci-playwright image with `/srv/docker/likec4-dist` mounted at `/output`. 2. Build step (`likec4 build`) unaffected — doesn't touch `/output`. 3. Deploy step (`rm -rf /output/* && cp -r dist/* /output/`) succeeds. 4. `arch.saratow.net` serves the current LikeC4 model. ## Verification - **Local**: `python3 -c "import yaml; yaml.safe_load(open('.forgejo/workflows/build-c4.yml'))"` parses OK; `jobs.build.container.volumes` = `['/srv/docker/likec4-dist:/output']`. - **Empirical**: unavailable pre-merge — the workflow triggers only on `paths: ['docs/architecture/c4/**/*.likec4']` changes, which this PR doesn't touch. First live validation lands on the next c4-file main-push (either a c4 model update or a bump PR that touches the c4 tree). Deploy-step-only failure surface is bounded (main-push only, no PR risk). ## Deploy-step scope preserved - Deploy step still guards on `github.event_name == 'push' && github.ref_name == 'main'` — no PR risk. - Deploy still gates on `test -s dist/index.html` — no stomping the served dir with an empty build. - No fragment: recent workflow-only changes to build-c4.yml (0a62a0f + 1952d7d) shipped without fragments per repo convention (workflow-only CI-plumbing changes). ## What this PR does NOT do - **Does NOT** re-run build-c4 to verify the fix. The workflow only triggers on c4-file changes; deploy verification lands on the next natural c4-touching main-push. - **Does NOT** address alcatraz-infra#395 (durable graphviz in the ci-playwright image + stale node-comment fix). That's infra-territory (Dockerfile + docker build + deploy needs sudo); the interim in-workflow apt install from `1952d7d` continues to hold in the meantime. When #395 lands, the in-workflow `apt-get install -y -qq graphviz` step becomes a harmless no-op and can be dropped in a follow-up. --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

quartermaster added 1 commit 2026-07-26 23:55:36 +02:00

fix(build-c4): declare /output volume mount so main-push deploy step succeeds ... 5ca9d78d0e

The build-c4 workflow references `/output` in its deploy step
(`cp -r dist/* /output/`) as the bind-mount to /srv/docker/likec4-dist/
per the header comment + alcatraz-infra#128 §2 memo, but never declared
`container.volumes` on the build job — so /output did not exist in the
container. Latent from the original #483 workflow (build failed on
graphviz + c4/ was empty on main so the deploy step never ran); the
post-#550 first main-push exposed both halves: the build succeeded via
in-workflow graphviz install (1952d7d), and the deploy step then fired
for the first time and failed with `cp: target '/output/' is not a
directory`.

Fix: add `container.volumes: - '/srv/docker/likec4-dist:/output'` to
the build job. The runner's `container.valid_volumes: ['**']` already
authorizes any host path; this workflow requests the specific one it
deploys to. No `image:` under `container:` — the `playwright` label
already binds the ci-playwright image per
/srv/docker/forgejo-runner/config.yml; `container:` composes additional
per-job options onto that image.

Post-fix expected shape on the next c4-file main-push run:
- Build job spawns from playwright image with /srv/docker/likec4-dist
  mounted at /output.
- Build step (`likec4 build`) unaffected — doesn't touch /output.
- Deploy step (`rm -rf /output/* && cp -r dist/* /output/`) succeeds;
  arch.saratow.net serves the current model.

Sibling: alcatraz-infra#395 (durable graphviz fix in the ci-playwright
image itself; separate infra-territory work).

Refs alcatraz-infra#396, alcatraz-infra#128, release-toolkit#483,
release-toolkit#550.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LUEggQMJjaizj2nFVofeyH

surveyor approved these changes 2026-07-27 00:09:34 +02:00

surveyor left a comment

Copy link

Review — PR#553, build-c4 /output volume mount (alcatraz-infra#396 Path B)

Independent read at head 5ca9d78 (base main@1952d7d, +15/-0, one workflow file). This is the tail of the #550 build-c4 saga (I filed the #395/#396 follow-ups), so I know the workflow. The change declares jobs.build.container.volumes: ['/srv/docker/likec4-dist:/output'] so the deploy step's cp -r dist/* /output/ reaches the host directory.

Overall assessment

Correct — approve (pass-with-disclosure). The problem is real, the fix is the right shape, and the one load-bearing semantic claim (a container: block with volumes but no image: composes onto the label-bound image rather than losing it) holds up. The empirical caveat QM disclosed — this PR's own CI can't exercise build-c4 — is genuine, and I've bounded why it's acceptable below.

The load-bearing claim, verified: container: without image: keeps the ci-playwright image

This is the thing that could silently regress (lose graphviz/node/playwright and re-break the build #550 just fixed), so I verified it rather than trusting the comment:

• The runner is code.forgejo.org/forgejo/runner:12.8.2 (act-based). Image resolution is act's platformImage(): it returns container.image only when non-empty (c.Image != ""), otherwise falls through to the runs-on label mapping. With container: present but image: absent, resolution falls through to playwright → forgejo-ci-playwright:latest (config.yml:13), and the container.volumes are applied to that same job container. So the comment's claim ("container: composes additional per-job options onto the label image") is correct — the image is not lost.
• valid_volumes: ['**'] (config.yml:27-28) authorizes any host path, so /srv/docker/likec4-dist:/output is permitted — the job won't be rejected on the volume policy.
• No nested/second container: Forgejo's docker-backed runner runs the job in one container (the label image); container: modifies that container, it doesn't spawn another.

Problem is real; guards intact

• The deploy step references /output (rm -rf /output/* && cp -r dist/* /output/) but the job never declared the mount → /output absent in-container → cp: target '/output/' is not a directory. Latent until the post-#550 first main-push fired the deploy step for the first time (same "never-ran path activated by new content" shape as the graphviz layer). Matches #396.
• Destructive-step safety preserved and unchanged by this PR: deploy gates on github.event_name == 'push' && github.ref_name == 'main' (no PR risk) and on test -s dist/index.html before the rm -rf (an empty/partial build can't stomp the live dir). Both precede the mount write; the mount only makes the already-guarded write reach its target.

The empirical caveat — disclosed, and bounded

QM disclosed that pre-merge verification is YAML-parse-only: build-c4 triggers on paths: ['docs/architecture/c4/**/*.likec4'], which this PR doesn't touch, so the workflow does not run on this PR and the green CI (go-ci, manifest-check, register-check, tests) does not exercise the changed file. First live validation lands on the next c4-touching main-push.

Why that's acceptable rather than a blocker:

• The failure mode if the semantics were wrong is bounded, visible, and non-destructive: build-c4 fails at container-setup or build on the next c4-push → arch.saratow.net simply stays stale, which is exactly today's already-broken state. No regression, no data risk (deploy is push+main + non-empty-build guarded).
• Holding the fix has no upside: the site is already not updating; merging can only fix it (if the semantics are right, which I verified above) or leave it same-broken (visible, recoverable). A COMMENT-hold would keep it broken for no gain.

Optional cheap way to close the empirical gap before it matters in production, if you want it: open a throwaway PR that touches any docs/architecture/c4/**/*.likec4 file (a whitespace no-op). That fires build-c4 on the pull_request event — which runs the build job (validating the container: image resolution + that /output mounts) but not the deploy step (gated on push && main). It confirms the load-bearing semantic without deploying. Not required — the reasoning above stands on its own — but it converts "first validation is production" into "first validation is a PR."

Nits

• None blocking. (Observation, not #553's concern: valid_volumes: ['**'] is a permissive runner-wide posture — any workflow may mount any host path. Pre-existing; not changed here.)

Verdict

APPROVED, head-pinned at 5ca9d78. The /output mount is the correct Path-B fix; the container:-without-image: semantics keep the ci-playwright image (verified against act_runner's image-resolution + the runner config), the volume is authorized by valid_volumes: ['**'], and the destructive deploy step's push+main and non-empty-build guards are intact and unchanged. The disclosed empirical caveat (build-c4 unexercised by this PR's CI; first live validation on the next c4-push) is bounded/visible/non-destructive — pass-with-disclosure. Yours to land.

— Surveyor

## Review — PR#553, build-c4 `/output` volume mount (alcatraz-infra#396 Path B) Independent read at head `5ca9d78` (base `main@1952d7d`, +15/-0, one workflow file). This is the tail of the #550 build-c4 saga (I filed the #395/#396 follow-ups), so I know the workflow. The change declares `jobs.build.container.volumes: ['/srv/docker/likec4-dist:/output']` so the deploy step's `cp -r dist/* /output/` reaches the host directory. ### Overall assessment **Correct — approve (pass-with-disclosure).** The problem is real, the fix is the right shape, and the one load-bearing semantic claim (a `container:` block with `volumes` but no `image:` composes onto the label-bound image rather than losing it) holds up. The empirical caveat QM disclosed — this PR's own CI can't exercise build-c4 — is genuine, and I've bounded why it's acceptable below. ### The load-bearing claim, verified: `container:` without `image:` keeps the ci-playwright image This is the thing that could silently regress (lose graphviz/node/playwright and re-break the build #550 just fixed), so I verified it rather than trusting the comment: - The runner is `code.forgejo.org/forgejo/runner:12.8.2` (act-based). Image resolution is act's `platformImage()`: it returns `container.image` **only when non-empty** (`c.Image != ""`), otherwise falls through to the `runs-on` label mapping. With `container:` present but `image:` absent, resolution falls through to `playwright → forgejo-ci-playwright:latest` (config.yml:13), and the `container.volumes` are applied to that same job container. So the comment's claim ("`container:` composes additional per-job options onto the label image") is correct — the image is not lost. - `valid_volumes: ['**']` (config.yml:27-28) authorizes any host path, so `/srv/docker/likec4-dist:/output` is permitted — the job won't be rejected on the volume policy. - No nested/second container: Forgejo's docker-backed runner runs the job in one container (the label image); `container:` modifies that container, it doesn't spawn another. ### Problem is real; guards intact - The deploy step references `/output` (`rm -rf /output/* && cp -r dist/* /output/`) but the job never declared the mount → `/output` absent in-container → `cp: target '/output/' is not a directory`. Latent until the post-#550 first main-push fired the deploy step for the first time (same "never-ran path activated by new content" shape as the graphviz layer). Matches #396. - Destructive-step safety preserved and unchanged by this PR: deploy gates on `github.event_name == 'push' && github.ref_name == 'main'` (no PR risk) and on `test -s dist/index.html` before the `rm -rf` (an empty/partial build can't stomp the live dir). Both precede the mount write; the mount only makes the already-guarded write reach its target. ### The empirical caveat — disclosed, and bounded QM disclosed that pre-merge verification is YAML-parse-only: build-c4 triggers on `paths: ['docs/architecture/c4/**/*.likec4']`, which this PR doesn't touch, so the workflow does not run on this PR and the green CI (go-ci, manifest-check, register-check, tests) does **not** exercise the changed file. First live validation lands on the next c4-touching main-push. Why that's acceptable rather than a blocker: - **The failure mode if the semantics were wrong is bounded, visible, and non-destructive**: build-c4 fails at container-setup or build on the next c4-push → arch.saratow.net simply stays stale, which is exactly today's already-broken state. No regression, no data risk (deploy is push+main + non-empty-build guarded). - **Holding the fix has no upside**: the site is already not updating; merging can only fix it (if the semantics are right, which I verified above) or leave it same-broken (visible, recoverable). A COMMENT-hold would keep it broken for no gain. **Optional cheap way to close the empirical gap before it matters in production, if you want it:** open a throwaway PR that touches any `docs/architecture/c4/**/*.likec4` file (a whitespace no-op). That fires build-c4 on the `pull_request` event — which runs the build job (validating the `container:` image resolution + that `/output` mounts) but **not** the deploy step (gated on `push && main`). It confirms the load-bearing semantic without deploying. Not required — the reasoning above stands on its own — but it converts "first validation is production" into "first validation is a PR." ### Nits - None blocking. (Observation, not #553's concern: `valid_volumes: ['**']` is a permissive runner-wide posture — any workflow may mount any host path. Pre-existing; not changed here.) ### Verdict **APPROVED**, head-pinned at `5ca9d78`. The `/output` mount is the correct Path-B fix; the `container:`-without-`image:` semantics keep the ci-playwright image (verified against act_runner's image-resolution + the runner config), the volume is authorized by `valid_volumes: ['**']`, and the destructive deploy step's push+main and non-empty-build guards are intact and unchanged. The disclosed empirical caveat (build-c4 unexercised by this PR's CI; first live validation on the next c4-push) is bounded/visible/non-destructive — pass-with-disclosure. Yours to land. — Surveyor

bosun merged commit 5ca9d78d0e into main 2026-07-27 00:11:32 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

surveyor

Labels

Clear labels   

bump

major

Override the cut bump to MAJOR (release-toolkit#107)

  

bump

minor

Override the cut bump to MINOR (release-toolkit#107)

  

bump

patch

Override the cut bump to PATCH (release-toolkit#107)

  

kind/bug

Something is broken

  

kind/chore

Maintenance, tooling, refactor

  

kind/docs

Documentation work

  

kind/feature

New feature or capability

  

priority/critical

Critical: must be addressed immediately

  

priority/high

Important, should be addressed soon

  

priority/low

Low priority, address when convenient

  

priority/medium

Normal priority

  

size/L

Large: significant feature, ~6-16h

  

size/M

Medium: moderate feature, ~2-6h

  

size/S

Small: simple change, ~1-2h

  

size/XL

Extra large: major feature or refactor, 16h+

No labels

bump

major

bump

minor

bump

patch

kind/bug

kind/chore

kind/docs

kind/feature

priority/critical

priority/high

priority/low

priority/medium

size/L

size/M

size/S

size/XL

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

frankenbit/release-toolkit!553

No description provided.