Skip to content

feat(review): add boundary-safe test-generation advisory (#1972)#3794

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
worktree-wf_c9b16cbb-862-4
Jul 6, 2026
Merged

feat(review): add boundary-safe test-generation advisory (#1972)#3794
loopover-orb[bot] merged 1 commit into
mainfrom
worktree-wf_c9b16cbb-862-4

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Summary

Implements #1972 (boundary-safe test generation). Adds a new deterministic,
config-gated advisory: when a PR's diff touches a small, precise set of
boundary-condition patterns (off-by-one array/index bounds, null/undefined
branches, empty-collection checks) with no accompanying test evidence
anywhere in the PR, it surfaces a boundary_test_generation_available
finding plus a LOCAL-execution test-generation action spec (criteria and
framework-agnostic hints only — never generated test code, mirroring
src/mcp/local-write-tools.ts's no-cloud-write boundary: gittensory
supplies the content, the contributor's own agent scaffolds and runs it).

Full server-side test-code generation was explicitly scoped OUT per the
issue — generated test code would need a human to verify it is even
correct anyway, which risks false confidence. The pattern set is
deliberately small (3 kinds) since false positives are worse than a
narrow true-positive set here.

New .gittensory.yml knob review.test_generation (bool | null),
wired through src/signals/focus-manifest.ts exactly like the existing
review.effort_score / review.changed_files_summary boolean flags
(parse, empty-manifest defaults ×3, present check, serialize
round-trip, and a dedicated resolveTestGenerationEnabled resolver
mirroring resolveReviewPreMergeChecks). Default OFF; a repo with no
config is byte-identical to today since nothing calls the new detector
unless a caller opts in.

Exposed as a new gittensory_suggest_boundary_tests MCP tool alongside
the existing gittensory_check_slop_risk / gittensory_check_issue_slop
pure-metadata tools (no repo/auth access needed — the caller supplies
changed-file paths + patch text, never full source).

Design decision (conservative, smallest-scope): rather than wiring this
into the AI-review orchestration in src/queue/processors.ts (a very
large integration surface for a narrow, config-gated advisory), I kept
the new detector/finding/spec as standalone pure functions plus the
config knob and the MCP tool — the closer, smaller existing analogue
(gittensory_check_slop_risk's pure-metadata MCP-tool pattern) rather
than the heavier unified-comment rendering pipeline. It is NOT wired
into the scored slop.ts composite (buildSlopAssessment), since that
would change slopRisk for every repo that opts in and entangle with
slopGateMode: block thresholds in ways not asked for by the issue.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed. (Deliberately not auto-closed yet — see Notes.)

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally — full unsharded run: 510 test files (508 passed, 2 skipped by design), 10215 tests passed, 0 failed. New file src/signals/boundary-test-generation.ts: 100% lines (36/36), 100% branches (23/23). Touched src/signals/focus-manifest.ts: 99.92% lines, 99.38% branches (only pre-existing unrelated gaps remain). Touched src/mcp/server.ts: 98.12% lines, 94.14% branches (pre-existing gaps unrelated to this change; verified none of my added lines are in the uncovered set).
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — test/unit/boundary-test-generation.test.ts (26 tests covering every pattern kind, both branch sides, the fail-safe no-patch path, the MAX_TOUCHES cap, snippet truncation, and dedup), test/unit/mcp-suggest-boundary-tests.test.ts (3 tests for the new MCP tool), plus extended coverage in test/unit/focus-manifest.test.ts for the new review.test_generation parse/round-trip/resolver.

Also ran (not in the template checklist, but part of the full local gate): npm run db:migrations:check, npm run db:schema-drift:check, npm run cf-typegen:check, npm run selfhost:env-reference:check, npm run docs:drift-check, npm run command-reference:check, npm run ui:openapi:settings-parity, npm run ui:test, npm run test:engine-parity, npm run rees:test — all green. No regeneration was needed for any of them (no API/OpenAPI/Cloudflare-binding/Drizzle-schema/self-host-env changes in this PR).

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A — no auth/cookie/CORS/session code touched.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (New MCP tool gittensory_suggest_boundary_tests, tested; no OpenAPI/HTTP route change.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI changes.)
  • Visible UI changes include a UI Evidence section below with screenshots. (N/A — backend/MCP only, no visible UI change.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (Updated .gittensory.yml.example and config/examples/gittensory.full.yml documentation for the new review.test_generation knob; did not touch CHANGELOG.md.)

UI Evidence

N/A — this PR has no visible UI/frontend change (backend signal + MCP tool only).

Notes

  • Related to Boundary-safe test generation #1972, not auto-closed on purpose: the milestone groups several related deliverables and a maintainer should confirm Boundary-safe test generation #1972 is fully satisfied before it closes on merge.
  • Conservative scope decision (noted in the issue itself as a fallback for design ambiguity): full test-code generation is out of scope by the issue's own text ("Generating tests server-side would break the contributor boundary"); this PR ships the deterministic detector + advisory finding + LOCAL-execution action spec (criteria/hints only), gated off by default.

Detects a small, precise set of boundary-condition patterns (off-by-one
array/index bounds, null/undefined branches, empty-collection checks)
touched by a PR's diff with no accompanying test evidence, and returns an
advisory finding plus a LOCAL-execution test-generation action spec
(criteria/hints only, mirroring local-write-tools.ts's no-cloud-write
boundary -- gittensory never writes or runs test code).

Gated by the new review.test_generation .gittensory.yml knob, wired
through focus-manifest.ts exactly like review.effort_score/
changed_files_summary (default OFF, byte-identical when unset). Exposed
via a new gittensory_suggest_boundary_tests MCP tool alongside the
existing gittensory_check_slop_risk / gittensory_check_issue_slop
pure-metadata tools.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.34%. Comparing base (ca7a002) to head (d50b664).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3794   +/-   ##
=======================================
  Coverage   93.33%   93.34%           
=======================================
  Files         317      318    +1     
  Lines       32467    32515   +48     
  Branches    11901    11913   +12     
=======================================
+ Hits        30303    30351   +48     
  Misses       1530     1530           
  Partials      634      634           
Files with missing lines Coverage Δ
src/mcp/server.ts 95.79% <100.00%> (+0.05%) ⬆️
src/signals/boundary-test-generation.ts 100.00% <100.00%> (ø)
src/signals/focus-manifest.ts 99.11% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 6, 2026
@loopover-orb

loopover-orb Bot commented Jul 6, 2026

Copy link
Copy Markdown

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-06 10:54:41 UTC

9 files · 1 AI reviewer · no blockers · readiness 82/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Adds a new, self-contained, config-gated advisory (`review.test_generation`, default off) that scans changed-file patches for three narrow boundary-condition patterns and, when found with no accompanying test evidence, returns an advisory finding plus a local-execution test-generation hint spec via a new pure-metadata MCP tool. The wiring through focus-manifest.ts (parse/empty-defaults/present/serialize/resolver) exactly mirrors the existing `effort_score`/`changed_files_summary` boolean-flag pattern, is additive-only, and ships with solid unit and MCP-integration test coverage for both the detector and the config plumbing. The feature is deliberately not wired into the AI-review orchestration (queue/processors.ts), which the PR explains as an intentional smallest-scope decision rather than an oversight.

Nits — 6 non-blocking
  • src/signals/boundary-test-generation.ts: the array_index_bounds regex's second alternative `\.length\s*(?:-\s*1)?\s*[<>]=?` is broad enough to match plain length comparisons like `arr.length > 10` that have nothing to do with an off-by-one bound, which will occasionally mislabel/over-trigger the finding (acceptable given the advisory-only severity, but worth tightening if false positives show up in practice).
  • Magic numbers in src/mcp/server.ts (400/20000/500/2000 length caps) and the `paths.slice(0, 5)` truncation in boundary-test-generation.ts:109 are unexplained inline literals; naming them as constants would match the style used for MAX_SNIPPET_LENGTH/MAX_TOUCHES just above.
  • The detector never runs unless a caller (MCP tool) explicitly supplies file patches — worth a one-line doc note in README/docs alongside the other `review.*` flags so operators know this is MCP-only for now, not part of the automated review comment.
  • Consider extracting the path-count truncation threshold (5) in boundary-test-generation.ts:109 into a named constant alongside MAX_SNIPPET_LENGTH/MAX_TOUCHES for consistency.
  • If false positives from the broad `.length [<>]=?` sub-pattern become an issue, consider requiring the `-1` or `.length - 1` form more strictly for the array_index_bounds kind and moving plain length comparisons fully into empty_collection_check.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #1972
Related work ⚠️ 1 scoped overlap Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 54 registered-repo PR(s), 46 merged, 407 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 54 PR(s), 407 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 54 PR(s), 407 issue(s).
  • Related work: Titles/paths share 8 meaningful terms. (issue #2189, issue #2184)
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 199475e into main Jul 6, 2026
10 checks passed
@loopover-orb
loopover-orb Bot deleted the worktree-wf_c9b16cbb-862-4 branch July 6, 2026 10:54
JSONbored added a commit that referenced this pull request Jul 7, 2026
)

#3794 and #3795 both landed a review.test_generation entry in
.gittensory.yml.example and config/examples/gittensory.full.yml within
an hour of each other; the rebase collision was resolved for the field
itself in focus-manifest.ts but left two separate documentation blocks
per file (one per PR's framing) instead of one. Collapse each pair into
a single entry that keeps the fuller behavioral description alongside
the operator-flag callout, matching how every other kill-switched
review.* field documents itself once.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Development

Successfully merging this pull request may close these issues.

1 participant