Merge pull request 'Extension: fix credential-push 405, guard the publish path, add a unit suite' (#234 ) from dev into main

Merge pull request 'feat(agent): idle-unload GPU models to free VRAM when the queue is idle' (#233 ) from dev into main
Merge pull request 'DRY + stability pass — extension, ML/settings backend, frontend cards (#161 )' (#232 ) from dev into main
2026-08-03 08:29:09 -04:00 · 2026-07-17 13:07:15 -04:00 · 2026-07-13 23:08:18 -04:00 · 2026-07-13 18:32:31 -04:00 · 2026-07-13 17:56:32 -04:00 · 2026-07-13 16:36:26 -04:00
7 changed files with 63 additions and 291 deletions
@@ -40,11 +40,6 @@ jobs:
      image: git.fabledsword.com/bvandeusen/ci-python:3.14
    steps:
      - uses: actions/checkout@v4
        with:
          # Full history: the shadow-mode step below derives a version from a
          # commit count, which a depth-1 clone cannot produce. Harmless for
          # everything else in this job.
          fetch-depth: 0
      - name: Resolve extension version
        id: extver
@@ -53,25 +48,6 @@ jobs:
          echo "version=$VERSION" >> "$GITHUB_OUTPUT"
          echo "Resolved extension version: $VERSION"
      # --- shadow mode (milestone #271, step 2) ---------------------------
      # Informational ONLY — nothing downstream reads this, and it must never
      # fail the build. This is THE place the derived formula gets validated:
      # `sign-extension` only runs on main, so main pushes are the sole source
      # of truth for whether the derived version moves exactly when the shipped
      # extension changes. Compare these lines across several main builds
      # before step 4 lets the derived value control publishing.
      - name: Shadow — derived version (informational)
        run: |
          set -u
          DERIVED=$(sh extension/scripts/packaging.sh version 2>&1 || echo "UNAVAILABLE")
          MANUAL=${{ steps.extver.outputs.version }}
          echo "shadow: manual=$MANUAL derived=$DERIVED sha=$GITHUB_SHA"
          if [ "$MANUAL" = "$DERIVED" ]; then
            echo "shadow: manual and derived agree"
          else
            echo "shadow: DIVERGENT — expected until step 4 cuts over; derived is authoritative-to-be"
          fi
      - name: Check Forgejo release-asset cache
        id: cache
        env:
@@ -77,17 +77,6 @@ jobs:
          test -n "$PKG" || { echo "ERROR: no version found in extension/package.json"; exit 1; }
          test -n "$MAN" || { echo "ERROR: no version found in extension/manifest.json"; exit 1; }
          # --- shadow mode (milestone #271, step 2) -------------------------
          # Informational ONLY: nothing below reads DERIVED, and this must never
          # fail the job. `web-ext sign` is one-shot per version (AMO 409s on a
          # repeat), so a wrong formula would burn a real version number that
          # can't be reclaimed. Logging it against real pushes first is the only
          # way to validate it at zero cost.
          # Placed before every early-exit path so it reports on all runs.
          DERIVED=$(sh extension/scripts/packaging.sh version 2>&1 || echo "UNAVAILABLE")
          echo "shadow: manual=$PKG derived=$DERIVED"
          # -----------------------------------------------------------------
          # (1) Unconditional: the two version strings must agree. `web-ext sign`
          # reads manifest.json (package.json sits in --ignore-files and isn't
          # even inside the XPI), so AMO signs MAN and Firefox installs MAN.
@@ -132,16 +121,20 @@ jobs:
            echo "OK: extension version $PKG"
            exit 0
          fi
-          # The exclusion list is NOT written out here — it comes from
+          # Exclusions mirror --ignore-files in extension/package.json's web-ext
-          # extension/scripts/packaging.sh, the one definition of what ships,
+          # scripts: these files are not packaged into the XPI, so touching them
-          # shared with web-ext's --ignore-files and the derived-version patch
+          # (e.g. Renovate bumping the web-ext devDep, or editing a spec)
-          # count. Three hand-kept copies of that fact is how #2397 happened.
+          # changes nothing shipped and must not demand a version bump.
-          #
+          # KEEP IN SYNC with --ignore-files — a file packaged into the XPI but
-          # `set -f` is required around the substitution: without it the shell
+          # excluded here is exactly the silent-stale-ship this job exists to
-          # globs `test/**` against the working tree and silently narrows it.
+          # prevent. test/version.spec.js pins the two lists' shared intent.
-          set -f
+          CHANGED=$(git diff --name-only "$BASE" HEAD -- extension/ \
-          CHANGED=$(git diff --name-only "$BASE" HEAD -- extension/ $(sh extension/scripts/packaging.sh pathspec))
+            ':(exclude)extension/package.json' \
-          set +f
+            ':(exclude)extension/package-lock.json' \
            ':(exclude)extension/README.md' \
            ':(exclude)extension/.gitignore' \
            ':(exclude)extension/vitest.config.js' \
            ':(exclude)extension/test/**')
          if [ -z "$CHANGED" ]; then
            echo "No packaged extension files changed since $BASE — nothing to guard."
            echo "OK: extension version $PKG"
@@ -38,45 +38,3 @@ jobs:
      # package version-consistency checks. No browser, no network.
      - name: Unit tests
        run: cd extension && npm run test:unit
      # Everything else about packaging is asserted against our own declaration
      # of what ships. This is the only check that asks web-ext what it ACTUALLY
      # put in the archive. Until now that was an unverified assumption about
      # glob semantics — and a fragile one: `test/**` reaches web-ext intact
      # only because callers `set -f` first, so losing that quoting would
      # silently start shipping dev files with no other signal.
      - name: Verify XPI contents
        run: |
          set -eu
          command -v unzip >/dev/null 2>&1 || { apt-get update -qq && apt-get install -y -qq unzip; }
          cd extension
          npm run build
          ZIP=$(ls web-ext-artifacts/*.zip | head -1)
          echo "=== packaged entries in $ZIP ==="
          unzip -Z1 "$ZIP" | sort
          echo "=== end ==="
          ENTRIES=$(unzip -Z1 "$ZIP")
          fail=0
          # Must NOT ship: repo infrastructure with no business in a user's browser.
          for pat in 'test/' 'scripts/' 'vitest.config.js' 'package.json' 'package-lock.json' 'README.md' 'node_modules/' 'web-ext-artifacts/'; do
            if echo "$ENTRIES" | grep -q "^$pat"; then
              echo "ERROR: '$pat' was packaged into the XPI but must not be"
              fail=1
            fi
          done
          # Must ship: if an exclusion pattern ever over-matches, the extension
          # breaks at runtime rather than at build time, so assert presence too.
          for req in 'manifest.json' 'lib/url.js' 'lib/api.js' 'lib/platforms.js' 'lib/cookies.js'; do
            if ! echo "$ENTRIES" | grep -q "^$req$"; then
              echo "ERROR: '$req' is missing from the XPI"
              fail=1
            fi
          done
          for dir in 'background/' 'popup/' 'options/' 'content/' 'icons/'; do
            if ! echo "$ENTRIES" | grep -q "^$dir"; then
              echo "ERROR: nothing from '$dir' was packaged"
              fail=1
            fi
          done
          [ "$fail" -eq 0 ] || exit 1
          echo "XPI contents verified."
@@ -27,10 +27,6 @@ per `docs/process.md`'s "add deps to the image when used by >1 project".
 - `pip install -r requirements.txt pytest pytest-asyncio` — in `backend-lint-and-test` and `integration` jobs
 - `npm install --no-audit --no-fund` — in `frontend-build` job
 - `npm install --no-audit --no-fund` — in `extension.yml`'s `lint` job (web-ext + vitest)
 - `unzip` — in `extension.yml`'s "Verify XPI contents" step, installed via apt
  only when absent (`node:24-bookworm-slim` may or may not carry it). Debian
  package, ~2s. Not worth baking into a shared image for a single consumer, per
  `docs/process.md`'s ">1 project" rule.
 ## Notes
@@ -53,17 +49,7 @@ per `docs/process.md`'s "add deps to the image when used by >1 project".
  classic script (`test/helpers/loadLib.js`) rather than adding `module.exports`
  shims to production code — the libs ship as `background.scripts`, not ES
  modules, so the specs exercise exactly the bytes packaged into the XPI.
- **`extension/scripts/packaging.sh` is the single definition of what ships
+- Extension test files are excluded from the XPI via `--ignore-files` in
-  inside the XPI.** Three consumers read from it rather than keeping their own
+  `extension/package.json`, and the same paths are excluded from `ci.yml`'s
-  copy: web-ext's `--ignore-files` (`extension/package.json`), the `:(exclude)`
+  `extension-version` guard. Those two lists must agree — `test/version.spec.js`
-  pathspec in `ci.yml`'s `extension-version` guard, and the commit count that
+  asserts the guard never ignores a file web-ext actually packages.
  derives the extension version. Three hand-kept copies of that one fact is
  what allowed issue #2397.
 - `build.yml`'s `sign-extension` checks out with `fetch-depth: 0` — the derived
  extension version is a commit count, which a shallow clone cannot produce.
 - Callers MUST `set -f` before substituting the script's output. Without it the
  shell expands `test/**` against the working tree and silently narrows the
  pattern to whatever files exist at that moment — a failure that looks like
  nothing until dev files start appearing in the XPI. `test/version.spec.js`
  asserts every `--ignore-files` consumer sets it, and that no consumer has
  quietly reinstated a hardcoded list.
@@ -3,12 +3,11 @@
  "version": "1.0.10",
  "private": true,
  "description": "Firefox extension for FabledCurator",
  "comment_ignore_files": "The --ignore-files list comes from scripts/packaging.sh, the single source of truth shared with ci.yml's guard and the derived-version patch count. `set -f` is REQUIRED before the substitution: without it the shell globs `test/**` against the working tree and silently narrows the pattern to whatever files happen to exist.",
  "scripts": {
-    "lint":  "set -f; web-ext lint --source-dir=. --no-config-discovery --ignore-files $(sh scripts/packaging.sh ignore)",
+    "lint":  "web-ext lint --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\"",
-    "start": "set -f; web-ext run --source-dir=. --no-config-discovery --ignore-files $(sh scripts/packaging.sh ignore) --firefox=firefox",
+    "start": "web-ext run --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\" --firefox=firefox",
-    "build": "set -f; web-ext build --source-dir=. --no-config-discovery --ignore-files $(sh scripts/packaging.sh ignore) --overwrite-dest",
+    "build": "web-ext build --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\" --overwrite-dest",
-    "sign":  "set -f; web-ext sign --source-dir=. --no-config-discovery --ignore-files $(sh scripts/packaging.sh ignore) --channel=unlisted --api-key=$WEB_EXT_API_KEY --api-secret=$WEB_EXT_API_SECRET",
+    "sign":  "web-ext sign --source-dir=. --no-config-discovery --ignore-files package.json package-lock.json web-ext-artifacts node_modules README.md .gitignore vitest.config.js \"test/**\" --channel=unlisted --api-key=$WEB_EXT_API_KEY --api-secret=$WEB_EXT_API_SECRET",
    "test:unit": "vitest run"
  },
  "devDependencies": {
@@ -1,99 +0,0 @@
 #!/bin/sh
 # Single source of truth for "what ships inside the XPI", plus the version
 # derived from it.
 #
 # Three consumers used to hand-maintain their own copy of this list, and
 # keeping three copies of one fact in sync by hand is how issue #2397 happened:
 #
 #   1. web-ext's --ignore-files      (extension/package.json's four scripts)
 #   2. the :(exclude) pathspec       (ci.yml's extension-version guard)
 #   3. the rev-list pathspec         (the derived version, below)
 #
 # They now all read from here. POSIX sh only — CI's run shell is busybox.
 #
 # -f (no pathname expansion) is set for the whole script and is load-bearing:
 # the lists below are iterated with deliberate word-splitting, and without -f
 # the shell would also GLOB them, expanding `test/**` into whatever files
 # happen to exist and corrupting the output. A caller's own `set -f` does not
 # help here — this runs as a separate sh process and does not inherit it.
 # Callers still need their own `set -f` for the substituted result; the two
 # guards protect different expansions.
 set -euf
 # Paths under extension/ that are NOT packaged into the XPI.
 #
 # Split by whether git tracks them: node_modules and web-ext-artifacts are
 # build/dependency output that never appears in a commit, so they belong in
 # web-ext's ignore list but would be meaningless in a git pathspec.
 #
 # Directories need BOTH forms. `test/**` matches the files inside, but not the
 # directory entry itself — web-ext writes an entry for the directory too, so
 # with only the glob the XPI ends up carrying empty `test/` and `scripts/`
 # entries (caught by the XPI-content check on 2026-08-03). The bare name alone
 # is not enough either: minimatch's `test` does not match `test/url.spec.js`,
 # so dropping the glob would ship the contents. Keep both.
 NOT_PACKAGED_TRACKED='package.json package-lock.json README.md .gitignore vitest.config.js scripts scripts/** test test/**'
 NOT_PACKAGED_BUILD='web-ext-artifacts node_modules'
 usage() {
  echo "usage: packaging.sh {ignore|pathspec|version|major-minor|patch}" >&2
  exit 2
 }
 # web-ext --ignore-files values, space-separated.
 #
 # Callers MUST disable pathname expansion first (`set -f`), or the shell will
 # glob `test/**` against the working tree before web-ext ever sees the pattern
 # and silently narrow it to whatever happens to exist right now.
 cmd_ignore() {
  echo "$NOT_PACKAGED_TRACKED $NOT_PACKAGED_BUILD"
 }
 # git pathspec excluding the non-packaged tracked files, e.g.
 #   :(exclude)extension/package.json :(exclude)extension/test/**
 # Same `set -f` requirement as above.
 cmd_pathspec() {
  for entry in $NOT_PACKAGED_TRACKED; do
    printf ':(exclude)extension/%s ' "$entry"
  done
  echo
 }
 # MAJOR.MINOR stays hand-set in manifest.json — it's the part that carries
 # deliberate meaning. Only the patch component is derived.
 cmd_major_minor() {
  root=$(git rev-parse --show-toplevel)
  grep -E '"version"' "$root/extension/manifest.json" \
    | head -1 \
    | sed -E 's/.*"version"[[:space:]]*:[[:space:]]*"([0-9]+)\.([0-9]+).*/\1.\2/'
 }
 # Count of commits that touched a PACKAGED extension file. Monotonic on a
 # branch (the count only grows), which is a correctness requirement, not a
 # nicety: Firefox refuses to install a version lower than the one present.
 #
 # Merge commits need no special handling — git's history simplification already
 # prunes merges that don't change the pathspec, so --no-merges is a no-op here
 # (verified on main: both forms return the same count).
 cmd_patch() {
  root=$(git rev-parse --show-toplevel)
  # Unquoted on purpose: the pathspec must word-split into separate args.
  # Globbing is already off script-wide (set -euf above).
  # shellcheck disable=SC2046
  count=$(cd "$root" && git rev-list --count HEAD -- extension/ $(cmd_pathspec))
  echo "$count"
 }
 cmd_version() {
  echo "$(cmd_major_minor).$(cmd_patch)"
 }
 [ $# -ge 1 ] || usage
 case "$1" in
  ignore)      cmd_ignore ;;
  pathspec)    cmd_pathspec ;;
  version)     cmd_version ;;
  major-minor) cmd_major_minor ;;
  patch)       cmd_patch ;;
  *)           usage ;;
 esac
@@ -1,97 +1,32 @@
 import { describe, it, expect } from 'vitest'
 import { readFileSync } from 'node:fs'
 import { execFileSync } from 'node:child_process'
 import { fileURLToPath } from 'node:url'
 import path from 'node:path'
 const EXT_DIR = path.join(path.dirname(fileURLToPath(import.meta.url)), '..')
 const read = (name) => JSON.parse(readFileSync(path.join(EXT_DIR, name), 'utf8'))
 const readText = (...seg) => readFileSync(path.join(EXT_DIR, ...seg), 'utf8')
-// Only the git-free subcommands are exercised here: `version`/`patch` shell out
+describe('extension version consistency', () => {
-// to git, and the extension lane runs on node:24-bookworm-slim which may not
+  // Duplicates check (1) of ci.yml's extension-version job, deliberately.
-// ship it. Those two are covered where git is guaranteed — ci.yml and build.yml
+  // That job is the gate that can't be bypassed; this spec is the one that
-// run on ci-python.
+  // fails in a second on the developer's own CI lane with a readable diff.
-const packaging = (cmd) =>
+  // The two version strings feed different systems and nothing else reconciles
-  execFileSync('sh', [path.join(EXT_DIR, 'scripts', 'packaging.sh'), cmd], {
+  // them:
-    cwd: EXT_DIR,
+  //   manifest.json  -> what `web-ext sign` signs, so what Firefox installs
-    encoding: 'utf8'
+  //                     (package.json is in --ignore-files, not in the XPI)
-  })
+  //   package.json   -> build.yml's AMO cache key, the ext-<version> release
-    .trim()
+  //                     tag, the XPI filename, and therefore the version
-    .split(/\s+/)
+  //                     /api/extension/manifest reports to the update prompt
    .filter(Boolean)
 describe('packaging.sh — the single definition of what ships', () => {
  it('emits an ignore list and a pathspec that agree on the tracked files', () => {
    const ignore = packaging('ignore')
    const pathspec = packaging('pathspec').map((e) => e.replace(':(exclude)extension/', ''))
    // Every git-excluded path must also be hidden from web-ext. The reverse is
    // not required: node_modules and web-ext-artifacts are build output git
    // never tracks, so they appear only in the ignore list.
    for (const entry of pathspec) {
      expect(ignore, `pathspec has "${entry}" but --ignore-files does not`).toContain(entry)
    }
    expect(pathspec.length).toBeGreaterThan(0)
    expect(ignore).toContain('node_modules')
  })
  it('emits glob patterns literally, never expanded against the working tree', () => {
    // The script iterates its lists with deliberate word-splitting, so it must
    // run with pathname expansion off. Without that, invoking it from a cwd
    // where test/ exists (exactly how ci.yml and vitest call it) expands
    // `test/**` into the individual spec files, and the pathspec silently stops
    // covering anything added later.
    const pathspec = packaging('pathspec')
    expect(pathspec).toContain(':(exclude)extension/test/**')
    expect(pathspec).toContain(':(exclude)extension/scripts/**')
    expect(pathspec.some((e) => e.includes('.spec.js'))).toBe(false)
    expect(pathspec.some((e) => e.includes('helpers'))).toBe(false)
  })
  it('keeps its own scripts and specs out of the XPI', () => {
    // Both are repo infrastructure. web-ext packages everything not ignored, so
    // omitting either would ship dev tooling to users -- and `test/**` in
    // particular only survives because callers `set -f` before substituting it.
    const ignore = packaging('ignore')
    expect(ignore).toContain('vitest.config.js')
    // Both forms per directory. The glob covers the contents; the bare name
    // covers the directory ENTRY, which web-ext writes separately — with only
    // the glob, the XPI carries an empty `test/` and `scripts/`.
    for (const dir of ['test', 'scripts']) {
      expect(ignore, `${dir} contents`).toContain(`${dir}/**`)
      expect(ignore, `${dir} directory entry`).toContain(dir)
    }
  })
 })
 describe('consumers delegate rather than keeping their own copy', () => {
  // These assertions are the actual anti-regression value: it is easy for a
  // future edit to "simplify" by inlining a literal list again, which silently
  // reintroduces the drift that issue #2397 was about.
  it('package.json derives --ignore-files from the script', () => {
    for (const [name, script] of Object.entries(read('package.json').scripts)) {
      if (!script.includes('--ignore-files')) continue
      expect(script, `${name} should call packaging.sh`).toContain('scripts/packaging.sh ignore')
      expect(script, `${name} must set -f before the substitution`).toMatch(/set -f;/)
    }
  })
  it('ci.yml derives its pathspec from the script and hardcodes none', () => {
    const ci = readText('..', '.forgejo', 'workflows', 'ci.yml')
    expect(ci).toContain('extension/scripts/packaging.sh pathspec')
    // A literal :(exclude)extension/... in the workflow means someone bypassed
    // the shared definition.
    expect(ci).not.toMatch(/:\(exclude\)extension\//)
  })
 })
 describe('extension version', () => {
  it('keeps manifest.json and package.json in lockstep', () => {
-    expect(read('manifest.json').version).toBe(read('package.json').version)
+    const manifest = read('manifest.json')
    const pkg = read('package.json')
    expect(manifest.version).toBe(pkg.version)
  })
  it('uses a plain dotted numeric version AMO will accept', () => {
    // AMO rejects exotic version strings, and build.yml embeds this value in a
    // release tag and a filename — so anything needing escaping breaks the
    // publish path rather than the extension.
    expect(read('package.json').version).toMatch(/^\d+(\.\d+)*$/)
  })
@@ -99,6 +34,30 @@ describe('extension version', () => {
    expect(read('manifest.json').manifest_version).toBe(3)
  })
  it('never lets the CI guard ignore a file that actually ships', () => {
    // ci.yml's extension-version job skips its bump check for paths it deems
    // non-shipping. If it excludes something web-ext DOES package, a real
    // change to shipped code passes the guard unnoticed — precisely the
    // silent-stale-ship the guard exists to stop. The reverse drift (guard
    // stricter than web-ext) only costs a needless bump, so it isn't asserted.
    const ci = readFileSync(path.join(EXT_DIR, '..', '.forgejo', 'workflows', 'ci.yml'), 'utf8')
    const lint = read('package.json').scripts.lint
    const after = lint.split('--ignore-files')[1] ?? ''
    const ignored = new Set(
      after
        .split(/\s+/)
        .filter((tok) => tok && !tok.startsWith('--'))
        .map((tok) => tok.replace(/^["']|["']$/g, ''))
    )
    expect(ignored.size, 'parsed --ignore-files from the lint script').toBeGreaterThan(0)
    const guarded = [...ci.matchAll(/:\(exclude\)extension\/(\S+?)'/g)].map((m) => m[1])
    expect(guarded.length, 'parsed :(exclude) entries from ci.yml').toBeGreaterThan(0)
    for (const entry of guarded) {
      expect(ignored, `ci.yml excludes "${entry}" but web-ext packages it`).toContain(entry)
    }
  })
  it('lists every background script that exists, in dependency order', () => {
    // url.js must load BEFORE api.js: api.js calls normalizeApiUrl at
    // init()-time, and these are classic scripts sharing one scope, so a