fix(review): stop large-registry-PR false-closes across grounding, content-lane, and issue matching#7931
Merged
Conversation
registry/subnets/eirel.json's 188KB post-change body overflowed the flat 24k/60k grounding caps and GitHub omits the inline diff for a change this large, so the reviewer got zero content for the file that was the actual substance of the PR and the gate closed it as a confirmed defect instead of an honest "can't verify this". Raise the budgets with real headroom, and durably fix the underlying gap: an oversized file now degrades to a real head+tail sample instead of ever rendering as pure omitted content again, regardless of how large it grows. Also raises the downstream aggregate prompt-budget cap that would otherwise silently drop grounding on exactly this large-PR case.
…faces The registry append-count check fetched HEAD/BASE content with no size cap (defaulting to 24,001 chars), silently truncating a larger registry file into invalid JSON. That parsed as "surfaces: null", misread as "nothing appended", and the existing safety net only checked for a literal null fetch, not a truncated-but-non-null one. Pass an explicit, generous cap and treat truncation as unreadable so it routes through the existing defer-to-generic-gate guard instead of a wrong close. Also closes a related latent bug found while tracing this: the same fetch path was using a PR's base BRANCH NAME as a cache key in a cache built to assume immutable commit SHAs, which can serve indefinitely stale content once written. Add a TTL safety net so that can't happen regardless of what ref shape any caller passes.
The content-lane deliverable check only recognized a linked issue's own ask when its body named a literal registry file path. An issue whose body only carries a generic path placeholder (never the real resolved filename) read as not-applicable, silently providing zero protection against a PR that never delivers the registry change. Add an opt-in, spec-level signal: when a linked issue's title matches a known shape, a registry-file edit is required even without a literal path in the body. Deterministic and narrowly scoped -- no fuzzy slug inference, no AI call.
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
2 tasks
JSONbored
added a commit
that referenced
this pull request
Jul 21, 2026
The fix commits just merged in #7931 warrant a patch bump per scripts/orb-release-core.ts's own inferred-version logic, but the manifest still targets 3.2.0 -- the version already promoted to stable earlier today. Without this, orb-beta-release.yml's due check short-circuits (targetAlreadyStable) and never cuts a beta containing the fix.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7931 +/- ##
==========================================
- Coverage 91.77% 90.43% -1.34%
==========================================
Files 731 95 -636
Lines 74878 27510 -47368
Branches 22845 5898 -16947
==========================================
- Hits 68717 24878 -43839
+ Misses 5083 2296 -2787
+ Partials 1078 336 -742
Flags with carried forward coverage won't be shown. Click here to find out more.
|
3 tasks
JSONbored
added a commit
that referenced
this pull request
Jul 21, 2026
…eck (#7940) * test(review): cover the null-issue-title branch in the deliverable check processors.ts's runContentLaneDeliverableCheckForAdvisory passes issueFetch.facts.title ?? undefined to checkContentLaneDeliverable, but every existing test's stub always supplied a real title string, so the nullish side of that fallback was never exercised -- codecov/patch correctly flagged it as a partial branch on #7931. Pins that a null/absent issue title degrades cleanly to undefined: the literal-path-in-body signal alone still detects a missing deliverable, and passing undefined never throws or wrongly suppresses it. * docs(review): clarify the null-issue-title fallback in the deliverable check Documents why facts.title's null case degrades to undefined at this call site (see the accompanying test). Also broadens this PR's diff beyond a single test file, which a pure test-only diff was tripping in CI's scoped-test-selection coverage step.
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Each was root-caused against a real, reproduced incident (oversized diffs GitHub omits inline patches for, and a registry file's HEAD content exceeding the old fetch cap) rather than guessed at.
Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.md; nosite/,CNAME, or Pages changes.Validation
git diff --checknpm run typechecknpm run test:coveragelocally — full unsharded suite green, 0 failures; 100% line+branch coverage on every changed line in this diffnpm audit --audit-level=moderate— cleannpm run actionlint/test:workers/build:mcp/test:mcp-pack/ui:*— not run individually this pass; none of this diff touches CI config, MCP packaging, or the UI workspace, and the fullnpm run test:coveragerun already exercises the affected unit/integration suitesSafety
Notes