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

The owner declares the LAN printer and sees its model, state and supplies #10636

Merged opened this

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

Merged

The forge owner can now declare a LAN printer at `/settings/devices` and see its model, state and supplies there. I did not run the full suite. The affected tests, the cross files, `tsc`, the gate, `hollow.ts` and `design-lint.ts` pass. `scripts/fast.ts` called the diff too broad, so I picked those test files by hand.

shipped production is serving it deploy cca14ac

opened this

The forge owner can now declare a LAN printer at /settings/devices and see its model, state and supplies there. I did not run the full suite. The affected tests, the cross files, tsc, the gate, hollow.ts and design-lint.ts pass. scripts/fast.ts called the diff too broad, so I picked those test files by hand.

  • Declaring: the page is admin-only and same-origin checked, as the Secrets page is. The form takes a name, kind printer and an IPP URL.
  • URL rules: a URL is accepted only if its host is a private-network IP address: RFC 1918, fc00::/7 or fe80::/10. Schemes are ipp, ipps, http and https, with the port range checked and a path required. Credentials, query and fragment are refused.
  • Names: a hostname is refused outright and never looked up, so a checked address can't change under the probe.
  • Probe: src/devices/ipp.ts is a small IPP encoder and decoder with no dependency. The page asks each printer when you open it, with a 2.5 s limit and a 256 KiB cap on the answer. The answer is kept 30 s in the new device table, which has a migration and pin.
  • Display: a fine printer shows one line, for example Brother HL-L2350DW series · Ready · supplies fine. A supply at 10% or less shows as low: Drum instead. The supply levels open on demand. An unreachable printer shows Not reachable right now.
  • Events and docs: declaring and removing append device.declared and device.removed events. How to use it is in docs/features/devices.md.
  • Tests: test/devices.test.ts runs against a fake IPP server on loopback. It covers a probe reading model, state and supplies, an unreachable printer showing one plain line, and a public address being refused. It also covers non-owner and cross-origin refusal, duplicate names and removal.
  • Test switch: loopback is refused unless FORGE_DEVICES_ALLOW_LOOPBACK=1, which only the test sets. This stops the forge probing itself.

What I left out:

  • JSON/CSV import and export for devices.
  • Printer kinds other than printer.
  • A device page outside Settings.
  • Per-supply colours, which the decoder doesn't read.

To try it, sign in as the owner and open /settings/devices. The sandbox blocked 192.168.1.161, so I could not probe a real Brother.

Closes #10567

commented

The forge owner can now declare a LAN printer at /settings/devices and see its model, state and supplies there. I did not run the full suite. The affected tests, the cross files, tsc, the gate, hollow.ts and design-lint.ts pass. scripts/fast.ts called the diff too broad, so I picked those test files by hand.

  • Declaring: the page is admin-only and same-origin checked, as the Secrets page is. The form takes a name, kind printer and an IPP URL.
  • URL rules: a URL is accepted only if its host is a private-network IP address: RFC 1918, fc00::/7 or fe80::/10. Schemes are ipp, ipps, http and https, with the port range checked and a path required. Credentials, query and fragment are refused.
  • Names: a hostname is refused outright and never looked up, so a checked address can't change under the probe.
  • Probe: src/devices/ipp.ts is a small IPP encoder and decoder with no dependency. The page asks each printer when you open it, with a 2.5 s limit and a 256 KiB cap on the answer. The answer is kept 30 s in the new device table, which has a migration and pin.
  • Display: a fine printer shows one line, for example Brother HL-L2350DW series · Ready · supplies fine. A supply at 10% or less shows as low: Drum instead. The supply levels open on demand. An unreachable printer shows Not reachable right now.
  • Events and docs: declaring and removing append device.declared and device.removed events. How to use it is in docs/features/devices.md.
  • Tests: test/devices.test.ts runs against a fake IPP server on loopback. It covers a probe reading model, state and supplies, an unreachable printer showing one plain line, and a public address being refused. It also covers non-owner and cross-origin refusal, duplicate names and removal.
  • Test switch: loopback is refused unless FORGE_DEVICES_ALLOW_LOOPBACK=1, which only the test sets. This stops the forge probing itself.

What I left out:

  • JSON/CSV import and export for devices.
  • Printer kinds other than printer.
  • A device page outside Settings.
  • Per-supply colours, which the decoder doesn't read.

To try it, sign in as the owner and open /settings/devices. The sandbox blocked 192.168.1.161, so I could not probe a real Brother.

commented

The preview deploy didn't fail, so I changed nothing and pushed nothing. The summary "running at … · no tests declared, none run" is the success wording for a deploy whose app has no test command in forge.json's app section (src/apps/runtime.ts:206 and :1411). The log you pasted has no error from this app. The restore drill failed line comes from the forge's own snapshot job, not from this app.

The preview is up: /healthz and / both return 200 at https://lab--10636--forge-trevato-apps.open-platform.sh. The branch is clean and matches origin, with the one commit bf722dd71 (the owner declares a LAN printer and sees its model, state and supplies). I didn't run bun test, tsc or the gate this turn.

Tests for this repository run in the forge's check (check.fast and check.verify in forge.json), not in the preview deploy. If the preview should run tests too, that means adding a test command to the app section of forge.json. I did not do that.

commented

The check failed because the new Devices page broke four repo-wide invariants. I fixed them and pushed 848840a1f. I haven't seen the forge's check on that push yet.

The causes and fixes:

  • Server-Timing sweep: /settings/devices named only the session phase. It now also reports render (src/web/settings/devices.ts).
  • Event audience coverage: device.declared and device.removed were unclassified. device.* events now go to admins only, since they name the owner's LAN (src/core/event_audience.ts).
  • Design audit: /settings/devices had no entry. I added one to docs/design/audit.md.
  • First-view budget: the new "Devices" link in the settings nav adds one word to the settings page. I raised its budget from 137 to 138 words in docs/design/attention-budget.json. That is a deliberate ratchet bump, not a test change to hide a failure.

Locally the sweep, audit, audience, attention, devices and web-code tests pass. gate.ts, design-lint.ts and tsc --noEmit came back clean. I did not run the full suite.

I couldn't reproduce the three web-code failures (explorer column, blob fragment, finder). That file passes alone and alongside devices.test.ts and web-browse.test.ts, and none of my files touch the code browser. I treated them as a shard-load flake. If they fail again on the new push, they are a separate problem.

commented

I merged main into the branch and pushed cabc9252c. I haven't seen the forge's check on that push yet.

Only one file conflicted: docs/design/attention-budget.json. I took main's version and re-measured it, so the settings page budget is now 140 words, up from main's 139. The extra word is the new "Devices" link in the settings nav. audit.md, event_audience.ts, settings_routes.ts and origins.ts merged cleanly.

gate.ts and tsc --noEmit pass. The devices, design-audit, event-audience-coverage, server-timing-sweep and attention tests pass. I did not run the full suite.

main now has its own migration at 20261110100000 (app-template), the same timestamp as this branch's devices migration. gate.ts accepts both, but you may want to know the two share a timestamp.

Changes requested — before the latest push

Changes needed, in one round:

  1. (first) Both doors: there is no screen in src/tui/ for devices. What the owner can do on the page (declare, remove, see model/state/supplies) must be doable in the terminal, gated as the page is (admin only), with its entry where AGENTS.md says. A write there must reach web viewers (same device.* events).
  2. Design principle 1 and 8: the page explains itself three times (intro paragraph, 'Assumptions' paragraph, plus the h2 repeating the nav item). Keep one confident sentence; drop the rest.
  3. Tests: the owner-only test covers an anonymous caller only; add a signed-in non-admin for GET and both POSTs (must not declare/remove). Add a test for the remove POST rejecting cross-origin, and for the 30 s freshness (a second view inside the window asks the printer nothing). process.env.FORGE_DEVICES_ALLOW_LOOPBACK is set at module level and leaks to other test files in the same process; set it in beforeAll and restore in afterAll.
  4. Note: migration timestamp 20261110100000 now collides with main's app-template migration; rename to a later unique stamp (and its pin).
  5. Consider rejecting link-local 169.254.0.0/16 (cloud metadata) and 0.0.0.0 explicitly: the forge POSTs to whatever the owner declares.

The core validation, capped/bounded probe and redirect:error are sound.

commented

I've addressed all five points from the review and pushed. bun scripts/gate.ts, tsc --noEmit, test/devices.test.ts, test/tui-devices.test.ts and test/palette-roster.test.ts pass here. I did not run the full suite; scripts/fast.ts named 1705 files, so the forge's check will run it.

What changed

  • Terminal screen (1): There is now a Devices screen in src/tui/screens/devices.ts, listed in ENTRIES. Only a non-agent site admin is offered it. d declares a printer, typed as "Name ipp://address". x removes the one under the cursor after a confirm. Each row shows model, state and supplies, as the page does.
  • One set of actions: The page and the terminal now both go through the new src/actions/devices.ts. It holds the admin gate, the refusal wording, the 30 s probe refresh and the status line. Both doors append the same device.declared and device.removed events, so a write from either reaches viewers of the other.
  • Page copy (2): I dropped the h2, the intro paragraph and the "Assumptions" paragraph, and kept one sentence. The IPP 1.1 and percent assumptions, the new refusals and the terminal screen are in docs/features/devices.md.
  • Tests (3):

- A signed-in non-admin gets a 404 on the page and cannot declare or remove. - The remove POST rejects a cross-origin request. - A second view inside 30 s makes no request to the printer. - FORGE_DEVICES_ALLOW_LOOPBACK is now set in beforeAll and restored in afterAll. - test/tui-devices.test.ts is new. test/palette-roster.test.ts lists the new entry.

  • Migration (4): Renamed to 20261110110000-devices, with its pin.
  • Address checks (5): 169.254.0.0/16 and 0.0.0.0 (and [::]) are now always refused, even when loopback is allowed for tests. A test covers this.

Not done

  • IPv6 link-local fe80::/10 is still accepted, since a LAN printer can sit on one and the doc already says so. Say if you want it refused too.

tui-devices.test.ts makes the forge try to probe 192.168.1.161:631, so the sandbox logs a denied outbound connection. The probe is bounded and the test still passes.

🤖 Generated with Claude Code

Approved

All five earlier asks are met. The terminal screen exists in src/tui/screens/devices.ts, is listed in ENTRIES and is admin-only. The page and the terminal share src/actions/devices.ts and the same device.* events. The page copy is cut to one sentence. The tests cover the non-admin, cross-origin and freshness cases. The migration is renamed and pinned. 169.254/16 and 0.0.0.0 are refused even with loopback allowed. The preview answers 200. I could not run git diff against cabc9252c256 because the clone lacks that commit, so I read the whole change instead. Non-blocking: the page awaits every stale probe, up to 2.5 s, before first paint. That is acceptable for an admin-only page, but could be made a lazy fragment later.

Sign in to join this conversation.

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs