docs(app-blocks): de-line six stale file:line cross-references, and correct a false justification (#4838)

* docs(app-blocks): de-line six stale file:line cross-references

Six citations named a file:line that no longer held what the citing
sentence claimed. Each was re-measured against main and replaced with the
SYMBOL, function or branch it means — a fresh line number would be wrong
again on the next edit, which is exactly how these six got here.

docs/features/app-blocks.md (4):
  - CACHE_TTL_SECONDS         :39   was an import stmt  -> name the file only
  - invalidateModelCache      :461  was a kill-list note-> name the file only
  - KILL_LIST_CACHE_TTL_MS    :480  was a catch comment -> name the file only
  - MAX_BLOCKS_PER_SLOT       :1849 was an NSFW comment -> installOnModel

Two bare line-lists in the SAME two sentences were also stale and are
de-lined with them: the four invalidateModelCache callers (cited 1950 /
2019 / 2055 / 2108, actually installOnModel / uninstallFromModel /
toggleEnabled / updateSettings) and the kill-list filter on a cache hit
(cited 657-664, actually the kill.has(r.blockId) filter on listForModel's
cache-hit branch). Leaving a known-wrong number attached to a clause whose
other half was just corrected would ship a statement measured false.

src/shared/constants/block-effective-scopes.ts (2):
  - blocks.router.ts:2787-2789     was a pinned-install count mapping
    -> grantScopes' `const ceiling = new Set(effectiveBlockScopes(...))`
  - scope-grant.service.ts:222-224 was the buzzBudgetPerDay opts field
    -> recordScopeGrant's `const incoming = Array.from(new Set(...))`

Matches the de-lining style three citations in that same file already use.

NOT changed, because they were re-measured and are ACCURATE — they are the
positive control proving this audit discriminates rather than rewriting
everything it touches: block-registry.service.ts:329 (the
OwnedNonApprovedPageBlockResolution docblock sentence), block-tokens
:1054 / :469 / :650 / :375-389 / :455-459, block-registry:2117-2119 and
:2104-2114, and block-manifest-validator.service.ts:478-496. Nine of the
eleven citations checked in that file are exact.

Comment/docs only; no behaviour change.

* docs(app-blocks): the full-bleed ledger's reason is an operator decision

The `WHY THIS IS CSS AND NOT A MANIFEST FIELD` block argued from a real
premise to a conclusion that does not follow. It said the manifest schema
is mirrored across three repos and this host is pinned to
@civitai/app-sdk@^0.14.0 while guests ship 0.35.x, "so a new manifest field
would be UNTYPED at exactly the point the host consumes it."

The pin is real. The conclusion was measured false: the host never reads a
manifest through an SDK type at all.
  - src/server/services/block-manifest-validator.service.ts imports nothing
    from @civitai/app-sdk (0 matches; 6 import statements as a positive
    control) and validates against this repo's own rules.
  - src/components/AppBlocks/types.ts DECLARES the host's own BlockManifest.
    Its one "@civitai/app-sdk" hit is prose in a docblock ("Matches
    @civitai/app-sdk/blocks v1"); the file has no import statements at all.

The conclusion — keep full bleed in CSS — stands, but on its actual author
of record: the repo owner decided full bleed should be managed by styling,
not a manifest field. PR #4812 built the manifest field and was closed
unmerged on that call. That decision is now stated as the reason, and the
false technical reason is kept alongside it, explicitly retracted, so it is
not rediscovered and acted on. Deliberately NOT replaced with a freshly
constructed technical argument: inventing a replacement reason is the
failure that produced the false one.

The three-repo mirroring IS true and is kept, now as the measured COST of
the alternative rather than as the reason. Verified first-hand that all
three copies declare `page` with additionalProperties:false and the
identical four keys: public/schemas/app-block/v1.json (canonical), the Go
CLI's schema/app-block.manifest.schema.json, and app-sdk@0.14.0's vendored
schemas/app-block/v1.json. Against the released CLI 0.1.101, adding an
undeclared key under `page` makes `civitai app validate` exit 1 with
"page: additional properties 'fullBleed' not allowed", locally, before any
network call (negative control); the unmodified manifest exits 0 (positive
control).

Comment-only; no CSS rule, selector or declaration changed. `/*` and `*/`
counts equal at 31/31 before and after.
ledgerSelectorSurvivesProdStrip.test.ts parses selectors out of this file
INCLUDING its comments, so it was instrument-validated rather than merely
run green: injecting a data-testid-keyed selector into this comment turns
it red naming data-testid (1 failed / 6 passed), and it is green at
16/16 with pageBlockHostMaxWidth.test.ts once reverted.

* docs(globals): cut the retraction to what is measured, not a fourth draft

The replacement prose asserted three things beyond the measurement, in the
paragraph that replaced a claim retracted for exactly that:

- "and it is strict" of the three schema mirrors. Measured false today:
  `@civitai/app-sdk@0.14.0`'s vendored copy is missing five top-level
  properties the canonical declares, and the Go CLI is missing one. The
  mirror is loose, and the drift sat there unnoticed.
- a re-vendor cost attributed to all three copies. Only the CLI was ever
  measured to block; nothing consumes the SDK's copy as a validator, and a
  manifest's $schema names the canonical URL.
- "its 'Matches @civitai/app-sdk/blocks v1' line" bound to `BlockManifest`,
  which has no docblock — that line belongs to the BLOCK_INIT payload type.

Cut rather than redrafted. Every sentence here is a claim that can rot, and
this block has now been wrong twice; a shorter one has less to be wrong
about. The comparative clauses ("one line", "needs no schema motion") went
with it: they argued for the mechanism under a label saying they did not.

Guard re-validated on this tree, not inherited: a literal `*/` planted in
the new paragraph turns ledgerSelectorSurvivesProdStrip and
pageBlockHostMaxWidth red (2 failed / 14 passed); restored, 16/16 green and
the comment balance is 31/31.
This commit is contained in:
Zachary Lowden
2026-09-14 17:37:07 -05:00
committed by GitHub
parent 959ebbbc1f
commit 26b47fa98e
3 changed files with 43 additions and 20 deletions
+11 -10
View File
@@ -574,13 +574,13 @@ that takedown tool is built, not a live bug today.
### Cache invalidation on `app_blocks.status` transitions (audit-10 H3)
`BlockRegistry.listForModel` caches its per-`(model, slot)` result for 60s
(`CACHE_TTL_SECONDS`, `block-registry.service.ts:39`) and the SQL only filters
(`CACHE_TTL_SECONDS` in `block-registry.service.ts`) and the SQL only filters
on `ab.status = 'approved'` at query time, not at cache-read time.
`invalidateModelCache(modelId)` (`block-registry.service.ts:461`) is called only
from the four per-model mutation paths — install, uninstall, toggleEnabled,
updateSettings (lines 1950 / 2019 / 2055 / 2108). It is NOT called on any
`app_blocks.status` transition, because a status change on a block doesn't know
which models the block is installed on.
`invalidateModelCache(modelId)` (same file) is called only from the four
per-model mutation paths — `BlockRegistry.installOnModel`,
`uninstallFromModel`, `toggleEnabled` and `updateSettings`, which are its only
callers. It is NOT called on any `app_blocks.status` transition, because a
status change on a block doesn't know which models the block is installed on.
Two facts bound the actual risk today:
@@ -594,9 +594,10 @@ Two facts bound the actual risk today:
late; it never keeps rendering after it should have stopped.
- **The emergency kill list is applied fresh on every cache hit**, so it is NOT
subject to the 60s cache. `getKillList()` has its own 5s in-process TTL
(`KILL_LIST_CACHE_TTL_MS`, `block-registry.service.ts:480`) and `listForModel`
filters the cached rows against it on every read (lines 657664). "Stop this
block right now" (`sysRedis SET system:blocks:emergency-kill-list`) therefore
(`KILL_LIST_CACHE_TTL_MS` in `block-registry.service.ts`) and `listForModel`
filters the cached rows against it on every read (the `kill.has(r.blockId)`
filter on `listForModel`'s cache-hit branch). "Stop this block right now"
(`sysRedis SET system:blocks:emergency-kill-list`) therefore
takes effect within ~5s regardless of the registry cache.
So the real work item is: before shipping a moderator `suspend`/`deprecate`
@@ -653,6 +654,6 @@ documented above, so they're dropped from this list.
- **DNS-rebinding gate at `assetBundleUrl` fetch time** (still lexical-only at
submit — see the SSRF note in the threat model).
- **Per-slot install-cap race hardening.** The cap is enforced at install time
(`MAX_BLOCKS_PER_SLOT`, `block-registry.service.ts:1849`) via a
(`MAX_BLOCKS_PER_SLOT`, checked in `BlockRegistry.installOnModel`) via a
count-then-insert, not a row-locked transaction, so a rare concurrent double
install can still exceed the cap; accepted for now.