diff --git a/AGENTS.md b/AGENTS.md index b4546dc..cad7372 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -43,14 +43,44 @@ workspace's `Research/` directory. They are references, not vendored code. - No telemetry is enabled by default. Any future telemetry must be documented, opt-in, redacted, and independently disableable. +## Dependencies that are not an agent's decision + +The native UI framework is settled and its dependency is pinned by a decision +record. Do not change the `gpui` (or `gpui_platform`) source, revision, or +version; do not edit `rust-toolchain.toml` or the workspace `rust-version`; do +not run `cargo update` or relax `deny.toml` to make a build succeed. A headless +test asserts the pinned source and revision on every push, so a silent bump +fails CI rather than landing. + +Changing any of them is a new decision record with a probe run at the new +revision, not a commit. If a UI build fails, the failure is the finding: report +it rather than routing around it. + +## Every crate is a workspace member + +A crate directory in neither `workspace.members` nor `workspace.exclude` +inherits no lints, is never built, and its tests never run -- silently. This has +happened twice here, and both times the code looked finished and did not +compile. Add a crate to `members` in the same change that creates it, and never +move code into an excluded directory to make a build pass. CI fails on a crate +that is in neither list. + ## Verification Before committing Rust changes, run: ```bash +export PATH="$HOME/.cargo/bin:$PATH" # a non-interactive shell has no cargo ./scripts/ci.sh ``` +`cargo` is on `PATH` only via an interactive shell profile, so an agent, an +`ssh` command, or a hook gets `cargo: command not found` without that export. +Never conclude a gate passed from an exit code obtained through a pipe -- a +pipeline reports its last command's status, which has hidden a failing gate +here already. `--headless` deliberately excludes `apps/lumbridge`, so a headless +pass is not a product pass. + Keep `bacon` running during development. Tests must use synthetic fixtures; real agent transcripts, subscription state, private source, and credentials are never test data. diff --git a/apps/lumbridge/src/main.rs b/apps/lumbridge/src/main.rs index 3b61880..16a942a 100644 --- a/apps/lumbridge/src/main.rs +++ b/apps/lumbridge/src/main.rs @@ -3743,12 +3743,12 @@ fn main() { #[cfg(test)] mod tests { + use super::CellMetrics; use super::{ KeyModifiers, TerminalEngine, TerminalEngineOptions, TerminalKey, TerminalScroll, indexed_terminal_color, terminal_dimensions_for_window, terminal_key_from_parts, terminal_paint_rows, terminal_scroll_from_parts, visible_panel_count, visible_panel_range, }; - use super::CellMetrics; use crate::sidebar::model::DEFAULT_WIDTH; use gpui::{px, size}; diff --git a/deny.toml b/deny.toml index 8a914d1..9e42d46 100644 --- a/deny.toml +++ b/deny.toml @@ -55,3 +55,15 @@ yanked = "deny" unknown-registry = "deny" unknown-git = "deny" allow-registry = ["https://github.com/rust-lang/crates.io-index"] +# Every Git source Lumbridge is allowed to build from, named one by one, so that +# adding a fourth is a decision rather than an accident. `unknown-git = "deny"` +# above is what gives this list teeth. +# +# Until now this policy was never enforced: `scripts/ci.sh` ran only +# `cargo deny check licenses`, and running `check sources` failed immediately on +# buzz-sdk. A gate that has never been executed is not a gate. +# +# block/buzz is the upstream Buzz SDK, rev-pinned in crates/lumbridge-buzz. +# It has no crates.io release; BUZZ_INTEGRATION.md records why we use its signed +# protocol semantics rather than inventing a dialect. +allow-git = ["https://github.com/block/buzz"] diff --git a/docs/decisions/0023-gpui-moves-to-a-pinned-zed-revision.md b/docs/decisions/0023-gpui-moves-to-a-pinned-zed-revision.md new file mode 100644 index 0000000..4026c64 --- /dev/null +++ b/docs/decisions/0023-gpui-moves-to-a-pinned-zed-revision.md @@ -0,0 +1,131 @@ +# 0023: The GPUI dependency moves to a pinned Zed revision, and the accessibility adapter is written before it moves + +Status: accepted. Supersedes the dependency choice in decision 0017; that +record's disposition of Floem, its account of the licence closure, and its +reasoning about accent colour all stand. + +Decision 0017 chose published `gpui 0.2.2` from crates.io over a Zed git +revision, accepted a known-unmet accessibility gate as the price, and described +the way back: "Reversing this decision means changing one dependency line and +one adapter implementation." Both halves of that sentence have since been +falsified. The published crate is not a release line that can be migrated from +later, and the adapter that was supposed to make the migration cheap was never +written. + +## The published crate is not a channel + +Measured against the crates.io API and Zed's `main` on 2026-09-01: + +- `gpui`'s newest version is `0.2.2`, published **2025-10-22**. Every non-yanked + release was published in a seventeen-day window in October 2025, and nothing + has been published in the ten months since. +- Zed's `main` still declares `version = "0.2.2"` in `crates/gpui/Cargo.toml`. + Ten months of development, no version bump: nobody upstream is preparing a + publish. +- The platform layer has since been split into `gpui_platform`, `gpui_linux`, + `gpui_macos`, `gpui_windows`, `gpui_wgpu`, `gpui_web` and `gpui_apple`. All of + them inherit `publish = false` from Zed's workspace; only `crates/gpui` sets + `publish = true`. On `main` the `x11` and `wayland` features of `gpui` itself + are now empty markers, because the backends they used to select live in + `gpui_linux`. +- Published 0.2.2 has no `accesskit` dependency at all, and + `docs.rs/gpui/0.2.2/gpui/enum.Role.html` is a 404. At `ce48461e`, + `crates/gpui/Cargo.toml:50` carries `accesskit.workspace = true`. +- Published 0.2.2 renders through `blade-graphics`. Upstream rendering has moved + to `gpui_wgpu`. + +So the choice was never "0.2.2 now, a newer published version later". Even if +Zed published tomorrow, the publishable crate no longer contains a Linux +platform backend. Staying means freezing permanently on an October-2025 +artifact, and permanently failing the hard gate in `UX_VERTICAL_SLICE.md` that +says the accessibility tree must name each pane, its selected state, its +execution target, and its waiting state. + +## A pinned revision is not an unpinned upstream + +Decision 0017's objection was "a git dependency on an unpinned upstream". That +is not what is adopted here. With a full-SHA `rev`, a committed `Cargo.lock`, +and `deny.toml` naming the permitted Git sources, a git dependency is *more* +pinned than `gpui = "0.2.2"`, which is a caret requirement that would silently +accept a `0.2.3` nobody reviewed. The real cost is that bumping the revision +becomes a deliberate scheduled act. That cost is accepted and named here rather +than avoided. + +The target revision is `ce48461eaadd16c65c31f835511ab96bd3b6e746` +(2026-08-31), which is the revision `spikes/gpui-accessibility-probe` already +pins and already proves compiles with stable accessibility IDs, workspace/pane +roles, selected state, and real `EntityInputHandler`/`ElementInputHandler` +implementations. Its toolchain is Rust 1.97.1, moving the workspace from 1.94.1. + +## The adapter comes first, and it does not exist yet + +Decision 0017 said the mitigation was "a narrow `a11y` adapter trait, introduced +now while there are few call sites", so that the migration would be "a swap +rather than a rewrite". No such trait, module, or file exists anywhere in the +workspace. The only trace is `apps/lumbridge/src/sidebar/model.rs`, where +`describe()` produces the screen-reader sentence, is tested, is called by +nothing, and carries an `#[allow(dead_code, reason = "wired up with the +accessibility adapter, decision 0017")]`. + +Meanwhile the call sites stopped being few: `apps/lumbridge/src` now builds 147 +elements and gives only 20 of them a stable identity. + +Writing the adapter is therefore the first step and happens **before** the +dependency changes, on 0.2.2, where its bodies are no-ops. That keeps the +dependency swap a swap, as 0017 intended, instead of paying for the swap and +the wiring in one unreviewable change. The `#[allow(dead_code)]` is deleted by +that step rather than carried. + +## Order, and why each step is alone + +Each stage lands separately because each fails differently, and a combined +failure is unattributable: + +1. **The `a11y` adapter, still on 0.2.2.** No-op bodies; wire the elements that + carry identity; give `describe()` a caller. +2. **The toolchain, still on 0.2.2.** Move `rust-toolchain.toml`, the workspace + `rust-version`, and the toolchain named in `.gitea/workflows/ci.yml` — which + hardcodes it twice rather than reading the file — to 1.97.1. Three minor + versions of new `clippy::pedantic` lints arrive as hard errors under + `-D warnings`. Isolating this stage is what stops a lint avalanche being + mistaken for a GPUI API break. +3. **`deny.toml`.** Add `zed-industries/zed` and `zed-industries/font-kit` to + `allow-git`. The second is not optional: `gpui_wgpu` reaches `font-kit` + through a Git source, so the move brings two Git sources, not one. +4. **The dependency swap.** `gpui` and `gpui_platform` at the pinned rev; + `wayland`/`x11` requested on `gpui_platform`, where they still mean + something; and `Application::new()` — which does not exist at that revision — + becomes `gpui_platform::application()`. +5. **Real semantics in the adapter**, and the probe's AccessKit tree test ported + into `apps/lumbridge`. +6. **The probe changes job**: it stops proving AccessKit exists and starts + proving the next revision still compiles. + +The renderer swap from blade to wgpu in stage 4 is the least characterised risk. +`scripts/native-libs.sh` addresses one missing `libxkbcommon-x11` symlink and +says nothing about a Vulkan or GL loader, and the Gitea job installs no wgpu +dependency. Expect to learn something here. + +## What is knowingly still unmet + +A passing unit test over an AccessKit `TreeUpdate` is not an assistive-technology +claim. The gate is not met until AT-SPI/Orca on Linux and VoiceOver on macOS +have been driven by hand. Stage 5 makes the semantics real; it does not close +the gate, and nothing in this record should be read as closing it. + +## Enforcement + +This record is not self-enforcing, and the previous one was not obeyed. Three +mechanical checks carry it instead: + +- `scripts/workspace-guard.sh` asserts the gpui source and version in + `Cargo.lock` — the lockfile, because a manifest states an intent while the + lockfile states what would actually compile. It runs in the fast headless job, + which is not `continue-on-error`, so it gates every push. +- `deny.toml`'s `unknown-git = "deny"` plus an explicit `allow-git` list makes a + fourth Git source a CI failure. `scripts/ci.sh` now actually runs + `cargo deny check sources`; it previously ran only `check licenses`, and the + source policy had never once been executed. +- `AGENTS.md` names the gpui dependency, `rust-toolchain.toml`, the workspace + `rust-version`, and `deny.toml` as things a contributor or agent may not + change without a decision record. diff --git a/scripts/ci.sh b/scripts/ci.sh index 7fc368c..f6b295b 100755 --- a/scripts/ci.sh +++ b/scripts/ci.sh @@ -25,20 +25,43 @@ esac # headless by default rather than silently joining the slow job. ui_packages=(lumbridge lumbridge-ui-fixture) +# In CI a missing tool is a failure, not a warning. Locally it stays a warning so +# a contributor without cargo-deny is not blocked. The difference matters: the +# licence closure DISTRIBUTION.md depends on was, until this flag existed, being +# skipped on a runner that had never installed cargo-deny -- and reported green. +# Set LUMBRIDGE_CI_STRICT=1 (the workflow does) to require every gate to run. +strict="${LUMBRIDGE_CI_STRICT:-0}" + +require_tool() { + command -v "$1" >/dev/null 2>&1 && return 0 + if [ "$strict" = 1 ]; then + echo "$1 is required when LUMBRIDGE_CI_STRICT=1 and is not installed" >&2 + exit 1 + fi + return 1 +} + +# Structural gates that need no compiler: crate membership, and the pinned UI +# dependency. First, because they cost milliseconds and catch the failure that +# makes every later gate meaningless -- code that is silently not being built. +"$(dirname "$0")/workspace-guard.sh" + cargo fmt --all --check -# The product's licence closure. Graduating GPUI into the workspace made this -# the product's problem rather than a spike's; deny.toml records which licences -# were reviewed and why. Skipped rather than failed when the tool is absent, so -# a contributor without it is not blocked. -if command -v cargo-deny >/dev/null 2>&1; then - cargo deny check licenses +# The product's licence closure and its source allowlist. Graduating GPUI into +# the workspace made both the product's problem rather than a spike's; deny.toml +# records which licences were reviewed and which Git sources are permitted. +# +# `sources` is checked here for the first time. It had been declared in deny.toml +# and never run, which is why nobody noticed it was failing. +if require_tool cargo-deny; then + cargo deny check licenses sources else - echo "cargo-deny not installed; skipping the licence gate (cargo install cargo-deny)" >&2 + echo "cargo-deny not installed; skipping the licence and source gates (cargo install cargo-deny)" >&2 fi test_runner() { - if command -v cargo-nextest >/dev/null 2>&1; then + if require_tool cargo-nextest; then cargo nextest run "$@" --all-features --profile "${NEXTEST_PROFILE:-default}" else echo "cargo-nextest not installed; using cargo test" >&2 diff --git a/scripts/workspace-guard.sh b/scripts/workspace-guard.sh new file mode 100755 index 0000000..81e6874 --- /dev/null +++ b/scripts/workspace-guard.sh @@ -0,0 +1,80 @@ +#!/usr/bin/env bash +# Structural gates that need no compiler, so they run first and finish instantly. +# +# Both of these exist because the repository has already been wrong in exactly +# these two ways, and in both cases the mistake was invisible: the build stayed +# green while the code was not being built. +# +# scripts/workspace-guard.sh +set -euo pipefail + +root="$(cd "$(dirname "$0")/.." && pwd)" +cd "$root" +status=0 + +# --------------------------------------------------------------------------- +# 1. Every crate is a member or is explicitly excluded. +# +# A crate directory named in neither list is not a build error. Cargo simply +# never looks at it: no lints, no tests, no compilation. lumbridge-devices sat +# that way with 1,127 lines, 19 tests that had never run, and a `mod` statement +# pointing at a file that did not exist. Decision 0017 records the same failure +# a month earlier with the GPUI shell in `spikes/`. Neither was caught by a +# human reading a diff, so it is caught here instead. +members="$(sed -n '/^members = \[/,/^]/p' Cargo.toml)" +excludes="$(sed -n '/^exclude = \[/,/^]/p' Cargo.toml)" +# A single-line `exclude = [...]` does not match the range above; catch it too. +excludes="$excludes$(grep -E '^exclude = \[.*\]' Cargo.toml || true)" + +while IFS= read -r manifest; do + dir="$(dirname "$manifest")" + dir="${dir#./}" + # A crate carrying its own [workspace] table is deliberately independent. + if grep -qE '^\[workspace\]' "$manifest"; then + continue + fi + if printf '%s' "$members" | grep -qF "\"$dir\""; then + continue + fi + if printf '%s' "$excludes" | grep -qF "\"$dir\""; then + continue + fi + echo "workspace-guard: $dir/Cargo.toml is in neither workspace.members nor workspace.exclude." >&2 + echo " Cargo will never build it, lint it, or run its tests. See AGENTS.md." >&2 + status=1 +done < <(find apps crates tools -mindepth 2 -maxdepth 2 -name Cargo.toml 2>/dev/null | sort) + +# --------------------------------------------------------------------------- +# 2. The GPUI dependency is the one the decision record chose. +# +# Checked against Cargo.lock rather than the manifest, because the manifest +# states an intent and the lockfile states what would actually be compiled -- +# and a caret requirement in the manifest silently accepts a version nobody +# decided on. Asserted in the fast headless job so it gates every push, and not +# in the UI job, which is `continue-on-error` and therefore cannot fail one. +# +# Changing this line means writing a decision record. See AGENTS.md, +# "Dependencies that are not an agent's decision". +expected_gpui_source='registry+https://github.com/rust-lang/crates.io-index' +expected_gpui_version='0.2.2' + +gpui_block="$(awk '/^name = "gpui"$/{found=1} found{print} found&&/^$/{exit}' Cargo.lock)" +if [ -z "$gpui_block" ]; then + echo "workspace-guard: no gpui package in Cargo.lock; the shell cannot build." >&2 + status=1 +else + actual_version="$(printf '%s' "$gpui_block" | sed -n 's/^version = "\(.*\)"$/\1/p')" + actual_source="$(printf '%s' "$gpui_block" | sed -n 's/^source = "\(.*\)"$/\1/p')" + if [ "$actual_version" != "$expected_gpui_version" ] || [ "$actual_source" != "$expected_gpui_source" ]; then + echo "workspace-guard: the gpui dependency is not the one the decision record chose." >&2 + echo " expected $expected_gpui_version from $expected_gpui_source" >&2 + echo " found $actual_version from $actual_source" >&2 + echo " A bump is a decision record, not a commit. See AGENTS.md." >&2 + status=1 + fi +fi + +if [ "$status" = 0 ]; then + echo "workspace-guard: crate membership and the pinned UI dependency are as recorded." +fi +exit "$status" diff --git a/skills/lumbridge-development/SKILL.md b/skills/lumbridge-development/SKILL.md index e4d0907..5eea4fb 100644 --- a/skills/lumbridge-development/SKILL.md +++ b/skills/lumbridge-development/SKILL.md @@ -27,20 +27,61 @@ Preserve these product boundaries: not cross-platform proof. - Usage and quota values always retain provenance and uncertainty. +## The UI framework is decided + +GPUI graduated out of `spikes/` into `apps/lumbridge` and is part of the product +workspace, its lints, and its licence closure. `spikes/floem-shell` is frozen +evidence at the revision the comparison was made; it is not maintained in parity +and is not a candidate. Do not reopen the comparison, do not add Floem to the +workspace, and do not read `UI_SPIKE_SCORECARD.md`'s provisional wording as a +live question. See `docs/decisions/0017-*` and any record superseding it. + +## The dependencies that are not an agent's decision + +Do not change the `gpui` (or `gpui_platform`) source, revision, or version. Do +not edit `rust-toolchain.toml` or the workspace `rust-version`. Do not run +`cargo update`, or relax `deny.toml`, to make a UI build succeed. Each of these +is fixed by a decision record and checked mechanically in the fast headless CI +job; a change to any of them requires a new decision record, not a commit. + +If a UI build fails, the failure is the finding. Report it. + +## Every crate is a workspace member + +A crate directory that appears in neither `workspace.members` nor +`workspace.exclude` inherits no lints, is not built, and its tests never run -- +silently. This has happened twice in this repository, and both times the code +looked finished and was not. When you add a crate, add it to `members` in the +same change, and never move code into an excluded directory to make a build +pass. + ## Development loop -Before editing, inspect Git status and preserve unrelated work. Use: +Before editing, inspect Git status and preserve unrelated work. + +`cargo` is installed at `~/.cargo/bin` and is put on `PATH` by an interactive +shell profile. A non-interactive shell -- which is what an agent, an `ssh` +command, and a hook all get -- does not source that profile and will fail with +`cargo: command not found`. Export it explicitly: ```bash -bacon -cargo xtest -./scripts/ci.sh +export PATH="$HOME/.cargo/bin:$PATH" ``` -Root CI must remain independent from experimental UI dependencies. GPUI and -Floem live in separate workspaces under `spikes/` and consume the same -`ui-shell-model` fixture. Record only observed results in the scorecard; do not -select a framework until both pass the hard gates. +Then: + +```bash +bacon # continuous check; t tests, c lints, v the full gate +cargo xtest # fast isolated tests with nextest +./scripts/ci.sh # format + strict Clippy + tests + doctests + cargo deny +``` + +`scripts/ci.sh --headless` skips the GPUI crates and finishes in seconds; +`--ui` runs only them. A headless pass is not a product pass: it deliberately +excludes `apps/lumbridge`. Never report the gate as green from `--headless` +alone, and never conclude a gate passed from an exit code you obtained through +a pipe -- a pipeline reports the exit status of its last command, which has +already hidden a failing gate here once. For Buzz integration, use its Apache-2.0 Rust SDK and signed protocol semantics rather than inventing a webhook dialect. Test against a local relay and fixture @@ -48,6 +89,6 @@ identity. Pane screenshots or transcripts require a visible preview, redaction, explicit destination, and user confirmation before upload. Add tests at the lowest deterministic layer first, then platform or rendered -tests where behavior crosses a real boundary. Before handoff, run the relevant -spike build plus `./scripts/ci.sh`, `git diff --check`, and report anything not -validated on all target systems. +tests where behavior crosses a real boundary. Before handoff, run +`./scripts/ci.sh`, `git diff --check`, and report anything not validated on all +target systems.