Judge r2 (grade C) — all 3 blockers + both polish items:
1. REQUEST-vs-NOTIFICATION by PRESENCE, never truthiness: validateJsonRpc
uses hasOwnProperty('id'); id 0 and id '' are VALID request ids echoed
back BY VALUE (strict-equality tested), id:null treated as a request
under the documented error-id convention. mcp-core default branch fixed
the same way (`if (id)` → hasOwnProperty — id 0 / '' unknown-method now
answer -32601 with the id echoed instead of being swallowed as falsy).
Routes' malformed-notification silence path also switched to
presence-based detection (id:null + bad params → 400 envelope, not 204).
2. params:null REJECTED (-32600): contract is object-or-omitted ONLY
(strict JSON-RPC; the old code's m.params !== null guard contradicted
the documented contract). Boundary tests pin the policy.
3. Unknown-method boundary tests: id 0 → -32601 id:0, id '' → -32601 id:'',
id null → -32601 id:null — asserted at BOTH layer levels (route).
Polish:
- 16-case TABLE-DRIVEN boundary matrix covering every allowed/rejected
id/params combination incl. 202 vs 204 notification split.
- Verifier now probes writable /tmp up front (exit 73 + clear message if
unavailable; documented requirement) instead of dying at mktemp.
- Verifier target count 57 → 73 (DC-162 suites) with the boundary table
named in the assertion label.
Full DC-162 suite: 73 passed (route 50 + core 23). No transport/credential
semantics changed from rev2.
urn:ump:or2qqhcytp6lfzlyi7xru7hdqy3uar2hbjllsrrbxrbray3c5kpa
All three r3 blocking issues fixed + polish applied:
- Radicale provision route now FAILS CLOSED (409) unless the target container
is present, inspectable, and its image matches /radicale/i. Missing
containerId, failed inspect, or non-Radicale image → nothing is written to
the shared config and no source/credential is registered. 4 new tests cover
the three refusal paths + the verified-image happy path.
- ICS feed fetch now streams with a hard 5 MiB body cap (same bound as the
CalDAV path) via readBodyCapped(); oversized feeds reject mid-stream and
degrade to per-source error. Test covers a >5 MiB feed end-to-end.
- Verifier mode separation: `--verify` runs the same checks fully read-only
(all output under mktemp -d in /tmp, transcript printed to stdout, scratch
removed on exit — proven rc=0 with zero repo/tmp-side writes); default
transcript mode is explicitly declared a TRANSCRIPT GENERATOR in its header
and writes only under the script's own directory.
- Full-suite count assertions replaced with the documented invariant
(failed ∈ [5,7], failing set == known-flaky billing suite, extras pass
isolated rerun) — judge r3 polish.
- Stale credential-key comment in radicale-provisioner.js header corrected.
Calendar suites: 43 passed (38 + 5 new). Perf note: readBodyCapped streams;
no full-buffer read of unbounded bodies on the ICS path anymore.
All three r2 blocking issues fixed + polish applied:
- useExisting (adopted) Radicale containers no longer get a source registered
with credentials the operator never provisioned. Registration happens ONLY
when pre-start provisioning succeeded; adopted deployments get a
calendarHint in the deploy response pointing at the manual-wiring modal.
- Weekly BYDAY expansion: candidates earlier than DTSTART are skipped (they
do not consume COUNT); window inclusion now uses interval-overlap
semantics (occurrence span vs window span) for both recurring and
non-recurring events. Three new tests cover midweek DTSTART, a straddling
multi-day event, and a boundary-exactly-ending event.
- Verifier is now read-only on the graded tree: no stash/checkout/rewrite.
Base proof runs in a temporary git worktree of 448f535 under /tmp (removed
on exit); the flaky pdf-parse suite is proven pre-existing via "red at
base" + "patch touches no billing file" (empty diff), and a race-flaky
suite (DC-116, witnessed once under parallel load) is given one isolated
rerun which must pass.
Polish: PUT /sources normalizes the user field (and clamps name) like POST.
Calendar suites: 38 passed (35 + 3 new recurrence tests). The 3558/3564
full-suite numbers vary by one flaky pdf-parse member between runs; the
evidence now asserts the invariant (suite red at base, no billing file in
the patch, flaky suites green on isolated rerun) instead of an exact count.
All five r1 blocking issues fixed + polish items applied:
- Unified credential-key convention: registerAutoWiredSource now stores at
calendar:source:<source.id> — the same key the events fetcher resolves —
so auto-wired sources authenticate on first fetch (was calendar:radicale:
<subdomain>, fetched key never written = empty-password CalDAV auth).
- calendarStateManager is now passed INTO assembleContext(...) (the r1 patch
only extended the function signature, so ctx.calendarStateManager was
undefined and the deploy hook silently skipped auto-wiring).
- Radicale config is provisioned BEFORE container start (Radicale reads its
config at startup): the deploy hook writes config+htpasswd ahead of
deployContainer() and passes the pre-start credentials to the post-deploy
registration step; useExisting path logs an explicit restart warning.
- PUT /sources/:id now validates URL updates through the same shared
validateSourceUrl() used by POST (protocol + remote-plaintext rules).
- New end-to-end contract test (calendar-autowire.e2e.test.js): spins a real
HTTP CalDAV server, registers an auto-wired source, then fetches events
through the API — the server verifies Basic auth against the provisioned
password, so a credential-key or wiring regression fails the suite.
Polish: provisioning is idempotent (deterministic source id, update instead
of duplicate), unbiased rejection-sampling for password/salt generation,
writeRadicaleConfig docstring no longer claims idempotence.
Calendar suites: 35 passed (32 prior + 3 new). Lint: 0 errors; complexity
warnings in deploy.js pre-exist at base (61 vs 68 now — the hook did not
worsen the trend line materially; calendar files themselves lint clean).
- __tests__/e2e-node/composition-root.e2e.test.js: `boot()` takes {timeoutMs, code}
and the timeout rejection carries the child it killed; a NEW test drives the real
timeout against a child that never reports PORT/READY and asserts the OS-observed
SIGKILL on that child (marker e2e-timeout-kill) — the r4 claim is now exercised.
- routes/deploys.js: bridge() coerces any non-object JSON body (null/[]/string) into
the same failure shape as a non-JSON body, so no `body.ok` deref can 500 or report a
false success; the subdomain label regex rejects leading AND trailing hyphens.
- __tests__/routes/deploys.catalog.r4.test.js: trailing-dash rejection case + three
malformed-successful-body cases (clean 502, never 500).
The e2e child now calls app.listen(0) and reports the OS-assigned port on
stdout (PORT:<n>); the parent reads it. The probe-then-release helper is
deleted, so no other process can take the port between probe and bind.
BLOCK0: catalog deploy audit now uses the REAL AuditLogger contract
(auditLogger.log({action,resource,details,outcome,ip})) fire-and-forget
via Promise.resolve().catch — async audit failures can never fail the
deploy response (tested with a throwing audit backend).
BLOCK1: verify.sh targeted-jest block captures jest exit code explicitly
(set +e / rc / loud FAIL) — no silent termination under set -e.
BLOCK2: subdomain advertised by the App Selector modal is now FORWARDED
end-to-end (selector -> /deploys/catalog/deploy validated charset ->
bridge --subdomain -> CLI CatalogInstallOptions, implemented in
dc-catalog-2 r3); invalid subdomains 400 before the bridge (4 cases).
POLISH: boot-timeout SIGKILLs the child (no leaked process).
New r4 suite: 7 tests (audit shape success/failure, subdomain forward +
4 rejections). Full suite 157 suites / 3519 tests green.
Judge r2 blockers addressed:
- MOUNTED vs UNKNOWN: probes now carry a REAL JWT minted by the same
authManager the app wires (sub claim; generated in the boot child,
emitted on READY). Every mounted-route assertion is compared against a
live notFoundHandler-404 baseline for a known-nonexistent route on the
same instance — a fall-through can no longer pass as mounted.
- Route-level behavior asserted with real payloads: /catalog/plex 200
must contain plexinc/pms-docker AND {{PORT}}:32400; /security/shield 200
must be success:true.
- OS-assigned ephemeral port (net server listen(0)) replaces the random
30000-49999 guess; boot-timeout cleared on readiness; SIGTERM+SIGKILL
shutdown with bounded wait.
- 7 tests (was 5). Suite: 156 suites / 3512 tests green.
GET /deploys/catalog/apps -> bridge /api/catalog/apps (73 embedded apps,
single source of truth). POST /deploys/catalog/deploy {app,port,env} ->
validated (service-id charset, port 1024-65535, env name/value rules) then
proxied to bridge /api/catalog/deploy which runs the shipdeck CLI catalog
deploy (OCI image -> hardened systemd service; NO Docker daemon). Audited
as deploy.catalog with ok/seconds. 11 table-driven route tests incl.
validation rejections never reaching the bridge + bridge-failure 502 shape.
App Selector submit rewired from /apps/deploy (Docker chain) to
/deploys/catalog/deploy — same modal, same fields (port, mediaPath, plex
claim token as env), no new UI screens. Full suite 3510/3510 (156 suites).
- DC-160: /catalog/:appId read app.image/ports/env/volumes which live under
app.docker.* — every detail response was empty. Now reads docker.* and
surfaces defaultPort/subdomain/healthCheck/mediaMount/claimToken/difficulty/
setupInstructions for the deploy form. List/search use the templates own
category field (old hand-rolled CATEGORY_MAP mis-categorized apps).
- Shield (routes/shield.js + 5-line app.js mount) committed to source after
weeks as prod-only drift; regression test asserts the mount exists.
- New composition-root e2e suite: boots REAL createApp() on an ephemeral
port, probes the historically-broken routes through the REAL mount table
(wizard wiring, shield, catalog detail, deploys bridge) — the class of bug
all 3 P7 dogfood findings belonged to. 5 tests.
- Full suite 3499/3499 (155 suites); table-driven DC-160 test covers all 73
image templates.
Dogfood finding #3: upsertPerson resolved existing records by id only.
The wizard upserts a bootstrap admin AFTER a provider sync has already
created a person for the same Tailscale user (no id passed), so the same
providerUserId was duplicated into two records. Resolve existing by
providerUserId when no id is given; bootstrap Admin group + protectedAt are
preserved on the merge. New regression test; full suite 3414/3414.
Dogfood finding #2: createTailscaleProvider() built with coordClient=null;
nothing in prod ever called attachCoordClient() (the wiring the provider
docblock promised never landed), so wizard credential validation always
failed not_configured even with a valid stored token. Pass a delegating
proxy that fetches a fresh coord client per call via the sanctioned
ctx.tailscaleCoord.getClient pattern (token re-read from credentialManager
each call). Full suite 3413/3413.
The /network/wizard mount in app.js omitted providers, so the wizard built
with providers={} and every mode selection returned 503 provider_unavailable
in production. Unit tests never caught it because they inject their own
provider map. Pass built tailscale + wireguard instances.
Full suite: 3413/3413 (152 suites).
Hermes
sami7777