diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ac083824..d23ca39e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,3 +38,21 @@ jobs: - name: Lint, suite and gates run: bash tools/ci.sh "$GITHUB_WORKSPACE" + + # The end-to-end tier (3.5.0-DEV21): the same scenarios `make e2e` runs under + # podman, here directly in the job's fresh container, sharing one system. A + # `pre-release` prerequisite outside GATES, so listed in the Makefile's + # CI_JOBS; tests/test_ci_tooling.py holds the two to each other. + e2e: + runs-on: ubuntu-latest + container: docker.io/library/archlinux:base-devel + steps: + - name: Install git and make + run: pacman -Syu --noconfirm --needed git make + + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - name: End-to-end scenarios + run: make e2e E2E_HERE=1 diff --git a/DESIGN.md b/DESIGN.md index 1df53af3..3b66f6c8 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -1388,16 +1388,16 @@ spelling per meaning, across every verb: **Result types** (`PreCheckResult` and `ExecResult`) are plain dataclasses with `ctx` / `artifacts` dicts; the runner does not inspect their contents. This keeps phase boundaries loose enough for ad-hoc data flow within a verb without inventing a per-verb context class. -**Sentinel handling.** Verbs whose `execute` mutates the live system set `requires_sentinel = True`. The runner wraps `execute + post_validate` in `sentinel_scope(state_dir, verb.name, recovery=…, retry_cmd=…, **metadata)` (`recovery` is a `stage_sentinel.Recovery` from `verb.sentinel_recovery`, stored as a kind plus arguments, `3.5.0-B43`) from `primitives/stage_sentinel.py`. On entry, the sentinel writes `stage_in_progress.toml`; on normal completion (both phases pass), it clears. On `RuntimeError` or `CleanExitRequested`, the sentinel is left in place so the next sysforge invocation blocks at the CLI-entry recovery prompt. `sentinel_scope` also installs an `InterruptScope`, so verbs participate in the same first-Ctrl-C-defers-to-safe-boundary behaviour as the toolchain stage: `build` and `update` stop before their next package (`interrupt.checkpoint()` in `build_core`), and a verb with no checkpoint stops when its `execute` + `post_validate` complete (`3.5.0-B44`). It additionally holds the `stage_in_progress.lock` liveness lock for the scope: if another run already holds it, entering the scope raises `RuntimeError` **before** `mark_started`, which the runner's existing handler converts to exit 1 without clobbering the live run's sentinel (§Pipeline layer, *Liveness guard*). The toolchain pipeline stage uses the same primitive — there is one implementation, shared. +**Sentinel handling.** Verbs whose `execute` mutates the live system set `requires_sentinel = True`. The runner wraps `execute + post_validate` in `sentinel_scope(state_dir, verb.name, recovery=…, retry_cmd=…, **metadata)` (`recovery` is a `stage_sentinel.Recovery` from `verb.sentinel_recovery`, stored as a kind plus arguments, `3.5.0-B43`) from `primitives/stage_sentinel.py`, with `arm_on_mutation=True` (`3.5.0-B138`): the scope takes the liveness lock on entry but writes `stage_in_progress.toml` only at the first escalation that changes the system, which `privilege.privileged_argv` announces to `privilege.on_mutation` listeners (§Privilege seam). A verb's `execute` does its checks first (`build` probes for `pkgctl`, resolves targets, builds), and a failure there leaves nothing for the next run to refuse on; before the fix it left `stage = "build"` behind and blocked a timer or cron run. On normal completion (both phases pass), the record clears. On `RuntimeError` or `CleanExitRequested` after the record was written, it is left in place so the next sysforge invocation blocks at the CLI-entry recovery prompt; a Ctrl-C before it says nothing changed. The pipeline stages keep the default (record on entry), since they open their scope around the install itself. `sentinel_scope` also installs an `InterruptScope`, so verbs participate in the same first-Ctrl-C-defers-to-safe-boundary behaviour as the toolchain stage: `build` and `update` stop before their next package (`interrupt.checkpoint()` in `build_core`), and a verb with no checkpoint stops when its `execute` + `post_validate` complete (`3.5.0-B44`). It additionally holds the `stage_in_progress.lock` liveness lock for the scope: if another run already holds it, entering the scope raises `RuntimeError` **before** `mark_started`, which the runner's existing handler converts to exit 1 without clobbering the live run's sentinel (§Pipeline layer, *Liveness guard*). The toolchain pipeline stage uses the same primitive — there is one implementation, shared. **Read-only verbs** (`env`, `help`, `resolve`, `log`, `config check`, `state list`, `state orphans` without `--delete`, `state failed` without `--clear`/`--clear-all`, `state profiles` without `--delete`, `state boot-entries`, `bench toolchain`, `packages list`, `doctor` without `--apply`) implement `execute` (the work is printing) and return `ExecResult()`; `post_validate` defaults to no-op and `requires_sentinel = False`. They use the same dispatch path as mutating verbs — no second code path. **`state boot-entries`** (`3.3.0-F5`, `StateBootEntriesVerb`, read-only, `requires_sentinel = False`) lists every `vmlinuz-*` in `/boot` with the loader entry that boots it. The table goes through the pager like `state profiles` (`--no-pager` disables it): columns KERNEL / ENTRY / OWNER (`you`, `sysforge` or `none`) / TEMPLATE, plus `(missing)` rows for managed entries whose kernel is gone. Under `bootloader = grub`/`none` it prints a one-line note instead; with `boot_entries = "off"` it notes that and still lists. Rows come from `boot_entries.rows_for`. **Error model.** -- `RuntimeError` raised from any phase → `_log.error(msg)`, return 1. Sentinel preserved if active. +- `RuntimeError` raised from any phase → `_log.error(msg)`, return 1. Sentinel preserved if written (a change had begun). - `SystemExit` → propagate verbatim (lets tests and signal handlers see the raw exit). -- `CleanExitRequested` → caught inside `sentinel_scope`, logged, re-raised as `RuntimeError` with the verb's `retry_cmd` and recovery command in the message; sentinel preserved. +- `CleanExitRequested` → caught inside `sentinel_scope`, logged, re-raised as `RuntimeError` with the verb's `retry_cmd` and recovery command in the message; sentinel preserved. Before the first change, the message says nothing changed and no record is left. **Per-verb phase mapping** (current shape; phases reflect where each piece of work lives, not which primitives it calls): @@ -1920,6 +1920,8 @@ Bottom-anchored status line for batch operations (`[3/10] building · htop`). Du - **When the bar repaints** — `_draw()` is the sole writer. It builds the whole frame (any pending resize, region establishment or release, and the painted row, glyph-downgraded then width-truncated) as one string and emits it with a **single write**, so another thread's log line can land before or after a frame but never inside it; an unchanged frame writes nothing. A module lock guards mutate-then-draw in every public entry point and the ticker, and is never held across a blocking call or a yield body. `SIGWINCH` only sets `resize_pending` (no I/O, no lock — a signal can interrupt a paint holding both); the next frame applies it. **A resize release erases from the content cursor down (`ESC[J`), never the old absolute row** (`3.5.0-B4`): a taller window makes the emulator pull scrollback in and shift everything, cursor and bar alike, down by the rows gained, so the old row then holds a content line, and establishment's index would scroll the stale bar into the output. Order is what a resize preserves, and the bar is always below the cursor. The emulator also resets DECSTBM on resize, so `pty_runner` repaints before forwarding the first chunk after a `SIGWINCH` instead of waiting out its one-second throttle. Events (tick, phase, yield exit) draw immediately; **exactly one scheduled painter** keeps the clock moving between them, chosen by whether a raw byte stream is being forwarded to the terminal. Single-write frames are safe against line-oriented writers (`log.*`, `print`, TTY-inheriting pacman/git), but a raw forwarder can be mid-escape or mid-UTF-8. By default the painter is a daemon **ticker** thread, started on the first TTY draw and stopped by `shutdown()`/`clear()`, which redraws every second unless yielded or forwarding; exceptions are swallowed and traced. While `pty_runner` forwards makepkg's raw output it enters `forwarding_output()` and becomes the painter itself, calling `refresh()` at most once a second and only at a stream boundary (§`pty_runner.py`). Cost: one wakeup a second, and about one short write a second only while a tracker is past the elapsed floor. **`heartbeat(detail)`** (`3.2.0-B13`) only sets `detail` — the scheduled painter shows it. It exists because a whole package build sits inside a single `tick()`: a trace once showed a 379-second gap between paints. The detail replaces rather than appends and the next tick/note/resume/phase clears it. - **Lifecycle** — `cli.main` wraps `_main()` in `progress.session()`, whose exit calls `shutdown()` (stop the ticker, release the region). The session exits before the `KeyboardInterrupt`/`BrokenPipeError` handlers, so the region is gone before `aborted (Ctrl-C)` prints, and a plain `sys.exit` or `log.fatal` releases on the way out too. `log` does not import `ui`. The `atexit` hook remains as the backstop for importers that don't go through `cli.main`; `shutdown()` is idempotent. +**Tested on a terminal model** (`3.5.0-DEV22`). `tests/test_progress.py` checks the sequences the bar writes; `tests/test_terminal_model.py` replays them, and `update`'s `_TERMINAL_RESET`, through `tests/term_model.py`, a VT model of the sequences sysforge emits that raises on any other, and asserts what the screen holds: the command line survives, output follows it in order, the bar leaves nothing behind, the next prompt lands after the last line. Each runs under both rules terminals have for a saved cursor (DECSC) across a scroll: xterm keeps the screen row, xterm.js (VS Code's terminal, vhs) follows the content line. `3.5.0-B132` and `B133` both turned on that difference, and each has a test replaying its pre-fix sequence to show the model reproduces it. A new escape sequence in the bar is taught to the model in the same change. + **One tracker at a time.** A tracker opened while another is open is a no-op that yields `progress_hooks._NoOpTick` and logs at debug, leaving the outer untouched; under tests (`_strict_nesting`, set by an autouse `tests/conftest.py` fixture that also gives every test a fresh `_bar` and no ticker) it raises `RuntimeError("tracker '' opened inside ''")`. A sub-step inside an item uses `tick.note()`/`tick.resume()` instead. The functions that open trackers call `require_no_tracker(owner)` (same strict/lenient policy) first thing, so a mis-nested caller fails before any sync or resolution side effect — the build-⊂-update invariant invites routing more callers through `build_and_install`. A stale `tick` after its tracker exits is a no-op. Tracker owners: | Tracker | Owner | Runs under | @@ -4101,6 +4103,20 @@ the gates — they need a booted VM or a working podman and network, neither of prerequisite of a lint run — but a minor or major release that skips them is untested against a real install. See `tools/vm/README.md` and `tools/container/README.md`. +The **end-to-end tier** (`make e2e`, `tools/e2e/`, `3.5.0-DEV21`) runs the documented paths the way +a new user meets them: the Quick start in one login session, a repo package's opt-in, flag drift +after a profile change, a failed build's cleanup. Each runs as an unprivileged user in a fresh +container built from the demo image (`tools/demo/`, shared with `make demo-gif`), asserting on exit +codes, `pacman -Qi`, the state files and leftover sentinels, with prompts answered on a +pseudo-terminal. It covers what the unit suite stubs: root versus the user, group membership that +applies only at the next login, the state one command leaves for the next. It builds sysforge from +the checkout itself, so it needs no `vm-pkg-stable`. `pre-release` runs it; the release preflight +does not (`_RELEASE_GATE_EXEMPT`: minutes long, podman, and one networked scenario, so not +deterministic). CI runs the same scenarios in an `e2e` job of its own, directly in the job's +container (`make e2e E2E_HERE=1`), listed in the Makefile's `CI_JOBS` beside `GATES`. A scenario +marked `xfail: ` fails until that item is fixed and reports `XFAIL`; once it passes it fails +the run, so the marker comes off with the fix. + **CI** (`.github/workflows/ci.yml`, `3.4.0-DEV1`) runs every `pre-release` gate on each pull request (a newer push to it cancels the superseded run) and on each push to `main`, in a fresh `archlinux:base-devel` container, as an unprivileged user, with only the core dev tier (`make dev-deps-core`) installed. Its job is @@ -4363,7 +4379,7 @@ subsection is only presentation. | 24 | Deprecation registry (declared removal version + warn-on-use) | Every config key, state token, CLI flag, or path sysforge still honours only for backwards compatibility | enforced | `primitives/deprecations.py` is the single home: each record carries `surface`/`kind`/`function`/`deprecated_in`/`removed_in`/`replacement`, and each compat read path calls `warn_used(surface)` so the warning text is built from the record and cannot drift from the version the gate enforces. Two `function` values, because removal is not uniformly breaking — a `compat` surface still works (removing it is breaking, so `removed_in` must be `X.0.0`, and presence is proven by its `warn_used` call sites), while a `shim` already fails and is kept only to name its replacement (removal is not breaking, `removed_in` may be `X.Y.0`, presence proven by an exact repo-relative `anchor`). `check_standards` `deprecations` group: registry↔call-site bijection both ways, major-only removal for `compat`, and an error when the release target is at or past a declared `removed_in` with the surface still present (`--target-version`); a registry the gate cannot read is itself an error, because a check that cannot fail is worse than no check; one written as an empty tuple (`_REGISTRY = ()`) is accepted, since that is what removing the last record leaves (`3.5.0-F16`). The release-note removal parity check matches the registry's literal surface string against the `## Removed` section body, so a removal note must spell the canonical identifier (e.g. `build_state.build_mode=profiled`), not just prose describing it. `tests/test_deprecations.py` (behavioural, incl. once-per-run dedup and per-surface resolution) + `tests/test_standards_compliance.py`. **First application:** the 2.6.1-F5 sweep removed all five `compat` surfaces the registry catalogued at `2.6.1-STD2` shipping time, proving the major-only removal gate end to end. The `shim` half was proven the same way at `3.0.0-STD3`: `doctor.flat_flags` declared `removed_in = 3.1.0`, and the release-target gate refused the 3.1.0 release until the hint table, its `cli.py` hook and the record itself were deleted — a scheduled removal the gate enforced rather than a reviewer remembering. The registry is empty as of 4.0.0: its last record, the `compat` `profiles.build_mode=patched_pkgbuild` (`2.6.1-STD3`), was removed on schedule (`3.5.0-F16`), and `normalize_build_mode` now refuses the token. That record predated the registry and `2.6.1-STD2` missed it, because the bijection walks registry→call-site and never code→registry: **an unregistered compat surface is invisible to the gate by construction.** A catalogued-empty `compat` half is therefore never proof that none exist, and finding the next one is a review obligation, not a tooling guarantee — a code→registry direction is unimplemented (no reliable static signal distinguishes a compat alias table from any other lookup dict). Its `removed_in` was `4.0.0` rather than `3.0.0` because 3.0.0 shipped with the surface present, and the gate refuses a release at or past a declared removal with the surface still live. | | 25 | Log-level rubric (UI / ERROR / WARN / INFO / DEBUG) | Every call site that emits user-visible output | followed | `docs/design/12-logging.md` is the single home: the five-level table (fn, gate, reserved-for), the decision test that resolves most call sites (*is this the answer, or narration about producing the answer?*), and the explicit anti-pattern that `ui()` is not a "make this always show up" escape hatch — `ui()` is the answer the user ran the command to see, `info()`/`warn()` are narration and are gated behind `-vv`/`-v`. Enforcement is deliberately partial and the row does not claim otherwise. The one mechanical guard is the `quiet_at_default` fixture (`tests/conftest.py`), a golden-output regression that runs a stage at the shipped default (`verbosity=0`) and again at `-vv`, and fails if the default run emitted an `[INFO]` or `[WARN]` line. That catches the failure mode that matters — narration leaking into always-visible output — but nothing checks WARN-vs-INFO or INFO-vs-DEBUG classification, which stays a review obligation. Coverage is per-stage opt-in: the fixture is anchored on `packages` (`_load_packages`, `tests/test_stage_packages.py`) and `configure`'s dry run (`tests/test_stage_bootstrap.py`). A stage added without requesting the fixture is unguarded by construction, so **coverage thins with every stage that does not opt in** — requesting it is part of adding a stage, not a later cleanup. | | 26 | Lint scope (every Python tree in the repo) | `sysforge/`, `tests/`, and `tools/` — all Python the repo owns, not only the shipped package | enforced | `make lint-py` runs `ruff check sysforge/ tests/ tools/`, and `.claude/hooks/ruff-on-edit.sh` blocks on the same three trees. The two must name the identical set: before 2.6.1-STD8 the gate covered `sysforge/` alone while the hook already blocked on `tests/`, so 189 violations accumulated in a tree nothing reported on and the hook fired on pre-existing debt in files an author had merely touched — enforcement strict enough to interrupt, scoped so it could only ever surface someone else's backlog. Rule selection is uniform (`[tool.ruff.lint] select`); the only per-tree relaxation is `[tool.ruff.lint.per-file-ignores]` `"tests/**" = ["S", "PTH", "SIM117"]`, each with a stated reason — `S` because test code is not production attack surface (2.3.0-F1), `PTH` because it fires on the `sys.path` import bootstrap repeated across ~25 test files, `SIM117` because collapsing stacked monkeypatch/mock context managers costs a line-length violation per test and reads worse. A new relaxation is a per-file-ignores entry with a comment, never a directory dropped from the gate. Scope note: `tools/` is deliberately **not** relaxed — it holds `check_standards.py`, `check_shipped.py`, and `release.sh`'s Python siblings, which the release process depends on. | -| 27 | Release-gate completeness | Static-analysis gates of `make pre-release` vs the `tools/release.sh` preflight | enforced | Every `pre-release` prerequisite except the full `test` suite (the heavier tier, by design) is also run by the release preflight, which fails the release on it. `pre-release` is advisory and the script is the gate a release cannot skip, so a check only the former runs is one a tag can ship without: `v3.2.0` shipped four pyright errors — one a live crash (`3.2.0-B30`) the type-checker had named weeks earlier — and the 3.3.0 cycle reached eighteen (`3.2.0-STD1`). `check_standards` `release_gates` group diffs the `pre-release:` prerequisite list against the targets `release.sh` invokes, so `docs/RELEASE-CHECKLIST.md`'s gate table cannot silently diverge. A new `pre-release` gate is added to the script in the same change, or to `_RELEASE_GATE_EXEMPT` with its reason. Because the script *blocks*, each gate must also be deterministic: a `uv run --with` overlay in a gate's recipe is pinned (`PYRIGHT_VERSION`, `REUSE_VERSION` in the Makefile header), since an unpinned overlay resolves the newest release on every run and a tool update alone could turn a release red; the same group flags an unpinned `--with` in any gate recipe. Pins are bumped deliberately in their own commit, and the header keeps the no-`[dependency-groups] dev` rule (the overlays stay the one record). Known residue: `lint` runs the pacman-provided `ruff`, which is kept on `PATH` for editor/hook integration and so follows system upgrades. | +| 27 | Release-gate completeness | Static-analysis gates of `make pre-release` vs the `tools/release.sh` preflight | enforced | Every `pre-release` prerequisite except the full `test` suite and the `e2e` tier (the heavier tiers, by design; `e2e` also needs podman and the network, so it could not be deterministic) is also run by the release preflight, which fails the release on it. `pre-release` is advisory and the script is the gate a release cannot skip, so a check only the former runs is one a tag can ship without: `v3.2.0` shipped four pyright errors — one a live crash (`3.2.0-B30`) the type-checker had named weeks earlier — and the 3.3.0 cycle reached eighteen (`3.2.0-STD1`). `check_standards` `release_gates` group diffs the `pre-release:` prerequisite list against the targets `release.sh` invokes, so `docs/RELEASE-CHECKLIST.md`'s gate table cannot silently diverge. A new `pre-release` gate is added to the script in the same change, or to `_RELEASE_GATE_EXEMPT` with its reason. Because the script *blocks*, each gate must also be deterministic: a `uv run --with` overlay in a gate's recipe is pinned (`PYRIGHT_VERSION`, `REUSE_VERSION` in the Makefile header), since an unpinned overlay resolves the newest release on every run and a tool update alone could turn a release red; the same group flags an unpinned `--with` in any gate recipe. Pins are bumped deliberately in their own commit, and the header keeps the no-`[dependency-groups] dev` rule (the overlays stay the one record). Known residue: `lint` runs the pacman-provided `ruff`, which is kept on `PATH` for editor/hook integration and so follows system upgrades. | | 30 | No em-dashes in user-facing prose | `README.md`, `docs/guides/`, `docs/concepts/` (`3.5.0-DOC3`), the man page (`man/sysforge.1.scd.in` and the generated `man/sysforge.1`), `completions/`, and every non-docstring string literal under `sysforge/`: `--help` text, log and prompt messages, exception text, and the headers of files sysforge generates | enforced | Rewrite with a comma, colon, semicolon, period or parentheses as the sentence needs, never a blind substitution (`3.4.0-DOC2`). `check_standards` `em_dash` group: a line scan of the files and an AST scan of every string constant under `sysforge/`, so implicit concatenation and f-string pieces are covered without importing `sysforge`. Exempt: docstrings (developer prose), a literal that is only the glyph (an empty-cell placeholder, or a key in `log`'s ASCII glyph-fallback table), ROADMAP and release-note entries, whose ``- **`` — .**`` lead is a lint-enforced format (changing it is its own `STD`), internal docs, shipped-config and hook comments, and code comments. `tests/test_standards_compliance.py` (`test_em_dash_*`). | | 31 | Subprocess goes through `primitives/run` | Every external command `sysforge/` runs | enforced | Numbered 28 until `3.4.0-STD2`, when it was found sharing that number with the `pam.d(5)` row, which took 28 first and keeps it. ruff `banned-api` (TID251, `pyproject.toml`) bans `subprocess.run`/`Popen`/`call`/`check_call`/`check_output` outside the seam modules `primitives/run.py`, `pty_runner.py`, `privilege.py` and `sudo_session.py` (`3.2.0-F13`). Probes use `run.capture` (missing binary → `None`) or `run.probe` (missing binary → status 127), raise-on-failure calls use `run_or_raise`, and escalation uses the §22 privilege seam. A call that legitimately stays raw (inherits the TTY, streams, needs its own `preexec_fn`, or is a privileged command whose status or `CalledProcessError` its caller inspects) carries `# noqa: TID251 — <why>`, and the reason is mandatory. Enforced by the ruff rule plus `tests/test_standards_compliance.py` (`test_subprocess_fence_*`), which pins the banned set, the exemption list and the per-call reasons. Complements row 17, which governs argv form; this row governs *where* the call is made. | | 32 | Machine-readable output contract (`--json`) | `state list --json`, `doctor` (`system`, `pkg`, bare) `--json`, `update --dry-run --json`, `config check --json` (`3.5.0-F18`) | enforced | `primitives/json_out.py` is the one home (`3.4.0-F7`). Stdout carries exactly one JSON document ([RFC 8259](https://www.rfc-editor.org/rfc/rfc8259)) and nothing else: `cli.main` enters `json_out.reserved_stdout()` right after parsing and `verbs.runner.run_verb` enters it again (re-entrant), so every `ui()`/`print()`, startup logs and progress phases included, goes to stderr; the document goes out through `json_out.emit`. Every document has a top-level `"schema"` integer (`SCHEMA_VERSION`, now 1) and a `"kind"` (`state-list`, `doctor`, `update-dry-run`). **Adding a field is a minor change; renaming or removing one, or changing its type, is a major change and bumps `SCHEMA_VERSION`.** Each document is built from the result objects the human renderer reads (`AxisResult` via `diagnostics.axis_document`, `run.results` and the drift lists via `update._dry_run_document`, build_state rows through an explicit field list in `state_cmd._JSON_ENTRY_FIELDS`), so the two outputs cannot disagree. Exit codes are unchanged by `--json`. Mutating runs are out of scope: `update --json` without `--dry-run` and `doctor pkg --apply --json` are refused (exit 2). The schemas are committed as JSON Schema draft 2020-12 under `tests/data/json-schema/`; `tests/test_json_output.py` validates each verb's document against its schema (a subset validator, so the suite needs no new dependency), checks that stdout parses as one document with `log` in dry-run mode at `-vv`, and that a doctor skip appears under `roster.skipped`, never as a finding. A new field updates its schema in the same change. | @@ -4537,23 +4553,23 @@ branch, no hand-rolled `["sudo", ...]` list. ### Two entry points -- **`privileged_argv(argv, *, noninteractive=False) -> list[str]`** — builds +- **`privileged_argv(argv, *, noninteractive=False, mutates=True) -> list[str]`** — builds the escalated argv and hands it back; the caller runs it with its own `subprocess.run` (or equivalent). Use this at sites that need to stream output to the TTY, inspect the return code themselves, or otherwise need control over how the command executes. When already root (`euid == 0`) the argv is returned unchanged (no `sudo` prefix); `noninteractive=True` inserts `-n` so `sudo` fails fast instead of prompting, when not already root. -- **`run_privileged(argv, *, tag, **kwargs) -> subprocess.CompletedProcess`** — +- **`run_privileged(argv, *, tag, mutates=True, **kwargs) -> subprocess.CompletedProcess`** — escalates via `privileged_argv` and executes it through `primitives/run.py`'s `run_or_raise` (raise-on-failure). This is the - available convenience for a new site that just wants "run this, raise if it - fails" with no further inspection. Note that the migrated sites do **not** - use it: each deliberately retains its own `subprocess.run` call to preserve - its established error handling and — critically — the per-module - `subprocess.run` monkeypatch seams the test suite relies on. `run_privileged` - therefore currently has no callers; it exists as the sanctioned raise-on-failure - path for future code, and routing an escalation through it must not replace a + convenience for a site that just wants "run this, raise if it fails" with no + further inspection; `atomic_write`, `artifacts`, `snapshot` and + `build_sandbox` use it. The sites migrated onto the seam earlier (pacman, + the kernel install, reconfigure) do **not**: each deliberately retains its + own `subprocess.run` call to preserve its established error handling and, + critically, the per-module `subprocess.run` monkeypatch seams the test suite + relies on. Routing an escalation through `run_privileged` must not replace a site whose tests patch that site's own `subprocess.run`. - **`ensure_credentials() -> bool`** — the one place a password prompt sysforge @@ -4583,6 +4599,37 @@ This mirrors the streaming/returncode carve-out already established by `run_privileged` is to `privileged_argv` what `run_or_raise` is to a raw `subprocess.run` call. +### Announcing system changes — `on_mutation` + +The seam is also where a change to the system begins, so `privileged_argv` +tells every `on_mutation(listener)` listener of each escalation before +returning its argv (`3.5.0-B138`), with the unescalated argv and even when +already root. The one listener is the verb runner's stage sentinel: +`sentinel_scope(arm_on_mutation=True)` writes its record at the first +announcement rather than on entry, so a verb that fails before changing +anything leaves no recovery record (§CLI Verb Framework). A list of +callables, not a call into `stage_sentinel`, because that module imports this +one. + +`mutates=True` is the default, so a new escalation site arms the sentinel +unless it says otherwise. A site passes `mutates=False` only for a command +that cannot leave the system inconsistent if interrupted, each with its reason +beside it: + +| Site | Why it is no change to recover from | +|------|-------------------------------------| +| `atomic_write.replace_file` (`install`, `mv`, `rm` of the staged file) | stage + rename is atomic: the old file or the new one | +| `fs_provision._run_priv` | provisioning is idempotent; the next run redoes it | +| `provides_lookup` `pacman -Fy` | a files-database download | +| `build_sandbox`: chroot `install`, `arch-nspawn … pacman -Syu` | the build chroot, not the host | +| `build_sandbox`: `systemctl set-property --runtime` (cap and reset) | runtime-only, reset after the build | +| `build_sandbox` `pacman -Sw` | a download into the package cache | +| `stage_sentinel.Recovery.argv(to_run=False)` (`display`, record validation) | built only to validate or print | + +The sandbox's slice reset runs in a `finally` after the build, so without the +marker a failed sandbox build would arm the sentinel on its cleanup, which is +`3.5.0-B138` again. + ### Escalate / probe / drop-priv taxonomy Not every `sudo`-prefixed argv is an escalation, and the seam only owns the @@ -4692,7 +4739,8 @@ the operator is still present. The kernel stage, the PGO sequence and `--bolt-on each raise before building on a refused or timed-out prompt (`3.5.0-B55`). The kernel and toolchain stages additionally probe *before* entering -`sentinel_scope`. By the sentinel contract any exception inside the scope leaves +`sentinel_scope`. Their scopes write the record on entry (only the verb +runner's waits for the first change), so any exception inside them leaves the record in place, so an auth failure raised inside it would demand a recovery (`mkinitcpio`, or a stock `pacman -S` of the LLVM suite that replaces a working PGO toolchain) for a mutation that never began. The toolchain's refusal names diff --git a/Makefile b/Makefile index b68fdd06..088841a8 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ preflight release-major release-minor release-patch release-resume \ vm-deps vm-image vm-boot vm-boot-gui vm-snapshot vm-loadvm vm-iso vm-monitor vm-console vm-savevm vm-snapshots vm-ssh vm-ssh-builder vm-stop vm-clean \ vm-pkg-stable vm-pkg-git vm-pkg-all vm-install-stable vm-install-git vm-smoke vm-test \ - container-build container-smoke container-smoke-cachyos container-shell container-clean \ + container-build container-smoke container-smoke-cachyos container-shell container-clean e2e \ ci-local # --------------------------------------------------------------------------- @@ -42,7 +42,7 @@ DEV_DEPS_CORE = python-pytest ruff shellcheck scdoc uv git base-devel zsh DEV_DEPS_PKG = devtools # QEMU test VM: vm-boot, vm-snapshot, vm-console, vm-monitor (socat), … DEV_DEPS_VM = qemu-desktop edk2-ovmf gtk-vnc socat -# Container test tier: container-smoke, container-smoke-cachyos. +# Container test tier: container-smoke, container-smoke-cachyos, e2e. DEV_DEPS_CONTAINER = podman # Release: GPG-signed commit/tag/tarball + the GitHub release upload. DEV_DEPS_RELEASE = gnupg github-cli @@ -374,10 +374,13 @@ sync-config: ## Merge new shipped defaults into the live config dir ##@ Release # The gate list, in run order, written once (3.5.0-DEV8): `pre-release` # depends on it and tools/ci.sh reads it through `print-gates`, so the release -# gate and CI cannot drift apart. +# gate and CI cannot drift apart. CI_JOBS are the rest of `pre-release`: CI +# runs each in a job of its own (`make <job>` in .github/workflows/ci.yml), not +# as a gate of tools/ci.sh (3.5.0-DEV21). GATES := lint typecheck test check-shipped check-personal check-design check-roadmap-table check-standards check-surface +CI_JOBS := e2e -pre-release: $(GATES) ## Composite gate: lint, types, tests, all checks +pre-release: $(GATES) $(CI_JOBS) ## Composite gate: lint, types, tests, all checks, end-to-end tier print-gates: ## Print the gate list pre-release and CI run @echo $(GATES) @@ -647,6 +650,14 @@ container-shell: ## Open a shell in the test container container-clean: ## Remove the test container and image ./tools/container/harness.sh clean --distro=$(or $(DISTRO),arch) +# End-to-end tier (3.5.0-DEV21): the documented paths run as an unprivileged +# user on a fresh system, each scenario in its own container from the demo +# image (tools/e2e/README.md). SCENARIO= runs only the named ones; E2E_COMMIT= +# builds that revision instead of HEAD. E2E_HERE=1 runs them on this system +# as root, without podman: CI's e2e job, in its own fresh container. +e2e: ## Run the end-to-end scenarios in fresh Arch containers (SCENARIO=, needs podman) + E2E_COMMIT=$(E2E_COMMIT) bash tools/e2e/run.sh $(if $(E2E_HERE),--here) $(SCENARIO) + # Replay the CI job (.github/workflows/ci.yml, 3.4.0-DEV1) against the working # tree, uncommitted edits included, in the same base image. The tree is mounted # read-only: tools/ci.sh copies it to an unprivileged user before running the diff --git a/REUSE.toml b/REUSE.toml index 446663b3..b9e07329 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -15,6 +15,7 @@ path = [ "tools/vm/**", "tools/container/**", "tools/demo/**", + "tools/e2e/**", "**.md", "**.toml", "**.lock", diff --git a/ROADMAP.md b/ROADMAP.md index 037c9740..394d480d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -111,46 +111,12 @@ canonical ordering. <!-- BEGIN roadmap-table (generated by tools/gen_roadmap_table.py — do not edit by hand; run `make roadmap-table`) --> | ID | Item | Priority | Effort | Bump | |----|------|----------|--------|------| -| `3.5.0-DEV21` | an end-to-end test tier that runs the documented paths on a fresh system | high | large | patch | -| `3.5.0-B138` | a verb that fails before installing anything leaves a stale sentinel | med | medium | patch | <!-- END roadmap-table --> ### Features ### Bugs -- **`3.5.0-B138` — a verb that fails before installing anything leaves a stale sentinel.** - `verbs.runner.run_verb` wraps all of a `requires_sentinel` verb's `execute` in - `stage_sentinel.sentinel_scope`, which keeps the record on any exception. So `sysforge build` - failing on a missing `pkgctl` (a `RuntimeError` raised before any install) leaves - `stage = "build"` behind, and the next run refuses to start ("Stale stage-in-progress sentinel - present"): a recovery prompt at a terminal, a hard stop for a timer or cron run. Found by the - demo-recording container on 2026-10-23. Two shapes to choose between: arm the sentinel at the - first mutation (the install) rather than at `execute` entry, or move pre-mutation checks such - as the `pkgctl` probe into `pre_check`, which runs before the scope opens. The first fixes the - class; the second only the cases found so far. - *Priority: med · Effort: medium · Bump: patch* — blocks unattended runs after an ordinary, - harmless failure. - ### Developer tooling -- **`3.5.0-DEV21` — an end-to-end test tier that runs the documented paths on a fresh system.** - The unit suite stubs out exactly where six bugs found on 2026-10-23 lived: root versus the user - and group membership that only applies at the next login (`3.5.0-B134`, `B135`, `B137`), state one - command leaves for the next (`B136`, whose test called `mark_done` directly, a state the real - code never produced) and real terminal behaviour (`B132`, `B133`). The demo-recording container - (`tools/demo/`, `make demo-gif`) caught them by following the README. Add `make e2e`: shell - scenarios run as an unprivileged user in that same image, asserting on exit codes, `pacman -Qi`, - the state files and leftover sentinels (not vhs). Start with four: the Quick start as one login - session (setup, AUR build, `update`); a repo package's opt-in; flag drift after a profile change; - and a failure before install leaving no sentinel (`3.5.0-B138`'s regression test once fixed). - Prefer local PKGBUILDs, with at most one networked scenario, so outages don't flake the tier. CI - already runs in a fresh `archlinux:base-devel` container, so the same scripts can run there - directly without podman; wire `make e2e` into `pre-release` and `docs/RELEASE-CHECKLIST.md`. A - cheaper complement for the terminal class: a test helper that replays the progress bar's output - through a terminal model and asserts where the cursor ends up, under both xterm's and xterm.js's - saved-cursor rules, which would have caught B132 and B133 in the unit suite. - *Priority: high · Effort: large · Bump: patch* — the only tier that exercises privilege - boundaries, multi-command sequences and the documented first run, where users meet sysforge. - ### Open questions diff --git a/docs/RELEASE-CHECKLIST.md b/docs/RELEASE-CHECKLIST.md index 02d0daea..bc23c242 100644 --- a/docs/RELEASE-CHECKLIST.md +++ b/docs/RELEASE-CHECKLIST.md @@ -15,6 +15,7 @@ make next-bump # 1. what level the release notes require make design roadmap-table # 2. regenerate, then commit anything that moved make ci-local # 3. every static gate + full suite, clean container make coverage-ratchet audit # 3. the two checks no gate runs +make e2e # 4a make vm-pkg-stable && make container-smoke && make container-smoke-cachyos # 4a make vm-snapshot && make vm-test && make vm-stop # 4b (+ git flavor) # 5. /release-notes skill @@ -70,6 +71,7 @@ In more detail: | `lint` (ruff + shellcheck) | yes | yes | yes | yes | | `typecheck` (pyright) | yes | yes | no | yes | | full test suite | yes (`test`) | yes (`test`) | yes (`coverage`) | no | +| end-to-end tier (`e2e`) | yes | no (its own CI job) | no | no | | `check-shipped` (includes version markers, completions) | yes | yes | subset | yes | | `check-personal`, `check-design`, `check-roadmap-table`, `check-standards` | yes | yes | no | yes | | clean environment (not your workstation) | no | yes | no | no | @@ -143,6 +145,20 @@ pass: fix it or record the tier as unrun. ### 4a. Container tier: fast, run first +The end-to-end tier first: the documented paths (the Quick start, a repo package's opt-in, flag +drift, a failed build's cleanup) run as an unprivileged user on a fresh system, one container per +scenario. It builds sysforge from the checkout itself, so it needs no `vm-pkg-stable`. CI runs the +same scenarios on every pull request; run it here for the release commit. An `XFAIL` is a known, +filed bug and does not block; an `XPASS` fails the run (remove the scenario's `xfail` marker). + +```bash +make e2e # tools/e2e/README.md +``` + +- [ ] `make e2e` exits 0 + +The smokes then check the packaging itself. + Seconds rather than a boot. The CachyOS arm is the one that actually exercises portability (repo/AUR shadowing, a different `makepkg.conf` baseline, bumped pkgrels on core packages). diff --git a/docs/design/08-cli-verb-framework.md b/docs/design/08-cli-verb-framework.md index f8cae6a3..5b0957ec 100644 --- a/docs/design/08-cli-verb-framework.md +++ b/docs/design/08-cli-verb-framework.md @@ -75,16 +75,16 @@ spelling per meaning, across every verb: **Result types** (`PreCheckResult` and `ExecResult`) are plain dataclasses with `ctx` / `artifacts` dicts; the runner does not inspect their contents. This keeps phase boundaries loose enough for ad-hoc data flow within a verb without inventing a per-verb context class. -**Sentinel handling.** Verbs whose `execute` mutates the live system set `requires_sentinel = True`. The runner wraps `execute + post_validate` in `sentinel_scope(state_dir, verb.name, recovery=…, retry_cmd=…, **metadata)` (`recovery` is a `stage_sentinel.Recovery` from `verb.sentinel_recovery`, stored as a kind plus arguments, `3.5.0-B43`) from `primitives/stage_sentinel.py`. On entry, the sentinel writes `stage_in_progress.toml`; on normal completion (both phases pass), it clears. On `RuntimeError` or `CleanExitRequested`, the sentinel is left in place so the next sysforge invocation blocks at the CLI-entry recovery prompt. `sentinel_scope` also installs an `InterruptScope`, so verbs participate in the same first-Ctrl-C-defers-to-safe-boundary behaviour as the toolchain stage: `build` and `update` stop before their next package (`interrupt.checkpoint()` in `build_core`), and a verb with no checkpoint stops when its `execute` + `post_validate` complete (`3.5.0-B44`). It additionally holds the `stage_in_progress.lock` liveness lock for the scope: if another run already holds it, entering the scope raises `RuntimeError` **before** `mark_started`, which the runner's existing handler converts to exit 1 without clobbering the live run's sentinel (§Pipeline layer, *Liveness guard*). The toolchain pipeline stage uses the same primitive — there is one implementation, shared. +**Sentinel handling.** Verbs whose `execute` mutates the live system set `requires_sentinel = True`. The runner wraps `execute + post_validate` in `sentinel_scope(state_dir, verb.name, recovery=…, retry_cmd=…, **metadata)` (`recovery` is a `stage_sentinel.Recovery` from `verb.sentinel_recovery`, stored as a kind plus arguments, `3.5.0-B43`) from `primitives/stage_sentinel.py`, with `arm_on_mutation=True` (`3.5.0-B138`): the scope takes the liveness lock on entry but writes `stage_in_progress.toml` only at the first escalation that changes the system, which `privilege.privileged_argv` announces to `privilege.on_mutation` listeners (§Privilege seam). A verb's `execute` does its checks first (`build` probes for `pkgctl`, resolves targets, builds), and a failure there leaves nothing for the next run to refuse on; before the fix it left `stage = "build"` behind and blocked a timer or cron run. On normal completion (both phases pass), the record clears. On `RuntimeError` or `CleanExitRequested` after the record was written, it is left in place so the next sysforge invocation blocks at the CLI-entry recovery prompt; a Ctrl-C before it says nothing changed. The pipeline stages keep the default (record on entry), since they open their scope around the install itself. `sentinel_scope` also installs an `InterruptScope`, so verbs participate in the same first-Ctrl-C-defers-to-safe-boundary behaviour as the toolchain stage: `build` and `update` stop before their next package (`interrupt.checkpoint()` in `build_core`), and a verb with no checkpoint stops when its `execute` + `post_validate` complete (`3.5.0-B44`). It additionally holds the `stage_in_progress.lock` liveness lock for the scope: if another run already holds it, entering the scope raises `RuntimeError` **before** `mark_started`, which the runner's existing handler converts to exit 1 without clobbering the live run's sentinel (§Pipeline layer, *Liveness guard*). The toolchain pipeline stage uses the same primitive — there is one implementation, shared. **Read-only verbs** (`env`, `help`, `resolve`, `log`, `config check`, `state list`, `state orphans` without `--delete`, `state failed` without `--clear`/`--clear-all`, `state profiles` without `--delete`, `state boot-entries`, `bench toolchain`, `packages list`, `doctor` without `--apply`) implement `execute` (the work is printing) and return `ExecResult()`; `post_validate` defaults to no-op and `requires_sentinel = False`. They use the same dispatch path as mutating verbs — no second code path. **`state boot-entries`** (`3.3.0-F5`, `StateBootEntriesVerb`, read-only, `requires_sentinel = False`) lists every `vmlinuz-*` in `/boot` with the loader entry that boots it. The table goes through the pager like `state profiles` (`--no-pager` disables it): columns KERNEL / ENTRY / OWNER (`you`, `sysforge` or `none`) / TEMPLATE, plus `(missing)` rows for managed entries whose kernel is gone. Under `bootloader = grub`/`none` it prints a one-line note instead; with `boot_entries = "off"` it notes that and still lists. Rows come from `boot_entries.rows_for`. **Error model.** -- `RuntimeError` raised from any phase → `_log.error(msg)`, return 1. Sentinel preserved if active. +- `RuntimeError` raised from any phase → `_log.error(msg)`, return 1. Sentinel preserved if written (a change had begun). - `SystemExit` → propagate verbatim (lets tests and signal handlers see the raw exit). -- `CleanExitRequested` → caught inside `sentinel_scope`, logged, re-raised as `RuntimeError` with the verb's `retry_cmd` and recovery command in the message; sentinel preserved. +- `CleanExitRequested` → caught inside `sentinel_scope`, logged, re-raised as `RuntimeError` with the verb's `retry_cmd` and recovery command in the message; sentinel preserved. Before the first change, the message says nothing changed and no record is left. **Per-verb phase mapping** (current shape; phases reflect where each piece of work lives, not which primitives it calls): diff --git a/docs/design/09-primitives-layer.md b/docs/design/09-primitives-layer.md index fb2da469..26d4b623 100644 --- a/docs/design/09-primitives-layer.md +++ b/docs/design/09-primitives-layer.md @@ -32,6 +32,8 @@ Bottom-anchored status line for batch operations (`[3/10] building · htop`). Du - **When the bar repaints** — `_draw()` is the sole writer. It builds the whole frame (any pending resize, region establishment or release, and the painted row, glyph-downgraded then width-truncated) as one string and emits it with a **single write**, so another thread's log line can land before or after a frame but never inside it; an unchanged frame writes nothing. A module lock guards mutate-then-draw in every public entry point and the ticker, and is never held across a blocking call or a yield body. `SIGWINCH` only sets `resize_pending` (no I/O, no lock — a signal can interrupt a paint holding both); the next frame applies it. **A resize release erases from the content cursor down (`ESC[J`), never the old absolute row** (`3.5.0-B4`): a taller window makes the emulator pull scrollback in and shift everything, cursor and bar alike, down by the rows gained, so the old row then holds a content line, and establishment's index would scroll the stale bar into the output. Order is what a resize preserves, and the bar is always below the cursor. The emulator also resets DECSTBM on resize, so `pty_runner` repaints before forwarding the first chunk after a `SIGWINCH` instead of waiting out its one-second throttle. Events (tick, phase, yield exit) draw immediately; **exactly one scheduled painter** keeps the clock moving between them, chosen by whether a raw byte stream is being forwarded to the terminal. Single-write frames are safe against line-oriented writers (`log.*`, `print`, TTY-inheriting pacman/git), but a raw forwarder can be mid-escape or mid-UTF-8. By default the painter is a daemon **ticker** thread, started on the first TTY draw and stopped by `shutdown()`/`clear()`, which redraws every second unless yielded or forwarding; exceptions are swallowed and traced. While `pty_runner` forwards makepkg's raw output it enters `forwarding_output()` and becomes the painter itself, calling `refresh()` at most once a second and only at a stream boundary (§`pty_runner.py`). Cost: one wakeup a second, and about one short write a second only while a tracker is past the elapsed floor. **`heartbeat(detail)`** (`3.2.0-B13`) only sets `detail` — the scheduled painter shows it. It exists because a whole package build sits inside a single `tick()`: a trace once showed a 379-second gap between paints. The detail replaces rather than appends and the next tick/note/resume/phase clears it. - **Lifecycle** — `cli.main` wraps `_main()` in `progress.session()`, whose exit calls `shutdown()` (stop the ticker, release the region). The session exits before the `KeyboardInterrupt`/`BrokenPipeError` handlers, so the region is gone before `aborted (Ctrl-C)` prints, and a plain `sys.exit` or `log.fatal` releases on the way out too. `log` does not import `ui`. The `atexit` hook remains as the backstop for importers that don't go through `cli.main`; `shutdown()` is idempotent. +**Tested on a terminal model** (`3.5.0-DEV22`). `tests/test_progress.py` checks the sequences the bar writes; `tests/test_terminal_model.py` replays them, and `update`'s `_TERMINAL_RESET`, through `tests/term_model.py`, a VT model of the sequences sysforge emits that raises on any other, and asserts what the screen holds: the command line survives, output follows it in order, the bar leaves nothing behind, the next prompt lands after the last line. Each runs under both rules terminals have for a saved cursor (DECSC) across a scroll: xterm keeps the screen row, xterm.js (VS Code's terminal, vhs) follows the content line. `3.5.0-B132` and `B133` both turned on that difference, and each has a test replaying its pre-fix sequence to show the model reproduces it. A new escape sequence in the bar is taught to the model in the same change. + **One tracker at a time.** A tracker opened while another is open is a no-op that yields `progress_hooks._NoOpTick` and logs at debug, leaving the outer untouched; under tests (`_strict_nesting`, set by an autouse `tests/conftest.py` fixture that also gives every test a fresh `_bar` and no ticker) it raises `RuntimeError("tracker '<inner>' opened inside '<outer>'")`. A sub-step inside an item uses `tick.note()`/`tick.resume()` instead. The functions that open trackers call `require_no_tracker(owner)` (same strict/lenient policy) first thing, so a mis-nested caller fails before any sync or resolution side effect — the build-⊂-update invariant invites routing more callers through `build_and_install`. A stale `tick` after its tracker exits is a no-op. Tracker owners: | Tracker | Owner | Runs under | diff --git a/docs/design/17-release-plan.md b/docs/design/17-release-plan.md index ca7ba04f..0bf74fd7 100644 --- a/docs/design/17-release-plan.md +++ b/docs/design/17-release-plan.md @@ -35,6 +35,20 @@ the gates — they need a booted VM or a working podman and network, neither of prerequisite of a lint run — but a minor or major release that skips them is untested against a real install. See `tools/vm/README.md` and `tools/container/README.md`. +The **end-to-end tier** (`make e2e`, `tools/e2e/`, `3.5.0-DEV21`) runs the documented paths the way +a new user meets them: the Quick start in one login session, a repo package's opt-in, flag drift +after a profile change, a failed build's cleanup. Each runs as an unprivileged user in a fresh +container built from the demo image (`tools/demo/`, shared with `make demo-gif`), asserting on exit +codes, `pacman -Qi`, the state files and leftover sentinels, with prompts answered on a +pseudo-terminal. It covers what the unit suite stubs: root versus the user, group membership that +applies only at the next login, the state one command leaves for the next. It builds sysforge from +the checkout itself, so it needs no `vm-pkg-stable`. `pre-release` runs it; the release preflight +does not (`_RELEASE_GATE_EXEMPT`: minutes long, podman, and one networked scenario, so not +deterministic). CI runs the same scenarios in an `e2e` job of its own, directly in the job's +container (`make e2e E2E_HERE=1`), listed in the Makefile's `CI_JOBS` beside `GATES`. A scenario +marked `xfail: <ID>` fails until that item is fixed and reports `XFAIL`; once it passes it fails +the run, so the marker comes off with the fix. + **CI** (`.github/workflows/ci.yml`, `3.4.0-DEV1`) runs every `pre-release` gate on each pull request (a newer push to it cancels the superseded run) and on each push to `main`, in a fresh `archlinux:base-devel` container, as an unprivileged user, with only the core dev tier (`make dev-deps-core`) installed. Its job is diff --git a/docs/design/21-standards.md b/docs/design/21-standards.md index c52be11b..46b16434 100644 --- a/docs/design/21-standards.md +++ b/docs/design/21-standards.md @@ -60,7 +60,7 @@ subsection is only presentation. | 24 | Deprecation registry (declared removal version + warn-on-use) | Every config key, state token, CLI flag, or path sysforge still honours only for backwards compatibility | enforced | `primitives/deprecations.py` is the single home: each record carries `surface`/`kind`/`function`/`deprecated_in`/`removed_in`/`replacement`, and each compat read path calls `warn_used(surface)` so the warning text is built from the record and cannot drift from the version the gate enforces. Two `function` values, because removal is not uniformly breaking — a `compat` surface still works (removing it is breaking, so `removed_in` must be `X.0.0`, and presence is proven by its `warn_used` call sites), while a `shim` already fails and is kept only to name its replacement (removal is not breaking, `removed_in` may be `X.Y.0`, presence proven by an exact repo-relative `anchor`). `check_standards` `deprecations` group: registry↔call-site bijection both ways, major-only removal for `compat`, and an error when the release target is at or past a declared `removed_in` with the surface still present (`--target-version`); a registry the gate cannot read is itself an error, because a check that cannot fail is worse than no check; one written as an empty tuple (`_REGISTRY = ()`) is accepted, since that is what removing the last record leaves (`3.5.0-F16`). The release-note removal parity check matches the registry's literal surface string against the `## Removed` section body, so a removal note must spell the canonical identifier (e.g. `build_state.build_mode=profiled`), not just prose describing it. `tests/test_deprecations.py` (behavioural, incl. once-per-run dedup and per-surface resolution) + `tests/test_standards_compliance.py`. **First application:** the 2.6.1-F5 sweep removed all five `compat` surfaces the registry catalogued at `2.6.1-STD2` shipping time, proving the major-only removal gate end to end. The `shim` half was proven the same way at `3.0.0-STD3`: `doctor.flat_flags` declared `removed_in = 3.1.0`, and the release-target gate refused the 3.1.0 release until the hint table, its `cli.py` hook and the record itself were deleted — a scheduled removal the gate enforced rather than a reviewer remembering. The registry is empty as of 4.0.0: its last record, the `compat` `profiles.build_mode=patched_pkgbuild` (`2.6.1-STD3`), was removed on schedule (`3.5.0-F16`), and `normalize_build_mode` now refuses the token. That record predated the registry and `2.6.1-STD2` missed it, because the bijection walks registry→call-site and never code→registry: **an unregistered compat surface is invisible to the gate by construction.** A catalogued-empty `compat` half is therefore never proof that none exist, and finding the next one is a review obligation, not a tooling guarantee — a code→registry direction is unimplemented (no reliable static signal distinguishes a compat alias table from any other lookup dict). Its `removed_in` was `4.0.0` rather than `3.0.0` because 3.0.0 shipped with the surface present, and the gate refuses a release at or past a declared removal with the surface still live. | | 25 | Log-level rubric (UI / ERROR / WARN / INFO / DEBUG) | Every call site that emits user-visible output | followed | `docs/design/12-logging.md` is the single home: the five-level table (fn, gate, reserved-for), the decision test that resolves most call sites (*is this the answer, or narration about producing the answer?*), and the explicit anti-pattern that `ui()` is not a "make this always show up" escape hatch — `ui()` is the answer the user ran the command to see, `info()`/`warn()` are narration and are gated behind `-vv`/`-v`. Enforcement is deliberately partial and the row does not claim otherwise. The one mechanical guard is the `quiet_at_default` fixture (`tests/conftest.py`), a golden-output regression that runs a stage at the shipped default (`verbosity=0`) and again at `-vv`, and fails if the default run emitted an `[INFO]` or `[WARN]` line. That catches the failure mode that matters — narration leaking into always-visible output — but nothing checks WARN-vs-INFO or INFO-vs-DEBUG classification, which stays a review obligation. Coverage is per-stage opt-in: the fixture is anchored on `packages` (`_load_packages`, `tests/test_stage_packages.py`) and `configure`'s dry run (`tests/test_stage_bootstrap.py`). A stage added without requesting the fixture is unguarded by construction, so **coverage thins with every stage that does not opt in** — requesting it is part of adding a stage, not a later cleanup. | | 26 | Lint scope (every Python tree in the repo) | `sysforge/`, `tests/`, and `tools/` — all Python the repo owns, not only the shipped package | enforced | `make lint-py` runs `ruff check sysforge/ tests/ tools/`, and `.claude/hooks/ruff-on-edit.sh` blocks on the same three trees. The two must name the identical set: before 2.6.1-STD8 the gate covered `sysforge/` alone while the hook already blocked on `tests/`, so 189 violations accumulated in a tree nothing reported on and the hook fired on pre-existing debt in files an author had merely touched — enforcement strict enough to interrupt, scoped so it could only ever surface someone else's backlog. Rule selection is uniform (`[tool.ruff.lint] select`); the only per-tree relaxation is `[tool.ruff.lint.per-file-ignores]` `"tests/**" = ["S", "PTH", "SIM117"]`, each with a stated reason — `S` because test code is not production attack surface (2.3.0-F1), `PTH` because it fires on the `sys.path` import bootstrap repeated across ~25 test files, `SIM117` because collapsing stacked monkeypatch/mock context managers costs a line-length violation per test and reads worse. A new relaxation is a per-file-ignores entry with a comment, never a directory dropped from the gate. Scope note: `tools/` is deliberately **not** relaxed — it holds `check_standards.py`, `check_shipped.py`, and `release.sh`'s Python siblings, which the release process depends on. | -| 27 | Release-gate completeness | Static-analysis gates of `make pre-release` vs the `tools/release.sh` preflight | enforced | Every `pre-release` prerequisite except the full `test` suite (the heavier tier, by design) is also run by the release preflight, which fails the release on it. `pre-release` is advisory and the script is the gate a release cannot skip, so a check only the former runs is one a tag can ship without: `v3.2.0` shipped four pyright errors — one a live crash (`3.2.0-B30`) the type-checker had named weeks earlier — and the 3.3.0 cycle reached eighteen (`3.2.0-STD1`). `check_standards` `release_gates` group diffs the `pre-release:` prerequisite list against the targets `release.sh` invokes, so `docs/RELEASE-CHECKLIST.md`'s gate table cannot silently diverge. A new `pre-release` gate is added to the script in the same change, or to `_RELEASE_GATE_EXEMPT` with its reason. Because the script *blocks*, each gate must also be deterministic: a `uv run --with` overlay in a gate's recipe is pinned (`PYRIGHT_VERSION`, `REUSE_VERSION` in the Makefile header), since an unpinned overlay resolves the newest release on every run and a tool update alone could turn a release red; the same group flags an unpinned `--with` in any gate recipe. Pins are bumped deliberately in their own commit, and the header keeps the no-`[dependency-groups] dev` rule (the overlays stay the one record). Known residue: `lint` runs the pacman-provided `ruff`, which is kept on `PATH` for editor/hook integration and so follows system upgrades. | +| 27 | Release-gate completeness | Static-analysis gates of `make pre-release` vs the `tools/release.sh` preflight | enforced | Every `pre-release` prerequisite except the full `test` suite and the `e2e` tier (the heavier tiers, by design; `e2e` also needs podman and the network, so it could not be deterministic) is also run by the release preflight, which fails the release on it. `pre-release` is advisory and the script is the gate a release cannot skip, so a check only the former runs is one a tag can ship without: `v3.2.0` shipped four pyright errors — one a live crash (`3.2.0-B30`) the type-checker had named weeks earlier — and the 3.3.0 cycle reached eighteen (`3.2.0-STD1`). `check_standards` `release_gates` group diffs the `pre-release:` prerequisite list against the targets `release.sh` invokes, so `docs/RELEASE-CHECKLIST.md`'s gate table cannot silently diverge. A new `pre-release` gate is added to the script in the same change, or to `_RELEASE_GATE_EXEMPT` with its reason. Because the script *blocks*, each gate must also be deterministic: a `uv run --with` overlay in a gate's recipe is pinned (`PYRIGHT_VERSION`, `REUSE_VERSION` in the Makefile header), since an unpinned overlay resolves the newest release on every run and a tool update alone could turn a release red; the same group flags an unpinned `--with` in any gate recipe. Pins are bumped deliberately in their own commit, and the header keeps the no-`[dependency-groups] dev` rule (the overlays stay the one record). Known residue: `lint` runs the pacman-provided `ruff`, which is kept on `PATH` for editor/hook integration and so follows system upgrades. | | 30 | No em-dashes in user-facing prose | `README.md`, `docs/guides/`, `docs/concepts/` (`3.5.0-DOC3`), the man page (`man/sysforge.1.scd.in` and the generated `man/sysforge.1`), `completions/`, and every non-docstring string literal under `sysforge/`: `--help` text, log and prompt messages, exception text, and the headers of files sysforge generates | enforced | Rewrite with a comma, colon, semicolon, period or parentheses as the sentence needs, never a blind substitution (`3.4.0-DOC2`). `check_standards` `em_dash` group: a line scan of the files and an AST scan of every string constant under `sysforge/`, so implicit concatenation and f-string pieces are covered without importing `sysforge`. Exempt: docstrings (developer prose), a literal that is only the glyph (an empty-cell placeholder, or a key in `log`'s ASCII glyph-fallback table), ROADMAP and release-note entries, whose ``- **`<ID>` — <title>.**`` lead is a lint-enforced format (changing it is its own `STD`), internal docs, shipped-config and hook comments, and code comments. `tests/test_standards_compliance.py` (`test_em_dash_*`). | | 31 | Subprocess goes through `primitives/run` | Every external command `sysforge/` runs | enforced | Numbered 28 until `3.4.0-STD2`, when it was found sharing that number with the `pam.d(5)` row, which took 28 first and keeps it. ruff `banned-api` (TID251, `pyproject.toml`) bans `subprocess.run`/`Popen`/`call`/`check_call`/`check_output` outside the seam modules `primitives/run.py`, `pty_runner.py`, `privilege.py` and `sudo_session.py` (`3.2.0-F13`). Probes use `run.capture` (missing binary → `None`) or `run.probe` (missing binary → status 127), raise-on-failure calls use `run_or_raise`, and escalation uses the §22 privilege seam. A call that legitimately stays raw (inherits the TTY, streams, needs its own `preexec_fn`, or is a privileged command whose status or `CalledProcessError` its caller inspects) carries `# noqa: TID251 — <why>`, and the reason is mandatory. Enforced by the ruff rule plus `tests/test_standards_compliance.py` (`test_subprocess_fence_*`), which pins the banned set, the exemption list and the per-call reasons. Complements row 17, which governs argv form; this row governs *where* the call is made. | | 32 | Machine-readable output contract (`--json`) | `state list --json`, `doctor` (`system`, `pkg`, bare) `--json`, `update --dry-run --json`, `config check --json` (`3.5.0-F18`) | enforced | `primitives/json_out.py` is the one home (`3.4.0-F7`). Stdout carries exactly one JSON document ([RFC 8259](https://www.rfc-editor.org/rfc/rfc8259)) and nothing else: `cli.main` enters `json_out.reserved_stdout()` right after parsing and `verbs.runner.run_verb` enters it again (re-entrant), so every `ui()`/`print()`, startup logs and progress phases included, goes to stderr; the document goes out through `json_out.emit`. Every document has a top-level `"schema"` integer (`SCHEMA_VERSION`, now 1) and a `"kind"` (`state-list`, `doctor`, `update-dry-run`). **Adding a field is a minor change; renaming or removing one, or changing its type, is a major change and bumps `SCHEMA_VERSION`.** Each document is built from the result objects the human renderer reads (`AxisResult` via `diagnostics.axis_document`, `run.results` and the drift lists via `update._dry_run_document`, build_state rows through an explicit field list in `state_cmd._JSON_ENTRY_FIELDS`), so the two outputs cannot disagree. Exit codes are unchanged by `--json`. Mutating runs are out of scope: `update --json` without `--dry-run` and `doctor pkg --apply --json` are refused (exit 2). The schemas are committed as JSON Schema draft 2020-12 under `tests/data/json-schema/`; `tests/test_json_output.py` validates each verb's document against its schema (a subset validator, so the suite needs no new dependency), checks that stdout parses as one document with `log` in dry-run mode at `-vv`, and that a doctor skip appears under `roster.skipped`, never as a finding. A new field updates its schema in the same change. | diff --git a/docs/design/22-privilege-seam.md b/docs/design/22-privilege-seam.md index 8050be5e..7671d86f 100644 --- a/docs/design/22-privilege-seam.md +++ b/docs/design/22-privilege-seam.md @@ -10,23 +10,23 @@ branch, no hand-rolled `["sudo", ...]` list. ### Two entry points -- **`privileged_argv(argv, *, noninteractive=False) -> list[str]`** — builds +- **`privileged_argv(argv, *, noninteractive=False, mutates=True) -> list[str]`** — builds the escalated argv and hands it back; the caller runs it with its own `subprocess.run` (or equivalent). Use this at sites that need to stream output to the TTY, inspect the return code themselves, or otherwise need control over how the command executes. When already root (`euid == 0`) the argv is returned unchanged (no `sudo` prefix); `noninteractive=True` inserts `-n` so `sudo` fails fast instead of prompting, when not already root. -- **`run_privileged(argv, *, tag, **kwargs) -> subprocess.CompletedProcess`** — +- **`run_privileged(argv, *, tag, mutates=True, **kwargs) -> subprocess.CompletedProcess`** — escalates via `privileged_argv` and executes it through `primitives/run.py`'s `run_or_raise` (raise-on-failure). This is the - available convenience for a new site that just wants "run this, raise if it - fails" with no further inspection. Note that the migrated sites do **not** - use it: each deliberately retains its own `subprocess.run` call to preserve - its established error handling and — critically — the per-module - `subprocess.run` monkeypatch seams the test suite relies on. `run_privileged` - therefore currently has no callers; it exists as the sanctioned raise-on-failure - path for future code, and routing an escalation through it must not replace a + convenience for a site that just wants "run this, raise if it fails" with no + further inspection; `atomic_write`, `artifacts`, `snapshot` and + `build_sandbox` use it. The sites migrated onto the seam earlier (pacman, + the kernel install, reconfigure) do **not**: each deliberately retains its + own `subprocess.run` call to preserve its established error handling and, + critically, the per-module `subprocess.run` monkeypatch seams the test suite + relies on. Routing an escalation through `run_privileged` must not replace a site whose tests patch that site's own `subprocess.run`. - **`ensure_credentials() -> bool`** — the one place a password prompt sysforge @@ -56,6 +56,37 @@ This mirrors the streaming/returncode carve-out already established by `run_privileged` is to `privileged_argv` what `run_or_raise` is to a raw `subprocess.run` call. +### Announcing system changes — `on_mutation` + +The seam is also where a change to the system begins, so `privileged_argv` +tells every `on_mutation(listener)` listener of each escalation before +returning its argv (`3.5.0-B138`), with the unescalated argv and even when +already root. The one listener is the verb runner's stage sentinel: +`sentinel_scope(arm_on_mutation=True)` writes its record at the first +announcement rather than on entry, so a verb that fails before changing +anything leaves no recovery record (§CLI Verb Framework). A list of +callables, not a call into `stage_sentinel`, because that module imports this +one. + +`mutates=True` is the default, so a new escalation site arms the sentinel +unless it says otherwise. A site passes `mutates=False` only for a command +that cannot leave the system inconsistent if interrupted, each with its reason +beside it: + +| Site | Why it is no change to recover from | +|------|-------------------------------------| +| `atomic_write.replace_file` (`install`, `mv`, `rm` of the staged file) | stage + rename is atomic: the old file or the new one | +| `fs_provision._run_priv` | provisioning is idempotent; the next run redoes it | +| `provides_lookup` `pacman -Fy` | a files-database download | +| `build_sandbox`: chroot `install`, `arch-nspawn … pacman -Syu` | the build chroot, not the host | +| `build_sandbox`: `systemctl set-property --runtime` (cap and reset) | runtime-only, reset after the build | +| `build_sandbox` `pacman -Sw` | a download into the package cache | +| `stage_sentinel.Recovery.argv(to_run=False)` (`display`, record validation) | built only to validate or print | + +The sandbox's slice reset runs in a `finally` after the build, so without the +marker a failed sandbox build would arm the sentinel on its cleanup, which is +`3.5.0-B138` again. + ### Escalate / probe / drop-priv taxonomy Not every `sudo`-prefixed argv is an escalation, and the seam only owns the @@ -165,7 +196,8 @@ the operator is still present. The kernel stage, the PGO sequence and `--bolt-on each raise before building on a refused or timed-out prompt (`3.5.0-B55`). The kernel and toolchain stages additionally probe *before* entering -`sentinel_scope`. By the sentinel contract any exception inside the scope leaves +`sentinel_scope`. Their scopes write the record on entry (only the verb +runner's waits for the first change), so any exception inside them leaves the record in place, so an auth failure raised inside it would demand a recovery (`mkinitcpio`, or a stock `pacman -S` of the LLVM suite that replaces a working PGO toolchain) for a mutation that never began. The toolchain's refusal names diff --git a/docs/release-notes/unreleased.md b/docs/release-notes/unreleased.md index 16d12684..be873f9e 100644 --- a/docs/release-notes/unreleased.md +++ b/docs/release-notes/unreleased.md @@ -209,6 +209,14 @@ https://keepachangelog.com/en/1.1.0/ --- +- **`3.5.0-DEV21` — `make e2e` runs the documented paths on a fresh system.** Four scenarios under `tools/e2e/` run as an unprivileged user, each in a fresh Arch container from the demo image: the README's Quick start in one login session (the one-time setup, an AUR package through the review gate, `update`), a repo package's opt-in prompt and its saved answer, flag drift after a default-profile change, and a build that fails before installing anything. They check exit codes, `pacman -Qi`, the build record, `packages.toml` and leftover sentinels, answering prompts on a pseudo-terminal so `build` takes its interactive paths. They build local PKGBUILDs except the Quick start, the one scenario that needs the network. Built from the commit before `3.5.0-B134`, the Quick start fails on the lost build record. The last is the regression test for `3.5.0-B138`. A scenario can be marked as failing until a filed bug is fixed; once it passes it fails the run, so the marker comes off with the fix. `E2E_COMMIT=` builds an older revision with today's scenarios, `pre-release` runs the tier, CI runs it in a job of its own on every pull request, and the release checklist runs it before the container smokes. + +--- + +- **`3.5.0-DEV22` — the progress bar is tested on a model terminal.** `3.5.0-B132` and `B133` drew output over earlier lines in xterm and VS Code's terminal, which the progress bar's tests could not see: they check the escape sequences it writes, not what those do to a screen. A small terminal model under `tests/` now replays the bar's real output, and `update`'s end-of-run reset, and checks the screen: the command line survives, output follows it in order, the bar leaves nothing behind and the next prompt lands after the last line. Every check runs under both rules terminals follow for a saved cursor when the screen scrolls, xterm's and xterm.js's. Replayed through it, the sequences each fix replaced show their bug. The model refuses a sequence it has not been taught, so a new one cannot be misread silently. + +--- + - **`3.5.0-DOC1` — the package description says what sysforge optimizes.** The `pkgdesc` shown by `pacman -Si` and on the AUR now names what people search for in one short line: "AUR helper that builds and tracks packages tuned to your CPU with PGO, BOLT and LTO", instead of a generic "build and maintenance suite" line. The README's opening paragraph says the same. --- @@ -844,6 +852,10 @@ https://keepachangelog.com/en/1.1.0/ - **`3.5.0-B137` — sysforge notices a second run again after a `sudo sysforge` command.** sysforge keeps a lock file so that two runs that build or install cannot overlap. The first `sudo sysforge run …` created it readable only, and from then on every run as your user failed to open it, so the check that stops a second run was off for good (with a warning shown only under `-v`). The lock file is now created group-writable, and a run that cannot write an older one still takes the lock. +--- + +- **`3.5.0-B138` — a command that fails before changing anything no longer blocks the next run.** `build`, `update`, `uninstall`, `revert-to-stock` and the other commands that change the system write a record when they start, so that if one is interrupted mid-install the next run stops and offers to restore the system. The record was written before the command's own checks, so a harmless early failure, such as `sysforge build` on a system without `pkgctl`, left it behind: the next run then refused to start ("Stale stage-in-progress sentinel present"), asking for a recovery at a terminal and simply failing under a timer or cron. The record is now written at the first change to the system, the first package install, removal or boot update. Downloads, the build sandbox's own setup and config-file writes do not count, since an interruption cannot leave them half done. A second run is still refused while the first is running, and a Ctrl-C before anything changed says so. + ## Security - **`3.5.0-B37` — the root pacman hook no longer follows links in the state directory.** The hook helper runs as root in every pacman transaction and wrote into `/var/lib/sysforge/sentinels`, whose parent any `sysforge` group member can write to. A member could swap `sentinels` for a symlink to another directory, which the next transaction made group-writable, or symlink one of its files so the hook appended to any file as root. The helper now enters the directory and acts only if it is the real `sentinels`, owned by root. It appends only to a regular file, opened so that a symlink is refused and a planted FIFO cannot stall the transaction. As before, it never fails a transaction. diff --git a/sysforge/CLAUDE.md b/sysforge/CLAUDE.md index ac87af1e..7ce1bff7 100644 --- a/sysforge/CLAUDE.md +++ b/sysforge/CLAUDE.md @@ -196,7 +196,9 @@ Mechanism lives in the cited §DESIGN section. - **Install sentinel**: `stage_sentinel.sentinel_scope()` — record *and* its liveness lock (`stage_in_progress.lock`, via `build_lock`; never roll a second flock path or a PID field). Scopes are sequential, never nested. Contention is strict (refuse, no override, no prompt); - `OSError` stays lenient. §Toolchain stage. + `OSError` stays lenient. The verb runner's scope (`arm_on_mutation=True`, `3.5.0-B138`) takes + the lock on entry but writes the record at the first mutating `privileged_argv`, announced via + `privilege.on_mutation`, so a pre-install failure leaves none; stages arm on entry. §Toolchain stage. - **`doctor` axes**: each a producer → `list[diagnostics.Finding]`, read-only (no `pacman -Sy` / `BuildState.save()`; never import `pipeline`). Register in `doctor.py` (the producer plus its flag in `_add_system_axis_flags`) + both completions + manpage + each axis's `clean_msg` in the @@ -208,7 +210,10 @@ Mechanism lives in the cited §DESIGN section. `stage_ownership.owner_of`; `lib32-*` excluded from prefix-ownership. §Toolchain stage. - **Privilege escalation**: `primitives/privilege.py` (`privileged_argv`/ `run_privileged`) — never hand-roll `["sudo", …]`; auth probes (`-v`/`-n true`) - and drop-priv (`-u`) are the only exceptions. §22. + and drop-priv (`-u`) are the only exceptions. Every escalation arms the verb sentinel + unless it passes `mutates=False`, which is only for a command an interruption cannot leave + inconsistent (download, runtime setting, atomic/idempotent write, the sandbox chroot) or an + argv only displayed, with the reason beside it. §22. - **Sudo credential *lifetime*** (orthogonal to escalation): `primitives/sudo_session.py` — `authenticate()` (returns usability, so "not authorized, nothing ran" is distinguishable from "ran and failed") + `keepalive(tag=…, enabled=…)` context diff --git a/sysforge/primitives/atomic_write.py b/sysforge/primitives/atomic_write.py index e56bb9ce..7185d54f 100644 --- a/sysforge/primitives/atomic_write.py +++ b/sysforge/primitives/atomic_write.py @@ -162,13 +162,17 @@ def _replace_privileged(target: Path, data: bytes, mode: int, uid, gid, tag: str owner = ["-o", str(uid if uid is not None else 0), "-g", str(gid if gid is not None else 0)] _log.info(f" Writing (sudo): {target}") + # mutates=False: stage + rename is atomic, so an interrupted write + # leaves the old file or the new one, nothing to recover (3.5.0-B138). try: run_privileged(["install", "-m", f"{mode:o}", *owner, str(src), str(staged)], - tag=tag, operation="install") - run_privileged(["mv", "-fT", str(staged), str(target)], tag=tag, operation="mv") + tag=tag, mutates=False, operation="install") + run_privileged(["mv", "-fT", str(staged), str(target)], + tag=tag, mutates=False, operation="mv") except RuntimeError as e: with contextlib.suppress(RuntimeError): - run_privileged(["rm", "-f", str(staged)], tag=tag, operation="rm") + run_privileged(["rm", "-f", str(staged)], tag=tag, mutates=False, + operation="rm") raise OSError(f"{e}: {target} unchanged") from e finally: src.unlink(missing_ok=True) diff --git a/sysforge/primitives/build_sandbox.py b/sysforge/primitives/build_sandbox.py index a1e95c8f..0ee69cfa 100644 --- a/sysforge/primitives/build_sandbox.py +++ b/sysforge/primitives/build_sandbox.py @@ -600,7 +600,7 @@ def provision_profile_data(policy: SandboxPolicy, exports: dict | None, # 644: the container builds as builduser, which is not this uid. run_privileged( ["install", "-Dm644", str(src), str(dest)], - tag="SANDBOX", + tag="SANDBOX", mutates=False, # the build chroot, not the host ) except Exception as exc: raise SandboxUnavailable( @@ -658,7 +658,7 @@ def provision_toolchain(policy: SandboxPolicy, exports: dict | None, run_privileged( ["arch-nspawn", str(root), "pacman", "-Syu", "--needed", "--noconfirm", *packages], - tag="SANDBOX", + tag="SANDBOX", mutates=False, # the build chroot, not the host ) except Exception as exc: raise SandboxUnavailable( @@ -822,7 +822,7 @@ def slice_limits(policy: SandboxPolicy, throttle: BuildThrottle, env: dict): _log.info(f"Capping the sandbox slice {slice_name}: {' '.join(props)}") try: run_privileged(["systemctl", "set-property", "--runtime", slice_name, *props], - tag="SANDBOX") + tag="SANDBOX", mutates=False) # runtime only except Exception as exc: _log.warn(f"could not cap the sandbox slice {slice_name} ({exc}): " "the build runs without its cpu_quota/mem_limit") @@ -834,7 +834,7 @@ def slice_limits(policy: SandboxPolicy, throttle: BuildThrottle, env: dict): resets = [f"{key}={CGROUP_PROPERTY_DEFAULTS[key]}" for key in keys] try: run_privileged(["systemctl", "set-property", "--runtime", slice_name, *resets], - tag="SANDBOX") + tag="SANDBOX", mutates=False) # runtime only except Exception as exc: _log.warn(f"could not lift the cap on {slice_name} ({exc}): the next " "sandboxed build clears it, or a reboot does") @@ -1333,7 +1333,8 @@ def fetch_repo_packages(names, why: str = _RUST_WHY) -> list[Path]: ) try: run_privileged(["pacman", "-Sw", "--noconfirm", - *[f"{rows[n][0]}/{n}" for n in names]], tag="SANDBOX") + *[f"{rows[n][0]}/{n}" for n in names]], + tag="SANDBOX", mutates=False) # download only except Exception as exc: raise SandboxUnavailable( f"refusing to sandbox: could not download the repo build of " diff --git a/sysforge/primitives/fs_provision.py b/sysforge/primitives/fs_provision.py index 1ec5ce55..9900d963 100644 --- a/sysforge/primitives/fs_provision.py +++ b/sysforge/primitives/fs_provision.py @@ -75,7 +75,8 @@ def _run_priv(argv: list[str]) -> None: """Run a privileged command via sudo, translating failure into FsProvisionError so callers get one exception type to catch.""" try: - subprocess.run(privileged_argv(argv), check=True, preexec_fn=lift_for_child) # noqa: TID251 — privileged; CalledProcessError translated to FsProvisionError + # mutates=False: provisioning is idempotent, the next run redoes it. + subprocess.run(privileged_argv(argv, mutates=False), check=True, preexec_fn=lift_for_child) # noqa: TID251 — privileged; CalledProcessError translated to FsProvisionError except FileNotFoundError as e: # sudo not installed raise FsProvisionError(f"sudo not available: {e}") from e except subprocess.CalledProcessError as e: diff --git a/sysforge/primitives/privilege.py b/sysforge/primitives/privilege.py index 991c4a10..393f3105 100644 --- a/sysforge/primitives/privilege.py +++ b/sysforge/primitives/privilege.py @@ -27,11 +27,23 @@ terminal yielded so the progress bar is hidden and its clock paused (3.3.0-F3). Call it right before running something privileged — never from :func:`privileged_argv`, which also builds argv that a ``--dry-run`` only prints. + +The seam is also where a system change begins, so it tells +:func:`on_mutation` listeners of every escalation that changes the live system +before it runs (3.5.0-B138): the verb runner's stage sentinel records its run +only then, so a verb that fails before changing anything leaves no recovery +record behind. A site passes ``mutates=False`` only for a command that cannot +leave the system inconsistent if interrupted (a download, a runtime-only +setting, an atomic or idempotent write, a change inside the build sandbox's +chroot) or an argv built only to validate or display; anything else arms the +sentinel, so a new site is safe by default. """ from __future__ import annotations import os import subprocess +from collections.abc import Callable, Generator +from contextlib import contextmanager from sysforge.primitives import progress_hooks from sysforge.primitives.run import run_or_raise @@ -63,25 +75,49 @@ def ensure_credentials() -> bool: preexec_fn=lift_for_child).returncode == 0 -def privileged_argv(argv: list[str], *, noninteractive: bool = False) -> list[str]: +# Called with the unescalated argv of each system-changing escalation. A list +# rather than a call into stage_sentinel, which imports this module. +_mutation_listeners: list[Callable[[list[str]], None]] = [] + + +@contextmanager +def on_mutation(listener: Callable[[list[str]], None]) -> Generator[None, None, None]: + """Call *listener* before each escalation that changes the live system + (every :func:`privileged_argv` without ``mutates=False``) for the block.""" + _mutation_listeners.append(listener) + try: + yield + finally: + _mutation_listeners.remove(listener) + + +def privileged_argv( + argv: list[str], *, noninteractive: bool = False, mutates: bool = True, +) -> list[str]: """Return *argv* escalated to root. When already root (euid 0) the argv is returned unchanged. Otherwise it is prefixed with ``sudo``; ``noninteractive=True`` inserts ``-n`` so sudo fails - fast instead of prompting (moot when already root). + fast instead of prompting (moot when already root). Unless ``mutates`` is + False (see the module docstring for when), :func:`on_mutation` listeners + are told first, even when already root. """ + if mutates: + for listener in list(_mutation_listeners): + listener(list(argv)) if os.geteuid() == 0: return list(argv) return ["sudo", *(["-n"] if noninteractive else []), *argv] def run_privileged( - argv: list[str], *, tag: str, **kwargs + argv: list[str], *, tag: str, mutates: bool = True, **kwargs ) -> subprocess.CompletedProcess: """Escalate *argv* and run it through :func:`run_or_raise` (raise on non-zero). Credentials are acquired first via :func:`ensure_credentials`, so a prompt - never lands under the progress bar. + never lands under the progress bar. ``mutates`` as for + :func:`privileged_argv`. """ ensure_credentials() - return run_or_raise(privileged_argv(argv), tag=tag, **kwargs) + return run_or_raise(privileged_argv(argv, mutates=mutates), tag=tag, **kwargs) diff --git a/sysforge/primitives/provides_lookup.py b/sysforge/primitives/provides_lookup.py index 07bbc0b2..e90af144 100644 --- a/sysforge/primitives/provides_lookup.py +++ b/sysforge/primitives/provides_lookup.py @@ -66,7 +66,9 @@ def sync_files_db() -> bool: if not shutil.which("pacman"): return False try: - result = subprocess.run(privileged_argv(["pacman", "-Fy"]), preexec_fn=lift_for_child) # noqa: TID251 — privileged files-DB refresh streams, status inspected + # mutates=False: a files-DB download, no installed package changes. + argv = privileged_argv(["pacman", "-Fy"], mutates=False) + result = subprocess.run(argv, preexec_fn=lift_for_child) # noqa: TID251 — privileged files-DB refresh streams, status inspected except FileNotFoundError: return False return result.returncode == 0 diff --git a/sysforge/primitives/stage_sentinel.py b/sysforge/primitives/stage_sentinel.py index 6cf3f8ee..3e799d6e 100644 --- a/sysforge/primitives/stage_sentinel.py +++ b/sysforge/primitives/stage_sentinel.py @@ -14,7 +14,10 @@ This primitive writes a small TOML sentinel into ``<state_dir>/ stage_in_progress.toml`` just before the stage performs the first mutation, and clears it only after the stage's post-install verification -passes. Any subsequent sysforge invocation that loads state checks for a +passes. (A stage writes it when its scope opens, around the install; the +verb runner's scope writes it at the first escalation that changes the +system, via :func:`~sysforge.primitives.privilege.on_mutation`, 3.5.0-B138.) +Any subsequent sysforge invocation that loads state checks for a stale sentinel via :func:`get_active` and surfaces a recovery prompt before proceeding — converting "silently broken system" into "loud blocker at next run". @@ -75,7 +78,7 @@ import subprocess import tomllib from collections.abc import Generator, Iterable -from contextlib import ExitStack, contextmanager, suppress +from contextlib import ExitStack, contextmanager, nullcontext, suppress from dataclasses import dataclass from datetime import datetime, timezone from pathlib import Path @@ -84,7 +87,7 @@ from sysforge.primitives import atomic_write from sysforge.primitives.build_lock import build_lock from sysforge.primitives.interrupt import CleanExitRequested, InterruptScope -from sysforge.primitives.privilege import privileged_argv +from sysforge.primitives.privilege import on_mutation, privileged_argv from sysforge.primitives.prompt import is_interactive, prompt_choice from sysforge.primitives.resource_guard import lift_for_child @@ -198,9 +201,13 @@ def _command(self) -> list[str]: return ["mkinitcpio", "-P"] raise ValueError(f"unknown recovery kind {self.kind!r}") - def argv(self) -> list[str]: - """The escalated argv to run. Raises ``ValueError`` when invalid.""" - return privileged_argv(self._command()) + def argv(self, *, to_run: bool = True) -> list[str]: + """The escalated argv to run. Raises ``ValueError`` when invalid. + + ``to_run=False`` for an argv only validated or shown, which is no + system change (``privileged_argv(mutates=False)``). + """ + return privileged_argv(self._command(), mutates=to_run) def display(self) -> str: """The command as a copy-pasteable shell line, for messages. @@ -209,7 +216,7 @@ def display(self) -> str: string. An invalid record renders as its kind alone. """ try: - return shlex.join(self.argv()) + return shlex.join(self.argv(to_run=False)) except ValueError: return f"<invalid {self.kind} recovery>" @@ -231,7 +238,7 @@ def recovery_from_record(record: dict) -> "Recovery | None": return None rec = Recovery(kind, tuple(args)) try: - rec.argv() + rec.argv(to_run=False) except ValueError as e: _log.warn(f"Recorded recovery refused: {e}") return None @@ -540,6 +547,7 @@ def sentinel_scope( *, recovery: Recovery | None = None, retry_cmd: str | None = None, + arm_on_mutation: bool = False, **metadata, ) -> Generator["StageSentinel", None, None]: """Install a stage sentinel for the duration of the block. @@ -582,6 +590,13 @@ def sentinel_scope( retry_cmd: command the operator can re-run to resume from a clean state, surfaced in the ``CleanExitRequested`` → ``RuntimeError`` message (e.g. ``sysforge run toolchain``). + arm_on_mutation: write the record at the body's first system change + (the first :func:`~sysforge.primitives.privilege.privileged_argv` + that mutates) instead of on entry (3.5.0-B138). The verb runner + sets it: a verb's ``execute`` does its checks first, and a failure + there, before anything changed, must leave no record for the next + run to refuse on. The lock is still taken on entry. The stages + keep the default: they open their scope around the install itself. **metadata: extra fields persisted in the sentinel (e.g. ``compiler="llvm"``, ``pgo=True``). @@ -626,19 +641,33 @@ def sentinel_scope( "detection will be unavailable for this run" ) - try: - sentinel.mark_started(stage_name, **extra, **metadata) - except (OSError, PermissionError) as e: - _log.warn( - f"Cannot write stage sentinel ({e}); interrupted-run " - "detection will be unavailable for this run" - ) - + armed = False + + def arm(_argv: list[str] | None = None) -> None: + nonlocal armed + if armed: + return + armed = True + try: + sentinel.mark_started(stage_name, **extra, **metadata) + except (OSError, PermissionError) as e: + _log.warn( + f"Cannot write stage sentinel ({e}); interrupted-run " + "detection will be unavailable for this run" + ) + + if not arm_on_mutation: + arm() scope = InterruptScope() try: - with scope: + with scope, on_mutation(arm) if arm_on_mutation else nullcontext(): yield sentinel except CleanExitRequested: + if not armed: + raise RuntimeError( + f"[{stage_name.upper()}] interrupted by user (Ctrl-C) before " + "anything on the system changed." + ) from None _log.warn( f"{stage_name} interrupted at a safe boundary. Stage sentinel " "left in place; the next sysforge invocation will detect the " diff --git a/sysforge/verbs/runner.py b/sysforge/verbs/runner.py index 168de002..6467d124 100644 --- a/sysforge/verbs/runner.py +++ b/sysforge/verbs/runner.py @@ -14,7 +14,7 @@ Error model: • :class:`RuntimeError` raised from any phase → ``_log.error(msg)``, - return 1. Sentinel is preserved if active (the next sysforge run + return 1. Sentinel is preserved if written (the next sysforge run blocks at the CLI-entry recovery prompt). • :class:`SystemExit` propagates verbatim so tests and CLI hooks can see the raw exit code. @@ -24,8 +24,12 @@ Sentinel handling (only when ``verb.requires_sentinel`` and pre_check proceeds): the runner enters :func:`~sysforge.primitives.stage_sentinel.sentinel_scope` for the -duration of ``execute + post_validate``. The sentinel is written on -entry, cleared on a clean exit, and left in place on any exception. +duration of ``execute + post_validate``. The stage lock is taken on entry; +the sentinel record is written at the first system change (``arm_on_mutation``, +3.5.0-B138), cleared on a clean exit, and left in place on any exception +after it was written. A verb that fails before changing anything, such as +``build`` on a missing ``pkgctl``, so leaves nothing for the next run to +refuse on. """ from __future__ import annotations @@ -89,6 +93,7 @@ def _run_verb_inner(verb: Verb, args, _log) -> int: verb.name, recovery=verb.sentinel_recovery(args, pre), retry_cmd=f"sysforge {verb.name}", + arm_on_mutation=True, **verb.sentinel_metadata(args, pre), ) else: diff --git a/tests/term_model.py b/tests/term_model.py new file mode 100644 index 00000000..2208eceb --- /dev/null +++ b/tests/term_model.py @@ -0,0 +1,235 @@ +""" +term_model.py — a small terminal model for testing what sysforge's output does +to a real screen (3.5.0-DEV22). + +The progress bar's tests check the escape sequences it writes; this checks +where they leave the text and the cursor. ``Terminal.feed`` interprets the +subset of VT100/xterm control sequences sysforge emits and raises on any other, +so a new sequence cannot be silently misread: teach the model it first. + +Two flavours differ where real terminals do, in the rule both 3.5.0-B132 and +B133 turned on: + +- ``"xterm"``: a saved cursor (DECSC, ``ESC 7``) keeps its screen row across a + scroll. +- ``"xterm.js"`` (VS Code's terminal, vhs): a saved cursor follows its content + line, so a scroll of the region holding it moves it up with the text. + +Both restore the saved cursor on ``CSI ?1049l`` even when the alternate screen +was never entered, and keep one saved cursor per screen buffer. + +Lines a scroll pushes off a region whose top is the first row go to +``scrollback``, as in both. Newlines are fed as the terminal receives them +from a tty with ONLCR (``\\n`` → CR LF), the default. +""" +from __future__ import annotations + +import re + +FLAVORS = ("xterm", "xterm.js") + +_CSI_RE = re.compile(r"\[([0-?]*)([ -/]*)([@-~])") + + +class UnknownSequence(AssertionError): + """An escape sequence the model does not interpret.""" + + +class Terminal: + def __init__(self, rows: int = 10, cols: int = 40, *, flavor: str = "xterm") -> None: + if flavor not in FLAVORS: + raise ValueError(f"flavor must be one of {FLAVORS}") + self.rows, self.cols, self.flavor = rows, cols, flavor + self.scrollback: list[str] = [] + self._normal = self._blank_screen() + self._alt = self._blank_screen() + self.alt_active = False + self.row = self.col = 0 + self._wrap_pending = False + self.top, self.bottom = 0, rows - 1 + # One saved cursor per buffer: normal, alternate. + self._saved: dict[bool, tuple[int, int] | None] = {False: None, True: None} + + # -- the screen --------------------------------------------------------- + + def _blank_screen(self) -> list[list[str]]: + return [[" "] * self.cols for _ in range(self.rows)] + + @property + def _screen(self) -> list[list[str]]: + return self._alt if self.alt_active else self._normal + + def line(self, row: int) -> str: + """Row ``row`` of the visible screen, trailing blanks stripped.""" + return "".join(self._screen[row]).rstrip() + + def lines(self) -> list[str]: + return [self.line(r) for r in range(self.rows)] + + def transcript(self) -> list[str]: + """Every non-empty line, scrollback then screen, in order.""" + return [ln for ln in self.scrollback + self.lines() if ln] + + @property + def cursor(self) -> tuple[int, int]: + return self.row, self.col + + # -- input -------------------------------------------------------------- + + def feed(self, data: str) -> None: + i = 0 + while i < len(data): + ch = data[i] + if ch == "\x1b": + i = self._escape(data, i + 1) + continue + if ch == "\n": + self.col = 0 + self._linefeed() + elif ch == "\r": + self.col = 0 + self._wrap_pending = False + elif ch == "\b": + self.col = max(0, self.col - 1) + self._wrap_pending = False + elif ch == "\a": + pass + elif ch < " ": + raise UnknownSequence(f"control character {ch!r}") + else: + self._put(ch) + i += 1 + + def _put(self, ch: str) -> None: + if self._wrap_pending: + self.col = 0 + self._linefeed() + self._screen[self.row][self.col] = ch + if self.col == self.cols - 1: + self._wrap_pending = True + else: + self.col += 1 + + def _linefeed(self) -> None: + self._wrap_pending = False + if self.row == self.bottom: + self._scroll_up() + elif self.row < self.rows - 1: + self.row += 1 + + def _scroll_up(self) -> None: + screen = self._screen + gone = "".join(screen[self.top]).rstrip() + if self.top == 0 and not self.alt_active: + self.scrollback.append(gone) + del screen[self.top] + screen.insert(self.bottom, [" "] * self.cols) + saved = self._saved[self.alt_active] + if (self.flavor == "xterm.js" and saved is not None + and self.top <= saved[0] <= self.bottom): + self._saved[self.alt_active] = (max(self.top, saved[0] - 1), saved[1]) + + # -- escape sequences --------------------------------------------------- + + def _escape(self, data: str, i: int) -> int: + """Interpret the sequence after an ESC at ``data[i]``; return the + index after it.""" + if i >= len(data): + raise UnknownSequence("truncated escape sequence") + ch = data[i] + if ch == "7": + self._saved[self.alt_active] = (self.row, self.col) + elif ch == "8": + self._restore() + elif ch == "D": + self._linefeed() + elif ch == "]": + end = min((j for j in (data.find("\a", i), data.find("\x1b\\", i)) if j >= 0), + default=-1) + if end < 0: + raise UnknownSequence("unterminated OSC") + return end + (1 if data[end] == "\a" else 2) + elif ch == "[": + m = _CSI_RE.match(data, i) + if m is None: + raise UnknownSequence(f"malformed CSI at {data[i:i + 10]!r}") + self._csi(m.group(1), m.group(2), m.group(3)) + return m.end() + else: + raise UnknownSequence(f"ESC {ch!r}") + return i + 1 + + def _restore(self) -> None: + saved = self._saved[self.alt_active] or (0, 0) + self.row, self.col = saved + self._wrap_pending = False + + def _csi(self, params: str, inter: str, final: str) -> None: + if inter: + raise UnknownSequence(f"CSI {params}{inter}{final}") + if params.startswith("?"): + self._private_mode(params[1:], final) + return + nums = [int(p) if p else 0 for p in params.split(";")] if params else [] + + def arg(k: int, default: int) -> int: + return nums[k] if len(nums) > k and nums[k] else default + + self._wrap_pending = False + if final == "A": + floor = self.top if self.row >= self.top else 0 + self.row = max(floor, self.row - arg(0, 1)) + elif final == "B": + ceiling = self.bottom if self.row <= self.bottom else self.rows - 1 + self.row = min(ceiling, self.row + arg(0, 1)) + elif final == "C": + self.col = min(self.cols - 1, self.col + arg(0, 1)) + elif final == "D": + self.col = max(0, self.col - arg(0, 1)) + elif final in ("H", "f"): + self.row = min(self.rows, arg(0, 1)) - 1 + self.col = min(self.cols, arg(1, 1)) - 1 + elif final == "K": + mode = arg(0, 0) + cells = self._screen[self.row] + span = {0: range(self.col, self.cols), 1: range(0, self.col + 1), + 2: range(self.cols)}.get(mode) + if span is None: + raise UnknownSequence(f"CSI {params}K") + for c in span: + cells[c] = " " + elif final == "J": + if arg(0, 0) != 0: + raise UnknownSequence(f"CSI {params}J") + for c in range(self.col, self.cols): + self._screen[self.row][c] = " " + for r in range(self.row + 1, self.rows): + self._screen[r] = [" "] * self.cols + elif final == "r": + top, bottom = arg(0, 1) - 1, arg(1, self.rows) - 1 + if top < bottom <= self.rows - 1: + self.top, self.bottom = top, bottom + self.row = self.col = 0 # DECSTBM homes the cursor + elif final == "m": + pass # colour and attributes: not modelled + else: + raise UnknownSequence(f"CSI {params}{final}") + + def _private_mode(self, params: str, final: str) -> None: + if final not in ("h", "l"): + raise UnknownSequence(f"CSI ?{params}{final}") + for mode in params.split(";"): + if mode in ("25", "7", "2004"): + continue # cursor visibility, autowrap, bracketed paste + if mode != "1049": + raise UnknownSequence(f"CSI ?{mode}{final}") + if final == "h": + self._saved[False] = (self.row, self.col) + self.alt_active = True + self._alt = self._blank_screen() + self.row = self.col = 0 + else: + # Restores the cursor even when the alternate screen was never + # entered (the 3.5.0-B132 mechanism). + self.alt_active = False + self._restore() diff --git a/tests/test_ci_tooling.py b/tests/test_ci_tooling.py index af74e6f8..e9ccac32 100644 --- a/tests/test_ci_tooling.py +++ b/tests/test_ci_tooling.py @@ -30,12 +30,26 @@ def _print_gates() -> list[str]: def test_pre_release_and_ci_resolve_to_the_same_gates(): """`pre-release` depends on `$(GATES)` and ci.sh runs `print-gates`, so the - two lists are one list (3.5.0-DEV8).""" + two lists are one list (3.5.0-DEV8); the rest of `pre-release` is + `$(CI_JOBS)`, which CI runs as jobs of their own (3.5.0-DEV21).""" cs = _load_check_standards() text = (REPO / "Makefile").read_text(encoding="utf-8") m = re.search(r"^pre-release:([^#\n]*)", text, re.MULTILINE) assert m is not None - assert cs._expand_make_vars(m.group(1).split(), text) == _print_gates() + jobs = cs._expand_make_vars(["$(CI_JOBS)"], text) + assert cs._expand_make_vars(m.group(1).split(), text) == _print_gates() + jobs + + +def test_every_ci_job_gate_has_its_own_ci_job(): + """A `pre-release` prerequisite outside GATES (`CI_JOBS`) is run by a step + of .github/workflows/ci.yml, or CI would not run what `pre-release` does.""" + cs = _load_check_standards() + text = (REPO / "Makefile").read_text(encoding="utf-8") + workflow = (REPO / ".github" / "workflows" / "ci.yml").read_text(encoding="utf-8") + jobs = cs._expand_make_vars(["$(CI_JOBS)"], text) + assert jobs and jobs != ["$(CI_JOBS)"] + for job in jobs: + assert re.search(rf"^\s*run: make {re.escape(job)}\b", workflow, re.MULTILINE), job def test_ci_sh_reads_the_gate_list_instead_of_spelling_it(): diff --git a/tests/test_privilege.py b/tests/test_privilege.py index b4fbf8a0..5aac809f 100644 --- a/tests/test_privilege.py +++ b/tests/test_privilege.py @@ -52,6 +52,52 @@ def fake_run_or_raise(cmd, *, tag, **kw): assert seen["tag"] == "TEST" +# --------------------------------------------------------------------------- +# on_mutation (3.5.0-B138) — the stage sentinel arms at the first change +# --------------------------------------------------------------------------- + +def test_listener_sees_each_mutating_escalation_unescalated(): + seen = [] + with patch("sysforge.primitives.privilege.os.geteuid", return_value=1000), \ + privilege.on_mutation(seen.append): + argv = privilege.privileged_argv(["pacman", "-U", "x.pkg"]) + assert seen == [["pacman", "-U", "x.pkg"]] + assert argv == ["sudo", "pacman", "-U", "x.pkg"] + + +def test_listener_told_when_already_root(): + seen = [] + with patch("sysforge.primitives.privilege.os.geteuid", return_value=0), \ + privilege.on_mutation(seen.append): + privilege.privileged_argv(["mkinitcpio", "-P"]) + assert seen == [["mkinitcpio", "-P"]] + + +def test_non_mutating_escalation_is_not_announced(): + seen = [] + with privilege.on_mutation(seen.append): + privilege.privileged_argv(["pacman", "-Sw", "x"], mutates=False) + assert seen == [] + + +def test_run_privileged_forwards_mutates(monkeypatch): + seen = [] + monkeypatch.setattr(privilege, "ensure_credentials", lambda: True) + monkeypatch.setattr(privilege, "run_or_raise", lambda cmd, **kw: "OK") + with privilege.on_mutation(seen.append): + privilege.run_privileged(["systemctl", "set-property", "x"], tag="T", mutates=False) + privilege.run_privileged(["systemctl", "daemon-reload"], tag="T") + assert seen == [["systemctl", "daemon-reload"]] + + +def test_listener_removed_after_the_block(): + seen = [] + with privilege.on_mutation(seen.append): + pass + privilege.privileged_argv(["pacman", "-U", "x.pkg"]) + assert seen == [] + + # --------------------------------------------------------------------------- # ensure_credentials (3.3.0-F3) — prompt with the progress bar yielded # --------------------------------------------------------------------------- diff --git a/tests/test_revert_cmd.py b/tests/test_revert_cmd.py index c8cc9957..da92f355 100644 --- a/tests/test_revert_cmd.py +++ b/tests/test_revert_cmd.py @@ -199,9 +199,15 @@ def test_failed_derename_keeps_the_sentinel(tmp_path, monkeypatch): "pkgbase": "linux-sysforge", "origin_pkgbase": "linux"} bs.save() args = _args(packages=["linux-sysforge"], yes=True) - with patch.object(revert_cmd.pacman, "reinstall_repo_pkgs", - side_effect=subprocess.CalledProcessError(1, ["pacman"])), \ - patch.object(revert_cmd.pacman, "remove_pkgs"): + + # Stubbed below the real helpers, so their escalations go through the + # privilege seam: the record is written at the `-R` (3.5.0-B138). + def pacman_run(argv, **kw): + if "-S" in argv: + raise subprocess.CalledProcessError(1, argv) + return subprocess.CompletedProcess(argv, 0) + + with patch.object(revert_cmd.pacman.subprocess, "run", side_effect=pacman_run): assert runner.run_verb(revert_cmd.RevertToStockVerb(), args) == 1 assert StageSentinel(tmp_path).get_active() is not None diff --git a/tests/test_sentinel_scope.py b/tests/test_sentinel_scope.py index 8ea45cc8..3ea56ab7 100644 --- a/tests/test_sentinel_scope.py +++ b/tests/test_sentinel_scope.py @@ -5,7 +5,8 @@ Mirrors the behaviour previously tested only via the toolchain stage's inline copy; the shared primitive must guarantee: - • Sentinel is written on entry and cleared on a clean exit. + • Sentinel is written on entry (or, with arm_on_mutation, at the first + system change) and cleared on a clean exit. • Sentinel is left in place on any exception inside the body. • CleanExitRequested is translated to a RuntimeError that surfaces both ``retry_cmd`` and ``recovery_cmd`` to the operator. @@ -137,3 +138,82 @@ def test_verb_over_stale_record_keeps_it_non_interactive(tmp_path): with sentinel_scope(tmp_path, "state-repair"): pass assert StageSentinel(tmp_path).get_active()["stage"] == "toolchain" + + +# --------------------------------------------------------------------------- +# arm_on_mutation: the verb runner's record waits for the first change +# (3.5.0-B138) +# --------------------------------------------------------------------------- + +def _install_argv(): + from sysforge.primitives.privilege import privileged_argv + return privileged_argv(["pacman", "-U", "--noconfirm", "--", "x.pkg.tar.zst"]) + + +def test_armed_scope_writes_nothing_until_the_first_change(tmp_path): + with sentinel_scope(tmp_path, "build", arm_on_mutation=True): + assert StageSentinel(tmp_path).get_active() is None + _install_argv() + assert StageSentinel(tmp_path).get_active()["stage"] == "build" + assert StageSentinel(tmp_path).get_active() is None + + +def test_armed_scope_failure_before_a_change_leaves_no_record(tmp_path): + with pytest.raises(RuntimeError), sentinel_scope(tmp_path, "build", arm_on_mutation=True): + raise RuntimeError("pkgctl not found on PATH") + assert StageSentinel(tmp_path).get_active() is None + + +def test_armed_scope_failure_after_a_change_keeps_the_record(tmp_path): + rec = Recovery.pacman_sync(["llvm"]) + with pytest.raises(RuntimeError), sentinel_scope( + tmp_path, "update", recovery=rec, arm_on_mutation=True, compiler="llvm"): + _install_argv() + raise RuntimeError("install failed") + record = StageSentinel(tmp_path).get_active() + assert record["stage"] == "update" + assert record["recovery_kind"] == "pacman_sync" + assert record["compiler"] == "llvm" + + +def test_armed_scope_writes_the_record_once(tmp_path): + with sentinel_scope(tmp_path, "update", arm_on_mutation=True): + _install_argv() + with patch.object(StageSentinel, "mark_started") as again: + _install_argv() + again.assert_not_called() + + +def test_armed_scope_holds_the_lock_before_any_change(tmp_path): + """Liveness does not wait for the record: a second run is refused while + the first is still in its checks.""" + from sysforge.primitives.stage_sentinel import check_and_recover_stale_sentinel + with sentinel_scope(tmp_path, "build", arm_on_mutation=True): + assert check_and_recover_stale_sentinel(tmp_path) is False + + +def test_armed_scope_ctrl_c_before_a_change_says_nothing_changed(tmp_path): + with pytest.raises(RuntimeError) as exc_info, sentinel_scope( + tmp_path, "build", recovery=Recovery.pacman_sync(["llvm"]), + retry_cmd="sysforge build", arm_on_mutation=True): + raise CleanExitRequested() + msg = str(exc_info.value) + assert "before anything on the system changed" in msg + assert "pacman" not in msg + assert StageSentinel(tmp_path).get_active() is None + + +def test_armed_scope_stops_listening_after_the_body(tmp_path): + with sentinel_scope(tmp_path, "build", arm_on_mutation=True): + pass + _install_argv() + assert StageSentinel(tmp_path).get_active() is None + + +def test_recovery_display_is_not_a_change(tmp_path): + """The stages print their recovery command mid-body; rendering it must + not arm the record.""" + rec = Recovery.pacman_sync(["llvm"]) + with sentinel_scope(tmp_path, "build", recovery=rec, arm_on_mutation=True): + rec.display() + assert StageSentinel(tmp_path).get_active() is None diff --git a/tests/test_stage_sentinel.py b/tests/test_stage_sentinel.py index 824029e1..b8f3195b 100644 --- a/tests/test_stage_sentinel.py +++ b/tests/test_stage_sentinel.py @@ -302,7 +302,7 @@ def test_symlinked_sentinel_is_refused(tmp_path): def test_each_recovery_kind_rebuilds_its_command(tmp_path, monkeypatch): """pacman_sync and mkinitcpio rebuild from templates; pacman_upgrade accepts a root-owned archive in a pacman cache dir only.""" - monkeypatch.setattr(ss, "privileged_argv", lambda argv: ["sudo", *argv]) + monkeypatch.setattr(ss, "privileged_argv", lambda argv, **k: ["sudo", *argv]) assert Recovery.pacman_sync(["llvm", "lib32-llvm-libs"]).argv() == [ "sudo", "pacman", "-S", "--", "llvm", "lib32-llvm-libs"] assert Recovery.mkinitcpio().argv() == ["sudo", "mkinitcpio", "-P"] diff --git a/tests/test_terminal_model.py b/tests/test_terminal_model.py new file mode 100644 index 00000000..cb6fbbf6 --- /dev/null +++ b/tests/test_terminal_model.py @@ -0,0 +1,195 @@ +""" +test_terminal_model.py — the progress bar and update's terminal reset, replayed +through a terminal model under both saved-cursor rules (3.5.0-DEV22). + +``test_progress.py`` checks the sequences the bar writes; these check what they +do to the screen: the command line survives, output follows it in order, the +bar leaves nothing behind and the next prompt lands after the last line, under +xterm's and xterm.js's rules for a saved cursor (``tests/term_model.py``). The +``*_pre_fix_*`` tests replay the sequences 3.5.0-B132 and B133 replaced, to +show the model sees both bugs. +""" +import os +import shutil +import sys + +import pytest + +from sysforge import log +from sysforge.ui import progress +from sysforge.update import _TERMINAL_RESET +from term_model import FLAVORS, Terminal, UnknownSequence + +ROWS, COLS = 10, 50 +COMMAND = "$ sysforge build cbonsai" + + +class _TermStream: + """A terminal stderr: isatty() is true and every write reaches the model.""" + + def __init__(self, term: Terminal) -> None: + self._term = term + + def isatty(self) -> bool: + return True + + def write(self, s: str) -> int: + self._term.feed(s) + return len(s) + + def flush(self) -> None: + pass + + +@pytest.fixture(autouse=True) +def _tty_env(monkeypatch): + monkeypatch.setattr(log, "_DRY_RUN", False) + for var in ("CI", "NO_COLOR"): + monkeypatch.delenv(var, raising=False) + monkeypatch.setenv("TERM", "xterm-256color") + monkeypatch.setattr(shutil, "get_terminal_size", + lambda fallback=None: os.terminal_size((COLS, ROWS))) + yield + progress.shutdown() + + +@pytest.fixture(params=FLAVORS) +def term(request) -> Terminal: + return Terminal(ROWS, COLS, flavor=request.param) + + +def _shell(term: Terminal, earlier: int) -> list[str]: + """``earlier`` lines of earlier output, then the command typed and Enter + pressed; returns what the transcript holds so far.""" + before = [f"earlier {n}" for n in range(1, earlier + 1)] + for line in before: + term.feed(line + "\n") + term.feed(COMMAND + "\n") + return [*before, COMMAND] + + +def _run(term: Terminal, output: list[str]) -> None: + """A command's run: the bar comes up, output scrolls past while the bar + repaints, and the bar is released. + + stderr is bound here, in the test's own phase: pytest's capture puts its + own back between a fixture's setup and the test body.""" + with pytest.MonkeyPatch.context() as mp: + mp.setattr(sys, "stderr", _TermStream(term)) + progress.init() + assert progress._bar.mode == "tty" + progress.phase("syncing sources") + for n, line in enumerate(output): + term.feed(line + "\n") + if n % 3 == 0: + progress.phase(f"building {n}") # a repaint saves the cursor + progress.shutdown() + + +def _no_bar_left(term: Terminal) -> None: + assert not any("PROGRESS" in line for line in term.transcript()) + + +# --- B133: the first line of output and the command line ---------------------- + +def test_output_follows_a_command_mid_screen(term): + seen = _shell(term, earlier=2) + _run(term, ["line 1", "line 2"]) + assert term.transcript() == [*seen, "line 1", "line 2"] + assert term.scrollback == [] # free rows below: nothing pushed off the top + _no_bar_left(term) + + +def test_output_follows_a_command_on_the_bottom_row(term): + seen = _shell(term, earlier=ROWS - 1) # Enter on the bottom row scrolls + _run(term, ["line 1", "line 2"]) + assert term.transcript() == [*seen, "line 1", "line 2"] + _no_bar_left(term) + + +def test_long_output_scrolls_in_order_past_the_bar(term): + seen = _shell(term, earlier=4) + output = [f"line {n}" for n in range(1, 3 * ROWS)] + _run(term, output) + assert term.transcript() == [*seen, *output] + _no_bar_left(term) + + +def test_establish_pre_fix_overwrites_the_command_on_xterm_js(term, monkeypatch): + """The establish sequence B133 replaced (save, jump to the bottom, scroll, + set region, restore, up one): fine on xterm, one line high on xterm.js.""" + def pre_fix() -> str: + if progress._bar.reserved: + return "" + progress._refresh_size() + rows = progress._bar.rows + progress._bar.reserved = True + e = progress._ESC + return (f"{progress._SAVE}{e}[{rows};1H{progress._INDEX}{e}[1;{rows - 1}r" + f"{progress._RESTORE}{e}[1A") + + monkeypatch.setattr(progress, "_establish_seq", pre_fix) + seen = _shell(term, earlier=ROWS - 1) + _run(term, ["line 1", "line 2"]) + if term.flavor == "xterm.js": + assert COMMAND not in term.transcript() + else: + assert term.transcript() == [*seen, "line 1", "line 2"] + + +# --- B132: the end-of-run reset and the next prompt --------------------------- + +def _summary_then_prompt(term: Terminal, reset: str) -> list[str]: + """update's ending: the summary printed after the bar is released, the + terminal reset, then the shell's next prompt.""" + summary = ["Update complete: 1 built", " cbonsai: 1.4.2-1"] + for line in summary: + term.feed(line + "\n") + term.feed(reset) + term.feed("$ next") + return summary + + +def test_next_prompt_lands_after_the_summary(term): + seen = _shell(term, earlier=3) + output = [f"line {n}" for n in range(1, 6)] + _run(term, output) + summary = _summary_then_prompt(term, _TERMINAL_RESET) + assert term.transcript() == [*seen, *output, *summary, "$ next"] + + +def test_reset_pre_fix_sends_the_prompt_back_over_the_output(term): + """B132's reset, without the DECSC ahead of ?1049l, restores the bar's + last saved cursor on both terminals.""" + seen = _shell(term, earlier=3) + output = [f"line {n}" for n in range(1, 6)] + _run(term, output) + summary = _summary_then_prompt(term, "\x1b[?1049l\x1b[?25h\x1b[0m") + assert term.transcript() != [*seen, *output, *summary, "$ next"] + + +def test_reset_after_a_killed_pager_returns_to_the_normal_screen(term): + """A pager killed on the alternate screen: the reset leaves it, and the + cursor goes back to where the pager started.""" + seen = _shell(term, earlier=2) + term.feed("\x1b[?1049h" + "diff --git a/PKGBUILD b/PKGBUILD\n+pkgver=2\n") + term.feed(_TERMINAL_RESET) + term.feed("$ next") + assert not term.alt_active + assert term.transcript() == [*seen, "$ next"] + + +# --- the model itself -------------------------------------------------------- + +def test_model_refuses_a_sequence_it_does_not_know(): + with pytest.raises(UnknownSequence): + Terminal().feed("\x1b[5S") # scroll up: not modelled + + +@pytest.mark.parametrize("flavor", FLAVORS) +def test_saved_cursor_across_a_scroll(flavor): + t = Terminal(3, 10, flavor=flavor) + t.feed("a\nb\nc") # cursor on the bottom row, on "c" + t.feed("\x1b7\n\x1b8") # save, scroll one, restore + assert t.lines() == ["b", "c", ""] + assert t.cursor == ((2, 1) if flavor == "xterm" else (1, 1)) diff --git a/tests/test_verb_runner.py b/tests/test_verb_runner.py index e1efff16..5eb22c04 100644 --- a/tests/test_verb_runner.py +++ b/tests/test_verb_runner.py @@ -12,6 +12,7 @@ import pytest from sysforge.primitives.config import ConfigError +from sysforge.primitives.privilege import privileged_argv from sysforge.primitives.stage_sentinel import Recovery, StageSentinel from sysforge.verbs import ExecResult, PreCheckResult, Verb, run_verb @@ -260,14 +261,18 @@ class _SentinelVerb(Verb): name = "sentverb" requires_sentinel = True - def __init__(self, *, fail_execute=False, recovery=None): + def __init__(self, *, fail_execute=False, recovery=None, mutate=True): self._fail_execute = fail_execute self._recovery = recovery or Recovery.mkinitcpio() + self._mutate = mutate def pre_check(self, args) -> PreCheckResult: return PreCheckResult() def execute(self, args, pre) -> ExecResult: + if self._mutate: + # The escalation a real install builds right before running it. + privileged_argv(["pacman", "-U", "--noconfirm", "--", "x.pkg.tar.zst"]) if self._fail_execute: raise RuntimeError("execute failed") return ExecResult() @@ -285,7 +290,8 @@ def test_sentinel_cleared_on_success(tmp_path): def test_sentinel_preserved_on_failure(tmp_path): - """Verb RuntimeError → sentinel left in place for next-run recovery.""" + """Verb RuntimeError after a system change → sentinel left in place for + next-run recovery.""" v = _SentinelVerb(fail_execute=True) rc = run_verb(v, _args(state_dir=str(tmp_path))) assert rc == 1 @@ -295,6 +301,29 @@ def test_sentinel_preserved_on_failure(tmp_path): assert record["recovery_kind"] == "mkinitcpio" +def test_failure_before_any_change_leaves_no_sentinel(tmp_path): + """3.5.0-B138: `build` failing on a missing pkgctl, before installing + anything, left `stage = "build"` behind and the next run refused.""" + v = _SentinelVerb(fail_execute=True, mutate=False) + rc = run_verb(v, _args(state_dir=str(tmp_path))) + assert rc == 1 + assert StageSentinel(tmp_path).get_active() is None + + +def test_non_mutating_escalation_does_not_arm_the_sentinel(tmp_path): + """A download or an atomic config write (mutates=False) is no change a + recovery record is for.""" + + class _DownloadVerb(_SentinelVerb): + def execute(self, args, pre): + privileged_argv(["pacman", "-Sw", "--noconfirm", "x"], mutates=False) + raise RuntimeError("build failed") + + rc = run_verb(_DownloadVerb(mutate=False), _args(state_dir=str(tmp_path))) + assert rc == 1 + assert StageSentinel(tmp_path).get_active() is None + + def test_sentinel_skipped_when_requires_sentinel_false(tmp_path): """Read-only verbs never touch the sentinel file.""" v = _CountingVerb() diff --git a/tools/check_standards.py b/tools/check_standards.py index b2b653c0..ac64eb9e 100644 --- a/tools/check_standards.py +++ b/tools/check_standards.py @@ -1801,8 +1801,9 @@ def check_semver_bump(repo: Path, # `pre-release` prerequisites deliberately left to the heavier tier: the full # suite is minutes, not seconds (docs/RELEASE-CHECKLIST.md "Which command -# does what"). -_RELEASE_GATE_EXEMPT = {"test"} +# does what"), and the end-to-end tier (3.5.0-DEV21) is minutes too, needs +# podman, and fetches from the AUR, so it could not be a deterministic gate. +_RELEASE_GATE_EXEMPT = {"test", "e2e"} _PRE_RELEASE_RE = re.compile(r"^pre-release:([^#\n]*)", re.MULTILINE) _RELEASE_MAKE_RE = re.compile(r"\bmake\s+(?:--no-print-directory\s+)?([a-z][\w-]*)") _MAKE_VAR_REF_RE = re.compile(r"\$\((\w+)\)") diff --git a/tools/demo/Containerfile b/tools/demo/Containerfile index 6ba028f9..6c1c81ff 100644 --- a/tools/demo/Containerfile +++ b/tools/demo/Containerfile @@ -1,18 +1,12 @@ -# Container for the README/guide terminal recordings (docs/assets/*.tape). +# Container for the terminal recordings (docs/assets/*.tape) and the +# end-to-end tier (tools/e2e/). # -# A fresh Arch system with an unprivileged `demo` user who can sudo without a -# password, as a new user would be right after installing sysforge. sysforge -# itself is built from the checkout by tools/record-demo.sh, not here, so the -# image doesn't go stale with every commit. +# A fresh Arch system with an unprivileged `demo` user (provision.sh). sysforge +# itself is built from the checkout by install.sh, not here, so the image +# doesn't go stale with every commit. FROM docker.io/library/archlinux:base-devel -# devtools: pkgctl, which `sysforge build` uses to check out a repo package's -# PKGBUILD. less: the pager a review's [v]iew diff opens, as on most systems. -RUN pacman -Syu --noconfirm --needed git sudo python pyalpm uv python-installer devtools less \ - && pacman -Scc --noconfirm - -RUN useradd -m -G wheel demo \ - && echo 'demo ALL=(ALL:ALL) NOPASSWD: ALL' > /etc/sudoers.d/demo \ - && chmod 0440 /etc/sudoers.d/demo +COPY provision.sh /usr/local/sbin/sysforge-provision +RUN /usr/local/sbin/sysforge-provision CMD ["sleep", "infinity"] diff --git a/tools/demo/install.sh b/tools/demo/install.sh new file mode 100755 index 00000000..96230f07 --- /dev/null +++ b/tools/demo/install.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# install.sh -- as the demo user: install sysforge from the checkout at +# /src/sysforge, as Quick start step 1 does from the AUR. +# +# Usage: install.sh COMMIT +# +# PKGBUILD-git is pointed at the checkout, so the package is built from COMMIT +# (uncommitted edits are not in it). +set -euo pipefail + +commit=$1 +rm -rf ~/pkg +mkdir -p ~/pkg +cd ~/pkg +cp /src/sysforge/PKGBUILD-git PKGBUILD +cp /src/sysforge/sysforge.install . +sed -i "s|^source=.*|source=(\"\$pkgname::git+file:///src/sysforge#commit=$commit\")|" PKGBUILD +makepkg -si --noconfirm diff --git a/tools/demo/prepare.sh b/tools/demo/prepare.sh index 1d1fb2a6..7786f921 100755 --- a/tools/demo/prepare.sh +++ b/tools/demo/prepare.sh @@ -1,23 +1,15 @@ #!/usr/bin/env bash -# prepare.sh -- inside the demo container, as the demo user: install sysforge -# from the checkout mounted at /src/sysforge and run the Quick start's one-time -# setup, leaving the system where a new user is after steps 1-3. +# prepare.sh -- as the demo user: install sysforge from the checkout mounted at +# /src/sysforge and run the Quick start's one-time setup, leaving the system +# where a new user is after steps 1-3. # -# Usage: prepare.sh BRANCH +# Usage: prepare.sh COMMIT # -# PKGBUILD-git is pointed at the mounted checkout, so the package is built -# from BRANCH's committed HEAD (uncommitted edits are not in it). The setup -# runs with EDITOR=true, which accepts every shipped config file unchanged. +# The setup runs with EDITOR=true, which accepts every shipped config file +# unchanged. set -euo pipefail -branch=$1 -mkdir -p ~/pkg -cd ~/pkg -cp /src/sysforge/PKGBUILD-git PKGBUILD -cp /src/sysforge/sysforge.install . -sed -i "s|^source=.*|source=(\"\$pkgname::git+file:///src/sysforge#branch=$branch\")|" PKGBUILD -makepkg -si --noconfirm - +"$(dirname "$0")/install.sh" "$1" cd ~ sudo sysforge run hardware </dev/null sudo EDITOR=true sysforge run reconfigure </dev/null diff --git a/tools/demo/provision.sh b/tools/demo/provision.sh new file mode 100755 index 00000000..114e482c --- /dev/null +++ b/tools/demo/provision.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# provision.sh -- as root on a fresh archlinux:base-devel system: the packages +# and unprivileged `demo` user the recordings (tools/record-demo.sh) and the +# end-to-end tier (tools/e2e/) start from. The Containerfile runs it at image +# build; `tools/e2e/run.sh --here` runs it directly, as in CI. +# +# demo can sudo without a password, as a new user would right after +# installing sysforge. sysforge itself is not installed here (install.sh). +set -euo pipefail + +# devtools: pkgctl, which `sysforge build` uses to check out a repo package's +# PKGBUILD. less: the pager a review's [v]iew diff opens, as on most systems. +pacman -Syu --noconfirm --needed git sudo python pyalpm uv python-installer devtools less +pacman -Scc --noconfirm + +id demo >/dev/null 2>&1 || useradd -m -G wheel demo +echo 'demo ALL=(ALL:ALL) NOPASSWD: ALL' > /etc/sudoers.d/demo +chmod 0440 /etc/sudoers.d/demo diff --git a/tools/e2e/README.md b/tools/e2e/README.md new file mode 100644 index 00000000..bf67a979 --- /dev/null +++ b/tools/e2e/README.md @@ -0,0 +1,65 @@ +# SysForge End-to-End Tier + +The documented paths, run the way a new user meets them: on a fresh Arch system, as an +unprivileged user, one command after another. The unit suite stubs out exactly the places this +tier exercises: root versus the user, group membership that applies only at the next login, the +state one command leaves for the next, and a real terminal. + +```bash +make e2e # every scenario, each in its own container (needs podman) +make e2e SCENARIO="drift optin" # only the named ones +make e2e E2E_HERE=1 # as root on this system, no podman: what CI runs +``` + +sysforge is built from the checkout's committed `HEAD`. Uncommitted edits under `sysforge/` are +not in the build; edits to the scenarios are, since they run from the working tree. To find when a +scenario broke, build an older revision with today's scenarios: +`make e2e E2E_COMMIT=<rev> SCENARIO=quickstart`. + +## How it runs + +`run.sh` builds the demo image (`tools/demo/`, shared with `make demo-gif`), installs sysforge in +it as the `demo` user (`tools/demo/install.sh`) and saves two start states: + +| Start state | What is done | Scenarios | +|-------------|--------------|-----------| +| `installed` | Quick start step 1: the package installed | the Quick start itself | +| `ready` | the one-time setup too (`run reconfigure`, `run hardware`) | everything else | + +Each scenario then gets a fresh container from its start state and runs in one `podman exec`, +that is one login session. With `--here` every scenario shares the one system in order, and the +first `ready` scenario runs the setup itself when the Quick start did not. + +The scenarios assert on exit codes, `pacman -Qi`, `build_state.toml`, `packages.toml` and leftover +sentinels (`lib.sh`), not on rendered output. Prompts are answered on a pseudo-terminal +(`answer.py`), so `build` takes the same interactive paths it takes at a real terminal. + +## Scenarios + +| Scenario | Covers | +|----------|--------| +| `10-quickstart` | the README Quick start in one login: setup, an AUR build through the review gate, `update` | +| `20-repo-optin` | a repo package refused without a terminal, the opt-in prompt, the saved answer | +| `30-flag-drift` | a default-profile change reported by `update --dry-run` and rebuilt by `--rebuild-on-flag-drift` | +| `40-pre-install-failure` | a build failing before any install leaves no sentinel | + +Packages come from `pkgs/`, local PKGBUILDs with no network sources, so an outage can't flake the +tier. `10-quickstart` is the one networked scenario (the AUR, and pacman for build dependencies). +`pkgs/tree` is a stand-in named after a repo package, which puts `build` on its repo opt-in path +without fetching Arch's packaging repo. + +## Writing a scenario + +A scenario is a bash fragment in `scenarios/`, sourced with `set -euo pipefail` after `lib.sh`, +running as `demo` in `~`. Its header names where it starts, and optionally a roadmap item it +fails until fixed: + +```bash +# What the scenario shows, in a sentence or two. +# start: ready +# xfail: <roadmap ID> +``` + +An `xfail` scenario's failure reports `XFAIL` and keeps the run green; once it passes it reports +`XPASS` and fails the run, so the marker comes off in the fix's own change. Keep it local: build +from `stage_pkg <name>`, not a networked package, unless the network is the point. diff --git a/tools/e2e/answer.py b/tools/e2e/answer.py new file mode 100755 index 00000000..bac00eca --- /dev/null +++ b/tools/e2e/answer.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python3 + +# SPDX-FileCopyrightText: 2026 Keith Raghubar +# +# SPDX-License-Identifier: MIT + +"""answer.py -- run a command on a terminal and answer its prompts. + +Usage: answer.py [PATTERN REPLY]... -- COMMAND [ARG...] + +The command runs on a pseudo-terminal, so it takes its interactive paths (the +review gate, the repo opt-in) as at a real one. Each PATTERN is a regular +expression waited for in order on the output seen since the previous match; +on a match its REPLY is typed (``\\n`` in REPLY is Enter). Output is copied to +stdout, and the exit status is the command's. + +A reply with no Enter answers a single-key prompt, which switches the terminal +to cbreak mode only after printing itself and discards what was typed before +(``tty.setcbreak`` flushes input). So, as a person would, it is typed once the +terminal has left line mode, not as soon as the prompt shows. + +A prompt that never appears is not an error here: the command just runs on, +and the scenario's assertions catch what it did instead. A command still +running after E2E_TIMEOUT seconds (default 1800) is killed, exit 124. +""" + +import os +import pty +import re +import select +import signal +import sys +import termios +import time + +ANSI = re.compile(rb"\x1b\[[0-9;?]*[A-Za-z]|\x1b[()][A-Za-z0-9]|\r") + + +def _wait_for_key_mode(fd: int, deadline: float) -> None: + """Wait until the terminal leaves line mode (the master reports the slave's + settings), or 10 seconds pass: a prompt that never switches gets the reply + anyway.""" + give_up = min(deadline, time.monotonic() + 10) + while time.monotonic() < give_up: + if not termios.tcgetattr(fd)[3] & termios.ICANON: + time.sleep(0.1) # past the flush, should the mode predate the prompt + return + time.sleep(0.05) + + +def main(argv: list[str]) -> int: + if "--" not in argv: + sys.exit(__doc__) + split = argv.index("--") + pairs, command = argv[:split], argv[split + 1:] + if len(pairs) % 2 or not command: + sys.exit(__doc__) + script = [(re.compile(pairs[i].encode()), pairs[i + 1].replace("\\n", "\r").encode()) + for i in range(0, len(pairs), 2)] + deadline = time.monotonic() + float(os.environ.get("E2E_TIMEOUT", "1800")) + + pid, fd = pty.fork() + if pid == 0: + os.execvp(command[0], command) # noqa: S606 — the scenario's own argv, no shell + + seen = b"" + while True: + if time.monotonic() > deadline: + os.kill(pid, signal.SIGKILL) + os.waitpid(pid, 0) + print("\nanswer.py: timed out", file=sys.stderr) + return 124 + ready, _, _ = select.select([fd], [], [], 1.0) + if not ready: + continue + try: + chunk = os.read(fd, 4096) + except OSError: # EIO: the child closed the terminal + break + if not chunk: + break + sys.stdout.buffer.write(chunk) + sys.stdout.buffer.flush() + seen += ANSI.sub(b"", chunk) + if script and script[0][0].search(seen): + reply = script.pop(0)[1] + if not reply.endswith(b"\r"): + _wait_for_key_mode(fd, deadline) + os.write(fd, reply) + seen = b"" + _, status = os.waitpid(pid, 0) + return os.waitstatus_to_exitcode(status) + + +if __name__ == "__main__": + sys.exit(main(sys.argv[1:])) diff --git a/tools/e2e/lib.sh b/tools/e2e/lib.sh new file mode 100755 index 00000000..ff566d3b --- /dev/null +++ b/tools/e2e/lib.sh @@ -0,0 +1,72 @@ +# shellcheck shell=bash +# lib.sh -- assertions for the end-to-end scenarios (tools/e2e/scenarios/). +# Sourced by run.sh's per-scenario wrapper; every scenario runs as the +# unprivileged demo user with `set -euo pipefail`, so a failed assertion or an +# unexpected non-zero exit ends it. + +E2E_DIR=${E2E_DIR:-/src/sysforge/tools/e2e} +STATE_DIR=/var/lib/sysforge + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +step() { + echo "--- $*" +} + +# answer [PATTERN REPLY]... -- COMMAND...: run COMMAND on a terminal, typing +# REPLY when PATTERN shows (tools/e2e/answer.py). +answer() { + python3 "$E2E_DIR/answer.py" "$@" +} + +# expect_exit CODE COMMAND...: run COMMAND, fail unless it exits CODE. +expect_exit() { + local want=$1 got=0 + shift + "$@" || got=$? + [[ $got == "$want" ]] || fail "\`$*\` exited $got, expected $want" +} + +assert_installed() { + pacman -Qi "$1" >/dev/null 2>&1 || fail "$1 is not installed" +} + +# assert_built_here PKG: PKG's installed copy was built here by makepkg, not +# downloaded from a repo (whose packages name an Arch packager). +assert_built_here() { + local packager + packager=$(pacman -Qi "$1" | sed -n 's/^Packager *: //p') + [[ $packager == "Unknown Packager" ]] || fail "$1 was not built here (Packager: $packager)" +} + +# state_get PKG KEY: KEY of PKG's build_state.toml record, empty when absent. +state_get() { + python3 - "$STATE_DIR/build_state.toml" "$1" "$2" <<'PY' +import sys, tomllib +with open(sys.argv[1], "rb") as f: + print(tomllib.load(f).get(sys.argv[2], {}).get(sys.argv[3], "")) +PY +} + +assert_recorded() { + [[ -f $STATE_DIR/build_state.toml ]] || fail "no $STATE_DIR/build_state.toml" + [[ -n $(state_get "$1" build_mode) ]] || fail "$1 has no build_state.toml record" +} + +# No stage left its in-progress record behind (the next run would refuse). +assert_no_sentinel() { + [[ ! -e $STATE_DIR/stage_in_progress.toml ]] \ + || fail "stale sentinel: $(cat "$STATE_DIR/stage_in_progress.toml")" +} + +# stage_pkg NAME: copy tools/e2e/pkgs/NAME to ~/e2e/NAME (makepkg writes +# beside the PKGBUILD) and print the copy's path. +stage_pkg() { + mkdir -p ~/e2e + rm -rf ~/e2e/"$1" + cp -r "$E2E_DIR/pkgs/$1" ~/e2e/"$1" + echo ~/e2e/"$1" +} diff --git a/tools/e2e/pkgs/e2e-hello/PKGBUILD b/tools/e2e/pkgs/e2e-hello/PKGBUILD new file mode 100644 index 00000000..58607458 --- /dev/null +++ b/tools/e2e/pkgs/e2e-hello/PKGBUILD @@ -0,0 +1,18 @@ +# A local package for the end-to-end tier: compiles one C file with the +# profile's CFLAGS, needing nothing from the network. +pkgname=e2e-hello +pkgver=1.0 +pkgrel=1 +pkgdesc="sysforge end-to-end test package" +arch=('x86_64') +license=('MIT') +source=('hello.c') +sha256sums=('SKIP') + +build() { + ${CC:-cc} $CPPFLAGS $CFLAGS $LDFLAGS -o e2e-hello hello.c +} + +package() { + install -Dm755 e2e-hello "$pkgdir/usr/bin/e2e-hello" +} diff --git a/tools/e2e/pkgs/e2e-hello/hello.c b/tools/e2e/pkgs/e2e-hello/hello.c new file mode 100644 index 00000000..31ef1dda --- /dev/null +++ b/tools/e2e/pkgs/e2e-hello/hello.c @@ -0,0 +1,6 @@ +#include <stdio.h> + +int main(void) { + puts("hello from sysforge's end-to-end tier"); + return 0; +} diff --git a/tools/e2e/pkgs/tree/PKGBUILD b/tools/e2e/pkgs/tree/PKGBUILD new file mode 100644 index 00000000..87b7ef2b --- /dev/null +++ b/tools/e2e/pkgs/tree/PKGBUILD @@ -0,0 +1,19 @@ +# A local stand-in for the repo package `tree`: a PKGBUILD whose name is a repo +# package's takes build's repo opt-in path without fetching Arch's packaging +# repo or tree's sources. +pkgname=tree +pkgver=0.0.1 +pkgrel=1 +pkgdesc="sysforge end-to-end stand-in for the tree repo package" +arch=('x86_64') +license=('MIT') +source=('tree.c') +sha256sums=('SKIP') + +build() { + ${CC:-cc} $CPPFLAGS $CFLAGS $LDFLAGS -o tree tree.c +} + +package() { + install -Dm755 tree "$pkgdir/usr/bin/tree" +} diff --git a/tools/e2e/pkgs/tree/tree.c b/tools/e2e/pkgs/tree/tree.c new file mode 100644 index 00000000..1c9a490a --- /dev/null +++ b/tools/e2e/pkgs/tree/tree.c @@ -0,0 +1,6 @@ +#include <stdio.h> + +int main(void) { + puts("tree (sysforge end-to-end stand-in)"); + return 0; +} diff --git a/tools/e2e/run.sh b/tools/e2e/run.sh new file mode 100755 index 00000000..ef63499a --- /dev/null +++ b/tools/e2e/run.sh @@ -0,0 +1,131 @@ +#!/usr/bin/env bash +# run.sh -- the end-to-end tier (3.5.0-DEV21): the documented paths run on a +# fresh Arch system as an unprivileged user, asserting on exit codes, pacman, +# the state files and leftover sentinels. +# +# Usage: run.sh [SCENARIO...] each scenario in its own podman container +# run.sh --here [SCENARIO...] on this system, as root (CI: a fresh +# archlinux:base-devel container) +# +# SCENARIO is a name under tools/e2e/scenarios/ (`quickstart` or +# `10-quickstart.sh`); default all, in order. sysforge is built from HEAD, or +# E2E_COMMIT (any revision: an older one to bisect a regression with today's +# scenarios); uncommitted edits are not in it. The scenarios run from the +# working tree. +# +# A scenario's header says where it starts: `# start: installed` (sysforge +# just installed, Quick start step 1) or `# start: ready` (the one-time setup +# done too). `# xfail: <ID>` marks a scenario that fails until that roadmap +# item is fixed: its failure reports XFAIL, and a pass fails the run so the +# marker is removed with the fix. +set -euo pipefail + +repo=$(cd "$(dirname "$0")/../.." && pwd) +here=false +if [[ ${1:-} == --here ]]; then + here=true + shift +fi + +scenarios=() +if [[ $# -gt 0 ]]; then + for name in "$@"; do + match=("$repo"/tools/e2e/scenarios/*"${name%.sh}"*.sh) + [[ -f ${match[0]} ]] || { echo "e2e: no scenario matches $name" >&2; exit 2; } + scenarios+=("${match[0]}") + done +else + scenarios=("$repo"/tools/e2e/scenarios/*.sh) +fi + +header() { # header FILE KEY: the value of the scenario's `# KEY: value` line + sed -n "s/^# $2: *//p" "$1" | head -1 +} + +# CI's checkout belongs to another user than the one reading it; git refuses +# such a repository unless it is marked safe (the demo user's clone too). +if $here; then + [[ $EUID == 0 ]] || { echo "e2e: --here runs as root (it provisions the system)" >&2; exit 2; } + git config --system --add safe.directory "$repo" + git config --system --add safe.directory /src/sysforge +fi +commit=$(git -C "$repo" rev-parse "${E2E_COMMIT:-HEAD}^{commit}") +if [[ -z ${E2E_COMMIT:-} ]] && [[ -n $(git -C "$repo" status --porcelain -- sysforge etc PKGBUILD-git sysforge.install pyproject.toml) ]]; then + echo "e2e: note: uncommitted changes to sysforge are not in the build (it builds HEAD)" >&2 +fi + +# run_one FILE: run a scenario in its start state; return its exit status. +if $here; then + # The demo scripts read the checkout at /src/sysforge, as in the container. + if [[ $repo != /src/sysforge ]]; then + mkdir -p /src + rm -rf /src/sysforge + cp -a "$repo" /src/sysforge + fi + bash /src/sysforge/tools/demo/provision.sh + runuser -l demo -c "/src/sysforge/tools/demo/install.sh $commit" + set_up=false + run_one() { + # One system for every scenario: the first `ready` one after no + # Quick start runs the setup the Quick start would have. + if [[ $(header "$1" start) == ready ]] && ! $set_up; then + runuser -l demo -c 'sudo sysforge run hardware </dev/null && sudo EDITOR=true sysforge run reconfigure </dev/null' + fi + set_up=true + runuser -l demo -c "bash /src/sysforge/tools/e2e/scenario.sh /src/sysforge/tools/e2e/scenarios/$(basename "$1")" + } +else + command -v podman >/dev/null || { echo "e2e: podman is not installed (sudo pacman -S podman)" >&2; exit 1; } + base=sysforge-demo:base + prep=sysforge-e2e-prep + cleanup() { podman rm -f -t 0 "$prep" sysforge-e2e-run >/dev/null 2>&1 || true; } + trap cleanup EXIT + cd "$repo" + echo "==> Building $base" + podman build -q -t "$base" -f tools/demo/Containerfile tools/demo >/dev/null + echo "==> Installing sysforge ($commit)" + cleanup + podman run -d --name "$prep" -v "$repo:/src/sysforge:ro" "$base" >/dev/null + podman exec -u demo -w /home/demo "$prep" /src/sysforge/tools/demo/install.sh "$commit" >/dev/null + podman commit -q "$prep" sysforge-e2e:installed >/dev/null + echo "==> Running the one-time setup" + podman exec -u demo -w /home/demo "$prep" \ + bash -c 'sudo sysforge run hardware </dev/null && sudo EDITOR=true sysforge run reconfigure </dev/null' >/dev/null + podman commit -q "$prep" sysforge-e2e:ready >/dev/null + podman rm -f -t 0 "$prep" >/dev/null + run_one() { + local rc=0 + podman run -d --name sysforge-e2e-run -v "$repo:/src/sysforge:ro" \ + "sysforge-e2e:$(header "$1" start)" >/dev/null + podman exec -u demo -w /home/demo sysforge-e2e-run \ + bash /src/sysforge/tools/e2e/scenario.sh "/src/sysforge/tools/e2e/scenarios/$(basename "$1")" || rc=$? + podman rm -f -t 0 sysforge-e2e-run >/dev/null + return "$rc" + } +fi + +results=() +failed=0 +for file in "${scenarios[@]}"; do + name=$(basename "$file" .sh) + xfail=$(header "$file" xfail) + echo "==> $name" + if run_one "$file"; then + if [[ -n $xfail ]]; then + results+=("XPASS $name (passes: remove its xfail: $xfail marker)") + failed=1 + else + results+=("PASS $name") + fi + elif [[ -n $xfail ]]; then + results+=("XFAIL $name ($xfail)") + else + results+=("FAIL $name") + failed=1 + fi +done + +echo +echo "== e2e results" +printf '%s\n' "${results[@]}" +exit "$failed" diff --git a/tools/e2e/scenario.sh b/tools/e2e/scenario.sh new file mode 100755 index 00000000..725be149 --- /dev/null +++ b/tools/e2e/scenario.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +# scenario.sh -- run one end-to-end scenario as the current (demo) user, in its +# own login session. Called by run.sh; not meant to be run by hand. +# +# Usage: scenario.sh SCENARIO_FILE +set -euo pipefail +E2E_DIR=$(cd "$(dirname "$0")" && pwd) +# shellcheck source=tools/e2e/lib.sh +source "$E2E_DIR/lib.sh" +cd ~ +# `podman exec` and runuser pass no terminal type; curses programs refuse to +# start without one. +export TERM=xterm-256color +# shellcheck disable=SC1090 +source "$1" diff --git a/tools/e2e/scenarios/10-quickstart.sh b/tools/e2e/scenarios/10-quickstart.sh new file mode 100644 index 00000000..0e7e3bce --- /dev/null +++ b/tools/e2e/scenarios/10-quickstart.sh @@ -0,0 +1,27 @@ +# shellcheck shell=bash +# The README's Quick start as one login session, from sysforge just installed: +# the one-time setup, an AUR build (the review gate accepted), then `update`. +# The setup adds the user to the sysforge group, which only applies at the next +# login, so every step after it runs without that membership (3.5.0-B134). +# The tier's one networked scenario: the AUR, and pacman for build deps. +# start: installed + +step "one-time setup" +sudo EDITOR=true sysforge run reconfigure </dev/null +sudo sysforge run hardware </dev/null +assert_no_sentinel +id -nG | grep -qw sysforge && fail "the sysforge group applied before the next login" + +step "build an AUR package" +answer '\[a\]ccept' a -- sysforge build cbonsai -si +assert_installed cbonsai +assert_built_here cbonsai +assert_recorded cbonsai +[[ $(state_get cbonsai build_mode) == source_built ]] || fail "cbonsai build_mode is not source_built" +assert_no_sentinel +cbonsai -p >/dev/null + +step "update" +sysforge update </dev/null +assert_no_sentinel +assert_recorded cbonsai diff --git a/tools/e2e/scenarios/20-repo-optin.sh b/tools/e2e/scenarios/20-repo-optin.sh new file mode 100644 index 00000000..a055a662 --- /dev/null +++ b/tools/e2e/scenarios/20-repo-optin.sh @@ -0,0 +1,25 @@ +# shellcheck shell=bash +# A repo package's opt-in: `build` on a package the repos also carry asks +# before building it from source, saves the answer, and records the build as a +# repo package's, so `update` keeps rebuilding it from source. +# start: ready + +dir=$(stage_pkg tree) + +step "without a terminal, build refuses an un-opted-in repo package" +sysforge build "$dir" -si </dev/null +pacman -Qi tree >/dev/null 2>&1 && fail "tree was installed without an opt-in" + +step "at a terminal, the opt-in prompt builds it and saves the answer" +answer 'build from source\?' 'y\n' -- sysforge build "$dir" -si +assert_installed tree +assert_built_here tree +assert_recorded tree +[[ $(state_get tree source) == repo ]] || fail "tree's record is not source=repo" +grep -q 'enable_build_from_source *= *true' /etc/sysforge/packages.toml \ + || fail "the opt-in was not saved to packages.toml" +assert_no_sentinel + +step "the saved opt-in answers the next build" +sysforge build "$dir" -si --rebuild </dev/null +assert_no_sentinel diff --git a/tools/e2e/scenarios/30-flag-drift.sh b/tools/e2e/scenarios/30-flag-drift.sh new file mode 100644 index 00000000..de97c164 --- /dev/null +++ b/tools/e2e/scenarios/30-flag-drift.sh @@ -0,0 +1,30 @@ +# shellcheck shell=bash +# Flag drift after a profile change: a package built with the default profile +# is reported as drifted once the default changes, and rebuilt with the new +# flags by `update --rebuild-on-flag-drift`. +# start: ready + +dir=$(stage_pkg e2e-hello) +profiles=/etc/sysforge/profiles.toml + +step "build with the shipped default profile" +sysforge build "$dir" -si </dev/null +assert_installed e2e-hello +assert_recorded e2e-hello +before=$(state_get e2e-hello flags_string) +[[ -n $before ]] || fail "e2e-hello has no recorded flags" + +step "switch the default profile" +grep -q '^profile = "standard"' "$profiles" || fail "the shipped default profile is not standard" +sudo sed -i 's/^profile = "standard"/profile = "optimized"/' "$profiles" + +step "update reports the drift" +out=$(sysforge update --dry-run --explain-drift </dev/null 2>&1) || fail "update --dry-run failed: $out" +echo "$out" +grep -q e2e-hello <<<"$out" || fail "update --dry-run did not report e2e-hello" + +step "update rebuilds it with the new flags" +sysforge update --rebuild-on-flag-drift --yes </dev/null +after=$(state_get e2e-hello flags_string) +[[ $after != "$before" ]] || fail "e2e-hello's recorded flags did not change" +assert_no_sentinel diff --git a/tools/e2e/scenarios/40-pre-install-failure.sh b/tools/e2e/scenarios/40-pre-install-failure.sh new file mode 100644 index 00000000..c9f37e5d --- /dev/null +++ b/tools/e2e/scenarios/40-pre-install-failure.sh @@ -0,0 +1,15 @@ +# shellcheck shell=bash +# A build that fails before installing anything leaves no sentinel, so the next +# run starts normally: a repo package's checkout needs pkgctl, and here it is +# missing (as on a system without devtools). +# start: ready + +sudo pacman -Rdd --noconfirm devtools + +step "the build fails on the missing pkgctl" +expect_exit 1 sysforge build cowsay --force </dev/null +pacman -Qi cowsay >/dev/null 2>&1 && fail "cowsay was installed" +assert_no_sentinel + +step "the next run is not refused" +sysforge update --dry-run </dev/null diff --git a/tools/record-demo.sh b/tools/record-demo.sh index e60b13be..28a48ca6 100755 --- a/tools/record-demo.sh +++ b/tools/record-demo.sh @@ -31,6 +31,7 @@ for tape in "${tapes[@]}"; do vhs validate "$tape" >/dev/null || { vhs validate "$tape"; exit 1; } done branch=$(git branch --show-current) +commit=$(git rev-parse HEAD) if [[ -n $(git status --porcelain) ]]; then echo "record-demo: note: uncommitted changes are not in the recording (it builds $branch's HEAD)" >&2 fi @@ -42,7 +43,7 @@ echo "==> Installing sysforge ($branch) and running the one-time setup" podman rm -f -t 0 sysforge-demo-prep >/dev/null 2>&1 || true podman run -d --name sysforge-demo-prep -v "$repo:/src/sysforge:ro" "$base" >/dev/null trap 'podman rm -f -t 0 sysforge-demo-prep >/dev/null 2>&1 || true' EXIT -podman exec -u demo -w /home/demo sysforge-demo-prep /src/sysforge/tools/demo/prepare.sh "$branch" \ +podman exec -u demo -w /home/demo sysforge-demo-prep /src/sysforge/tools/demo/prepare.sh "$commit" \ >/dev/null podman commit -q sysforge-demo-prep "$ready" >/dev/null podman rm -f -t 0 sysforge-demo-prep >/dev/null