ci(dependabot): cover the designer lockfile, document the vendor exclusion (BLO-31437) - #1622
Conversation
…usion (BLO-31437) `.github/dependabot.yml` declared one npm entry, `directory: "/"`. The repo has three npm lockfiles, so two got security alerts (which come from the dependency graph, not this file) but never an automated bump PR — remediation was manual every time. The same advisory landed by hand twice: GHSA-7p8r-x3mc-p8w7 (alert #114, BLO-21608) and GHSA-5jgf-p345-68v8 (alerts #148/#149, BLO-31415/BLO-31416, fixed in #1619). A decision is now recorded for each uncovered directory. packages/services/designer -> COVERED. It sits outside the pnpm workspace (pnpm-workspace.yaml excludes packages/services/**), so it carries its own package-lock.json that `directory: "/"` cannot reach. Safe to bump automatically: UPSTREAM.md excludes package-lock.json from the vendoring rsync and regenerates it locally/in CI — "lockfile insulation" is one of the three stated reasons for vendoring the service — and no integrity manifest covers the directory. The only CI check reading this lockfile, scripts/designer-fast-uri-security.test.js, asserts a version FLOOR (fast-uri >= 3.1.5), which an upward bump satisfies by construction. vendor/paperclip-adapter-claude-k8s -> DELIBERATELY EXCLUDED, because an automated bump there cannot pass CI. The `vendor_claude_k8s` job in .github/workflows/pr.yml recomputes a sha256 manifest over `git ls-files` for that directory, excluding only LICENSE and PROVENANCE.md, and fails unless it matches the hash recorded in PROVENANCE.md. package-lock.json is a tracked file inside that manifest, so any bump changes the hash; Dependabot cannot update PROVENANCE.md in the same PR; and vendor_claude_k8s is on the required `verify` path. Every such PR would be born red and unmergeable. That tree also carries a manual `-blockcast.N` version convention. It stays a hand-bumped in-tree PR per vendor/README.md. Both the table and the exclusion rationale are recorded in the config file itself, so the next person who asks why vendor/ is absent gets the answer without digging. The added entry mirrors the root entry's guardrails — `dependencies` label and the version-update:semver-major ignore rule — with one stated deviation: open-pull-requests-limit is 5 rather than 10, since this is a single service with a much smaller dependency surface than the monorepo root, which bounds the weekly PR volume the entry can add. Refs BLO-31437.
|
Hey @allyblockcast[bot]! Before this PR can be reviewed, a few things need attention: Missing or incomplete:
Once updated, push a new commit and these checks will re-run automatically. — commitperclip |
![allyblockcast[bot]](https://avatars.githubusercontent.com/u/57363972?s=60&v=4){kind=small}
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 56ac414
Config-only change to .github/dependabot.yml (+49/-0, one file). I re-verified every factual
claim the new comments make, against the tree at this exact head rather than against the PR
description — all of them hold. Details under Strengths.
Critical Issues (0)
None.
Important Issues (0)
None. The change is additive, syntactically well-formed (3 updates entries parse; npm
directories are exactly / and /packages/services/designer; no tabs), and the new entry's
target directory carries both package.json (1930 B) and package-lock.json (61418 B) at this
head, so the npm ecosystem resolves there.
Suggestions (3)
-
[native-codex]
.github/dependabot.yml:49— Dependabot has never opened a PR in this
repo, so this entry may be inert until version updates are enabled..github/dependabot.yml
has declared weekly npm andgithub-actionsversion updates sincec204d117(2026-06-12,
~12 weeks), yetsearch/issues?q=repo:Blockcast/paperclip+is:pr+author:app/dependabotreturns
total_count: 0. I ran the positive control — the same predicate returns792for
author:app/allyblockcast— so this is a real zero, not a wrong predicate spelling.
Corroborating: the repo has 9 labels anddependenciesis not among them, and that is a label
Dependabot creates itself when it opens its first PR. Twelve weeks ofactions/checkout-class
updates with zero PRs is hard to explain by "nothing was out of date."I could not determine the cause: the App token 403s on
repos/{r}/vulnerability-alerts,
repos/{r}/automated-security-fixes, andrepos/{r}/dependabot/alerts
(Resource not accessible by integration). I did not retry under the merge-seat token. So the
observation is confident and the diagnosis is not — this could be repo/org-level version-update
enablement, an App installation scope, or something else.This is not a defect in the diff: the entry is a necessary precondition either way, and
adding it is correct regardless of enablement. But the issue's stated goal is automated bumps,
and if version updates are off, merging this closes the config gap without closing the
remediation gap — the next designer advisory would still be hand-fixed, and the PR would read as
having solved it. Worth confirming enablement (Settings → Code security) before treating
BLO-31437 as fully discharged, and worth a line on the issue either way. -
[gstack/review]
.github/dependabot.yml:61-64— theignorerule
(dependency-name: "*",version-update:semver-major) is copied from the root entry, which is
the right instinct for consistency. Flagging one interaction worth confirming rather than
assuming: GitHub documentsignoreconditions independabot.ymlas applying to Dependabot
security updates as well as version updates. If that holds here, an advisory whose only fix
is a major bump would produce no automated PR on this directory — a narrower instance of exactly
the manual-remediation gap this PR exists to close. Neither cited advisory was major
(GHSA-5jgf-p345-68v8 fixed atfast-uri3.1.5), so nothing is broken today, and majors do want
human review. Not something to change in this PR — but if the security-update interaction is
confirmed, the durable fix is a narrower ignore on both entries, and that is a separate change. -
[pr-review-toolkit/comments]
.github/dependabot.yml:8-10— micro-nit, take or leave: the
three-line lockfile table is column-aligned for the first two rows and the third overruns, so
the->markers don't line up. Two extra spaces on rows 1-2 (or dropping alignment entirely)
would restore it. Zero functional impact.
Strengths
- Every load-bearing claim in the new comments verifies at this head. I checked each against
the tree rather than trusting the prose, and found no drift:- "THREE npm lockfiles" — exact. A non-truncated recursive tree read at
56ac414creturns
preciselypnpm-lock.yaml,packages/services/designer/package-lock.json,
vendor/paperclip-adapter-claude-k8s/package-lock.json, and noyarn.lock/
npm-shrinkwrap.jsonanywhere. - "pnpm-workspace.yaml excludes packages/services/**" —
pnpm-workspace.yamlcarries the
explicit- "!packages/services/**"negation, with a comment giving the same rationale. - "UPSTREAM.md excludes package-lock.json from the vendoring rsync" —
packages/services/designer/UPSTREAM.md:56is literally--exclude=package-lock.json, and
"Lockfile insulation" is the stated reason at line 15. - "package-lock.json is a tracked file inside that manifest" —
pr.yml:841-843computes
git ls-files | grep -vxE 'LICENSE|PROVENANCE\.md' | sort | xargs sha256sum | sha256sum, so
yes, the lockfile is inside the hashed set and any bump breaks it. - "vendor_claude_k8s is on the required verify path" — it is in
verify'sneeds:list
(pr.yml:873). - "asserts a version FLOOR (fast-uri >= 3.1.5)" —
scripts/designer-fast-uri-security.test.js:23
ismajor > 3 || (major === 3 && (minor > 1 || (minor === 1 && patch >= 5))), a true floor
that an upward bump satisfies by construction, including across a 4.x major.
- "THREE npm lockfiles" — exact. A non-truncated recursive tree read at
- The exclusion is argued from CI mechanics, not taste. "Every such PR would be born red and
unmergeable" is the correct standard for excluding a directory, and it is demonstrated rather
than asserted — the manifest genuinely covers the lockfile and Dependabot genuinely cannot update
PROVENANCE.mdin the same PR. - The two vendored directories are distinguished rather than lumped together. The easy error
here was "designer is vendored too, so exclude it as well"; the PR checks the specific property
that makes the adapter unsafe (an integrity manifest), finds designer has none, and covers it.
That is the difference between a rule and a cargo-culted rule. - The deviation is stated at the point of deviation.
open-pull-requests-limit: 5carries an
inline reason for differing from the root's 10, so the next reader doesn't have to guess whether
it was deliberate. - The rationale lives in the file it governs. The next person wondering why
vendor/is absent
gets the answer without digging through PR archaeology — which is exactly the failure mode that
let this gap persist for three months.
Recommended Action
- No Critical or Important issues — mergeable as-is from a review standpoint.
- Before closing BLO-31437, confirm Dependabot version updates are actually enabled for this repo
(Suggestion 1). If they are not, this PR is a correct precondition but not yet the fix, and the
issue should say so rather than closing on the config change alone. - Suggestions 2 and 3 are optional and out of scope for this PR.
Self-review: this PR is authored by app/allyblockcast, so GitHub bars an approval from the
author. Submitted as a formal COMMENTED review with the App credential, which is the artifact of
record for the ally gate. No Critical or Important findings — the comment state reflects the
authorship constraint, not a blocker.
Ally review nit on #1622: rows 1-2 were padded to a shorter column than row 3, so the `->` markers did not line up in a comment block whose whole purpose is legibility. Comment-only; the YAML parses to the same three `updates` entries.
…t-designer-coverage
There was a problem hiding this comment.
Ally — Consolidated PR Review
Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: ded5919
Re-review after synchronize. Two commits landed since my last pass, and neither changes what
this PR contributes:
9ff3fd94"style(dependabot): align the lockfile-table arrows in the header comment" —
+2/-2, both lines comment-only whitespace. That is the cosmetic nit I raised last time, now
fixed: all three table arrows sit at column 59.ded59196— a merge oforigin/master(parents9ff3fd94+a3fba29a). This pulls in
unrelated master work, notably ~188 lines in
packages/db/src/heartbeat-dispatch-query-plan.test.ts. That churn is not part of this
PR and I have not reviewed it here; the PR's own three-dot diff against the merge base is
still one file,.github/dependabot.yml, 64 diff lines, and.github/dependabot.ymlis
byte-identical to the revision I reviewed at9ff3fd94.
Because master moved underneath the branch, I re-ran every tree-level claim at this merged
head rather than carrying it forward — the header comment asserts things about files this diff
does not touch, so a master-side change could have falsified them. All still hold, at identical
line numbers. Details under Strengths.
Critical Issues (0)
None.
Important Issues (0)
None. Still additive and config-only (+49/-0 against the merge base, one file). Three
updates entries, no tabs, no trailing whitespace, and no duplicate
(package-ecosystem, directory) pair — (npm, /), (npm, /packages/services/designer),
(github-actions, /).
Suggestions (2)
Both carried forward from my previous pass; neither is a defect in this diff, and neither is
affected by the whitespace commit or the merge.
-
[native-codex]
.github/dependabot.yml:49— the entry may be inert until Dependabot
version updates are actually enabled for this repo, and the evidence for that got slightly
stronger. Re-measured at this head:is:pr author:app/dependabotstill returns
total_count: 0against a positive control of796forauthor:app/allyblockcast, so the
zero is real and not a mis-spelled predicate. Corroborating, and the part that moved: the
repo's 9 labels are stillbug, documentation, duplicate, enhancement, good first issue, help wanted, invalid, question, wontfix—dependenciesis still absent, and that is a
label Dependabot creates itself when it opens its first PR. The root entry has declared
weekly npm andgithub-actionsupdates sincec204d117(~12 weeks) with the same
labels: ["dependencies"]block, so twelve weeks ofactions/checkout-class updates have
produced neither a PR nor the label.I still could not determine the cause — the App token 403s on
repos/{r}/vulnerability-alerts,repos/{r}/automated-security-fixes, and
repos/{r}/dependabot/alerts(Resource not accessible by integration), and I did not
retry under the merge-seat token. So the observation is confident and the diagnosis is not.This is not a reason to hold the PR: the entry is a necessary precondition either way. It is
a reason not to close BLO-31437 on the config change alone — if version updates are off,
merging closes the config gap without closing the remediation gap, and the next designer
advisory is still hand-fixed while the PR reads as having solved it. -
[gstack/review]
.github/dependabot.yml:61-64— theignorerule
(dependency-name: "*",version-update:semver-major) mirrors the root entry, which is the
right instinct. Worth confirming rather than assuming: GitHub documentsignoreconditions
independabot.ymlas applying to Dependabot security updates as well as version
updates. If that holds here, an advisory whose only fix is a major bump would produce no
automated PR on this directory — a narrower instance of the manual-remediation gap this PR
exists to close. Neither cited advisory was major (GHSA-5jgf-p345-68v8 fixed atfast-uri
3.1.5), so nothing is broken today, and majors do want human review. Not a change for this
PR; if the interaction is confirmed, the durable fix is a narrower ignore on both entries.
Strengths
- The nit was fixed properly rather than papered over. Arrows now align at column 59 on all
three rows; the fix adjusted the two short rows rather than truncating the long path. - Every claim in the header comment re-verifies at this merged head, checked against the
tree and not the prose — this is the check that matters most on a merge commit, since these
are all assertions about files the diff does not touch:- "THREE npm lockfiles" — exact, on a non-truncated recursive tree read: precisely
pnpm-lock.yaml,packages/services/designer/package-lock.json, and
vendor/paperclip-adapter-claude-k8s/package-lock.json. Noyarn.lock,
npm-shrinkwrap.json, orbun.lockbanywhere. Master did not add a fourth. - The new entry's target resolves:
packages/services/designer/carries bothpackage.json
(1930 B) andpackage-lock.json(61418 B), so the npm ecosystem resolves there. - "There is no integrity manifest over this directory" — confirmed by absence; nothing
matchingPROVENANCE/MANIFEST/.sha256exists anywhere under
packages/services/designer/. - "pnpm-workspace.yaml excludes packages/services/**" —
pnpm-workspace.yaml:17is
literally- "!packages/services/**", with the rationale in the comment above it. - The exclusion rationale holds mechanically:
pr.yml:842-844still computes
git ls-files | grep -vxE 'LICENSE|PROVENANCE\.md' | LC_ALL=C sort | xargs sha256sum | sha256sum,
sopackage-lock.jsongenuinely is inside the hashed set and any bump breaks it — and
vendor_claude_k8sgenuinely is still on the required path (pr.yml:873, inside
verify'sneeds:list,verify:at 858 andneeds:at 864). - "asserts a version FLOOR" —
scripts/designer-fast-uri-security.test.js:23is still
major > 3 || (major === 3 && (minor > 1 || (minor === 1 && patch >= 5))), a true floor
that an upward bump satisfies by construction, including across a 4.x major.
- "THREE npm lockfiles" — exact, on a non-truncated recursive tree read: precisely
- The two vendored directories are distinguished rather than lumped together. The easy
error was "designer is vendored too, so exclude it as well"; this checks the specific
property that makes the adapter unsafe (an integrity manifest), finds designer has none, and
covers it. That is the difference between a rule and a cargo-culted rule. - The deviation is stated at the point of deviation.
open-pull-requests-limit: 5carries
an inline reason for differing from the root's 10. - The rationale lives in the file it governs, which is the failure mode that let this gap
persist for three months.
Recommended Action
- No Critical or Important issues — mergeable as-is from a review standpoint.
- Before closing BLO-31437, confirm Dependabot version updates are actually enabled for this
repo (Suggestion 1). If they are not, this PR is a correct precondition but not yet the fix,
and the issue should say so rather than closing on the config change alone. - Suggestion 2 is optional and out of scope here.
Self-review: this PR is authored by app/allyblockcast, so GitHub bars an approval from the
author. Submitted as a formal COMMENTED review with the App credential, which is the artifact
of record for the ally gate. No Critical or Important findings — the comment state reflects the
authorship constraint, not a blocker.
Thinking Path
Linked Issues or Issue Description
Tracked in Paperclip rather than GitHub Issues, so there is no
#-number to close:What Changed
Single file,
.github/dependabot.yml, config only./packages/services/designer, mirroring the root entry'sdependencieslabel andversion-update:semver-majorignore rule.open-pull-requests-limitto 5 rather than the root's 10 — the one deliberate deviation, stated inline.vendor/is absent gets the answer without re-deriving it.Per-directory decision
pnpm-lock.yamldirectory: "/"packages/services/designer/package-lock.jsonvendor/paperclip-adapter-claude-k8s/package-lock.jsonpackages/services/designer→ coveredIt sits outside the pnpm workspace (
pnpm-workspace.yamlexcludespackages/services/**), so it carries its ownpackage-lock.jsonand the rootpnpm.overridescannot reach it —directory: "/"does not cover it.It is also a vendored copy, so I checked whether that made it unsafe the way the adapter is. It does not:
UPSTREAM.mdexcludespackage-lock.jsonfrom the vendoring rsync and regenerates it locally/in CI, so the lockfile is not upstream-synced content.scripts/designer-fast-uri-security.test.js, asserts a version floor (fast-uri >= 3.1.5), which an upward bump satisfies by construction. (Worth checking explicitly — an exact pin would have behaved the opposite way.)vendor/paperclip-adapter-claude-k8s→ deliberately excludedNot a stylistic call — an automated bump there cannot pass CI.
vendor_claude_k8sin.github/workflows/pr.ymlrecomputes a sha256 manifest overgit ls-filesfor that directory, excluding onlyLICENSEandPROVENANCE.md, and fails unless it matches the hash recorded inPROVENANCE.md.package-lock.jsonis a tracked file inside that manifest, so:PROVENANCE.mdin the same PR;vendor_claude_k8sis inverify'sneeds:list, i.e. on the required path.So every such PR would be born red and unmergeable — a recurring stream of failing PRs, strictly worse than the status quo. That tree also carries a manual
-blockcast.Nversion convention Dependabot cannot follow. It stays a hand-bumped in-tree PR, pervendor/README.md.Verification
.github/dependabot.ymlparses to threeupdatesentries; npm directories are exactly/and/packages/services/designer, with novendordirectory present:mastervia the contents API (branching on exit status, not jq truthiness); the integrity-hash step read directly frompr.yml; thefast-uricheck read directly fromscripts/designer-fast-uri-security.test.jsand confirmed to be a floor.Vendored claude_k8s adapterpasses on this PR, confirming the vendored tree is untouched.packages/services/designer, and must not open any againstvendor/.Risks
Low risk. Config-only; no runtime surface, no code path, no migration.
open-pull-requests-limit: 5plus the mirrored semver-major ignore, so majors stay manual.open-pull-requests-limit: 5vs mirroring the root's10. I chose 5 on the designer's much smaller dependency surface and stated the reason inline; happy to make it 10 if you'd rather the entries be uniform.Model Used
Claude Opus 5 (
claude-opus-5), 1M context, extended thinking, with tool use (GitHub API reads, repo clone, shell) via Claude Code in the Paperclip agent harness.Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue templatedependabot.ymlitself, next to the config it explains