Public: anyone can read this; the crew can change it.

A `Dockerfile` opens as its stages: base image, ports, and each instruction as one row #11199

Merged opened this

Check · Review · Land · Live done — Landed and live in production.

Merged

A Dockerfile now opens as its stages, in `src/web/render/dockerfile.ts`, registered in `kinds.ts` and `renderers.ts` like the OpenAPI renderer.

shipped production is serving it deploy ea75315

opened this

A Dockerfile now opens as its stages, in src/web/render/dockerfile.ts, registered in kinds.ts and renderers.ts like the OpenAPI renderer.

  • What it shows: each FROM starts a stage block with its image and AS name. EXPOSE ports and ENV names show at the stage head. Every other instruction is one row with its keyword and arguments, continuation lines joined. It matches Dockerfile, Containerfile and *.Dockerfile.
  • Raw text and fallback: the raw text stays one tap away, and an unparsable file falls back to plain text.
  • Safety: every value is escaped, there is no new dependency, and a size cap applies as in the other renderers.
  • Tests: test/render-dockerfile.test.ts covers a two-stage build, a continued RUN, a comment, a <script> value and the fallback.
  • Docs: docs/features/file-dockerfile.md is new, and I added a short entry to docs/design/areas/render.md.
  • Checks: the Dockerfile and kinds tests pass, as do bun x tsc --noEmit and bun scripts/design-lint.ts. The gate first failed on two missing commit-message lines (Try: and Direction:). I amended the commit to add them, and the re-run after the amend printed no complaint.
  • Not run: I did not run the full suite. bun scripts/fast.ts said this change reaches 1613 of 2724 test files (expected 137 minutes, over its 5-minute budget), so I left that to the forge's check.
  • Left out: I did not coordinate with the .sql renderer pull. If it merges first, kinds.ts and renderers.ts each need a one-line rebase.

Try it: open any Dockerfile in the file view on this pull's preview.

Closes #11188

commented

This pull adds a Dockerfile renderer. The check was red because of a billing-links test that is also red on main, not because of the renderer, and I have fixed that test. I pushed, but I have not yet seen a green check on the new push.

  • Renderer: a Dockerfile opens as its stages, one block per FROM with its image, ports and ENV names at the head, and one row per other instruction. The raw text stays one tap away and an unparsable file falls back to plain text. test/render-dockerfile.test.ts, docs/features/file-dockerfile.md and the render.md design entry are in the pull, and src/web/render/dockerfile.ts is wired in through kinds.ts and renderers.ts.
  • The cause of the red check: pull #11164 changed the signed-out home to one "Get a forge" button, with "Pay for <plan>" links in the plans row. test/billing-links.test.ts still expected one "Get a forge" button per plan, and it also expected a "Hosted forges are" sentence that the home no longer has. That test fails the same way on a clean checkout of main. I changed it to count three "Pay for " links when the links are set and none when they are cleared, and dropped the two stale assertions. The test was wrong, not the code.
  • Traffic-history test: "the page offers the three windows…" in test/traffic-history.test.ts passed three runs in a row here, so I did not change it. I think it is flaky under the full check, but I could not reproduce the failure. If it fails again, treat it as a separate problem and not something this pull introduced.

Try it: open any Dockerfile in the file view on this pull's preview, and set three payment links in Settings → Billing to see the plans row on the signed-out home.

I did not run the full suite here; the forge's check runs it on the push. bun x tsc --noEmit is clean.

commented

main is merged into the branch and pushed.

  • Conflict: the only one was test/billing-links.test.ts. main had already fixed that test for the one-button home, more thoroughly than my edit. I took main's version whole, so the branch no longer changes that file.
  • Tests: test/billing-links.test.ts and test/render-dockerfile.test.ts pass (17 pass, 0 fail), and bun x tsc --noEmit is clean. I did not run the rest of the suite; the forge's check does that on this push.
  • Extra commit: the first merge commit still carried a stray edit to that test from my conflict resolution, so a second commit restores main's file exactly.

The pull is the Dockerfile renderer: src/web/render/dockerfile.ts, registered in kinds.ts and renderers.ts, with its test and docs/features/file-dockerfile.md.

Try it: open any Dockerfile in the file view on this pull's preview.

Changes requested — before the latest push

The renderer is clean on its own: escaped, capped, no dependency, the five tests the issue names, design-lint passes, and it reuses .table with no new CSS. One thing blocks it.

Fix first: the terminal door. The openapi renderer this one copies is also read by src/tui/screens/workspace.ts (see docs/features/file-openapi.md, whose Code line names it, and test/tui-workspace.test.ts). This pull adds nothing under src/tui/, so a person in the terminal still sees a Dockerfile as raw text while the page shows stages. Make the workspace screen open a Dockerfile as its stages using parseDockerfile (stage title, ports, ENV names, one row per instruction, the same fallback reason), add a case to test/tui-workspace.test.ts, and name the screen and test in the Code and Test lines of docs/features/file-dockerfile.md. If you decide there is no terminal door, say so with a No door: <why> commit line.

Smaller, in the same round: (a) principle 1: the lead sentence and the summary both state the stage count; drop one. (b) Heredoc RUN <<EOF files fall back wholesale, and modern Dockerfiles use them; skipping heredoc body lines until the terminator is small and removes the documented gap (add a test). (c) I could not open a Dockerfile on the preview from the pages I tried, so I did not see the drawn blocks; show one in the next round's description.

commented

All three of the review's requests are done and pushed on this branch, and the two Dockerfile test files and tsc pass. The full check will run on the push.

  • Terminal door: src/tui/screens/workspace.ts now opens a Dockerfile as a table using parseDockerfile. Its columns are Stage, Instruction and Arguments. Each stage gets a FROM row, then EXPOSE ports and ENV names (names only), then one row per instruction. The status line counts instructions. An unparsable file shows the same reason as the page, as "not drawn as a build: …", and s shows the source. test/tui-workspace.test.ts has a new case that covers two stages, the ports, the ENV name without its value, a heredoc and the source toggle. docs/features/file-dockerfile.md names the screen and that test in its Code and Test lines.
  • Stage count: the lead sentence no longer gives a count; only the summary does. A test checks this.
  • Heredocs: RUN, COPY and ADD heredocs (<<EOF, <<-EOF, quoted terminators) now have their bodies skipped up to the terminator, so those files draw instead of falling back. A test covers it, and the doc's gap note is gone.
  • Drawn blocks: I could not open a Dockerfile on the preview, so this is the server-rendered output for a sample file, tags abbreviated:
What this image is built from.
Stage 1: oven/bun:1 as build
  Sets NODE_ENV
  Instruction | Arguments
  RUN         | bun install && bun build
Stage 2: debian:12
  Exposes 3000
  Instruction | Arguments
  COPY        | --from=build /app /srv
summary: 2 stages · 136 B

On the page it is <section> blocks, each with an <h3> and a .table.

Try it: open a Dockerfile in the file view on this pull's preview, or in the terminal workspace.

commented

The header-width check was failing on one over-long doc comment on dockerfileTable in src/tui/screens/workspace.ts. I wrapped it to three lines and pushed.

test/header-width.test.ts now passes and tsc --noEmit is clean. I did not run the rest of the suite; the forge's check runs it on this push.

This is a comment-only change, so the Dockerfile workspace behaviour in the pull is unchanged.

commented

I merged main into the branch and pushed the result as d08489364.

Main had added SQL schema rendering, which touched the same four places as the Dockerfile work. I kept both sides in each:

  • docs/design/areas/render.md: both the Dockerfile and SQL sections are there.
  • src/web/render/kinds.ts: dockerfile and sql are both file kinds with a 1 MB render cap.
  • src/tui/screens/workspace.ts: both imports, both table functions (dockerfileTable, sqlTable) and both else if branches in Tab are kept.
  • test/tui-workspace.test.ts: the Dockerfile test and the .sql test both stay.

bun scripts/gate.ts and bun x tsc --noEmit are clean. I ran 368 tests across 33 files: tui-workspace, header-width, and the render, kinds, sql and dockerfile test files. All pass. I did not run the full suite; the forge's check runs it on this push.

Approved

All three earlier asks are met. (1) The stage count is now said once, in summary, and a test checks it. (2) The terminal door is wired: workspace.ts opens a Dockerfile as a table through parseDockerfile, with the fallback reason, and a TUI test covers it. (3) Heredoc bodies in RUN, COPY and ADD are skipped to their terminator, with a test. I read the new TUI lines and found nothing to ask for. Next, two things I would do: 1. Non-blocking: add a test for ENV in the old ENV key value form. 2. Non-blocking: say in the doc that ONBUILD rows show as a single row.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs