Repository navigation
feat(web,mobile,server): image icons, mobile picker, and container detection - #5
amanthanvi wants to merge 24 commits into
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @amanthanvi, you've used your own review budget of 250,000 diff characters for the last 7 days.
You can request another review in 1 day and 11 hours by commenting @sourcery-ai review. Upgrade to get a review now.
Reviewer's GuideAdds bounded image icons across web and mobile, a capability-aware mobile picker, and broader server machine detection for containers, Windows, and Apple silicon, with compatibility-preserving contract changes and tests. Sequence diagram for selecting and saving an environment image iconsequenceDiagram
actor User
participant Picker as ImagePicker
participant Encoder as ImageEncoder
participant Contract as IconImageDataUrlSchema
participant Settings as EnvironmentSettings
participant Server as EnvironmentServer
User->>Picker: Choose image
Picker->>Encoder: encodeEnvironmentIconImage(file)
Encoder->>Encoder: Crop and resize to 64x64 PNG
Encoder->>Contract: Validate data:image/png data URL
Contract-->>Encoder: Valid or invalid
Encoder-->>Picker: Encoded result
User->>Picker: Save
Picker->>Settings: updateSettings(environmentIcon)
Settings->>Server: Persist icon on machine
Server-->>Settings: Updated environment settings
Entity relationship diagram for environment icon compatibilityerDiagram
SERVER_ENVIRONMENT ||--o| ENVIRONMENT_ICON : stores
SERVER_ENVIRONMENT {
string environmentIcon
string environmentIconOverride
string machine
}
ENVIRONMENT_ICON {
string kind
string name
string dataUrl
}
ENVIRONMENT_ICON ||--o| LEGACY_MACHINE_KIND : bare_string_wire_form
ENVIRONMENT_ICON {
string compatibility_form
}
LEGACY_MACHINE_KIND {
string name
}
Flow diagram for capability-aware mobile environment icon selectionflowchart TD
A["Expand connected environment"] --> B["Tap Icon"]
B --> C{"environmentIcon capability?"}
C -->|No| D["Show locked picker"]
C -->|Yes| E{"Rich icon override supported?"}
E -->|No| F["Enable legacy machine kinds only"]
E -->|Yes| G["Enable curated icons and photo"]
F --> H["resolveMobileEnvironmentIconWrite"]
G --> H
H --> I["updateSettings"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
github-actions
Bot
added
vouch:trusted
f99eab5 to
fe267c1
Compare
Thread transfer impact
This comment will update automatically after the next completed run. |
fe267c1 to
3ef97b6
Compare
3ef97b6 to
8e6149b
Compare
8e6149b to
2b6c084
Compare
2b6c084 to
0011cfe
Compare
045d9aa to
e1a82ee
Compare
|
Two commits pushed, head is now The encoder reports a failed encode instead of throwing (
|
e1a82ee to
a240054
Compare
Rebased onto current mainRebased the whole stack onto main at
A clean textual replay does not prove the stack still holds together, so I went looking for the hazard that would hide behind one. Layer 2 renames Each layer is its own pull request, so I verified each one standing alone rather than only at the tip:
No review thread was open when I rebased, so the force push moved commits rather than answers. Line comments from earlier rounds now anchor to the old SHAs. |
|
@sourcery-ai review |
|
Sorry @amanthanvi, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 28 minutes by commenting |
a240054 to
0e38392
Compare
Rebased onto current main, and the monogram counter lost its
|
| layer | typecheck | lint | tests | restyle |
|---|---|---|---|---|
| 01 contracts | 4 packages, 0 errors | 0 errors | 4 files, 245 tests | 1207 |
| 02 rename | 5 packages, 0 errors | 0 errors | 5 files, 276 tests | 1207 |
| 03 curated | 5 packages, 0 errors | 0 errors | 6 files, 285 tests | 1207 |
| 04 rich | 5 packages, 0 errors | 0 errors | 9 files, 296 tests | 1207 |
| 05 lucide | 5 packages, 0 errors | 0 errors | 10 files, 300 tests | 1207 |
| 06 image, mobile, detect | 5 packages, 0 errors | 0 errors | 12 files, 327 tests | 1207 |
The force push moved commits, so line comments from earlier rounds now anchor to the old SHAs.
|
@sourcery-ai review |
Rebased onto main
|
…tection Three things were still missing after the picker learned emoji, monograms, and colors: an uploaded image, a way to pick on mobile, and detection for the machines that were left as a generic server. Image icons store a capped `data:` URL. Both clients already render inline images, so there is no new route, blob store, or access check. Each client crops to a square and downscales to 64 by 64 natively before writing (a canvas on web, `expo-image-manipulator` on mobile), and validates the result against the schema so it writes exactly what the server accepts. The prefix is pinned to PNG. An SVG can script, so the encoder never produces one and the write refuses it. The mobile picker lands in the expanded body of the environment row, beside the label and URL fields, as a sheet listing the curated icons and a photo option. Emoji and monograms are entered on web, where there is a keyboard; mobile draws whatever was picked there and says so. Writes go through the same settings command the settings screens use. Detection gains the two real gaps. Nothing read `/.dockerenv`, `/run/.containerenv`, or PID 1's cgroup, so a container fell through the chassis table. A container now detects as the new `container` kind, ahead of the host's DMI. Windows detection returned null by design, and it now reads the same SMBIOS enclosure table and hypervisor strings through one PowerShell CIM call. Apple silicon model identifiers, which name a generation rather than a product line, resolve through a table so `Mac16,10` is a Mac mini. Cloud VMs already detected as `cloud` through eighteen markers, so naming the vendor is left alone. That would be a contract widening to add one label. The T3 Connect discovery rows get no icon control on purpose, recorded in the code where the next reader will look for one. The only place an icon is stored is that machine's own settings, so a pick made before connecting would live on this device alone and would not follow the user to their other devices, which is the one property the icon exists for. Claude Fable 5.1 via Claude Code
`isContainerCgroup` treated `0::/` as proof of a container, and the container branch runs before the WSL check. Under cgroup v2 a container with a private namespace does read a bare root, but so does any host whose init leaves PID 1 there: stock WSL 2, Alpine on OpenRC, Void on runit. Those all reported as containers, and the WSL check below never ran. Systemd hosts read `0::/init.scope` and were unaffected, which is why the existing tests passed. Only a named runtime counts now. Docker and Podman drop a marker file, and Kubernetes, containerd and LXC name themselves in the cgroup path, so the only case lost is a raw containerd container with a private namespace and no marker file, which falls through to the generic glyph. Alongside it, four things the same review turned up: Image icons keyed their cached component on object identity, but every settings snapshot decodes a fresh object, so any settings write anywhere handed back a new component and remounted the subtree. They key on the data URL now, which removes the separate WeakMap. The WebP fallback in both encoders was unreachable. Incompressible noise at 64 by 64 encodes to 22,050 characters against a 32,768 cap, so the PNG always fits. Removing it leaves the encoders producing exactly what the contract accepts, which the first layer now pins to PNG. Its comment gains the reason the frame count is still open. APNG declares `image/png`, and only the canvas in these encoders rules it out. Picking a second image while the first was still encoding could let the first win. The handler ignores a stale result now. A source over 16 MB is refused before `createImageBitmap` decodes it at full resolution. The `machine` field still documented containers and Windows as undetectable. Both are detected here. Claude Opus 5 via Claude Code
…size one The mobile encoder built the data URL from an optional `base64` field with an empty string fallback, so a save that returned no payload failed the schema and told the user their image was too big for the cap. Report it as unreadable.
`encodeEnvironmentIconImage` documents a result type as its error channel and every other exit uses it, but `canvas.toDataURL` reports by throwing and the enclosing `try` carries only a `finally`. The picker dialog awaits the call with no `catch`, so a throw there would leave the dialog showing a pending upload with no error and no way forward. Contain it and return the existing `unreadable` reason, which the dialog already renders as a message. A 64 by 64 canvas drawn from a same-origin `Blob` should not taint or overflow, so this is about the function keeping the contract its callers rely on rather than a failure seen in practice.
…rong thing Review caught three claims that do not match the code. `IconImageDataUrl` said both encoders draw through a canvas. Only web does. Mobile hands the file to `expo-image-picker` and re-encodes through the platform image APIs. The conclusion still holds, since neither route can emit an APNG, so the sentence now names both mechanisms. The 16 MiB source guard was written as a bound on decoded pixels. It bounds compressed bytes, which only stands in for pixel count, so a dense 16 MiB PNG still allocates more than the comment implied. Say that it is coarse. `EnvironmentMachineIcon` said an inline image has no fallback state because nothing loads. Nothing loads over the network, but the schema checks the PNG signature and not the pixels, so bytes a peer wrote by hand can fail to decode and draw as an empty box. The comment now records that and why an `onError` fallback is not worth state in a component that renders once per row at 12 pixels.
Comment-only change. Splits four explanations that joined clauses with a colon into separate sentences.
…a false cgroup claim An independent minimalism audit found three defects in this layer. The mobile photo pipeline had no source bound. The web helper caps source bytes and says why in its doc comment, that `createImageBitmap` decodes at full resolution so a 40 megapixel photo allocates hundreds of megabytes to draw a 64 pixel tile. `renderAsync` does the same thing natively, where the memory is scarcer. The picker reports the dimensions before any decode runs, so mobile bounds pixels directly rather than estimating them from a compressed byte count. Picking a photo never set `pending`, so the row stayed enabled across the native picker, the decode, and the encode, while the sibling write guarded itself. The settings write is now its own function and each caller opens one pending window around the work it does. The image branch was the only icon kind on mobile with no accessibility treatment. Every other branch labels itself, so this one does too. `CONTAINER_MARKER_PATHS`'s comment still claimed a bare `0::/` cgroup is itself a container signal. Commit 75f3f48000 removed that check and rewrote the function's docstring to say the opposite, because a host whose init leaves PID 1 in the root cgroup reports the same value. The constant's comment was left behind asserting the bug that commit fixed. Model: Claude Opus 5 in T3 Code.
The mobile picker carried its own copy of the machine-kind-or-role ternary at the choice list and again at the write rule, which is the rule the contracts helper now owns. Call it in both places, and drop the memoization note from the write comment since the helper states it once.
Save stayed enabled while `encodeEnvironmentIconImage` ran. Replacing an image and saving before the encode finished wrote the previous image and closed the dialog, and the new pick was lost with nothing saying so. Save now waits while an image encodes in image mode, and the hint under the button reads "Preparing image…" until the encode lands. Opening the dialog also retires any encode still running from the last time it was open, which could otherwise land its image in the fresh dialog. Mobile needs none of this. Its sheet already holds `pending` across the whole pick, so every control is disabled until the photo is ready.
Adding the Windows probe cut the shared `runProbe` timeout from 5 seconds to 1.5 seconds, which also shortened main's budget for `ioreg` and `sysctl` on macOS. A Mac whose probe took between the two would have detected nothing and drawn the generic server. Each probe now passes its own timeout. The macOS probes keep main's 5 seconds, and only the PowerShell call, the slow one boot waits on, gets 1.5 seconds.
Main replaced `Effect.catch(() => Effect.succeed(...))` with `Effect.orElseSucceed` across the server (pingdotgg#13536), including this file's other two helpers, and the Effect language service now reports the old form. The container marker check added in this branch was the one call left.
Main now opens a page per environment from Settings → Environments (pingdotgg#13302) instead of expanding the row in place. The picker still sits in the row's expanded body, which that page shows under Connection, so the steps name the page and the section. The section's opening line also says a machine has an icon rather than wears one.
The line under the image button changes from the size hint to "Preparing image…" and then to an error or back, while focus stays on the button. It had no live region, so a screen reader announced none of it. It is now a status region, as in the other settings dialogs. Reported by Copilot on #5. Claude Opus 5.5 via Claude Code
The environment icons section said an older server keeps the machine kinds. Container is a machine kind, but it has no bare-string form, so an older server cannot store it and the picker locks it there. The sentence now names the original kinds. Reported by Copilot on #5. Claude Opus 5.5 via Claude Code
The mobile picker copied three rules from web: why the picker is locked, whether the server takes the object form, and that picking the detected kind clears the override. The strings and the write rule could drift apart. They now live in @t3tools/client-runtime/environment-icon, and both pickers use them. Web adds its session-scope check on top of the shared lock. The mobile logic module keeps only the list it builds for its sheet, and the selected-id helper is inlined at its one call site. The web dialog's save() reuses canSave instead of repeating its conditions. Claude Opus 5.5 via Claude Code
Splits the Change icon sentence in the user guide so the list of choices stands alone, and says Container and the richer icons stay locked on an older server. A comment in the web image encoder now says what the catch does instead of using a figure of speech. Claude Opus 5.5 via Claude Code
The mobile Icon button checked only the server's capabilities. A switched-off environment keeps its last config, and a session without the operate scope could still open the sheet and send a write the server would refuse. The button now locks until the environment is connected, and when the session cannot change settings, with the same messages web shows. The shared lock in client-runtime now takes the session's access, so web and mobile use one function instead of web wrapping it. The sheet also marked a colored named icon as its plain row. Tapping that row stored the plain icon and dropped the color without a word. A row is now selected only for a plain pick, and the note that a pick replaces an icon set on web covers colored and Lucide icons too. Found by an independent review of the rebased stack. Claude Opus 5.5 via Claude Code
The Apple silicon table had the 2025 Mac Studio with M4 Max (Mac16,9) but not the one with M3 Ultra (Mac15,14), so the fallback model lookup read it as unknown. The container cgroup comment also said a container has no DMI, which the probe's own comment contradicts: a container usually sees its host's DMI, which is why the marker check runs first. Found by an independent review of the rebased stack. Claude Opus 5.5 via Claude Code
Two comments and a test title said every machine kind encodes to the bare string an older server accepts. Since `container` joined the detected kinds without joining the legacy list, that holds only for the seven legacy kinds, and a container pick needs the object form like a role does. Found by an independent review of the stack. Claude Opus 5.5 via Claude Code
…where On web, a switched-off or dropped machine keeps its cached config, so the Change icon item stayed enabled and the dialog closed on a save the server never got. The menu item now takes the row's connection state and locks with the same message mobile shows. On mobile, the round before made every environment row subscribe to its session to compute that lock, so opening the Environments list fetched /api/auth/session once per connected machine with no row expanded. The Icon field and its lock now live in a child of the expanded row, so only an open row subscribes. Found by an independent review of the stack. Claude Opus 5.5 via Claude Code
The lock's comment said anything beyond a plain machine kind needs the object form. container is a machine kind without a string form, so both pickers gate it behind this lock, and the comment now says legacy machine kind. The mobile list's comment named only roles as gated, and now names container as well. Found by an independent review of the stack. Claude Opus 5.5 via Claude Code
…oth clients Main now separates settings writes from operating an environment (pingdotgg#9786). Mobile still read the session by hand and checked the operate scope, so the same session could be locked on one client and open on the other. Both clients now ask `useEnvironmentScope` for `AuthSettingsWriteScope`, which is what the server checks. The shared lock takes `connected` and `canWriteSettings` booleans, so neither caller nulls the config itself, and it waits for the grant as main's web lock does. Claude Opus 5.5 via Claude Code
…ot probes run together The Windows probe reports the same SMBIOS fields Linux reads, so it now feeds `machineKindFromDmi` instead of a copy of it. That also lets Boot Camp Macs match on their model name. The probe's JSON is used as decoded, without a reshaping step. Label, machine, and launcher resolution run concurrently at boot, so PowerShell no longer delays the other two. The Linux file reads run concurrently too. The bare-cgroup reasoning is now written once. Claude Opus 5.5 via Claude Code
…r, shorter comments After the emoji, monogram, and image branches return, the named variant is all that is left, so both renderers drop their repeated kind checks and web drops a helper that only did that. The web image encoder uses one catch instead of two. The dialog's image input is a required nullable like the Lucide one, its docstring lists every mode it has, and three comments shrink to the constraint they record. Claude Opus 5.5 via Claude Code
9b25ec8 to
75fd8e1
Compare
Rebased onto main
|
What changed
Three things were still missing: an uploaded image, a way to pick on mobile, and detection for the machines left as a generic server.
data:URL. Both clients already render inline images, so there is no new route, blob store, or access check. Each client crops to a square and downscales to 64 by 64 natively before writing (a canvas on web,expo-image-manipulatoron mobile) and validates the result against the schema. The contract pins the prefix to PNG, which is what both encoders ask the canvas for. An SVG can script, and on web the declared type is what picks the decoder, so the write refuses one. The web dialog holds Save while an image encodes, so a replacement picked just before saving is not dropped in favor of the old image.AuthSettingsWriteScope), using the same lock and messages as web, and only an expanded row checks the session./.dockerenv,/run/.containerenv, or PID 1's cgroup, so a container fell through the chassis table. It now detects as the newcontainerkind, checked before both the WSL kernel and the host's DMI, since a Docker Desktop container runs on the WSL 2 kernel. PID 1's cgroup counts only when it names a runtime, such as a Docker scope, akubepodspath, orlxc. Under cgroup v2 a container with a private cgroup namespace sees a bare0::/, but so do WSL 2 and any host whose init leaves PID 1 in the root cgroup, so that value alone says nothing. Such a container is detected only through/.dockerenvor/run/.containerenv. Otherwise it falls back to the host's hardware, and a pick fixes it.containerjoins the machine kinds a server can detect, but only the seven original kinds have a bare-string wire form. A pick of it travels as the object like any role, so a server withoutenvironmentIconOverridelocks it and an older client drops it rather than choking on an unknown string.cloud.Mac16,10is a Mac mini on thehw.modelfallback path.cloudthrough eighteen markers. Naming the vendor is left alone, because it would widen the contract to add one label.environmentIconForCuratedIdat layer 4, so both pickers call it here.Why
Stacked on #4. Layer 6 of 6; the rest of the scope from the original request.
Opened on the fork because GitHub only accepts a base branch that lives in the base repository, and stacks cannot span a fork and its upstream. Layers 2 through 6 form a native stack on the fork, so merging one layer there rebases the rest. Each will be re-targeted to pingdotgg/t3code once its base merges. The entry point upstream is pingdotgg#15511.
UI changes
Verification
data:image/png;base64,…at 806 characters against a 32,768 cap, and rendered as an<img>in the row.ServerEnvironmentMachine.test.tscovers the Windows probe (success, a missing enclosure, failure, and garbage output, with the script's array wrap and JSON flags pinned), container detection by marker file and by a runtime-named cgroup path, a bare cgroup v2 root not counting as one, and detection ahead of the WSL kernel check, and the Apple silicon table.@t3tools/client-runtime/environment-icon, covered byenvironmentIcon.test.ts. MobileenvironmentIconPicker.logic.test.tscovers the choice list gating.EnvironmentIconPicker.test.tsrefuses an SVG data URL and a missing image.Two things have not run on real hardware, the mobile sheet and photo picker on a device, and the Windows probe on a Windows host. The tests mock the process runner.
An adversarial review of this layer found and fixed, before opening: picking Container was no longer capability-gated and would have failed against every released server; Docker Desktop containers were read as WSL because the kernel check ran first; the cgroup markers were dead under cgroup v2; the Windows probe held boot for up to 5 s; a null enclosure discarded a usable vendor answer; and the web downscale ran at the default low smoothing. It also found the web icon component cache keying on the 32 KB data URL, which layer 4 now rules out by keying that cache on the icon object instead.
Claude Fable 5.1 via Claude Code
Summary by Sourcery
Add image-based environment icons, mobile icon selection, and broader machine detection while maintaining compatibility with existing servers and clients.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: