Skip to content

fix(orb): SKILL.md linked-issue trigger must key on the gate-mode enforcement authority (#9671) - #9923

Closed
kai392 wants to merge 1 commit into
JSONbored:mainfrom
kai392:fix/9671-skill-linked-issue-gate
Closed

fix(orb): SKILL.md linked-issue trigger must key on the gate-mode enforcement authority (#9671)#9923
kai392 wants to merge 1 commit into
JSONbored:mainfrom
kai392:fix/9671-skill-linked-issue-gate

Conversation

@kai392

@kai392 kai392 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What

repo-skill-render.ts asserted "a linked issue is required" from two different fields in the same generated file. hasStrictLinkedIssueRule keyed on requireLinkedIssue && linkedIssuePolicy !== "optional", while the Linked-issues section keyed on linkedIssueGateMode === "block". For a repo with requireLinkedIssue: true, linkedIssuePolicy: "required", linkedIssueGateMode: "advisory" — a real, common combo (advisory is the default gate mode) — the generated SKILL.md said "A linked issue is required, with a 'required' policy." a few lines above "Required: no". Since this file is written into contributor repos as .claude/skills/contributing-to-<repo>/SKILL.md and read by AI coding agents, that contradiction is directly actionable misinformation. Only "block" mode actually blocks (repo-profile.ts:68-70). hasStrictLinkedIssueRule is also one of shouldGenerateRepoSkill's three signals, so a skill file could be generated off a signal that enforces nothing.

How

  • hasStrictLinkedIssueRule now keys on linkedIssueGateMode === "block" — the documented enforcement authority, matching repo-doc-render.ts:106.
  • renderTriggerReasons' sentence states the blocking mode (...the linked-issue gate is in "block" mode.) and only fires when the gate actually blocks.
  • The Linked-issues section (Policy: / Required:) is unchanged.

Tests

  • The linked-issue signal counts toward shouldGenerateRepoSkill only under block mode (Deliverable 1).
  • REGRESSION: the full advisory-mode SKILL.md never claims a linked issue is required and says Required: no (Deliverable 2).
  • The block-mode doc states a linked issue is required and says Required: yes — the two agree (Deliverable 3).
  • Advisory linked-issue + multi-stage CI no longer reaches the 2-of-3 threshold (Deliverable 4).

Existing fixtures that meant a genuine strict rule now set block mode. All new/updated assertions fail against the current field choice.

Closes #9671

@kai392
kai392 requested a review from JSONbored as a code owner July 29, 2026 23:22
@loopover-orb

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Caution

🛑 LoopOver review result - fixes required

Review updated: 2026-07-29 23:32:08 UTC

2 files · 1 AI reviewer · no blockers · CI failing · unstable

🛑 Suggested Action - Fix Blockers

Review summary
This PR fixes a real self-contradiction bug: `hasStrictLinkedIssueRule` was keying on `requireLinkedIssue`/`linkedIssuePolicy` while the rendered Linked-issues section keyed on `linkedIssueGateMode === "block"`, so a common advisory-mode + required-policy combo produced a generated SKILL.md that claimed both 'A linked issue is required' and 'Required: no' in the same file. The fix correctly aligns the trigger signal on the actual enforcement authority (`linkedIssueGateMode === "block"`), matching the existing Linked-issues section and `repo-doc-render.ts:106`, and the test suite is updated thoroughly with fixtures reflecting the new semantics plus a dedicated regression describe block. The change is narrow, well-targeted, and directly traceable to the documented gate logic (`hasBlockingGate` mirrors the same field-authority pattern already used elsewhere).

Nits — 4 non-blocking
  • src/review/repo-skill-render.ts:27 — the comment cites 'repo-profile.ts:68-70' as a magic line reference; consider referencing the field/constant name instead of line numbers, which will drift as the file changes.
  • The PR body says 'Existing fixtures that meant a genuine strict rule now set block mode' — worth double-checking no other test file outside the two shown here also constructs `contributionWorkflow` with the old strict-rule shape, since that would silently diverge from the new semantics.
  • Consider extracting the repeated `{ requireLinkedIssue: true, linkedIssuePolicy: "required" as const, linkedIssueGateMode: ... }` literals in the new describe block (test/unit/repo-skill-render.test.ts:53-55) into a small shared builder if similar patterns recur elsewhere in the suite.
  • Given `linkedIssuePolicy` is now purely descriptive (only `linkedIssueGateMode` drives the trigger), a short doc/type comment on `RepoProfileContributionWorkflow.linkedIssuePolicy` clarifying that it's display-only relative to gating could prevent a future reintroduction of this exact bug.

CI checks failing

  • validate
  • validate-tests

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #9671
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: 101 registered-repo PR(s), 55 merged, 7 issue(s).
Contributor context ✅ Confirmed Gittensor contributor kai392; Gittensor profile; 101 PR(s), 7 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Addressed
The diff changes hasStrictLinkedIssueRule to key solely on linkedIssueGateMode === "block", updates the trigger sentence to state the blocking mode, leaves the Linked-issues section and repo-profile.ts/repo-doc-render.ts untouched, and adds named tests covering both arms of the condition, the rendered-body regression (advisory vs block), and the shouldGenerateRepoSkill threshold change.

Review context
  • Author: kai392
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Python, Cuda, JavaScript, Kotlin, MDX, Perl, Ruby, TypeScript
  • Official Gittensor activity: 101 PR(s), 7 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
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.
🧪 Chat with LoopOver

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

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

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 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 LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@superagent-security

Copy link
Copy Markdown
Contributor

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

…orcement authority (JSONbored#9671)

repo-skill-render.ts asserted "a linked issue is required" from two
different fields in the same generated file: hasStrictLinkedIssueRule keyed
on `requireLinkedIssue && linkedIssuePolicy !== "optional"`, while the
Linked-issues section keyed on `linkedIssueGateMode === "block"`. For a repo
with requireLinkedIssue:true, policy:"required", gateMode:"advisory" (a real,
common combo — advisory is the default), the generated SKILL.md said
"A linked issue is required, with a 'required' policy." a few lines above
"Required: no" — directly contradictory misinformation, since only "block"
mode actually blocks (repo-profile.ts:68-70). hasStrictLinkedIssueRule is
also one of shouldGenerateRepoSkill's three signals, so a skill file could be
generated off a signal that enforces nothing.

- hasStrictLinkedIssueRule now keys on `linkedIssueGateMode === "block"`, the
  documented enforcement authority, matching repo-doc-render.ts:106.
- renderTriggerReasons' sentence states the blocking mode ("...gate is in
  'block' mode.") and only fires when the gate actually blocks.
- The Linked-issues section (Policy/Required) is unchanged.

Tests: the signal counts only under block mode; a regression rendering the
full advisory-mode SKILL.md asserts it never claims a linked issue is
required while saying "Required: no"; the block-mode doc agrees in both
directions; and advisory linked-issue + multi-stage CI no longer reaches the
2-of-3 threshold. Existing fixtures that meant a genuine strict rule now set
block mode. All fail against the current field choice.

Closes JSONbored#9671

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

❌ 3 Tests Failed:

Tests completed Failed Passed Skipped
4502 3 4499 11
View the top 3 failed test(s) by shortest run time
test/unit/repo-doc-pr.test.ts > openRepoDocPullRequest (#3000) > #3001: adds a first-run skill file to the SAME tree/commit/PR as AGENTS.md when the trigger fires and scope includes "skills"
Stack Traces | 0.139s run time
AssertionError: expected [ 'AGENTS.md', 'CLAUDE.md' ] to deeply equal [ 'AGENTS.md', 'CLAUDE.md', …(1) ]

- Expected
+ Received

  [
    "AGENTS.md",
    "CLAUDE.md",
-   "..../skills/contributing-to-widgets/SKILL.md",
  ]

 ❯ test/unit/repo-doc-pr.test.ts:657:45
test/unit/repo-doc-pr.test.ts > openRepoDocPullRequest (#3000) > #3001: overwrites a hand-maintained skill file with a fresh generate when allowOverwriteExisting is set
Stack Traces | 0.21s run time
AssertionError: the given combination of arguments (undefined and string) is invalid for this assertion. You can use an array, a map, an object, a set, a string, or a weakset instead of a string
 ❯ test/unit/repo-doc-pr.test.ts:775:33
test/unit/repo-doc-pr.test.ts > openRepoDocPullRequest (#3000) > #3001: opens a PR for a skill-only change even when AGENTS.md itself is unchanged
Stack Traces | 0.223s run time
AssertionError: expected [ 'AGENTS.md', 'CLAUDE.md' ] to deeply equal [ 'AGENTS.md', 'CLAUDE.md', …(1) ]

- Expected
+ Received

  [
    "AGENTS.md",
    "CLAUDE.md",
-   "..../skills/contributing-to-widgets/SKILL.md",
  ]

 ❯ test/unit/repo-doc-pr.test.ts:693:45

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

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

loopover-orb Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests)). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

@loopover-orb loopover-orb Bot closed this Jul 29, 2026
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

orb(review): generated SKILL.md contradicts itself on whether a linked issue is required

2 participants