diff --git a/.gitattributes b/.gitattributes index ed05266b..03e18c9b 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,2 +1,9 @@ # Preserve exact diff bytes recorded in the dependency provenance lock. /flows/image-to-card/wasm/patches/*.patch -whitespace + +# The splashcheck golden corpus is byte-exact: the runner digests the fixture's +# sources and compares the golden files as text it rendered itself with \n. A +# core.autocrlf checkout that rewrote either to \r\n would show a phantom diff +# on every run, so both sides of that comparison are pinned. +/tools/splashcheck_corpus/fn-blindspot/* -text +/tools/splashcheck_corpus/golden/*.txt -text diff --git a/.github/workflows/release-splash-lsp.yml b/.github/workflows/release-splash-lsp.yml new file mode 100644 index 00000000..166ebf7f --- /dev/null +++ b/.github/workflows/release-splash-lsp.yml @@ -0,0 +1,257 @@ +name: Release splash-lsp + +# The Splash language server the editor extension downloads on first use. The +# binary is built here from the makepad revision recorded in +# tools/splash-server-toolchain.json, because the makepad fork has no CI of its +# own and distribution cannot depend on it. One tag, one version, one asset per +# platform, each named for the server version and the platform it runs on. +# +# Push a `splash-lsp-v*` tag to publish. The tag must be the tag the record +# names, or the release and the record disagree about which version is out. +on: + push: + tags: ['splash-lsp-v*'] + +permissions: {} + +concurrency: + group: release-splash-lsp-${{ github.ref }} + cancel-in-progress: false + +jobs: + build: + name: ${{ matrix.target }} + runs-on: ${{ matrix.os }} + timeout-minutes: 120 + permissions: + contents: read + strategy: + fail-fast: false + matrix: + include: + # The first platform binary. Adding a platform is the whole change: + # one entry here, and the matching "platforms" entry in + # tools/splash-server-toolchain.json. Nothing else in the release + # path is platform-specific: the record is read, not restated. + # + # - os: macos-14 + # target: aarch64-apple-darwin + # - os: ubuntu-latest + # target: x86_64-unknown-linux-gnu + # + # Two conditions come with them. Linux needs makepad's system + # libraries before cargo will link (see makepad's README: libx11, + # libgtk-3, libasound2, libudev and friends), so an apt step belongs + # in the matrix too. And macOS on Intel is a second entry with + # x86_64-apple-darwin: until it exists, a Mac build is Apple Silicon + # only, which the extension has to say rather than guess. + - os: windows-latest + target: x86_64-pc-windows-msvc + defaults: + run: + shell: bash + steps: + - uses: actions/checkout@v6 + + # One read of the record, and the rest of the run uses its output: the + # asset name, the revision to build, the version to verify against, the + # tag to publish under. A target the record does not publish fails here, + # so a runner cannot quietly produce an asset nobody recorded. + - name: Read the recorded release + id: record + run: python tools/splash_server_release.py --build-plan "${{ matrix.target }}" >> "$GITHUB_OUTPUT" + + - name: The release is published into the repository the record names + run: | + if [ "${{ steps.record.outputs.release-repository }}" != "$GITHUB_REPOSITORY" ]; then + echo "::error::the record names ${{ steps.record.outputs.release-repository }}, not $GITHUB_REPOSITORY" + exit 1 + fi + + - name: The tag is the one the record names + run: | + if [ "$GITHUB_REF_NAME" != "${{ steps.record.outputs.tag }}" ]; then + echo "::error::pushed $GITHUB_REF_NAME, the record names ${{ steps.record.outputs.tag }}" + exit 1 + fi + + - name: Check out the pinned makepad + uses: actions/checkout@v6 + with: + repository: ${{ steps.record.outputs.source-repository }} + ref: ${{ steps.record.outputs.source-revision }} + path: makepad + persist-credentials: false + + # makepad patches octoscript-node to a path outside its own tree, so a + # build with no sibling beside it does not build at all. The sibling is + # the shared runtime this repository already locks. + - name: Check out the pinned Octoscript-Makepad sibling + uses: actions/checkout@v6 + with: + repository: ${{ steps.record.outputs.runtime-repository }} + ref: ${{ steps.record.outputs.runtime-revision }} + path: ${{ steps.record.outputs.runtime-sibling }} + persist-credentials: false + + # The precondition for the build at all: the sibling must carry the crate + # the patch names. There is deliberately no stronger check that the sibling + # is "the right one" -- the shared runtime's makepad pin trails this + # record's, because the server source is a branch that has not been + # released into the runtime yet, and a build against a mismatched node + # model fails loudly here rather than shipping. + - name: The sibling carries the crate makepad patches to + shell: python + env: + SIBLING: ${{ steps.record.outputs.runtime-sibling }} + run: | + import os, pathlib + manifest = pathlib.Path(os.environ['SIBLING']) / 'crates' / 'octoscript-node' / 'Cargo.toml' + if not manifest.is_file(): + raise SystemExit(f'no {manifest}: the patched octoscript-node is missing') + name = next((line.split('=', 1)[1].strip().strip('"') + for line in manifest.read_text(encoding='utf-8').splitlines() + if line.startswith('name =')), None) + if name != 'octoscript-node': + raise SystemExit(f'{manifest} declares {name!r}, not octoscript-node') + + - name: Build the server + working-directory: makepad + run: | + arguments=(--release --target "${{ matrix.target }}" + -p "${{ steps.record.outputs.source-package }}" + --bin "${{ steps.record.outputs.source-bin }}") + if [ -n "${{ steps.record.outputs.source-features }}" ]; then + arguments+=(--features "${{ steps.record.outputs.source-features }}") + fi + cargo build "${arguments[@]}" + + - name: Find the built server + id: built + run: | + suffix="" + case "${{ matrix.target }}" in *windows*) suffix=".exe" ;; esac + binary="makepad/target/${{ matrix.target }}/release/${{ steps.record.outputs.source-bin }}${suffix}" + test -f "$binary" || { echo "::error::no $binary"; exit 1; } + echo "binary=$binary" >> "$GITHUB_OUTPUT" + echo "suffix=$suffix" >> "$GITHUB_OUTPUT" + + # The binary reports the framework's own package version, so one handshake + # checks both things that matter: that the server starts and speaks LSP at + # all, and that it is the version the record names. A makepad bump nobody + # recorded fails here rather than shipping a release whose name says one + # version and whose bytes are another. + - name: The server answers an initialize and reports the recorded version + shell: python + env: + BINARY: ${{ steps.built.outputs.binary }} + VERSION: ${{ steps.record.outputs.version }} + run: | + import json, os, subprocess, sys + server = subprocess.Popen([os.environ['BINARY']], stdin=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=subprocess.PIPE, + text=True, bufsize=1) + + def send(message): + body = json.dumps(message) + server.stdin.write(f'Content-Length: {len(body)}\r\n\r\n{body}') + server.stdin.flush() + + def read(): + headers = {} + while True: + line = server.stdout.readline() + if not line: + raise SystemExit('the server closed stdout without answering') + line = line.strip() + if not line: + break + name, value = line.split(':', 1) + headers[name.strip()] = value.strip() + return json.loads(server.stdout.read(int(headers['Content-Length']))) + + try: + send({'jsonrpc': '2.0', 'id': 1, 'method': 'initialize', + 'params': {'processId': None, 'rootUri': None, 'capabilities': {}}}) + # serverInfo sits inside capabilities here, not at the top level of + # the initialize result as the LSP spec allows. + info = read()['result']['capabilities']['serverInfo'] + if info.get('name') != 'splash-lsp': + raise SystemExit(f'no serverInfo in the initialize result: {info}') + if info.get('version') != os.environ['VERSION']: + raise SystemExit(f"the server reports {info.get('version')}, " + f"the record says {os.environ['VERSION']}") + send({'jsonrpc': '2.0', 'method': 'shutdown', 'params': None}) + read() + send({'jsonrpc': '2.0', 'method': 'exit', 'params': None}) + try: + server.wait(timeout=30) + except subprocess.TimeoutExpired: + server.kill() + raise SystemExit('the server did not exit after `exit`') + except BaseException as failure: + server.kill() + print(server.stderr.read(), file=sys.stderr) + raise SystemExit(f'the server did not answer an initialize: {failure}') + print(f"{info['name']} {info['version']} answers on stdio") + + - name: Pack the asset + run: | + python - "${{ steps.built.outputs.binary }}" "${{ steps.record.outputs.asset }}" \ + "${{ steps.record.outputs.source-bin }}" "${{ steps.built.outputs.suffix }}" <<'PY' + import sys, zipfile + binary, asset, name, suffix = sys.argv[1:5] + with zipfile.ZipFile(asset, 'w', zipfile.ZIP_DEFLATED) as pack: + # The entry keeps the platform's own name, so extracting the + # archive leaves something runnable without a rename. + pack.write(binary, f'{name}{suffix}') + PY + + - uses: actions/upload-artifact@v4 + with: + name: ${{ steps.record.outputs.asset }} + path: ${{ steps.record.outputs.asset }} + if-no-files-found: error + + release: + name: Publish + runs-on: ubuntu-24.04 + timeout-minutes: 20 + needs: [build] + permissions: + contents: write + steps: + - uses: actions/checkout@v6 + + - uses: actions/download-artifact@v4 + with: + path: assets + merge-multiple: true + + # The digests go in the notes, not in the record: a rebuilt binary is a + # different binary, so a digest committed in advance would only be a + # promise the next build breaks. Published here, it describes the bytes + # that were actually uploaded. + - name: Write the release notes + id: notes + shell: python + run: | + import os, pathlib, sys + sys.path.insert(0, 'tools') + import splash_server_release as record + pin = record.read_pin() + pathlib.Path('NOTES.md').write_text(record.release_notes(pin, 'assets'), encoding='utf-8') + with open(os.environ['GITHUB_OUTPUT'], 'a') as out: + out.write('tag=' + pin['server']['release']['tag'] + '\n') + + - name: Publish the release + env: + GH_TOKEN: ${{ github.token }} + run: | + if gh release view "${{ steps.notes.outputs.tag }}" --repo "$GITHUB_REPOSITORY" > /dev/null 2>&1; then + echo "::error::${{ steps.notes.outputs.tag }} is already published. A server release is" + echo "::error::immutable: delete it to publish these bytes again, or record a new version." + exit 1 + fi + gh release create "${{ steps.notes.outputs.tag }}" --repo "$GITHUB_REPOSITORY" \ + --verify-tag --title "${{ steps.notes.outputs.tag }}" --notes-file NOTES.md assets/*.zip diff --git a/docs/adr/0001-vm-query-api-two-functions.md b/docs/adr/0001-vm-query-api-two-functions.md new file mode 100644 index 00000000..807d6eba --- /dev/null +++ b/docs/adr/0001-vm-query-api-two-functions.md @@ -0,0 +1,89 @@ +# ADR 0001: The VM query API: two functions, and nothing else + +- **Date:** 2026-10-13 +- **Status:** Accepted (2026-10-13). Implemented as issue #9, in [OctoSense-org/makepad](https://github.com/OctoSense-org/makepad), `platform/script` (crate `makepad-script`, version 1.0.0), commit `136e775`: `query::object_keys` and `query::module_object`; the fixture corpus `platform/script/tests/query_corpus.txt` with a runner per installing crate (`cargo test -p makepad-script` for the modules the crate itself installs, `cargo test -p makepad-widgets` for the isolate's widget-facing modules); and the checked-in surface file `platform/script/tests/query_api_surface.txt`, which `tests/query_api_surface.rs` enforces — the semver check's equivalent, since there is no CI and no crates.io. The chain-walk proof §2 asks for is `tests/query_api.rs::the_prototype_chain_is_walkable_from_outside_with_the_existing_accessors`. +- **Scope:** How an editor-facing consumer asks the Splash VM which names exist — what the query API contains, what it deliberately leaves out, and how its stability is promised. It does not decide what the language server does with the answer; that is [ADR 0002](0002-analysis-posture-and-one-suppression-rule.md) and issue #8. This record lives in App Flow, where the spec and every ticket that cites it live; the code it governs lands in the makepad fork. +- **Relates to:** [ADR 0002](0002-analysis-posture-and-one-suppression-rule.md), [ADR 0003](0003-download-first-server-distribution.md); App Flow #1 (the spec), #8, #9; OctoSense [ADR 0005](https://github.com/OctoSense-org/OctoSense/blob/main/docs/adr/0005-app-contract.md), whose stability discipline this copies because it solved the same problem for a shared crate. + +## Context + +Completion has one rule that decides the whole design: **a suggestion must be a name that exists.** A wrong suggestion is worse than no suggestion, so the data cannot be a hand-written schema or a generated one — it has to be the registry the app itself runs against. That registry is a live `ScriptVm`: widget families in `mod.prelude.widgets`, the `sys`, `host`, `fs` and `agent` modules an isolate installs, and the standard library's `net`, `std`, `math`, `pod` and `shader` (`widgets/src/lib.rs:303`, `widgets/src/lib.rs:363`, `widgets/src/splash.rs:1315`, `widgets/src/splash.rs:1347`, `widgets/src/splash_host.rs:243`, `widgets/src/splash_storage.rs:243`). + +Every completion in the spec is one of two questions: + +1. **What names does this object have?** Widget names in the registry; the properties the current widget accepts, including the ones it inherits; an enum's values, which are the keys of the enum object; a module's members. +2. **Give me the object called X.** The module named `sys`; the widget proto named `Label`; the enum object a field's type points at. + +Everything the VM needs to answer both is already public, which is why this API is a wrapper and not a mechanism: + +| Existing accessor | What it gives | +| --- | --- | +| `ScriptHeap::map_ref` (`platform/script/src/object_heap.rs:1105`) with `ScriptObjectMap::keys()` (`platform/script/src/value_map.rs:262`) | an object's own entries | +| `ScriptHeap::iter_key_value` (`object_heap.rs:1193`) with `iter_len` (`object_heap.rs:1186`) | the same, spanning an object's vec and map entries | +| `ScriptHeap::module` / `ScriptVm::module` (`heap.rs:569`, `vm.rs:1418`) | the module object for a name | +| `ScriptVm::proto_map_iter_mut_with` (`vm.rs:1437`) | a prototype chain walked root to leaf | + +A wrapper that grows is how a shared crate loses its shape, so this ADR fixes the count and the semantics before the first caller exists. + +### What shaped this decision (measured) + +- **The fork has no CI.** `gh api repos/OctoSense-org/makepad/actions/workflows` answers `{"total_count": 0}`, and `gh api repos/OctoSense-org/makepad/contents/.github` answers 404. Nothing runs the tests on a push, so "CI will catch a breaking change" is not available as a stability mechanism for this API. +- **`makepad-script` is not published.** The crates.io API answers `crate 'makepad-script' does not exist`, so "semver on crates.io" is not available either. The `1.0.0` in `platform/script/Cargo.toml` is a promise this repository keeps by hand, exactly the position ADR 0005 §3 describes for a contract taken from a git revision. +- **Map order is not a contract.** `ValueMap` is a `Vec` up to 16 entries and a `HashMap` past that, and says so: "Key order is not part of the map contract (callers track ordering via ScriptMapTag::order)" (`value_map.rs:61-64`). A query API that returned keys in map order would hand a consumer an order that changes when an object grows past the spill threshold, and a golden fixture built on it would flake for no reason a reader could see. + +## Decision + +### 1. Two functions, and the count is the decision + +1. **Enumerate an object's own keys.** `object_keys(object) -> [name]`: the object's own entries, each name once, in a deterministic order (sorted), without values. Not the prototype chain — see §2. +2. **Get a module object by name.** `module_object(name) -> object | nil`. A name the VM does not have is nil. It is never a trap and never a panic. + +These two, because every question above reduces to one of them, and because each addition to a shared crate's public surface is a promise someone has to keep for as long as the crate is at that major version. The exact spelling is issue #9's to choose; the count and the semantics are fixed here. Issue #9 chose a `query` module holding `object_keys(heap, object) -> Vec` and `module_object(heap, name) -> Option`, so the promised surface is one small file a reviewer can read in a sitting. + +### 2. Prototype-chain traversal is not in the API + +`object_keys` returns own keys only. A widget's inherited properties — `draw_bg` on a `Label`, which comes from `View` — are the consumer's business, walked with the accessors that already exist: `ScriptVm::proto_map_iter_mut_with` (`vm.rs:1437`), or the object's own `proto` field. + +The reason is not minimalism for its own sake. A `keys` that walked the chain would have to answer, inside the VM, questions the VM has no business deciding: does a shadowed name appear once or twice, in which order, and does a native type's getter table count as a key? Those are policy, and policy belongs to the one consumer that has a use for it. Issue #9's fourth acceptance criterion is a test proving the chain is walkable from outside with the existing accessors — that test is what makes leaving it out safe rather than merely tidy. + +### 3. Read-only, and never a trap + +Neither function mutates the heap, runs script, or charges an instruction budget: they are reads an editor performs per keystroke. Neither traps. The default type getter traps with `no getter for field {:?} on type {:?}` (`platform/script/src/native.rs:326-334`), so a query API that inherited that behaviour would turn a completion request into a script error — and the one place a language server must not produce an error is the path it takes while the person is typing. + +### 4. The stability promise: a corpus and an additive-only rule, because there is no CI and no crates.io + +ADR 0005 pins the app contract with a fixture corpus of real manifests that every release must accept, plus additive-only within `1.x`. The same two mechanisms, and nothing that needs a CI runner: + +- **Additive only within `1.x`.** New functions and new optional results are `1.x`; removing or reinterpreting either function, or changing the order `object_keys` returns, is `2.0` and is called out in the fork's release notes. +- **A fixture corpus of real objects.** A checked-in corpus naming the modules and objects a real isolate installs — `mod.prelude.widgets`, `mod.host`, `mod.fs`, `mod.sys`, `mod.net`, `mod.std` — with the key sets each must answer. It lives with the crate (`platform/script/tests/`, which already holds the crate's behaviour tests) so it runs with one command anyone can type: `cargo test -p makepad-script`. A consumer that links the crate links the corpus. Issue #9 kept the file there and added a second runner in the widgets crate, because three of those six modules are installed by the isolate setup in `makepad-widgets` rather than by this crate: one corpus file, checked by `cargo test -p makepad-script` for the entries the crate itself installs and by `cargo test -p makepad-widgets` for the rest, each entry naming its runner. +- **The corpus is the review artifact.** Because no CI runs it, a change to either function names the corpus diff in its pull request. A green local run that nobody wrote down is not a promise. + +### 5. Where it lives + +In `makepad-script` (`platform/script`), the shared VM crate, not in `makepad-widgets` where the language server sits. The query is about the VM's own objects; a host that is not an editor has the same question, and a second copy in the widgets crate would be a second thing to keep stable. + +## Consequences + +- The completion provider (issue #8) becomes a reader: it asks for keys, walks protos itself, and formats. It cannot be wrong about whether a name exists, which was the whole point. +- A consumer cannot ask the VM "what properties does type X have", "what are this enum's variants as a list", or "where is this name declared". Those are either derivable from the two functions or are a separate decision with its own stability cost. If a third function is ever needed, this ADR is amended, not quietly extended. +- Every consumer pays for the corpus: a change to a module's members is a visible diff in a checked-in file, which is the intended friction. + +## Alternatives considered + +- **Expose the heap (`ScriptValue`, `ScriptObject`, `ScriptObjectMap`) directly.** Rejected: it publishes the heap's representation, so no internal change to it is possible without a breaking change, and it hands a caller the ability to mint values in a heap it does not own. +- **Emit a JSON schema of the registry.** Rejected: a schema is a snapshot that drifts from the code that produced it, and the failure mode is exactly the one the spec forbids — a suggestion for a name that no longer exists. +- **Put a "complete at position" function in the VM.** Rejected: that is parser and cursor policy, it belongs to the language server, and it would make every editor behaviour change a change to the shared crate's public API. +- **Walk the prototype chain inside `object_keys`.** Rejected; see §2. + +## Measurements + +Run on Windows, 2026-10-13: + +``` +$ gh api repos/OctoSense-org/makepad/actions/workflows --jq .total_count +0 +$ gh api repos/OctoSense-org/makepad/contents/.github +{"message":"Not Found", ... "status":"404"} +$ curl -s -H "User-Agent: adr-research" https://crates.io/api/v1/crates/makepad-script +{"errors":[{"detail":"crate `makepad-script` does not exist"}]} +``` diff --git a/docs/adr/0002-analysis-posture-and-one-suppression-rule.md b/docs/adr/0002-analysis-posture-and-one-suppression-rule.md new file mode 100644 index 00000000..91b8cf65 --- /dev/null +++ b/docs/adr/0002-analysis-posture-and-one-suppression-rule.md @@ -0,0 +1,108 @@ +# ADR 0002: The analysis posture: answer the host, re-run every top-level `fn`, suppress exactly one shape + +- **Date:** 2026-10-13 +- **Status:** Accepted (2026-10-13). Not yet implemented: answering host requests is issue #5 and the `fn` re-run is issue #6, both in [OctoSense-org/makepad](https://github.com/OctoSense-org/makepad) (`widgets/src/splashcheck.rs`, `widgets/src/splash.rs`). +- **Scope:** How deep the validator looks during a check, and which of the findings that depth produces it is allowed to dismiss. It covers the two blind spots the checker documents about itself, the shape of the answer stub, the single suppression rule, and the code that makes stub findings grep-able. It does not cover the discovery chain or the download (ADR 0003) or the completion data source (ADR 0001). This record lives in App Flow, where the spec and every ticket that cites it live; the code it governs lands in the makepad fork. +- **Relates to:** [ADR 0001](0001-vm-query-api-two-functions.md), [ADR 0003](0003-download-first-server-distribution.md); App Flow #1 (the spec), #4 (golden corpus), #5, #6. + +## Context + +The checker evaluates an app's body in a throwaway isolate — the same VM, the same widget registry, the same host bridge a real host installs (`widgets/src/splash.rs:580`, `validate_splash_body_records`) — and it is honest about two holes in that, in its own module docs (`widgets/src/splashcheck.rs:22-33`): + +> * Only top-level code is evaluated. A `fn` nothing calls at the top level is compiled but never run, so a fault inside it is not reported … +> * `host.request` calls queue and are never answered, so a callback's body does not run and a fault inside one is not reported. + +Both are measured below, and both are the difference between an error a person sees in their editor and an error their users see on a device. The fix for each is the same kind of thing: run more of the app during the check. + +Running more of an app during a check is also how a checker starts lying. A `fn` re-run has no arguments, so every parameter is `nil`; a stub answer to a host request is not the answer the real host would give. The findings that depth invents are the cost of the depth, so this ADR fixes the posture **and** the one rule that pays for it, together. A posture without its suppression rule is a tool that drowns people in false positives until they turn it off. + +### What shaped this decision (measured) + +Every run below was made on Windows, 2026-10-13, with the fork's prebuilt release binary (`target/release/splashcheck.exe`) over a one-file app. The body is evaluated inside the harness prefix (`widgets/src/splash.rs:158-162`), so the reported line is the app's own. + +**The `fn` blind spot.** `fn broken(){ let x = nope }`, with a `View{…}` root: + +``` +$ splashcheck . + ok .\main.splash — 101 bytes + note: no manifest.json above this file: capability checks skipped +splashcheck: 1 ok, 0 warn, 0 error +``` + +**The callback blind spot.** `fn load(){ host.request("news.list", {}, fn(r){ if r.is_ok { let bad = missing_name } }) }` called at the top level: + +``` +$ splashcheck . + ok .\main.splash — 203 bytes + note: no manifest.json above this file: capability checks skipped +splashcheck: 1 ok, 0 warn, 0 error +``` + +The request is queued and nothing drains it: `take_splash_host_requests` (`widgets/src/splash_host.rs:135`) is the host's call, and a validation run has no host. + +**A no-argument call binds parameters to `nil`, and `nil` only traps on a field.** One function, three uses of its parameter, called with no arguments: + +```splash +fn f(a){ + let b = a.field + let c = a * 2 + let d = "x" + a +} +f() +``` + +``` +$ splashcheck . + ERR .\main.splash — 142 bytes + 2:15 no getter for field field on type nil +splashcheck: 0 ok, 0 warn, 1 error +``` + +`a * 2` and `"x" + a` are silent, because the VM coerces rather than traps: `cast_to_f64` maps `nil` to `0.0` (`platform/script/src/heap.rs:703-705`) and `cast_to_string` appends nothing for `nil` (`platform/script/src/string_heap.rs:649-651`). A field access is the one operation with no coercion, because it falls through to the type's default getter, which traps (`platform/script/src/native.rs:326-334`). The same three lines at the top level, with `let a = nil`, give the same result: `nil * 2` and `"hi " + nil` are clean, `n.field` is `no getter for field field on type nil`. + +So the false-positive surface of a no-argument re-run is **one message shape**, and it is a shape the person can read: it names the field and says the value is `nil`. + +## Decision + +### 1. The posture: answer the host, then re-run every top-level `fn` + +During a check, in the same isolate that evaluates the body: + +1. **Host requests are answered deterministically.** After the body evaluates, the requests it queued are drained and each is answered once, with a success. The answer's shape is the one the bridge already builds — `{is_ok, data, error}` (`widgets/src/splash_host.rs:186-193`) — and the stub answers `is_ok` true, so the `if r.is_ok` branch runs. The success path is the one worth checking: it is where an app's real work is, and a checker that only ever exercised the failure branch would miss most of the code. +2. **Every top-level `fn` is re-run once with no arguments**, after the body and after the answers. The result is discarded; only the errors are collected. + +Only top-level `fn`. A `fn` nested inside another `fn`, or inside a proto, exists only when its enclosing code runs, so reaching it means evaluating the app rather than checking it — that is a different tool, and it is out of scope for this spec. The checker already says which checks it skipped (`report.notes`), and "nested `fn` not evaluated" joins that list rather than being left as silence. + +### 2. One suppression rule, and it is the one measured above + +`no getter for field on type nil`, produced while running a stub, is **downgraded to a note** — a line in the output channel, not a squiggle in the file. Nothing else is suppressed. Not `nil` arithmetic, which does not error; not a missing name, which is a real fault; not a type error in a callback, which is exactly what the posture exists to find. + +The rule is written as narrowly as the measurement allows: the message, the `nil`, and the stub mode. A finding that matches the words but arrives from top-level code is still an error, because top-level `let n = nil` followed by `n.field` is the app's own doing. + +### 3. Stub findings carry their own code + +Findings that exist only because of the stub carry `splash.fnstub`, beside the existing `splash.eval`, `splash.capability`, `splash.known` and `splash.io` (`widgets/src/splashcheck.rs:114-115`). That is what makes them greppable, baseline-able through the existing mechanism, and demotable in one move if the false-positive rate ever justifies it — a decision taken with data, rather than by deleting the posture. + +### 4. The golden corpus is the regression gate + +Issue #4 puts twelve real apps and the `fn` blind-spot fixture under golden files. The fixture's expectation flips from "no findings" to "findings", which is the measurable delivery of this posture; every other app's diff must be empty or explained. A posture that changes what a real app reports is a change a person must see. + +## Consequences + +- A fault inside any top-level `fn`, including one nothing calls, is reported at its own line. Dead code stops being a place to hide. +- A `host.request` callback's body runs during a check, so its faults are reported — and the checker now depends on the host bridge's answer shape. If that shape changes, the stub changes with it, in one place. +- The check costs more: one extra call per top-level `fn`, one answer per queued request, all inside the isolate's instruction budget (`SPLASH_EVAL_INSTRUCTION_LIMIT`, `widgets/src/splash.rs:162`). A pathological app can spend the budget in a stub re-run, which surfaces as the existing "crashed its isolate during validation" record rather than as a new failure mode. +- The one suppression rule will be wrong sometimes. A real `nil` field access inside a re-run `fn` is a real fault and will be a note. That is the trade, taken deliberately: rather miss a fault than invent a rule, because an invented rule suppresses shapes nobody has measured. + +## Alternatives considered + +- **Answer host requests with a failure instead of a success.** Rejected: it exercises the `else` branch and leaves the success path — the one carrying the app's work — unchecked. Double-answering (once ok, once failed) is out of scope for this spec; it is worth revisiting once there is real data on what the success pass finds. +- **Suppress every finding whose message mentions `nil`.** Rejected on the measurement: it would also suppress a real `nil` dereference the app performs itself, and it is the kind of broad rule that grows. +- **Give stub `fn`s synthetic arguments instead of `nil`.** Rejected: inventing a value per parameter type is a generator, and a generator is wrong in ways nobody can predict. `nil` is what the VM already does, it is visible in the message, and its false-positive surface is one shape. +- **Re-run `fn`s in a fresh isolate.** Rejected: the findings would come from a heap that never saw the app's own top-level state, and a `fn` that reads a `mod.state` its caller set would report faults that do not exist. + +## Measurements + +Windows, 2026-10-13, `C:\Users\Administrator\Desktop\makepad\target\release\splashcheck.exe`, over one-file apps in a temp directory. Quoted in full under [What shaped this decision](#what-shaped-this-decision-measured) above: the `fn` blind spot (`ok`, 0 findings), the callback blind spot (`ok`, 0 findings), the no-argument call (`a * 2` and `"x" + a` silent, `a.field` reporting `no getter for field field on type nil`), and the same three operations at the top level with `let a = nil`. + +Source evidence for the coercion, cited so a reader can check it without running anything: `platform/script/src/heap.rs:703-705` (`nil` → `0.0`), `platform/script/src/string_heap.rs:649-651` (`nil` → no text), `platform/script/src/native.rs:326-334` (the default getter's trap). diff --git a/docs/adr/0003-download-first-server-distribution.md b/docs/adr/0003-download-first-server-distribution.md new file mode 100644 index 00000000..222ad32f --- /dev/null +++ b/docs/adr/0003-download-first-server-distribution.md @@ -0,0 +1,86 @@ +# ADR 0003: Download-first distribution: a pinned server, one notice, and an activation that never fails + +- **Date:** 2026-10-13 +- **Status:** Accepted (2026-10-13). Not yet implemented: the release assets are issue #2 and the resolver with its download is issue #7, both in this repository. +- **Scope:** How the Splash language server reaches a person who installs the editor plugin — where the binary comes from, how its version is pinned, what happens when it cannot be fetched, and which part of that is a pure function with tests. It does not decide the server's behaviour (ADR 0002) or its query API (ADR 0001), and it does not cover embedding platform binaries in the `.vsix`, which is a separate spec. +- **Relates to:** [ADR 0001](0001-vm-query-api-two-functions.md), [ADR 0002](0002-analysis-posture-and-one-suppression-rule.md); App Flow #1 (the spec), #2, #7, #8. + +## Context + +Today the server exists only for its author. To use it, a person checks out the makepad fork, runs `cargo build` on it by hand, and points the plugin at the result. For everyone else the plugin does not exist: they get syntax highlighting, which is the one capability that needs no binary. The spec's third problem statement is exactly this, and its acceptance is a fresh VS Code on any of the three desktop platforms, with nothing built locally. + +Two facts, both measured, decide what the distribution can and cannot be: + +### What shaped this decision (measured) + +- **The fork has no CI.** `gh api repos/OctoSense-org/makepad/actions/workflows` answers `{"total_count": 0}`, and `gh api repos/OctoSense-org/makepad/contents/.github` answers 404. No workflow builds the server, so no release assets exist to point at, and a distribution plan that assumes the fork will produce them is a plan that waits on work nobody has scheduled. The assets are therefore produced here, where CI does run: `.github/workflows/ci.yml` already builds on `ubuntu-latest`, `windows-latest` and `macos-14`, which is the platform matrix the plugin needs (issue #2). +- **A Windows path with single backslashes does not survive JSON.** Node 24, today: + + ``` + $ node -e 'JSON.parse(String.raw`"C:\Users\me"`)' + SyntaxError: Bad escaped character in JSON at position 4 (line 1 column 5) + ``` + + A server path typed the way a Windows person types it either fails to parse or, once it has round-tripped through a JSON layer that escaped it once, arrives with its separators eaten. The symptom is the worst kind: the extension activates, finds nothing where the path points, and silently falls back to highlighting. That is why the discovery and download strategy is a pure function with every input injected and node tests over the whole priority chain (issue #7), and why the stale-value cases — a setting mangled by escaping, a host that cached a resolved path earlier in its session — each get a named test rather than a bug report. + +## Decision + +### 1. Download-first, as the model for a new user + +A person who installs the plugin and opens a `.splash` file gets the server without building anything: on first use the extension downloads the platform binary for its host and starts it. "Download-first" describes the model, not the resolver's order — see §2. + +### 2. The resolver is one pure function, and the download is its last source + +Discovery and download are a single pure function whose inputs are all injected: the workspace setting, the environment, `PATH`, the sibling checkouts, and a downloader. It returns either a resolved server path with the reason it was chosen, or "not found" with the reason it was not. + +The order is setting → environment variable → `PATH` → sibling checkout → download. The download is last because everything before it is a local statement of intent: a person who set a path, exported a variable, or has the fork checked out beside the plugin means that build, and a download that shadowed it would be a support problem with no diagnostic. The download is what the person who has none of those gets, which is the case this ADR exists for. + +The existing chain's behaviour does not change (issue #7's last criterion): a setting still wins, an environment variable still wins over `PATH`, and a sibling checkout is still found. The function is additive — the download is a new last source, not a rewrite of the four. + +### 3. The version is pinned, and moves only by a pull request + +The version is a constant in the plugin's manifest. It is overridable by a workspace setting, for a person testing a local build of the server against a released plugin. Nothing floats: no "latest", no range, no resolution at install time against a moving tag. + +The pin moves when a pull request moves it, and that pull request names what changed in the server. Each release asset records the makepad revision it was built from, and the output channel prints that revision beside the server's own version — so "which binary is this, from which source" is answerable from inside the editor instead of by bisecting a fork. + +### 4. Failure is one notice, one explanation, and a fallback — never a dead plugin + +When the server cannot be fetched: + +- **One notification**, once per session per reason, saying the server is unavailable. Not one per document, not one per retry. +- **One output-channel line** with the cause (network, HTTP status, checksum, unsupported platform) and the manual build path — the `cargo build` command and the setting that points at its result — so the person who wants to self-serve can. +- **Then the plugin carries on in degraded mode**: syntax highlighting, and whatever capabilities survive without a server. `activate()` does not throw, does not reject, and does not leave a half-registered client behind. A download failure is a missing feature, not a broken install. + +### 5. Three platforms, named assets, checksums + +The assets are named by platform and architecture and are checksummed, so a resolver can refuse a truncated or wrong download instead of starting a corrupt binary. A platform with no asset says so in the notice — "no server for this platform yet" is a different sentence from "download failed", and the person needs to know which one they got. + +## Consequences + +- A fresh install works with no local toolchain, which is the whole point; the cost is one download per version per platform, and a version bump re-downloads. +- The extension's activation path gains a network call, contained behind the resolver: the pure function decides, the caller performs. That seam is also where the "only highlighting left" failures are tested (issue #7), which is why it is the only new seam on the extension side. +- The plugin now ships a version of the server that can be older than the makepad revision a person has checked out. The pin's output-channel line is what keeps that visible. +- A person behind a proxy or offline gets highlighting plus one notice. That is the intended degraded mode, not a bug to be fixed by retrying harder. + +## Alternatives considered + +- **Embed the platform binaries in the `.vsix`.** Rejected for now, and named as out of scope in the spec: it triples the package every person downloads to serve the one platform they use, and it makes the server's version the package's version. It is the right answer later, when there is a release process worth embedding. +- **Depend on the fork's CI to publish assets.** Rejected on the measurement above: there is no CI in the fork, so this would be a distribution plan that cannot start. +- **Resolve "latest" at install time.** Rejected: a plugin and a server that can drift apart inside one session produce findings nobody can reproduce, and the spec's requirement is that the pair is pinned and overridable. +- **Try the download before the local chain.** Rejected: a person who set a path or exported a variable means it, and a download that wins over an explicit setting turns a configuration into a mystery. +- **Retry the download with backoff, then give up.** Rejected: a failure that is not reachable in one attempt is not reachable in five, and each retry is another chance to notify. One attempt, one notice, one explanation. + +## Measurements + +Windows, 2026-10-13: + +``` +$ gh api repos/OctoSense-org/makepad/actions/workflows --jq .total_count +0 +$ gh api repos/OctoSense-org/makepad/contents/.github +{"message":"Not Found", ... "status":"404"} +$ node -e 'JSON.parse(String.raw`"C:\Users\me"`)' +SyntaxError: Bad escaped character in JSON at position 4 (line 1 column 5) +``` + +The platform matrix is `.github/workflows/ci.yml` in this repository: `ubuntu-latest`, `windows-latest`, `macos-14`. diff --git a/tools/splash-server-toolchain.json b/tools/splash-server-toolchain.json new file mode 100644 index 00000000..24952ba4 --- /dev/null +++ b/tools/splash-server-toolchain.json @@ -0,0 +1,40 @@ +{ + "schema": 1, + "server": { + "name": "splash-lsp", + "version": "2.0.0", + "source": { + "repository": "woshuoduijiushidui/makepad", + "branch": "feat/splashcheck", + "revision": "0bbd43a7bb53ca997ad0f9bd3e8abca8a797bcc0", + "package": "makepad-widgets", + "bin": "splash-lsp", + "features": [ + "maps" + ] + }, + "release": { + "repository": "woshuoduijiushidui/OctoSense-App-Flow", + "tag": "splash-lsp-v2.0.0" + } + }, + "runtime": { + "lock": "native-runtime.lock.json", + "sibling": "octoscript-makepad" + }, + "platforms": [ + { + "platform": "win32", + "target": "x86_64-pc-windows-msvc", + "asset": "splash-lsp-2.0.0-x86_64-pc-windows-msvc.zip" + }, + { + "platform": "darwin", + "target": "aarch64-apple-darwin" + }, + { + "platform": "linux", + "target": "x86_64-unknown-linux-gnu" + } + ] +} diff --git a/tools/splash_server_release.py b/tools/splash_server_release.py new file mode 100644 index 00000000..65ffef4f --- /dev/null +++ b/tools/splash_server_release.py @@ -0,0 +1,194 @@ +#!/usr/bin/env python3 +"""The Splash language server's release: one naming convention, one version record. + +The `splash-lsp` binary is built from a makepad checkout, and the makepad fork +has no CI of its own — so the release is produced here, from the revision this +file pins, under a name that says which server and which platform it is. A +version nobody records is a version nobody can pin: the extension's download +(the server arrives with the plugin) reads this record rather than carrying its +own copy of the number. + +Run it with no arguments to print the release as it is recorded, or with +`--build-plan ` to emit the shell assignments one build needs — which is +also the guard: a target nothing was published for exits non-zero, so a runner +cannot quietly produce an asset the record does not name. +""" +import argparse +import hashlib +import json +from pathlib import Path +import sys + +HERE = Path(__file__).resolve().parent +PIN = HERE / 'splash-server-toolchain.json' + + +def asset_name(name, version, target): + """The release asset for one server build. + + Version and target both belong in the name: a downloader that fetches + `splash-lsp.zip` has no way to tell which of two servers it just got, and a + human reading a release page cannot tell which platform one is for. + """ + return f'{name}-{version}-{target}.zip' + + +def release_tag(name, version): + """The tag the whole server release lives under, namespaced per server.""" + return f'{name}-v{version}' + + +def download_url(repository, tag, asset): + """Where a release asset is fetched from, for a repository that holds it.""" + return f'https://github.com/{repository}/releases/download/{tag}/{asset}' + + +def repository_of(url): + """The `owner/name` a git URL names, which is what a checkout action takes.""" + return url.removeprefix('https://github.com/').removesuffix('.git') + + +def runtime_lock(pin): + """The sibling checkout a build needs, and the lock that pins it. + + makepad patches `octoscript-node` to a path outside its own tree, so the + build is only reproducible with that sibling beside it at a known revision. + This repository already locks the shared runtime for its flows, and CI runs + `tools/setup-native.py --check` over that lock, so its shape is guarded + elsewhere: this reads it rather than validating it a second time. + """ + path = HERE.parent / pin['runtime']['lock'] + locked = json.loads(path.read_text(encoding='utf-8')) + return {'repository': repository_of(locked['url']), 'revision': locked['revision']} + + +def read_pin(path=PIN): + return json.loads(Path(path).read_text(encoding='utf-8')) + + +def published(pin): + """The platforms that have an asset. Everything else is a promise.""" + return [platform for platform in pin['platforms'] if 'asset' in platform] + + +def unpublished(pin): + return [platform for platform in pin['platforms'] if 'asset' not in platform] + + +def download(pin, platform): + """The URL for a platform, or None when nothing was published for it. + + None is the honest answer: a URL that 404s is worse than an absent one, + because a downloader cannot tell "not built yet" from "the network is down". + """ + server = pin['server'] + for entry in published(pin): + if entry['platform'] == platform: + return download_url(server['release']['repository'], server['release']['tag'], entry['asset']) + return None + + +def build_plan(pin, target): + """Everything one build of one target needs, as shell assignments. + + Every value is a slug, a version, a revision or a name — nothing that could + carry a newline or a `=` — so the caller can append this straight to + `$GITHUB_OUTPUT`. A target nothing was published for returns None: the + release path must not be able to produce an asset the record does not name. + """ + for entry in published(pin): + if entry['target'] == target: + server, source = pin['server'], pin['server']['source'] + locked = runtime_lock(pin) + return { + 'name': server['name'], + 'version': server['version'], + 'tag': server['release']['tag'], + 'release-repository': server['release']['repository'], + 'source-repository': source['repository'], + 'source-revision': source['revision'], + 'source-package': source['package'], + 'source-bin': source['bin'], + 'source-features': ','.join(source['features']), + 'runtime-repository': locked['repository'], + 'runtime-revision': locked['revision'], + 'runtime-sibling': pin['runtime']['sibling'], + 'target': target, + 'asset': entry['asset'], + } + return None + + +def describe(pin): + """The record as a downloader or a release page reads it.""" + server = pin['server'] + return { + 'name': server['name'], + 'version': server['version'], + 'source-revision': server['source']['revision'], + 'tag': server['release']['tag'], + 'assets': [download(pin, entry['platform']) for entry in published(pin)], + 'missing': [entry['platform'] for entry in unpublished(pin)], + } + + +def release_notes(pin, assets): + """The release page for the assets actually uploaded, as Markdown. + + The digests are computed here rather than recorded in advance: a rebuilt + binary is a different binary, so a digest committed ahead of the build would + only be a promise the next build breaks. Written at publish time, the digest + describes the bytes that were uploaded. + """ + assets = Path(assets) + server, source = pin['server'], pin['server']['source'] + revision = source['revision'] + lines = [ + f"The `{server['name']}` language server, version {server['version']}.", + '', + 'Built from ' + f"[{revision[:12]}](https://github.com/{source['repository']}/commit/{revision})" + f" -- package `{source['package']}`, features `{', '.join(source['features'])}`.", + '', + '| Platform | Asset | SHA-256 |', + '| --- | --- | --- |', + ] + for entry in published(pin): + asset = assets / entry['asset'] + if not asset.is_file(): + raise FileNotFoundError(f'no {entry["asset"]} in {assets}') + digest = hashlib.sha256(asset.read_bytes()).hexdigest() + lines.append(f"| `{entry['platform']}` ({entry['target']}) | `{asset.name}` | `{digest}` |") + missing = ', '.join(entry['platform'] for entry in unpublished(pin)) + lines += [ + '', + 'Recorded in `tools/splash-server-toolchain.json`, which also names the' + f' platforms with no asset yet: {missing}.', + '', + ] + return '\n'.join(lines) + + +def main(argv=None): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument('--pin', default=str(PIN), help='the version record to read') + parser.add_argument('--build-plan', metavar='TARGET', + help='print the shell assignments one build of this Rust target needs') + args = parser.parse_args(argv) + pin = read_pin(args.pin) + if args.build_plan: + plan = build_plan(pin, args.build_plan) + if plan is None: + print(f"{pin['server']['name']}: no published asset for {args.build_plan}; " + f"publish it and record it in {args.pin} first", file=sys.stderr) + return 1 + for key, value in plan.items(): + print(f'{key}={value}') + return 0 + json.dump(describe(pin), sys.stdout, indent=2) + sys.stdout.write('\n') + return 0 + + +if __name__ == '__main__': + raise SystemExit(main()) diff --git a/tools/splashcheck_corpus/README.md b/tools/splashcheck_corpus/README.md new file mode 100644 index 00000000..df8f2a84 --- /dev/null +++ b/tools/splashcheck_corpus/README.md @@ -0,0 +1,93 @@ +# The splashcheck golden corpus + +The Splash checker's findings over the twelve real apps, pinned as text. One +command re-runs the checker and compares: + +```sh +python3 tools/splashcheck_golden.py # compare +python3 tools/splashcheck_golden.py --update # re-record after an intended change +python3 tools/splashcheck_golden.py --only mail # one app +``` + +Exit status: `0` every golden matches, `1` at least one differs (with a unified +diff on stdout), `2` a setup problem - no checker binary, no corpus app, no +golden file. A workspace that cannot run the corpus says so; it never passes +quietly. + +## Where the apps and the checker are + +Neither is in this repository, so the runner expects a workspace that holds the +sibling checkouts beside this one: + +```text +/ + OctoSense-App-Flow/ this repository + OctoSense/ the shell and its system apps: apps//bundle + OctoSense-AppCard/ the AppCard assistant: bundle + makepad/ splash-probe/ where the checker is built +``` + +Where that workspace is, in order: `--corpus-root`, `$OCTOSENSE_WORKSPACE`, then +this repository's parent directory. The checker is `splashcheck`, built from the +OctoSense fork of Makepad - `cargo build -p makepad-widgets --bin splashcheck +--features maps` - and is found in `$SPLASHCHECK`, then `splash-probe`, `makepad` +or `Octoscript-Makepad` under the workspace root, then `$PATH`: the order and the +`os.X_OK` test `tools/octo` applies to the tools it wraps. Build it with the +features of the host it checks for; this corpus is recorded with `maps`, or the +maps app reports a `MapView` its host does have. + +## Layout + +| Path | What it is | +| --- | --- | +| `golden/.txt` | One app's report: app id, a digest of the sources the checker read, and every finding with its line, column, code and message. | +| `fn-blindspot/` | The fn blind-spot fixture, shipped here so it is checked wherever the checker is. | + +The twelve apps are the OctoSense shell's eleven system apps (`news`, `photos`, +`maps`, `camera`, `mail`, `calendar`, `ai-providers`, `youtube`, `quickdeck`, +`pdftools`, `writer`) plus the AppCard assistant. `wasmlab` also ships a bundle +but is a lab rather than a system app, so it is not in the corpus. + +The digest covers the `.splash` files and the manifest and nothing else, because +those are the checker's whole input: an icon, a `tools.json` or a screenshot +cannot move a finding, and hashing them would make a screenshot update look like +a checker regression. + +## What a golden does not show + +Two reductions, both deliberate, both in the code with their reasons: + +- **Findings are sorted**, not listed in report order, so a reordering is not a + diff. +- **The VM's allocation counters are scrubbed** (`width(obj653)` becomes + `width(obj)`). They are heap indices: they move when anything ahead of them in + the registry moves, and a golden that kept them would churn on every unrelated + refactor and bury the change a reader came to see. + +## The fn blind spot + +`fn-blindspot/main.splash` is one line: `fn broken(){ let x = nope }` - a fault +inside a function nothing calls at the top level. The checker evaluates +top-level code only, so its golden records no findings. That is the measurement +this corpus exists to make: when the checker starts evaluating every `fn`, this +golden gains a finding and the diff says so, rather than the change arriving as +a sentence in a changelog. + +## Re-recording + +`--update` writes what the checker says now, so it is also how an intended +change is accepted. [ADR 0002](../../docs/adr/0002-analysis-posture-and-one-suppression-rule.md) +§4 makes this corpus the regression gate for the analysis depth: the fixture's +expectation flips from "no findings" to "findings", and every other app's diff +must be empty or explained. So a re-record is not a silent refresh - name what +moved and why, in the commit and in the pull request, the way +[ADR 0001](../../docs/adr/0001-vm-query-api-two-functions.md) §4 asks of the +corpus it governs: a green local run nobody wrote down is not a promise. + +## Boundary + +CI does not run the corpus: no runner has the `splashcheck` binary or the two app +checkouts, so `tools/test_splashcheck_golden.py` asserts the honest exit 2 there +and compares for real on a prepared developer machine. The corpus is a local +regression net, and a machine that has not run it has not been checked - which +is why the runner says so instead of passing quietly. diff --git a/tools/splashcheck_corpus/fn-blindspot/main.splash b/tools/splashcheck_corpus/fn-blindspot/main.splash new file mode 100644 index 00000000..cee92c4a --- /dev/null +++ b/tools/splashcheck_corpus/fn-blindspot/main.splash @@ -0,0 +1 @@ +fn broken(){ let x = nope } diff --git a/tools/splashcheck_corpus/fn-blindspot/manifest.json b/tools/splashcheck_corpus/fn-blindspot/manifest.json new file mode 100644 index 00000000..5250ce54 --- /dev/null +++ b/tools/splashcheck_corpus/fn-blindspot/manifest.json @@ -0,0 +1 @@ +{"schema": 1, "id": "dev.example.fnblindspot", "capabilities": []} diff --git a/tools/splashcheck_corpus/golden/ai-providers.txt b/tools/splashcheck_corpus/golden/ai-providers.txt new file mode 100644 index 00000000..d62e80cd --- /dev/null +++ b/tools/splashcheck_corpus/golden/ai-providers.txt @@ -0,0 +1,12 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.ai-providers +digest 5edb8b528eb90fda55aa32830e967ab54c54d7d87e1e01269ebc34c56be87fab +file main.splash +bytes 22147 +manifest found +declares no net +capabilities storage, llm +findings 1 + warn splash.capability 1 host.request call(s) pass a computed service name, which cannot be checked statically +notes 0 diff --git a/tools/splashcheck_corpus/golden/appcard.txt b/tools/splashcheck_corpus/golden/appcard.txt new file mode 100644 index 00000000..61bba03b --- /dev/null +++ b/tools/splashcheck_corpus/golden/appcard.txt @@ -0,0 +1,11 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app pantry-steward +digest 3f498d18fc7230f7f84744fc0202d0e8fb19a7e59492b6ae72ef75d9b66378f1 +file main.splash +bytes 131675 +manifest found +declares no net +capabilities storage, model +findings 0 +notes 0 diff --git a/tools/splashcheck_corpus/golden/calendar.txt b/tools/splashcheck_corpus/golden/calendar.txt new file mode 100644 index 00000000..e2a4acb7 --- /dev/null +++ b/tools/splashcheck_corpus/golden/calendar.txt @@ -0,0 +1,12 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.calendar +digest 623837f482d9dde9d9210c137b513a8ee3d8e19fbd010d06c0690938458fdd77 +file main.splash +bytes 17011 +manifest found +declares no net +capabilities calendar, glance +findings 1 + warn splash.capability 1 host.request call(s) pass a computed service name, which cannot be checked statically +notes 0 diff --git a/tools/splashcheck_corpus/golden/camera.txt b/tools/splashcheck_corpus/golden/camera.txt new file mode 100644 index 00000000..4a7ab576 --- /dev/null +++ b/tools/splashcheck_corpus/golden/camera.txt @@ -0,0 +1,11 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.camera +digest 8e09d61e71bcd632c279cabd5e625e4bb7d2417d51bd9150c894629ec58989d3 +file main.splash +bytes 10378 +manifest found +declares no net +capabilities storage, camera, microphone, library, glance +findings 0 +notes 0 diff --git a/tools/splashcheck_corpus/golden/fn-blindspot.txt b/tools/splashcheck_corpus/golden/fn-blindspot.txt new file mode 100644 index 00000000..dc46675f --- /dev/null +++ b/tools/splashcheck_corpus/golden/fn-blindspot.txt @@ -0,0 +1,11 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app dev.example.fnblindspot +digest 192da8407d15269d0fdb2ad4510c7bbd2f2dcf3f1178088959fe851148c9a884 +file main.splash +bytes 28 +manifest found +declares no net +capabilities none +findings 0 +notes 0 diff --git a/tools/splashcheck_corpus/golden/mail.txt b/tools/splashcheck_corpus/golden/mail.txt new file mode 100644 index 00000000..6b386c79 --- /dev/null +++ b/tools/splashcheck_corpus/golden/mail.txt @@ -0,0 +1,12 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.mail +digest 02e72bd64efccf0bacf989be2f8462cbb7845f0aec372cdf071483c6e7394225 +file main.splash +bytes 15573 +manifest found +declares no net +capabilities storage, mail, glance +findings 1 + 268:28 error splash.eval property visible not defined on type. Did you mean: distribute(nil) or abs_pos(nil), width(obj), height(obj), aspect(nil) (+55 more) +notes 0 diff --git a/tools/splashcheck_corpus/golden/maps.txt b/tools/splashcheck_corpus/golden/maps.txt new file mode 100644 index 00000000..e9184253 --- /dev/null +++ b/tools/splashcheck_corpus/golden/maps.txt @@ -0,0 +1,11 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.maps +digest 1b3b5fa68783c4386cce302b3ca87a37931c11999ca49260b4d5cd46def5592d +file main.splash +bytes 59187 +manifest found +declares net +capabilities storage, net, location, web, glance +findings 0 +notes 0 diff --git a/tools/splashcheck_corpus/golden/news.txt b/tools/splashcheck_corpus/golden/news.txt new file mode 100644 index 00000000..7dacc046 --- /dev/null +++ b/tools/splashcheck_corpus/golden/news.txt @@ -0,0 +1,11 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.news +digest 13158e3fae34c52fca19ddcd91656006bc5d242a3db553bbc2a2789ba54f0dd6 +file main.splash +bytes 23224 +manifest found +declares net +capabilities storage, net, images, web, news, glance +findings 0 +notes 0 diff --git a/tools/splashcheck_corpus/golden/pdftools.txt b/tools/splashcheck_corpus/golden/pdftools.txt new file mode 100644 index 00000000..8bdb8e66 --- /dev/null +++ b/tools/splashcheck_corpus/golden/pdftools.txt @@ -0,0 +1,12 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.pdftools +digest 0113fcc0c158844d1f2a3361bf6557a560d4f16eb190c0c9d45d34affbeefc72 +file main.splash +bytes 79599 +manifest found +declares no net +capabilities storage, files, pdf +findings 1 + warn splash.capability 1 host.request call(s) pass a computed service name, which cannot be checked statically +notes 0 diff --git a/tools/splashcheck_corpus/golden/photos.txt b/tools/splashcheck_corpus/golden/photos.txt new file mode 100644 index 00000000..1f283eba --- /dev/null +++ b/tools/splashcheck_corpus/golden/photos.txt @@ -0,0 +1,11 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.photos +digest 838df1a71bb012a6e0cbe8c78c6b4ab6d35eca1a15d3b3a6da5a2d9518c58d7b +file main.splash +bytes 60514 +manifest found +declares no net +capabilities storage, glance, model +findings 0 +notes 0 diff --git a/tools/splashcheck_corpus/golden/quickdeck.txt b/tools/splashcheck_corpus/golden/quickdeck.txt new file mode 100644 index 00000000..5dc9265b --- /dev/null +++ b/tools/splashcheck_corpus/golden/quickdeck.txt @@ -0,0 +1,12 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.quickdeck +digest 8e19ad81e418610ae5ac9c1903b2b738d3dcfb0ac3df6196f6552ed50087b036 +file main.splash +bytes 50962 +manifest found +declares no net +capabilities storage, deck +findings 1 + warn splash.capability 1 host.request call(s) pass a computed service name, which cannot be checked statically +notes 0 diff --git a/tools/splashcheck_corpus/golden/writer.txt b/tools/splashcheck_corpus/golden/writer.txt new file mode 100644 index 00000000..e684361d --- /dev/null +++ b/tools/splashcheck_corpus/golden/writer.txt @@ -0,0 +1,12 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.writer +digest 27c4307b3e37e4927ed0006311a2b665395633b1b8b5a4c20642290e93ddc7fe +file main.splash +bytes 46008 +manifest found +declares no net +capabilities storage, word +findings 1 + warn splash.capability 1 host.request call(s) pass a computed service name, which cannot be checked statically +notes 0 diff --git a/tools/splashcheck_corpus/golden/youtube.txt b/tools/splashcheck_corpus/golden/youtube.txt new file mode 100644 index 00000000..fc9b90e3 --- /dev/null +++ b/tools/splashcheck_corpus/golden/youtube.txt @@ -0,0 +1,11 @@ +# splashcheck_golden golden corpus +# checker: splashcheck 2.0.0 +app os.youtube +digest a52dda92c6888f68e5b9aa0d96c7cbe6afe16a486183dddfde5ba4f1840cbc20 +file main.splash +bytes 13168 +manifest found +declares net +capabilities storage, net, glance +findings 0 +notes 0 diff --git a/tools/splashcheck_golden.py b/tools/splashcheck_golden.py new file mode 100644 index 00000000..0ba4fd48 --- /dev/null +++ b/tools/splashcheck_golden.py @@ -0,0 +1,403 @@ +#!/usr/bin/env python3 +"""splashcheck golden corpus: the checker's findings over the real apps, compared. + +The Splash checker (`splashcheck`, built from the OctoSense fork of Makepad) +decides what an app's faults are. Every change to it - and every change to an +app - moves those findings, and a moved finding nobody noticed is how a +refactor silently starts missing things. This runner pins the answer: it runs +the checker over the twelve real apps, renders each report as text and compares +it with the golden file committed beside it. A difference is a readable diff and +a non-zero exit, never a silent pass. + + python3 tools/splashcheck_golden.py # compare + python3 tools/splashcheck_golden.py --update # re-record + python3 tools/splashcheck_golden.py --only mail # one app + +The corpus is the OctoSense shell's eleven system apps plus the AppCard +assistant. Neither is in this repository, so the runner expects a workspace +that holds the sibling checkouts beside this one: + + / + OctoSense-App-Flow/ this repository + OctoSense/ the shell and its system apps: apps//bundle + OctoSense-AppCard/ the AppCard assistant: bundle + makepad/ splash-probe/ where the checker is built + +Where that workspace is, in order: `--corpus-root`, `$OCTOSENSE_WORKSPACE`, then +this repository's parent directory. `docs/NATIVE-WORKSPACE.md` documents the +runtime workspace `tools/setup-native.py` prepares; the two app checkouts are +not part of it, which is why the corpus names its own layout above rather than +pointing at that one. + +The checker is `splashcheck`, built from the OctoSense fork of Makepad: +`cargo build -p makepad-widgets --bin splashcheck --features maps`. It is found +in `$SPLASHCHECK`, then `splash-probe`, `makepad` or `Octoscript-Makepad` under +the workspace root (`target/release` before `target/debug`), then `$PATH` - the +order, and the `os.X_OK` test, that `tools/octo`'s `find_binary` applies to the +tools it wraps. Build it with the features of the host it checks for: this +corpus is recorded with `maps`, or the maps app reports a `MapView` its host +does have. + +What the runner reads from the checker is one contract, and it is the checker's +own rather than this repository's invention: `--json`, whose shape `print_json` +in `widgets/src/splashcheck.rs` defines - `files[].bytes`, +`manifest_found`, `declares_net`, `capabilities`, `findings`, `notes` and +`fatal`; a finding's `severity`, `code`, `line`, `column` and `message`; and the +report's `tool` and `version` - together with the exit statuses `run` in the +same file returns, 0 nothing new, 1 a finding, 2 a usage or I/O problem. Both +were read from that source and confirmed against a recorded run of the binary; +the golden files in this directory are that run, so a change to either side of +the contract shows up here as a diff rather than as a surprise. + +The thirteenth entry is the fn blind spot: `fn broken(){ let x = nope }`, a +fault inside a function nothing calls at the top level. The checker evaluates +top-level code only, so its golden records no findings - and that is the +measurement this corpus exists to make: when the checker starts evaluating +every `fn`, this golden changes and the diff says so. +[ADR 0002](../../docs/adr/0002-analysis-posture-and-one-suppression-rule.md) +makes this corpus the regression gate for that change: the fixture's expectation +flips from "no findings" to "findings", and every other app's diff must be empty +or explained. Re-record with `--update`, and name what moved in the commit and +the pull request - no CI runs this corpus, so the written-down run is the only +promise there is. + +Exit status, the discipline `splashcheck` itself uses: 0 every golden matches, +1 at least one differs, 2 a setup problem - no checker binary, no corpus app, +no golden file to compare against. +""" +import argparse +import difflib +import hashlib +import json +import os +import re +import shutil +import subprocess +import sys +from pathlib import Path +from typing import NamedTuple + +REPO = Path(__file__).resolve().parent.parent +CORPUS = Path(__file__).resolve().parent / "splashcheck_corpus" +GOLDEN = CORPUS / "golden" + +TOOL = "splashcheck_golden" + +EXE = ".exe" if os.name == "nt" else "" + +# The OctoSense shell's eleven system apps, as its README lists them. `wasmlab` +# also ships a bundle but is a lab rather than a system app, so it is not in the +# corpus: a golden is a claim about what the checker should say, and a lab app +# is free to be wrong on purpose. +SYSTEM_APPS = ( + "news", "photos", "maps", "camera", "mail", "calendar", "ai-providers", + "youtube", "quickdeck", "pdftools", "writer", +) + +EXIT_OK = 0 +EXIT_DIFF = 1 +EXIT_SETUP = 2 + +# The VM's "Did you mean" tail names each suggestion with its kind and an +# allocation counter: `width(obj653)`. The counter is a heap index, so it moves +# when anything ahead of it in the registry moves - a golden that kept it would +# churn on every unrelated refactor and bury the change a reader came to see. +OBJECT_ID = re.compile(r"\(obj\d+\)") + + +class SetupError(Exception): + """A problem with the inputs rather than the comparison: exit 2.""" + + +def workspace_root(explicit=None): + """The directory holding the sibling repositories. See the module docstring.""" + if explicit: + return Path(explicit) + return Path(os.environ.get("OCTOSENSE_WORKSPACE") or REPO.parent) + + +def corpus(root): + """Every bundle the corpus checks, as (name, path), in a stable order.""" + entries = [("appcard", root / "OctoSense-AppCard" / "bundle")] + entries += [(name, root / "OctoSense" / "apps" / name / "bundle") for name in SYSTEM_APPS] + # The blind-spot fixture ships in this repository, so it is checked wherever + # the checker is. + entries.append(("fn-blindspot", CORPUS / "fn-blindspot")) + return sorted(entries) + + +def checker_candidates(root): + """Where a `splashcheck` binary may be, in the order the module docstring gives. + + Each item is the path searched, not only the ones that exist: a reader told + the binary is missing needs to see where the runner looked, which is what + `tools/octo`'s `need` prints for the tools it wraps. + """ + from_env = os.environ.get("SPLASHCHECK") + if from_env: + yield f"$SPLASHCHECK={from_env}", Path(from_env) + # `splash-probe` first: it is the standalone package that adopts + # `makepad-widgets` as a path dependency and builds the checker with the + # host's features, which is the build this corpus is recorded against. + for project in ("splash-probe", "makepad", "Octoscript-Makepad"): + for profile in ("release", "debug"): + path = root / project / "target" / profile / f"splashcheck{EXE}" + yield str(path), path + + +def find_checker(explicit=None, root=None): + """The checker binary and where it was found, or `(None, searched)`. + + `os.X_OK`, not `is_file`: a candidate that exists but is not executable is + not a checker, and picking it anyway would turn into an `OSError` from + `subprocess` rather than into the exit 2 this tool owes its caller. + """ + if explicit: + path = Path(explicit) + if path.is_file() and os.access(path, os.X_OK): + return path, [str(path)] + return None, [str(path)] + searched = [] + for label, candidate in checker_candidates(root or workspace_root()): + searched.append(label) + if candidate.is_file() and os.access(candidate, os.X_OK): + return candidate, searched + on_path = shutil.which("splashcheck") + searched.append("PATH (splashcheck)") + if on_path: + return Path(on_path), searched + return None, searched + + +def run_checker(checker, bundle): + """One checker invocation over one bundle: its JSON report. + + The JSON is the contract (`--json` is the machine face of the same analysis + the human output prints), and bytes are decoded as UTF-8 rather than with + the platform locale: a message quoting an app's own text is UTF-8 wherever + this runs. + """ + try: + proc = subprocess.run( + [str(checker), "--json", str(bundle)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) + except OSError as error: + # A path that will not run: no permission, not an executable at all. + raise SetupError(f"{checker} will not run: {error}") + if proc.returncode == EXIT_SETUP: + raise SetupError(f"{checker} refused {bundle}: {proc.stderr.decode('utf-8', 'replace').strip()}") + try: + return json.loads(proc.stdout.decode("utf-8")) + except (UnicodeDecodeError, ValueError) as error: + raise SetupError(f"{checker} said nothing usable about {bundle}: {error}") +def app_id(bundle): + """The app's own id, as its manifest declares it.""" + path = bundle / "manifest.json" + if not path.is_file(): + return "none" + try: + value = json.loads(path.read_text(encoding="utf-8")) + except (UnicodeDecodeError, ValueError): + return "unreadable" + return str(value.get("id") or "none") + + +def bundle_digest(bundle): + """A digest of the bundle's `.splash` files and its manifest. + + Recorded in the golden so that a changed app says so. Without it, a diff + after an app update reads as a checker regression, and the reader cannot + tell which of the two moved. + + Those files and no others, because they are the checker's whole input: it + evaluates the `.splash` files it scans and reads the manifest's + declarations, so nothing else in a bundle - an icon, a `tools.json`, a + screenshot - can move a finding. A bundle file that changes without moving + a finding therefore leaves the golden alone, which is the point. + """ + digest = hashlib.sha256() + for path in sorted(bundle.rglob("*"), key=lambda p: p.as_posix()): + if not path.is_file(): + continue + relative = path.relative_to(bundle).as_posix() + if path.suffix != ".splash" and relative != "manifest.json": + continue + digest.update(relative.encode("utf-8")) + digest.update(b"\0") + digest.update(path.read_bytes()) + digest.update(b"\0") + return digest.hexdigest() + + +def relative_to_bundle(bundle, path_text): + """A report's file path as a path inside its bundle, never as a checkout path.""" + path = Path(path_text) + try: + return path.relative_to(bundle).as_posix() + except ValueError: + return path.name + + +def scrub(text, bundle): + """Take the bundle's own path out of a message, so a golden is checkout-free.""" + for spelling in (str(bundle), str(bundle).replace("\\", "/")): + text = text.replace(spelling, "") + return text + + +def finding_message(finding): + """A finding's message, with the VM's allocation counters taken out.""" + return OBJECT_ID.sub("(obj)", finding.get("message") or "") + + +def finding_key(finding): + """Findings are ordered, not listed: a golden must not depend on report order.""" + return (finding.get("line") or 0, finding.get("column") or 0, finding.get("code") or "", finding_message(finding)) + + +def finding_line(finding): + """One finding as a golden line: `3:17 error splash.eval `.""" + at = "" + if finding.get("line"): + at = f"{finding['line']}:{finding.get('column') or 0} " + return f"{at}{finding.get('severity')} {finding.get('code')} {finding_message(finding)}" + + +def render(bundle, report): + """One bundle's report as the text its golden file holds.""" + lines = [ + f"# {TOOL} golden corpus", + f"# checker: {report.get('tool', '?')} {report.get('version', '?')}", + f"app {app_id(bundle)}", + f"digest {bundle_digest(bundle)}", + ] + files = sorted(report.get("files") or [], key=lambda entry: entry.get("path") or "") + for entry in files: + lines.append(f"file {relative_to_bundle(bundle, entry.get('path') or '')}") + lines.append(f"bytes {entry.get('bytes', 0)}") + lines.append(f"manifest {'found' if entry.get('manifest_found') else 'missing'}") + lines.append(f"declares {'net' if entry.get('declares_net') else 'no net'}") + lines.append("capabilities " + (", ".join(entry.get("capabilities") or []) or "none")) + findings = sorted(entry.get("findings") or [], key=finding_key) + lines.append(f"findings {len(findings)}") + lines += [f" {finding_line(finding)}" for finding in findings] + notes = list(entry.get("notes") or []) + lines.append(f"notes {len(notes)}") + lines += [f" {note}" for note in notes] + if entry.get("fatal"): + lines.append("fatal " + scrub(str(entry["fatal"]), bundle)) + return "\n".join(lines) + "\n" + + +def finding_count(report): + """How many findings the whole run carries, for the one-line status.""" + return sum(len(entry.get("findings") or []) for entry in report.get("files") or []) + + +def display(path): + """A path as a reader would type it: relative to the working directory.""" + path = Path(path) + try: + return path.resolve().relative_to(Path.cwd().resolve()).as_posix() + except ValueError: + return path.as_posix() + + +def setup_error(message): + print(f"{TOOL}: {message}", file=sys.stderr) + return EXIT_SETUP + + +def parse(argv): + parser = argparse.ArgumentParser( + prog=TOOL, + description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) + parser.add_argument("--update", action="store_true", help="re-record the golden files instead of comparing them") + parser.add_argument("--corpus-root", type=Path, help="the workspace holding OctoSense/ and OctoSense-AppCard/") + parser.add_argument("--checker", help="the splashcheck binary to run") + parser.add_argument("--golden-dir", type=Path, help=f"where the golden files live (default: {display(GOLDEN)})") + parser.add_argument("--only", action="append", metavar="APP", help="one corpus app, by name; repeatable") + return parser.parse_args(argv) + + +class Recorded(NamedTuple): + """One bundle's golden text, with the name and count a reader wants first.""" + + name: str + path: Path + text: str + findings: int + + +def main(argv=None): + args = parse(argv) + golden_dir = args.golden_dir or GOLDEN + root = workspace_root(args.corpus_root) + + entries = corpus(root) + if args.only: + known = {name for name, _ in entries} + unknown = [name for name in args.only if name not in known] + if unknown: + return setup_error(f"no corpus app {unknown[0]!r}: {', '.join(sorted(known))}") + wanted = set(args.only) + entries = [entry for entry in entries if entry[0] in wanted] + + checker, searched = find_checker(args.checker, root) + if checker is None: + if args.checker: + return setup_error(f"no splashcheck binary at {args.checker}") + return setup_error( + "no splashcheck binary. Searched:\n " + "\n ".join(searched) + + f"\nSet $SPLASHCHECK, or build it with `cargo build -p makepad-widgets" + f" --bin splashcheck --features maps` in a sibling checkout under {root}." + ) + for name, path in entries: + if not path.is_dir(): + return setup_error( + f"no corpus app {name!r}: {path} is not a directory " + f"(the workspace root is {root}: --corpus-root or $OCTOSENSE_WORKSPACE)" + ) + + recorded = [] + for name, path in entries: + try: + report = run_checker(checker, path) + except SetupError as error: + return setup_error(f"{name}: {error}") + recorded.append(Recorded(name, path, render(path, report), finding_count(report))) + + if args.update: + golden_dir.mkdir(parents=True, exist_ok=True) + for entry in recorded: + (golden_dir / f"{entry.name}.txt").write_text(entry.text, encoding="utf-8") + print(f" rec {entry.name} - {entry.findings} finding(s)") + print(f"{TOOL}: wrote {len(recorded)} golden file(s) to {display(golden_dir)}") + return EXIT_OK + + changed = 0 + for entry in recorded: + golden_path = golden_dir / f"{entry.name}.txt" + if not golden_path.is_file(): + return setup_error(f"no golden file for {entry.name!r}: {display(golden_path)} (record it with --update)") + expected = golden_path.read_text(encoding="utf-8") + if expected == entry.text: + print(f" ok {entry.name} - {entry.findings} finding(s)") + continue + changed += 1 + print(f" DIFF {entry.name} - {entry.findings} finding(s)") + for line in difflib.unified_diff( + expected.splitlines(), entry.text.splitlines(), + fromfile=f"golden/{entry.name}.txt", tofile="current", n=3, lineterm="", + ): + print(line) + print(f"{TOOL}: {len(recorded) - changed} ok, {changed} changed") + return EXIT_DIFF if changed else EXIT_OK + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/test_splash_server_release.py b/tools/test_splash_server_release.py new file mode 100644 index 00000000..d2fe27d4 --- /dev/null +++ b/tools/test_splash_server_release.py @@ -0,0 +1,189 @@ +"""The Splash language server release: naming, the version record, and what is missing. + +The seam under test is the recorded release — `tools/splash-server-toolchain.json` +— read through `tools/splash_server_release.py`, which is the one place the +release workflow and the extension's downloader both read. No network, no cargo, +no binary: the record is checked against the naming convention it was published +under, so a hand-edited asset name cannot drift away from what the release +actually holds. +""" +import importlib.machinery +import importlib.util +import hashlib +import json +from pathlib import Path +import re +import subprocess +import sys +import tempfile +import unittest + +HERE = Path(__file__).resolve().parent +WORKFLOW = HERE.parent / '.github' / 'workflows' / 'release-splash-lsp.yml' +loader = importlib.machinery.SourceFileLoader( + 'splash_server_release', str(HERE / 'splash_server_release.py')) +spec = importlib.util.spec_from_loader(loader.name, loader) +release = importlib.util.module_from_spec(spec) +loader.exec_module(release) + + +def workflow_targets(): + """The Rust targets the release workflow's matrix builds. + + Read out of the file rather than restated here: the record and the workflow + are two files that must agree, and a test that restated either would only + prove the restatement. + """ + text = WORKFLOW.read_text(encoding='utf-8') + return set(re.findall(r'^\s*target:\s*(\S+)\s*$', text, re.M)) + + +class Naming(unittest.TestCase): + def test_an_asset_name_carries_the_version_and_the_target(self): + self.assertEqual( + release.asset_name('splash-lsp', '2.0.0', 'x86_64-pc-windows-msvc'), + 'splash-lsp-2.0.0-x86_64-pc-windows-msvc.zip') + + def test_the_tag_is_namespaced_per_server(self): + self.assertEqual(release.release_tag('splash-lsp', '2.0.0'), 'splash-lsp-v2.0.0') + + def test_the_download_url_points_at_the_recorded_release(self): + self.assertEqual( + release.download_url('octo/flow', 'splash-lsp-v2.0.0', 'splash-lsp-2.0.0-x86_64-pc-windows-msvc.zip'), + 'https://github.com/octo/flow/releases/download/' + 'splash-lsp-v2.0.0/splash-lsp-2.0.0-x86_64-pc-windows-msvc.zip') + + +class RecordedRelease(unittest.TestCase): + def setUp(self): + self.pin = release.read_pin() + + def test_the_recorded_version_is_a_release_triple(self): + self.assertRegex(self.pin['server']['version'], r'^\d+\.\d+\.\d+$') + + def test_the_source_is_a_pinned_commit_rather_than_a_branch(self): + self.assertRegex(self.pin['server']['source']['revision'], r'^[0-9a-f]{40}$') + + def test_the_tag_is_derived_from_the_server_and_its_version(self): + self.assertEqual( + self.pin['server']['release']['tag'], + release.release_tag(self.pin['server']['name'], self.pin['server']['version'])) + + def test_every_published_asset_follows_the_naming_convention(self): + for platform in release.published(self.pin): + self.assertEqual( + platform['asset'], + release.asset_name(self.pin['server']['name'], + self.pin['server']['version'], platform['target'])) + + def test_a_published_platform_resolves_to_the_url_it_was_published_at(self): + for platform in release.published(self.pin): + self.assertEqual( + release.download(self.pin, platform['platform']), + release.download_url(self.pin['server']['release']['repository'], + self.pin['server']['release']['tag'], platform['asset'])) + + def test_a_platform_nothing_was_published_for_has_no_url(self): + for platform in release.unpublished(self.pin): + self.assertIsNone(release.download(self.pin, platform['platform'])) + self.assertNotIn('asset', platform) + + def test_every_published_platform_names_a_real_rust_target(self): + for platform in release.published(self.pin): + self.assertRegex(platform['target'], + r'^[a-z0-9_.]+(?:-[a-z0-9_]+){2,3}$') + + def test_the_first_platform_binary_is_published(self): + self.assertTrue(release.published(self.pin), 'the release holds no asset at all') + + def test_every_platform_the_extension_runs_on_is_declared(self): + self.assertEqual( + {platform['platform'] for platform in self.pin['platforms']}, + {'win32', 'darwin', 'linux'}) + + def test_the_sibling_the_build_needs_is_pinned_by_this_repository(self): + # makepad patches octoscript-node to a path outside its own tree, so a + # build with no sibling beside it does not build at all. + locked = release.runtime_lock(self.pin) + self.assertEqual(locked['repository'], 'OctoSense-org/Octoscript-Makepad') + self.assertRegex(locked['revision'], r'^[0-9a-f]{40}$') + self.assertEqual(self.pin['runtime']['sibling'], 'octoscript-makepad') + + def test_the_workflow_builds_exactly_the_platforms_the_record_publishes(self): + # Both directions matter. A platform the record publishes but nothing + # builds is a download URL that 404s; a platform the workflow builds but + # the record does not name is an asset nobody can pin. + self.assertEqual(workflow_targets(), + {platform['target'] for platform in release.published(self.pin)}) + + +class RecordedReleaseAsACLI(unittest.TestCase): + def build_plan(self, *args): + return subprocess.run([sys.executable, str(HERE / 'splash_server_release.py'), *args], + capture_output=True, text=True) + + def test_printing_the_record_shows_the_url_a_downloader_uses(self): + printed = json.loads(self.build_plan().stdout) + pin = release.read_pin() + self.assertEqual(printed['tag'], pin['server']['release']['tag']) + self.assertEqual(printed['assets'], + [release.download(pin, platform['platform']) + for platform in release.published(pin)]) + + def test_a_build_plan_names_the_asset_and_the_source_to_build_it_from(self): + plan = dict(line.split('=', 1) for line in + self.build_plan('--build-plan', 'x86_64-pc-windows-msvc').stdout.splitlines()) + pin = release.read_pin() + locked = release.runtime_lock(pin) + self.assertEqual(plan['asset'], + release.asset_name(pin['server']['name'], pin['server']['version'], + 'x86_64-pc-windows-msvc')) + self.assertEqual(plan['source-revision'], pin['server']['source']['revision']) + self.assertEqual(plan['source-features'], 'maps') + self.assertEqual(plan['runtime-revision'], locked['revision']) + self.assertEqual(plan['runtime-sibling'], 'octoscript-makepad') + + def test_a_build_plan_refuses_a_target_nothing_was_published_for(self): + done = self.build_plan('--build-plan', 'aarch64-apple-darwin') + self.assertEqual(done.returncode, 1) + self.assertIn('no published asset', done.stderr) + + +class ReleaseNotes(unittest.TestCase): + """The release page the publish job writes, which is where a digest belongs.""" + + def setUp(self): + self.pin = release.read_pin() + self.assets = Path(tempfile.mkdtemp()) / 'assets' + self.assets.mkdir() + for entry in release.published(self.pin): + (self.assets / entry['asset']).write_bytes(b'a stand-in for the built binary') + + def notes(self): + return release.release_notes(self.pin, self.assets) + + def test_the_notes_name_the_uploaded_asset_and_its_digest(self): + # The digest has to be computed from the bytes, not asserted equal to a + # constant, or the note would describe a build that never happened. + expected = hashlib.sha256(b'a stand-in for the built binary').hexdigest() + notes = self.notes() + for entry in release.published(self.pin): + self.assertIn(entry['asset'], notes) + self.assertIn(expected, notes) + + def test_the_notes_name_the_revision_the_binary_was_built_from(self): + revision = self.pin['server']['source']['revision'] + self.assertIn(revision[:12], self.notes()) + self.assertIn(revision, self.notes()) + + def test_the_notes_say_which_platforms_have_no_asset(self): + notes = self.notes() + for entry in release.unpublished(self.pin): + self.assertIn(entry['platform'], notes) + self.assertIn('no asset yet', notes) + + def test_a_missing_uploaded_asset_is_an_error_not_a_quiet_gap(self): + for entry in release.published(self.pin): + (self.assets / entry['asset']).unlink() + with self.assertRaises(FileNotFoundError): + self.notes() diff --git a/tools/test_splashcheck_golden.py b/tools/test_splashcheck_golden.py new file mode 100644 index 00000000..e9d07d30 --- /dev/null +++ b/tools/test_splashcheck_golden.py @@ -0,0 +1,302 @@ +"""tools/splashcheck_golden.py: the corpus runner's behavior, with no checker needed. + +The runner's seam is `run_checker`: one call, one bundle, one JSON report. The +tests below replace it with a canned checker and a synthetic corpus, so what is +tested is the runner's own contract - record, compare, diff, and say so when the +workspace is not prepared - and not the checker, which has its own tests beside +it. The real corpus is checked at the end, when this workspace has it. +""" +import contextlib +import importlib.machinery +import importlib.util +import io +import json +import os +import shutil +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + +HERE = Path(__file__).resolve().parent +_loader = importlib.machinery.SourceFileLoader("splashcheck_golden", str(HERE / "splashcheck_golden.py")) +_spec = importlib.util.spec_from_loader(_loader.name, _loader) +splashcheck_golden = importlib.util.module_from_spec(_spec) +_loader.exec_module(splashcheck_golden) + +# One finding, as the checker's JSON carries it. +FINDING = { + "severity": "error", + "code": "splash.eval", + "line": 3, + "column": 17, + "message": "variable nope not found in scope. Did you mean: width(obj653)", +} + + +class FakeChecker: + """A stand-in for the binary: canned reports, one recipe per app name.""" + + def __init__(self, recipes): + self.recipes = recipes + self.calls = [] + + def __call__(self, checker, bundle): + self.calls.append(bundle) + recipe = self.recipes.get(bundle.parent.name, {}) + return { + "tool": "splashcheck", + "version": recipe.get("version", "2.0.0"), + "files": [{ + "path": str(bundle / "main.splash"), + "bytes": recipe.get("bytes", 32), + "manifest_found": True, + "declares_net": recipe.get("declares_net", False), + "capabilities": list(recipe.get("capabilities", ("storage",))), + "findings": [dict(finding) for finding in recipe.get("findings", ())], + "notes": list(recipe.get("notes", ())), + "fatal": recipe.get("fatal"), + }], + } + + +class GoldenRunnerTests(unittest.TestCase): + def setUp(self): + self.root = Path(tempfile.mkdtemp(prefix="splashcheck-golden-")) + self.addCleanup(shutil.rmtree, self.root, ignore_errors=True) + self.golden = self.root / "golden" + # Only the runner looks at this path; the fake checker stands in for it. + self.checker = self.root / "splashcheck.exe" + self.checker.write_bytes(b"") + + def bundle(self, name, source="// a synthetic app\n", capabilities=("storage",)): + path = self.root / "OctoSense" / "apps" / name / "bundle" + path.mkdir(parents=True, exist_ok=True) + (path / "main.splash").write_text(source, encoding="utf-8") + (path / "manifest.json").write_text( + json.dumps({"schema": 1, "id": f"os.{name}", "capabilities": list(capabilities)}), + encoding="utf-8", + ) + return path + + def run_tool(self, *argv): + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + status = splashcheck_golden.main([str(arg) for arg in argv]) + return status, out.getvalue(), err.getvalue() + + def argv(self, *extra, only=("news",)): + argv = [ + "--corpus-root", str(self.root), + "--checker", str(self.checker), + "--golden-dir", str(self.golden), + ] + for name in only: + argv += ["--only", name] + return tuple(argv) + extra + + def test_a_recorded_corpus_compares_clean(self): + self.bundle("news") + self.bundle("mail") + recorder = FakeChecker({"mail": {"findings": [FINDING]}}) + with patch.object(splashcheck_golden, "run_checker", recorder): + status, out, err = self.run_tool(*self.argv("--update", only=("news", "mail"))) + self.assertEqual(status, 0, err) + self.assertIn("wrote 2 golden file(s)", out) + self.assertIn("rec mail - 1 finding(s)", out) + # One invocation per bundle, in the corpus's own order. + self.assertEqual([path.parent.name for path in recorder.calls], ["mail", "news"]) + with patch.object(splashcheck_golden, "run_checker", FakeChecker({"mail": {"findings": [FINDING]}})): + status, out, err = self.run_tool(*self.argv(only=("news", "mail"))) + self.assertEqual(status, 0, out) + self.assertIn("ok news - 0 finding(s)", out) + self.assertIn("ok mail - 1 finding(s)", out) + self.assertIn("2 ok, 0 changed", out) + + def test_a_new_finding_is_a_readable_diff(self): + self.bundle("news") + with patch.object(splashcheck_golden, "run_checker", FakeChecker({})): + status, _, err = self.run_tool(*self.argv("--update")) + self.assertEqual(status, 0, err) + with patch.object(splashcheck_golden, "run_checker", FakeChecker({"news": {"findings": [FINDING]}})): + status, out, err = self.run_tool(*self.argv()) + self.assertEqual(status, 1, out) + self.assertIn("DIFF news - 1 finding(s)", out) + self.assertIn("--- golden/news.txt", out) + self.assertIn("+++ current", out) + self.assertIn("-findings 0", out) + self.assertIn("+findings 1", out) + self.assertIn("+ 3:17 error splash.eval variable nope not found in scope", out) + self.assertIn("0 ok, 1 changed", out) + + def test_a_changed_app_says_so_on_the_digest_line(self): + self.bundle("news", source="// the first shape\n") + with patch.object(splashcheck_golden, "run_checker", FakeChecker({})): + self.assertEqual(self.run_tool(*self.argv("--update"))[0], 0) + # The app moved, the checker did not: the digest line is the difference. + self.bundle("news", source="// the second shape\n") + with patch.object(splashcheck_golden, "run_checker", FakeChecker({})): + status, out, _ = self.run_tool(*self.argv()) + self.assertEqual(status, 1, out) + self.assertIn("-digest ", out) + self.assertIn("+digest ", out) + self.assertNotIn("-findings", out) + + def test_findings_are_ordered_not_listed(self): + self.bundle("news") + second = dict(FINDING, line=9, column=4, code="splash.capability", severity="warn", + message="1 host.request call(s) pass a computed service name") + with patch.object(splashcheck_golden, "run_checker", FakeChecker({"news": {"findings": [FINDING, second]}})): + self.assertEqual(self.run_tool(*self.argv("--update"))[0], 0) + with patch.object(splashcheck_golden, "run_checker", FakeChecker({"news": {"findings": [second, FINDING]}})): + status, out, _ = self.run_tool(*self.argv()) + self.assertEqual(status, 0, out) + + def test_the_vm_object_counters_are_not_part_of_a_golden(self): + self.bundle("news") + with patch.object(splashcheck_golden, "run_checker", FakeChecker({"news": {"findings": [FINDING]}})): + self.assertEqual(self.run_tool(*self.argv("--update"))[0], 0) + moved = dict(FINDING, message=FINDING["message"].replace("obj653", "obj654")) + with patch.object(splashcheck_golden, "run_checker", FakeChecker({"news": {"findings": [moved]}})): + status, out, _ = self.run_tool(*self.argv()) + self.assertEqual(status, 0, out) + golden = (self.golden / "news.txt").read_text(encoding="utf-8") + self.assertIn("width(obj)", golden) + self.assertNotIn("obj653", golden) + + def test_a_workspace_without_the_checker_says_so(self): + self.bundle("news") + missing = self.root / "no-such-splashcheck" + status, out, err = self.run_tool(*self.argv("--checker", str(missing))) + self.assertEqual(status, 2, out) + self.assertIn(f"no splashcheck binary at {missing}", err) + + def test_a_checker_that_will_not_run_is_a_setup_problem(self): + # A file that is there but not a runnable program is not a checker, and + # saying so beats the crash subprocess would raise instead. Windows + # `os.access(X_OK)` only looks at the extension, so there the refusal + # arrives one step later, when the run itself fails; either way it is a + # named setup problem and exit 2, never a traceback. + self.bundle("news") + blocked = self.root / "splashcheck.exe" + blocked.write_bytes(b"") + os.chmod(blocked, 0o444) + status, out, err = self.run_tool(*self.argv("--checker", str(blocked))) + self.assertEqual(status, 2, out) + self.assertIn(str(blocked), err) + self.assertRegex(err, r"no splashcheck binary at|will not run") + + def test_a_workspace_with_no_checker_anywhere_says_where_it_looked(self): + self.bundle("news") + with patch.dict(os.environ, {"SPLASHCHECK": ""}), patch.object(shutil, "which", return_value=None): + status, out, err = self.run_tool( + "--corpus-root", str(self.root), + "--golden-dir", str(self.golden), + "--only", "news", + ) + self.assertEqual(status, 2, out) + self.assertIn("no splashcheck binary. Searched:", err) + # Every place it looked, then the two ways out: the variable and the build. + self.assertIn(str(self.root / "splash-probe" / "target" / "release" / "splashcheck.exe"), err) + self.assertIn(str(self.root / "makepad" / "target" / "debug" / "splashcheck.exe"), err) + self.assertIn("PATH (splashcheck)", err) + self.assertIn("$SPLASHCHECK", err) + self.assertIn("--features maps", err) + + def test_a_workspace_without_the_app_says_so(self): + status, out, err = self.run_tool(*self.argv()) + self.assertEqual(status, 2, out) + self.assertIn("no corpus app 'news'", err) + self.assertIn("OCTOSENSE_WORKSPACE", err) + + def test_a_missing_golden_file_is_a_setup_problem(self): + self.bundle("news") + self.bundle("mail") + with patch.object(splashcheck_golden, "run_checker", FakeChecker({})): + status, _, err = self.run_tool(*self.argv("--update", only=("news",))) + self.assertEqual(status, 0, err) + status, out, err = self.run_tool(*self.argv(only=("mail",))) + self.assertEqual(status, 2, out) + self.assertIn("no golden file for 'mail'", err) + self.assertIn("--update", err) + + def test_an_unknown_app_name_is_a_setup_problem(self): + status, out, err = self.run_tool(*self.argv(only=("wasmlab",))) + self.assertEqual(status, 2, out) + self.assertIn("no corpus app 'wasmlab'", err) + self.assertIn("fn-blindspot", err) + + +class CorpusTests(unittest.TestCase): + """What the corpus is, as committed: the twelve apps, the fixture, the goldens.""" + + def test_the_corpus_is_the_twelve_real_apps_and_the_fixture(self): + entries = splashcheck_golden.corpus(Path("/nowhere")) + self.assertEqual([name for name, _ in entries], self.names()) + self.assertEqual(len([name for name, _ in entries if name != "fn-blindspot"]), 12) + # The eleven system apps sit in the OctoSense checkout, AppCard beside it. + paths = dict(entries) + self.assertEqual(paths["news"], Path("/nowhere/OctoSense/apps/news/bundle")) + self.assertEqual(paths["appcard"], Path("/nowhere/OctoSense-AppCard/bundle")) + # The fixture ships here, so it is checked wherever the checker is. + self.assertEqual(paths["fn-blindspot"], splashcheck_golden.CORPUS / "fn-blindspot") + + def names(self): + """The roster, read from the module: one list, not one per file.""" + return [name for name, _ in splashcheck_golden.corpus(Path("/nowhere"))] + + def test_every_corpus_entry_has_a_golden_file(self): + golden = splashcheck_golden.GOLDEN + self.assertEqual( + sorted(path.name for path in golden.glob("*.txt")), + [f"{name}.txt" for name in self.names()], + ) + for name in self.names(): + text = (golden / f"{name}.txt").read_text(encoding="utf-8") + self.assertTrue(text.startswith("# splashcheck_golden golden corpus"), name) + self.assertIn("# checker: splashcheck ", text, name) + self.assertIn("digest ", text, name) + + def test_the_fn_blind_spot_is_the_fixture_the_ticket_names(self): + source = (splashcheck_golden.CORPUS / "fn-blindspot" / "main.splash").read_text(encoding="utf-8") + self.assertEqual(source.strip(), "fn broken(){ let x = nope }") + manifest = json.loads((splashcheck_golden.CORPUS / "fn-blindspot" / "manifest.json").read_text(encoding="utf-8")) + self.assertEqual(manifest["id"], "dev.example.fnblindspot") + + def test_the_fn_blind_spot_currently_reports_nothing(self): + """The expectation the analysis-depth tickets flip, made executable. + + The checker evaluates top-level code only, so a fault inside a `fn` + nothing calls at the top level is not reported yet. When it starts + evaluating every `fn`, this golden gains a finding and this test is + updated with it: the delivery is measured here, not asserted in prose. + """ + golden = (splashcheck_golden.GOLDEN / "fn-blindspot.txt").read_text(encoding="utf-8") + self.assertIn("app dev.example.fnblindspot", golden) + self.assertIn("findings 0", golden) + self.assertIn("notes 0", golden) + + +class RealCorpusTests(unittest.TestCase): + """The corpus itself, when this workspace has the checker and the twelve apps.""" + + def test_the_real_corpus_compares_clean(self): + root = splashcheck_golden.workspace_root() + checker, _ = splashcheck_golden.find_checker(None, root) + prepared = checker is not None and all( + path.is_dir() for _, path in splashcheck_golden.corpus(root) + ) + out, err = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(out), contextlib.redirect_stderr(err): + status = splashcheck_golden.main([]) + if not prepared: + # A workspace without the corpus says so; it never passes quietly. + self.assertEqual(status, 2, out.getvalue()) + self.assertIn("splashcheck_golden:", err.getvalue()) + return + self.assertEqual(status, 0, out.getvalue()) + self.assertIn("13 ok, 0 changed", out.getvalue()) + + +if __name__ == "__main__": + unittest.main()