5 Commits

Author SHA1 Message Date
xarmian a1716d8170 ci(web): decide the npm audit gate from the report, not the exit code, and run it last (BUG-2881) (#1247)
* ci(web): decide the npm audit gate from the report, not the exit code, and run it last (BUG-2881)

`npm audit` exits non-zero identically for "a HIGH/CRITICAL advisory
exists" and "the advisory service was unreachable". The Web job ran it
before Build / Type check / vitest under `bash -e`, so a registry
timeout (main, 03:50Z) and a 503 (#1246, 04:33Z) on 2026-09-04 each
produced a red row with every frontend verification step SKIPPED — a
lane that read like a failure and had asked nothing.

scripts/ci-audit.mjs runs the audit in --json mode and decides from the
report: metadata.vulnerabilities present → fail iff high+critical > 0,
naming the advisories; an error envelope or unparseable output → a
GitHub warning annotation saying the gate did not run, exit 0. The step
moves to the end of the job so the frontend's own verdict always exists
whatever the audit does.

Verified locally against five report shapes (transport timeout envelope,
E503 envelope, one high advisory, clean, garbage) and two live runs (the
real registry: clean; a dead registry: warning, exit 0). `--input <file>`
is the seam those checks use.

Fixes BUG-2881

* ci(web): the audit gate fails closed — retry an unreachable advisory service, then fail under its own title

Codex round 1 on #1247: the first draft warned and exited 0 when the
advisory service could not be asked, which made the only supply-chain
gate pass exactly when it had not run. A gate that passes when it cannot
run is not a gate.

Now: up to three attempts with backoff (registry blips are usually
seconds long), then `::error title=npm audit did not run` and exit 1.
The title is distinct from `::error title=npm audit` (a real advisory)
so the checks tab tells the two apart without opening the log; re-running
is the remedy for the first and never for the second. Because the step
runs last, Build / Type check / vitest have already produced their result
either way — the original blindness is gone regardless of which way this
step fails.

Verified against the same five saved shapes (transport and E503 envelopes
and garbage now exit 1 under the did-not-run title; a high advisory exits
1 under the advisory title; clean exits 0) and two live runs (real
registry: clean; dead registry: three attempts logged, exit 1).

Refs BUG-2881

* ci(web): the audit gate refuses counts it cannot read, and refuses bad tuning without crashing

Codex round 2 on #1247. (1) metadata.vulnerabilities was checked for
presence, not for shape: Number("x") + Number(null) > 0 is false, so a
malformed count read as a clean audit — a second fail-open, one layer
deeper than round 1's. high/critical must now be non-negative integers
or the report is unreadable, which is the fail-closed path. (2) The two
env knobs are operator-set, but CI_AUDIT_ATTEMPTS=NaN left the retry loop
unexecuted and threw a TypeError, and CI_AUDIT_BACKOFF_MS=Infinity parked
Atomics.wait forever; both now fall back to the default with a line
saying so.

Refs BUG-2881

* build: the local preflight runs the same audit gate CI does, and runs it last

Codex round 3 on #1247 (blast radius): `make web-check` still chained
bare `npm audit && npm run check`, so a registry blip stopped svelte-check
locally exactly as it had in CI, and CONTRIBUTING documented the bare
command as the way to reproduce the gate. New `web-audit` target runs
`npm run audit:ci`; `check` runs it after web-check and web-test, mirroring
the Web job's order. CONTRIBUTING and docs/architecture.md say so.

Refs BUG-2881

* build: web-audit stands alone — no `web` prerequisite, so `check` runs npm ci once and no new target reaches it

Codex round 4 on #1247: `web-audit: web` made `check` run `npm ci` twice
(`web` is .PHONY) and added a target CLAUDE.md's worktree rule did not
list as reaching `npm ci`. `npm audit` reads the lockfile and needs
neither node_modules nor a build — verified by running it with
node_modules removed — so the prerequisite goes; CLAUDE.md's safe list
gains `web-audit`.

Refs BUG-2881
2026-09-04 10:45:21 -04:00
xarmian 7c8ed3c815 fix: nine false or overstated claims in this diff's own prose (codex round 15)
An angle worth naming, because it found more than several code-shaped
ones did: check the COMMENTS against the CODE. This diff is
comment-heavy and its comments make specific factual claims. Nine were
wrong.

The one that mattered most was a false argument for a correct rule.
redisns.Parse rejects colons, and justified it with a collision example
that does not happen: ns "a:events" builds pad:a:events:events:<ws>, not
pad:a:events:<ws>, because the suffix is appended too. The rule stands on
its own grounds (a colon spans segments and makes the keyspace ambiguous
to read back) — but a false example is worse than none, because the next
reader trusts it.

Chasing that turned up a REAL collision needing no colon: a namespace
equal to one of Pad's own first segments nests this installation inside
the default one's keyspace. Namespace "events" puts every key under
pad:events:*, which is the default installation's activity channel space
— the exact cross-feed the namespace exists to prevent, arriving through
the namespace. Now rejected, with a control leg asserting that names
merely CONTAINING a reserved word ("events-eu", "prod-session") stay
valid.

The other eight:

- "The three keyspaces cannot drift" — overstated. Each constructor takes
  its own Keys; a source-reading test is what enforces it, which is
  weaker than a compiler and now says so.
- Two docs claimed both SSE endpoints incur a presence registration. Only
  the watch stream registers.
- The Redis metrics section said they "stay at zero" without Redis, while
  pad_redis_up is deliberately unregistered — the section contradicted
  the field three lines below it.
- The presence-failure metric's HELP string still carried the blanket
  "leaves sessions unlisted and untargetable" that the field comment had
  already been corrected away from. Two of the four ops fail in the
  opposite direction.
- A nil from MGET was described as proof the process died. Eviction, a
  restart and a manual DEL produce the same nil, and this file's own doc
  says eviction is indistinguishable from expiry.
- A test comment claimed to cover both corrupt-entry shapes; the second
  is unreachable and the subtest is deliberately absent, as the note ten
  lines down already said.
- "Enumerates every refusal path" covered per-instance and per-workspace
  and not per-user — the same undercount as round 13's, one round later.
  Both per-user paths added.
- The Observer contract said a go-redis drop is reported as a sequence
  gap. Only if a LATER notification arrives to expose the hole: drop the
  newest message on a bus that then goes quiet and nothing is reported.

Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X
2026-08-22 04:35:57 +00:00
xarmian 3e3170e915 fix(server,cli,docs): the consumer contract, per codex round 12 (BUG-2726)
An angle no earlier round took: what does a CLIENT see. Two of the five
findings were about consumers I had never opened.

- `pad project watch` returned "event stream returned 429: {json}" and
  exited, which sends the reader looking for a bug rather than at a
  limit. It now says what happened and which knobs govern it, and names
  the fact that those knobs cover this stream and the agent watch stream
  together. It still exits rather than backing off — it is interactive,
  and a human can decide — unlike the unattended monitor, which already
  folds 429 into its ladder.

- Both endpoints now answer a refusal through one helper: same status,
  same code, same message, plus `Retry-After`. `/api/v1/events` was
  setting `Content-Type: text/event-stream` BEFORE the admission check,
  so its 429 carried the JSON error envelope under an SSE content type —
  a different contract from its sibling's for the same refusal. Admission
  moved above the headers, which is where it belonged anyway.

- The anonymous-caller rule was documented as if it applied to both
  endpoints. It applies to `/api/v1/events` only; the watch stream
  requires a resolved user and answers 401 without one.

- docs/architecture.md described one SSE endpoint and one bus. It now has
  the table: two streams, two buses, different scopes and consumers, one
  shared connection budget, one Redis namespace.

FILED, not fixed: the web UI's `EventSource` cannot see a 429 or a
`Retry-After` — the spec exposes neither to the page — so a refused
browser tab reconnects at a constant rate while the CLI backs off. That
asymmetry means reaching the limit sheds load from the population that
respects it and not from the one that grows fastest under it. No
server-side change closes it; the fix is a client-side reconnect wrapper.
BUG-2733, and docs/deployment.md warns operators to size the limit with
it in mind.

Claude-Session: https://claude.ai/code/session_01JVDBKbgn3Xt7ndW1YoYd8X
2026-08-22 04:10:06 +00:00
xarmian afe721d202 feat(cli): add Cloud mode to pad init, drop Docker option (TASK-837, TASK-838) (#272)
Merging despite Go (PostgreSQL) red — those failures (TestListItems_FTS_HyphenatedSearchTerm/task-five + TestAdminBillingStats_SidecarSidecarError_DegradesToLocalOnly TempDir cleanup race) are pre-existing on main and tracked in BUG-842.

Codex reviewed in 3 rounds (round 1 clean → round 2 found a real semantic bug → fix → round 3 clean). Tests, vet, and lint all green; remaining check failures are documented pre-existing.
2026-04-28 09:41:50 -04:00
xarmian 062eef41b2 docs: architecture guide + full .env.example + gitattributes + Makefile note (TASK-687) (#222)
Grouped nice-to-haves called out in the pre-launch audit.

1. docs/architecture.md — new contributor-focused architecture doc.
   CLAUDE.md covers the same ground but is agent-oriented; this is the
   human companion. Covers backend layout, request flow, frontend /
   data model / CLI↔daemon model / agent integration / testing.

2. .env.example — extended to document every PAD_* variable in
   docs/deployment.md (core, database, real-time events, security,
   email). Existing Postgres/Redis + encryption secrets kept at the
   top; new variables grouped by concern with inline comments and
   safe defaults commented out.

3. .gitattributes — normalize LF line endings repo-wide, mark binary
   assets, and flag web/build + web/.svelte-kit as generated so they
   don't pollute GitHub linguist stats or PR diffs.

4. Makefile — CAUTION comment on `make install` noting that the
   `killall -9 pad` step is system-wide; anyone else's pad daemon on
   the same machine gets killed too. Designed for single-developer
   local setups; not for shared hosts.

Parent: PLAN-644.
2026-04-22 20:59:15 -04:00