Skip to content

feat(pmf): advisory newcomer-PR auto-guide (Phase-1-lite, advisory on… - #917

Closed
YB0y wants to merge 7 commits into
JSONbored:mainfrom
YB0y:feat/newcomer-pr-auto-guide-803
Closed

feat(pmf): advisory newcomer-PR auto-guide (Phase-1-lite, advisory on…#917
YB0y wants to merge 7 commits into
JSONbored:mainfrom
YB0y:feat/newcomer-pr-auto-guide-803

Conversation

@YB0y

@YB0y YB0y commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements the opt-in advisory newcomer-PR auto-guide feature, closing #803 (Phase-1-lite, advisory only). Adds a new newcomerGuideMode per-repo setting with two modes:

Mode Behavior
off (default) Feature disabled — no change to existing behaviour
enabled Post a one-time welcoming advisory comment on first-time-contributor PRs with specific, actionable guidance derived from the gate findings

The guide reuses the #552 newcomer detection (authorMergedPrCount === 0), surfaces gate findings in a newcomer-friendly way (anti-slop reminders, how-to-fix tips, merge-worthy checklist), and is strictly advisory — never blocks, never auto-merges.

Related Issue

Closes: #803

Change Type

  • New feature
  • Database migration (additive, non-breaking)
  • New signal / logic module
  • Config-as-code (.gittensory.yml settings block)
  • OpenAPI schema update
  • Unit tests added

Real Behaviour Proof

Newcomer guide comment generation (unit-testable, no network)

// Given: a first-time contributor PR with a missing-linked-issue finding
buildNewcomerGuideComment({
  authorLogin: "alice",
  pullNumber: 1,
  title: "Fix login bug",
  repoFullName: "JSONbored/gittensory",
  advisory: { findings: [{ code: "missing_linked_issue", ... }] },
  gateBlocking: true,
})
// → Comment body includes:
//   "<!-- gittensory-newcomer-guide:v1 -->"
//   "## Welcome, @alice! 👋"
//   "### Link a related issue"
//   "### What makes a PR merge-worthy"
//   "⚠️ The Gittensory Gate has flagged blockers..."

Non-newcomer guard (verified by processor logic)

// Author has 2 merged PRs → authorHistory.mergedPrCount = 2
// newcomerGuideMode = "enabled"
// Condition: mergedPrCount === 0 → FALSE → guide NOT posted

Idempotency (verified by marker-based comment system)

// First webhook: posts comment with marker <!-- gittensory-newcomer-guide:v1 -->
// Subsequent webhooks: finds existing comment by marker, updates in place (no duplicate)

Migration — additive only

ALTER TABLE repository_settings ADD COLUMN newcomer_guide_mode TEXT NOT NULL DEFAULT 'off';
-- Existing rows get 'off' automatically. No data loss. No backfill needed.

Checklist

  • Migration is additive (ALTER TABLE … ADD COLUMN … DEFAULT 'off') — zero downtime, safe rollback
  • newcomerGuideMode defaults to "off" everywhere (DB default, code default, getRepositorySettings missing-row path) — existing repos are unaffected
  • Guide fires independently of publicSurface/commentMode — works even when comments are off
  • Advisory only — never blocks, never auto-merges (explicitly NOT feat(agent): maintainer write-actions layer (request-changes/approve/merge/close/label) #778)
  • Best-effort: wrapped in try/catch, failures are audited via recordAuditEvent, never aborts the gate or public surface
  • Newcomer detection reuses authorHistory.mergedPrCount already computed for feat(github-app): first-time-contributor-aware gating #552 — no extra DB query
  • Idempotent: uses <!-- gittensory-newcomer-guide:v1 --> marker to find-and-update (never duplicates)
  • Public-safe: all guidance text is static and curated (no user-controlled content in the comment)
  • Config-as-code: gate.newcomerGuide: enabled in .gittensory.yml + settings.newcomerGuideMode override
  • OpenAPI schema updated for both RepositorySettings and RepoSettingsPreviewSchema
  • 9 unit tests cover: marker, welcome message, finding-specific guidance, merge-worthy checklist, gate blocking/non-blocking, dedup, truncation
  • All 2171 tests pass, typecheck clean

…ly) (JSONbored#803)

Add opt-in per-repo setting newcomerGuideMode (off/enabled, default off).
When enabled, the webhook posts a one-time welcoming advisory comment on
first-time-contributor PRs (0 merged PRs in the repo, reusing JSONbored#552 detection).

The guide surfaces gate findings in a newcomer-friendly way: what to fix,
what makes a PR merge-worthy, and an anti-slop reminder. Advisory only —
never blocks, never auto-merges. All text is public-safe.

- migrations/0047: add newcomer_guide_mode column (default 'off')
- src/types.ts: newcomerGuideMode field on RepositorySettings
- src/db/schema.ts + repositories.ts: column, defaults, parse, upsert
- src/signals/newcomer-guide.ts: buildNewcomerGuideComment message builder
- src/github/comments.ts: NEWCOMER_GUIDE_COMMENT_MARKER + posting helper
- src/queue/processors.ts: fires after gate finalization, before public surface;
  independent of publicSurface/commentMode so it works even when comments are off;
  best-effort (try/catch + audit), idempotent (marker-based update)
- src/signals/focus-manifest.ts: gate.newcomerGuide alias + settings override
- src/signals/settings-preview.ts + src/openapi/schemas.ts: preview/API surface
- test/unit/newcomer-guide.test.ts: 9 unit tests covering marker, welcome,
  finding-specific guidance, merge-worthy checklist, gate status, dedup, truncation

Closes JSONbored#803
@ghost

This comment has been minimized.

@ghost

ghost commented Jun 19, 2026

Copy link
Copy Markdown

Note

Gittensory Gate skipped

PR closed before full evaluation. No late first comment was created.

Signal Result Evidence Action
Gate result ⚠️ Skipped #917 is no longer open. No action.

💰 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.

@ghost ghost added the gittensory:reviewed label Jun 19, 2026
@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.61%. Comparing base (2973ff7) to head (69e4d48).
⚠️ Report is 31 commits behind head on main.

Files with missing lines Patch % Lines
src/queue/processors.ts 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #917   +/-   ##
=======================================
  Coverage   96.60%   96.61%           
=======================================
  Files         109      110    +1     
  Lines       14914    14973   +59     
  Branches     5390     5402   +12     
=======================================
+ Hits        14408    14466   +58     
  Misses        104      104           
- Partials      402      403    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jun 19, 2026
…est-effort audit catches

Adds tests across the newcomer-PR auto-guide surface (mode parse/round-trip,
focus-manifest gate + settings, comment wrapper, and processor post/fail/skip
scenarios) to satisfy the codecov patch gate. The two advisory .catch handlers
on the newcomer audit writes are marked v8 ignore next -- they only fire when
the audit write itself rejects, which is unreachable in tests without also
breaking the unhandled miner-detection audit earlier in the same flow.
@ghost

ghost commented Jun 19, 2026

Copy link
Copy Markdown

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

⛔ Maintainer review — Not merged

Reviewed 16 changed file(s) — two independent AI reviewers, synthesized.

ℹ️ This PR conflicts with main and can't be merged as-is. Conflicting file(s): apps/gittensory-ui/public/openapi.json, src/db/repositories.ts, src/github/comments.ts, src/queue/processors.ts, src/signals/focus-manifest.ts, src/signals/settings-preview.ts, test/unit/focus-manifest.test.ts, test/unit/github-comments.test.ts, test/unit/queue.test.ts. Resolve the conflict(s) and open a fresh PR.

Merge readiness: ❌ CI is red — approval withheld until it's green. Failing: see the checks tab.

Summary

The change introduces the newcomer‑guide feature across the stack. Core logic is sound and well‑tested; the new comment marker, DB column, settings handling, and queue processing are correctly wired. Tests cover the comment generation and the end‑to‑end guide posting, and they all pass. Security is unchanged – no new secret handling or external fetches beyond existing patterns. Maintainability is good: the code is modular and follows existing conventions.

However, the OpenAPI schemas were updated without marking the new newcomerGuideMode field as optional. This makes the field required in request/response validation, which will break existing clients that omit it and cause runtime validation errors. Additionally, the CI pipeline reports lint and security‑scan failures, so the PR cannot be merged in its current state.

💡 Nits — non-blocking, optional

  • newcomerGuideMode should be optional in OpenAPI schemas to match DB defaults.
  • Mark newcomerGuideMode as optional in RepositorySettingsSchema and RepoSettingsPreviewSchema (e.g., .optional()).
  • Run the linter and address the reported style/security issues.
  • Ensure the new OpenAPI field is documented consistently with other optional settings.
  • Consider adding more detailed comments in the code to explain the logic behind the newcomer guide feature.
  • Ensure that the newcomer guide feature is thoroughly documented in the project's documentation.
  • Consider adding more tests to cover edge cases, such as handling repositories with no previous PRs or repositories with a large number of contributors.

This repo uses one-shot review: a rejected PR is closed, not iterated in place. Address the above and open a new focused PR.

Route Viewport Before After
/ desktop before desktop after desktop
mobile before mobile after mobile

Before = production · After = this PR's preview deploy.

…-guide-803

# Conflicts:
#	src/queue/processors.ts
lines.push("");
lines.push(`## Welcome, @${input.authorLogin}! 👋`);
lines.push("");
lines.push(`Thanks for your first PR to **${input.repoFullName}** — "${truncate(input.title, 80)}".`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: User-controlled PR title rendered in bot comment without markdown sanitization

PR title from GitHub webhook is interpolated directly into a markdown comment body, allowing content injection.

Escape or sanitize user-controlled values before including them in GitHub markdown comments.

AI prompt
Check if this security scanner issue is valid. If so, understand the root cause and fix it. If appropriate, update or add tests. Keep the change focused and preserve intended behavior.

<file name="src/signals/newcomer-guide.ts">
<violation number="1" location="src/signals/newcomer-guide.ts:84">
<priority>P2</priority>
<title>User-controlled PR title rendered in bot comment without markdown sanitization</title>
<evidence>In buildNewcomerGuideComment, the user-supplied PR title (input.title) is interpolated directly into the guide comment body at lines.push(\`Thanks for your first PR to **${input.repoFullName}** — &quot;${truncate(input.title, 80)}&quot;.\`). The PR title originates from the GitHub webhook payload (pr.title) and is attacker-controllable. No markdown sanitization or escaping is applied. The JSDoc block incorrectly claims &apos;sanitized via sanitizePublicComment&apos;, but no such function exists in the codebase or is called. This allows a malicious contributor to inject arbitrary markdown (including links and formatting) into a comment posted by the trusted bot account, which could be used for phishing or social engineering.</evidence>
<recommendation>Sanitize input.title, input.authorLogin, and any other user-controlled strings before interpolating them into GitHub markdown. At minimum, escape markdown meta-characters (e.g., *, _, [, ], backtick, #, &gt;, |). Consider using a dedicated markdown-sanitization utility, or replace the quoted title with a plain-text representation that strips markdown syntax.</recommendation>
</violation>
</file>

@superagent-security superagent-security Bot added the pr:flagged PR flagged for review by security analysis. label Jun 20, 2026

@JSONbored JSONbored left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

merge conflicts, please resubmit as a clean PR.

@JSONbored JSONbored closed this Jun 20, 2026
@JSONbored JSONbored reopened this Jun 21, 2026
@ghost ghost closed this Jun 21, 2026
@JSONbored JSONbored reopened this Jun 21, 2026
@ghost ghost closed this Jun 21, 2026
@JSONbored JSONbored reopened this Jun 21, 2026
@ghost ghost closed this Jun 21, 2026
YB0y added a commit to YB0y/gittensory that referenced this pull request Jun 23, 2026
…d#917

- Rename newcomer-guide migration from 0047 to 0046 to close the
  0045 -> 0047 gap (check-migrations requires a contiguous sequence).
  The final number for merge into main will be reassigned by the
  maintainer at land time (next free after main).
- Regenerate apps/gittensory-ui/public/openapi.json so the OpenAPI
  artifact reflects the newcomerGuideMode schema additions.
This pull request was closed.
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. pr:flagged PR flagged for review by security analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(pmf): advisory newcomer-PR auto-guide (Phase-1-lite, advisory only)

2 participants