Skip to content

fix(review): keep recap cohorts private#5204

Merged
JSONbored merged 1 commit into
mainfrom
codex/fix-miner-cohort-gate-metrics-leak
Jul 12, 2026
Merged

fix(review): keep recap cohorts private#5204
JSONbored merged 1 commit into
mainfrom
codex/fix-miner-cohort-gate-metrics-leak

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The scheduled maintainer recap was opting loadGatePrecisionReport(..., { includeCohorts: true }) and rendering miner-vs-human cohort diagnostics into external Discord/Slack digests, which exposes sensitive maintainer-review/identity signals on public surfaces.
  • The change restores the public/private boundary by keeping cohort diagnostics as a maintainer-only diagnostic and preventing cohort labels/counts from appearing in externally-delivered recap bodies.
  • Add defense-in-depth by extending the shared public redaction vocabulary to catch cohort-related labels if they ever surface.

Description

  • Stop requesting cohort data for scheduled digests by removing includeCohorts: true at the scheduled loader call in src/review/maintainer-recap-wire.ts so periodic recap deliveries remain aggregate-only.
  • Keep the internal totals.cohorts shape available to callers that inject cohort-bearing reports, but remove cohort-derived lines from the public summary and omit the ## Cohorts section from the formatted public body in src/services/maintainer-recap.ts and formatMaintainerRecap.
  • Extend PUBLIC_UNSAFE_TERMS in src/signals/redaction.ts to include cohort diagnostic vocabulary (cohort, miner-originated, human-originated) as defense-in-depth against accidental public leakage.
  • Update unit tests to assert that cohort diagnostics are not exposed on the public recap path (test/unit/maintainer-recap.test.ts, test/unit/maintainer-recap-format.test.ts, test/unit/maintainer-recap-wire.test.ts).
  • Regenerate Cloudflare type output via the local typegen run resulting in an updated worker-configuration.d.ts to keep the type-check/generation gate current.

Testing

  • Ran targeted unit tests: npx vitest run test/unit/maintainer-recap.test.ts test/unit/maintainer-recap-format.test.ts test/unit/maintainer-recap-wire.test.ts --reporter=dot and all targeted tests passed (44 tests across 3 files).
  • Typecheck: npm run typecheck succeeded with no reported errors.
  • Cloudflare types: npm run cf-typegen executed and updated worker-configuration.d.ts to match the environment.
  • Full CI: npm run test:ci was attempted but failed in unrelated, existing queue/sweep integration tests (timing/remote setup) in this environment; the maintainer-recap-specific tests used for this fix passed.
  • Dependency audit: npm audit --audit-level=moderate could not complete in this environment (registry returned 403), so audit was not verified here.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

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

@JSONbored
JSONbored force-pushed the codex/fix-miner-cohort-gate-metrics-leak branch from c4cb091 to 0a15958 Compare July 12, 2026 09:47
@codecov

codecov Bot commented Jul 12, 2026

Copy link
Copy Markdown

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5204      +/-   ##
==========================================
- Coverage   94.35%   94.34%   -0.01%     
==========================================
  Files         473      473              
  Lines       39977    39968       -9     
  Branches    14574    14570       -4     
==========================================
- Hits        37719    37708      -11     
  Misses       1585     1585              
- Partials      673      675       +2     
Flag Coverage Δ
shard-1 46.42% <100.00%> (-0.04%) ⬇️
shard-2 34.58% <100.00%> (+0.02%) ⬆️
shard-3 31.97% <100.00%> (+1.00%) ⬆️
shard-4 32.20% <100.00%> (-1.11%) ⬇️
shard-5 33.69% <100.00%> (+<0.01%) ⬆️
shard-6 44.88% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/gittensory-engine/src/prompt-packet.ts 100.00% <100.00%> (ø)
src/review/maintainer-recap-wire.ts 98.07% <ø> (ø)
src/services/maintainer-recap.ts 96.77% <ø> (-3.23%) ⬇️
src/signals/redaction.ts 100.00% <100.00%> (ø)
🚀 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 12, 2026
@loopover-orb

loopover-orb Bot commented Jul 12, 2026

Copy link
Copy Markdown

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 Gittensory review result - fixes required

Review updated: 2026-07-12 09:54:57 UTC

6 files · 1 AI reviewer · 2 blockers · readiness 93/100 · CI failing · blocked

🛑 Suggested Action - Manual Review

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

Review summary
This PR removes the `includeCohorts: true` opt-in at the scheduled maintainer-recap call site (src/review/maintainer-recap-wire.ts) so periodic Discord/Slack digests no longer request miner-vs-human cohort data, strips the cohort summary line and `## Cohorts` section from the public formatter (src/services/maintainer-recap.ts), and adds `cohort`/`miner-originated`/`human-originated` to the shared `PUBLIC_UNSAFE_TERMS` redaction vocabulary as defense-in-depth (src/signals/redaction.ts). The fix is applied at the correct source (the request-time opt-in) rather than only patching the renderer, and the updated tests in maintainer-recap-wire.test.ts exercise the real production path by asserting `report.totals.cohorts` is undefined and the formatted body omits cohort text end-to-end. The internal `totals.cohorts` shape is deliberately retained for callers that inject cohort-bearing reports directly, but `formatMaintainerRecap` no longer renders it under any circumstance, and the new redaction terms provide a second layer of protection if a cohort string ever reaches a summary line.

Nits — 5 non-blocking
  • src/services/maintainer-recap.ts still computes and stores `totals.cohorts` (and `repo.cohorts`) internally even though no current call site ever populates it via `includeCohorts`, and `formatMaintainerRecap` never reads it — worth a one-line comment clarifying this is intentionally kept only for callers that inject reports directly, so a future reader doesn't assume it's dead code to delete.
  • test/unit/maintainer-recap-format.test.ts's new cohort test manually injects `totals.cohorts` and a hand-built `summary` array with cohort strings rather than driving the whole thing through `buildMaintainerRecap`, so it verifies the formatter's redaction defense-in-depth but not that `buildMaintainerRecap` itself would ever produce such a summary line — the wire-level test compensates for this by covering the real end-to-end path.
  • Consider a short code comment on `totals.cohorts`/`repo.cohorts` in src/services/maintainer-recap.ts explaining they're intentionally unrendered, to prevent a future cleanup pass from either deleting them or accidentally wiring them back into the formatter.
  • If any other call site besides the scheduled job still passes `includeCohorts: true` to `loadGatePrecisionReport` (e.g. an internal/maintainer-only diagnostic endpoint), it'd strengthen the review to confirm that surface is genuinely maintainer-authenticated and never reachable from the public formatter path.
  • src/services/maintainer-recap.ts:107: Keep the internal cohort aggregation only if another maintainer-authenticated caller still consumes `totals.cohorts`; otherwise consider removing that retained report shape in a follow-up to reduce private-field drift.

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
  • Maintainer requires a linked issue — Link the relevant issue (for example Closes #123) before opening the PR.

CI checks failing

  • validate
  • validate-tests (1)
Signal Result Evidence
Code review ❌ 2 blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
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 (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 45 registered-repo PR(s), 37 merged, 409 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 45 PR(s), 409 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence.
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: not available
  • Official Gittensor activity: 45 PR(s), 409 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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.
[BETA] Chat with Gittensory

Ask Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands

🟩 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 12, 2026
Removes includeCohorts: true from the scheduled maintainer-recap's call
to loadGatePrecisionReport -- cohort splits (miner-vs-human gate
false-positive rates) are maintainer-authenticated diagnostics via the
gate-precision API route, not content for the unauthenticated Discord/
Slack recap webhook. Extends PUBLIC_UNSAFE_TERMS with cohort/miner-
originated/human-originated as defense-in-depth.

Also syncs packages/gittensory-engine/src/prompt-packet.ts's hand-
duplicated mirror of PUBLIC_UNSAFE_TERMS (kept separate so the engine
package stays standalone) with the three new terms, and adds the two
new [-_\s]?-shaped terms to prompt-packet-redaction.test.ts's explicit
per-branch sample table -- both fell through to the test's generic
fallback (which uses the raw regex source itself as the sample text),
so neither the mirror drift nor the missing sample surfaced until the
adversarial redaction test's dynamically-generated cases failed in CI.
@JSONbored
JSONbored force-pushed the codex/fix-miner-cohort-gate-metrics-leak branch from 0a15958 to 7d14161 Compare July 12, 2026 09:56
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 12, 2026

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 7d14161 Commit Preview URL

Branch Preview URL
Jul 12 2026, 09:57 AM

@JSONbored
JSONbored merged commit c772d74 into main Jul 12, 2026
6 of 7 checks passed
@JSONbored
JSONbored deleted the codex/fix-miner-cohort-gate-metrics-leak branch July 12, 2026 09:57
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. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant