Skip to content

feat(review): add config-driven before/after screenshot-table gate - #3877

Merged
JSONbored merged 3 commits into
mainfrom
feat/screenshot-table-gate-2006
Jul 7, 2026
Merged

feat(review): add config-driven before/after screenshot-table gate#3877
JSONbored merged 3 commits into
mainfrom
feat/screenshot-table-gate-2006

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • Makes the previously documented-only requirement (a contributor visual/frontend PR needs a before/after screenshot table in its body or the gate auto-closes it) config-driven per repo, matching Config-driven gate rule: auto-close visual/frontend PRs missing a before/after screenshot table #2006. Follows the exact pattern of the existing anti-abuse mechanisms in this codebase (contributor_blacklist, review_nag, review_evasion_protection): a repository_settings row (not a .gittensory.yml review.* knob), off by default, opt-in per repo.
  • migrations/0117_screenshot_table_gate.sql + src/db/schema.ts: adds screenshot_table_gate_enabled (default off), screenshot_table_gate_when_labels_json/screenshot_table_gate_when_paths_json (either-match scoping, both empty = every PR), screenshot_table_gate_action (close/request_changes/comment, default close), screenshot_table_gate_message (nullable, null = built-in templated message).
  • src/review/screenshot-table-gate.ts: the deterministic (no AI) detector -- a PR "has a screenshot table" when its body contains a markdown table region with at least one image (![alt](url) or <img>). Also flags an image pasted outside any table, or a screenshot file committed to the repo under a scoped path (should be uploaded to the PR body instead).
  • src/db/repositories.ts + src/openapi/schemas.ts + src/settings/agent-actions.ts: settings round-trip (get/upsert), OpenAPI schema, and wiring into the agent-action planner so a violating PR gets the configured action.
  • .gittensory.yml.example / config/examples/gittensory.full.yml: documents the new setting.
  • Found and fixed en route: gittensory.full.yml (our own reference doc, round-trip parsed by config-templates.test.ts) had already reached 65522/65536 bytes on main before this PR's own doc addition -- any concurrent PR's config-doc growth would have tripped the old 64 KiB MAX_FOCUS_MANIFEST_BYTES ceiling next. Raised it to 128 KiB (src/signals/focus-manifest.ts) with a comment explaining why; a real per-repo .gittensory.yml never needs anywhere near this size, so the DoS-guard intent is unaffected.
  • Also added a regression test for parseJsonStringArray's non-array (valid JSON, wrong shape, e.g. {"a":1}) fail-closed branch, previously only covered by the JSON-syntax-error path.

Closes #2006.

Scope

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally -- 100% branch coverage on every changed line/branch in this diff (verified via targeted lcov branch inspection against the exact diff ranges).
  • 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.

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/session/CORS surface touched.)
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A -- no UI change.)
  • Visible UI changes include a UI Evidence section. (N/A -- backend-only change, no UI.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

Notes

  • The manifest-size-ceiling bump and the migration renumbering were both pre-existing/concurrent collisions discovered while finishing this PR, not scope creep -- both are one-line, low-risk fixes required to get a green gate.

JSONbored added 2 commits July 6, 2026 16:52
…2006)

Makes the previously documented-only requirement (a contributor visual/
frontend PR needs a before/after screenshot table or it gets auto-closed)
a per-repo, config-as-code gate: off by default, scoped by label/path, with
a configurable action and contract message, layered the same way as the
existing blacklist/review-nag/review-evasion anti-abuse mechanisms.
…g, close coverage gaps

Renumbers 0115_screenshot_table_gate.sql to 0117 after a concurrently-merged
PR claimed 0115. Raises MAX_FOCUS_MANIFEST_BYTES from 64 KiB to 128 KiB --
gittensory.full.yml (our own reference doc, round-trip tested by
config-templates.test.ts) had already reached 65522/65536 bytes on main
before this PR's own doc addition, so any concurrent PR's config-doc growth
would have tripped the old ceiling next; a real per-repo .gittensory.yml
never needs anywhere near this size, so the DoS-guard intent is unaffected.
Adds a regression test for parseJsonStringArray's non-array (valid JSON,
wrong shape) fail-closed branch, previously only covered by the JSON-syntax-
error path.
@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 5118371 Commit Preview URL

Branch Preview URL
Jul 07 2026, 12:07 AM

@superagent-security

Copy link
Copy Markdown
Contributor

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

@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.56%. Comparing base (daef526) to head (60ba041).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3877      +/-   ##
==========================================
+ Coverage   93.54%   93.56%   +0.02%     
==========================================
  Files         338      339       +1     
  Lines       33335    33445     +110     
  Branches    12192    12242      +50     
==========================================
+ Hits        31182    31292     +110     
  Misses       1528     1528              
  Partials      625      625              
Files with missing lines Coverage Δ
src/db/repositories.ts 96.61% <100.00%> (+0.01%) ⬆️
src/db/schema.ts 71.18% <ø> (ø)
src/openapi/schemas.ts 100.00% <ø> (ø)
src/queue/processors.ts 94.53% <100.00%> (+<0.01%) ⬆️
src/review/screenshot-table-gate.ts 100.00% <100.00%> (ø)
src/settings/agent-actions.ts 97.69% <100.00%> (+0.08%) ⬆️
src/signals/focus-manifest.ts 99.15% <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:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 7, 2026
@loopover-orb

loopover-orb Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Tip

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

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-07 00:38:18 UTC

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

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
This PR adds a deterministic, config-driven before/after screenshot-table gate (#2006), following the established repository_settings pattern (contributor_blacklist/review_nag shape): schema+migration, a pure evaluator (screenshot-table-gate.ts), DB round-trip, focus-manifest sparse-override wiring, OpenAPI schema, and short-circuit integration into the close planner ahead of merit/CI/AI analysis. The migration is D1-safe (plain ADD COLUMN, no forbidden statements), the planner short-circuit correctly excludes owner/admin/automation-bot authors and is exempt from the close-precision breaker matching sibling mechanisms, and test coverage is thorough (unit tests for the evaluator, DB round-trip including malformed-JSON fail-closed cases, planner short-circuit, and end-to-end queue processor tests for both the violating and compliant paths). The one notable gap is that `action: comment`/`request_changes` are exposed in the schema and documented in the example config as valid options but are silently no-ops today (only `action: close` is wired into any actual effect) — a self-hoster who configures either of those modes gets no visible enforcement at all, which isn't called out anywhere a config author would see it.

Nits — 6 non-blocking
  • src/queue/processors.ts and src/settings/agent-actions.ts only wire `action: close` to any real effect — `request_changes`/`comment` are documented in .gittensory.yml.example as valid actions but currently produce zero visible behavior, which should be called out in the example config comment so an operator doesn't silently configure a no-op.
  • src/review/screenshot-table-gate.ts:102 — the separator-row regex nests `\s*`/`:?`/`-{3,}` inside a `(...)+` group; while the disjoint character classes likely keep backtracking near-linear in practice, consider simplifying or adding a fuzz/perf test to remove any doubt given it runs on attacker-controlled PR body text.
  • src/signals/focus-manifest.ts — bumping MAX_FOCUS_MANIFEST_BYTES from 64 KiB to 128 KiB is bundled into this feature PR as a 'found en route' fix; it's a reasonable and well-justified change but is scope creep relative to the PR's stated purpose and could have shipped as its own small PR.
  • migrations/0117_screenshot_table_gate.sql — please confirm 0117 is in fact the next contiguous migration number on main (not verifiable from this diff alone) since a duplicate/gap here is a deploy-time blocker per the D1 migration ledger rules.
  • The external brief flags 'blacklist' terminology reused from existing code (agent-actions.ts, repositories.ts, schema.ts) — this PR only follows the existing convention rather than introducing new instances, so it's optional/out-of-scope for this diff.
  • 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 #2006
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low 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: 51 registered-repo PR(s), 43 merged, 352 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 51 PR(s), 352 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: 51 PR(s), 352 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
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 added the manual-review Gittensor contributor context label Jul 7, 2026
@JSONbored
JSONbored merged commit 1cd38c4 into main Jul 7, 2026
12 checks passed
@JSONbored
JSONbored deleted the feat/screenshot-table-gate-2006 branch July 7, 2026 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Config-driven gate rule: auto-close visual/frontend PRs missing a before/after screenshot table

1 participant