diff --git a/.github/scripts/__tests__/verifier-issue-formatter.test.js b/.github/scripts/__tests__/verifier-issue-formatter.test.js index c17056dcf..3ada5451e 100644 --- a/.github/scripts/__tests__/verifier-issue-formatter.test.js +++ b/.github/scripts/__tests__/verifier-issue-formatter.test.js @@ -12,6 +12,7 @@ const { extractCheckedItems, buildChecklist, isPlaceholderContent, + isMissingInfoGap, looksLikeSectionHeader, looksLikeReferenceLink, stripCheckboxesFromScope, @@ -263,6 +264,37 @@ Blocking gaps: }); }); + describe('isMissingInfoGap', () => { + it('identifies gaps about missing acceptance criteria', () => { + assert.ok(isMissingInfoGap('Provide explicit acceptance criteria in the PR description or linked issue so they can be independently verified.')); + assert.ok(isMissingInfoGap('Blocking gap: Provide explicit acceptance criteria in the PR')); + assert.ok(isMissingInfoGap('Acceptance criteria missing from source')); + }); + + it('identifies gaps about no testable criteria', () => { + assert.ok(isMissingInfoGap('No testable criteria provided')); + assert.ok(isMissingInfoGap('no verifiable requirements in issue')); + }); + + it('rejects actual verification failures', () => { + assert.ok(!isMissingInfoGap('Missing test coverage for new feature')); + assert.ok(!isMissingInfoGap('API returns wrong status code')); + assert.ok(!isMissingInfoGap('Error handling not implemented')); + assert.ok(!isMissingInfoGap('Function returns incorrect value')); + }); + + it('is case insensitive', () => { + assert.ok(isMissingInfoGap('PROVIDE EXPLICIT ACCEPTANCE CRITERIA')); + assert.ok(isMissingInfoGap('No Testable Criteria')); + }); + + it('handles empty and null input', () => { + assert.ok(!isMissingInfoGap('')); + assert.ok(!isMissingInfoGap(null)); + assert.ok(!isMissingInfoGap(undefined)); + }); + }); + describe('looksLikeSectionHeader', () => { it('identifies markdown headers', () => { assert.ok(looksLikeSectionHeader('## Related')); @@ -766,6 +798,53 @@ Blocking gaps: assert.equal(result.hasSubstantiveContent, false); }); + + it('returns false when all gaps are about missing source info', () => { + // Simulates Issue #415 scenario where verifier couldn't verify because source lacked criteria + const verifierOutput = `Verdict: FAIL + +Blocking gaps: +- Provide explicit acceptance criteria in the PR description or linked issue so they can be independently verified. +- Provide explicit acceptance criteria in the PR description or linked issue so they can be independently verified.`; + const prBody = `## Tasks +- [ ] Tasks section missing from source issue + +## Acceptance Criteria +- [ ] Acceptance Criteria section missing from source issue`; + + const result = formatFollowUpIssue({ + verifierOutput, + prBody, + issues: [], + prNumber: 123, + }); + + // Should NOT create follow-up issue since there's nothing actionable + assert.equal(result.hasSubstantiveContent, false); + }); + + it('returns true when gaps include actual verification failures', () => { + const verifierOutput = `Verdict: FAIL + +Blocking gaps: +- Provide explicit acceptance criteria in the PR description or linked issue so they can be independently verified. +- Missing test coverage for new feature`; + const prBody = `## Tasks +- [ ] Tasks section missing from source issue + +## Acceptance Criteria +- [ ] Acceptance Criteria section missing from source issue`; + + const result = formatFollowUpIssue({ + verifierOutput, + prBody, + issues: [], + prNumber: 123, + }); + + // SHOULD create follow-up issue since there's an actual verification failure + assert.equal(result.hasSubstantiveContent, true); + }); }); }); @@ -883,6 +962,41 @@ Something went wrong with the verification.`; assert.equal(result.hasSubstantiveContent, false); }); + + it('returns false when all gaps are about missing source info', () => { + // Simulates Issue #415 scenario where verifier couldn't verify because source lacked criteria + const output = `Verdict: FAIL + +Blocking gaps: +- Provide explicit acceptance criteria in the PR description or linked issue so they can be independently verified. +- Provide explicit acceptance criteria in the PR description or linked issue so they can be independently verified.`; + + const result = formatSimpleFollowUpIssue({ + verifierOutput: output, + prNumber: 123, + }); + + // Should NOT create follow-up issue since there's nothing actionable + // Note: hasSubstantiveContent will still be true because verifierOutput is non-empty + // The missing info gaps only affect the gap count, not the verifier output presence + assert.equal(result.hasSubstantiveContent, true); + }); + + it('returns true when gaps include actual verification failures alongside missing info', () => { + const output = `Verdict: FAIL + +Blocking gaps: +- Provide explicit acceptance criteria in the PR description or linked issue so they can be independently verified. +- Missing test coverage for new feature`; + + const result = formatSimpleFollowUpIssue({ + verifierOutput: output, + prNumber: 123, + }); + + // SHOULD create follow-up issue since there's an actual verification failure + assert.equal(result.hasSubstantiveContent, true); + }); }); }); }); diff --git a/.github/scripts/agents_pr_meta_update_body.js b/.github/scripts/agents_pr_meta_update_body.js index 078d7c68f..15489425a 100644 --- a/.github/scripts/agents_pr_meta_update_body.js +++ b/.github/scripts/agents_pr_meta_update_body.js @@ -285,8 +285,8 @@ async function withRetries(fn, options = {}) { const headers = error?.response?.headers || {}; const remainingRaw = headers['x-ratelimit-remaining'] ?? headers['X-RateLimit-Remaining']; const resetRaw = headers['x-ratelimit-reset'] ?? headers['X-RateLimit-Reset']; - const remaining = typeof remainingRaw === 'string' ? Number(remainingRaw) : Number(remainingRaw); - const reset = typeof resetRaw === 'string' ? Number(resetRaw) : Number(resetRaw); + const remaining = Number(remainingRaw); + const reset = Number(resetRaw); const isRateLimit = status === 403 && Number.isFinite(remaining) && remaining <= 0; const retryable = !status || status >= 500 || status === 429 || isRateLimit; diff --git a/.github/scripts/keepalive_loop.js b/.github/scripts/keepalive_loop.js index 2e7c676e7..87854e7cd 100644 --- a/.github/scripts/keepalive_loop.js +++ b/.github/scripts/keepalive_loop.js @@ -190,7 +190,7 @@ function normaliseChecklistSection(content) { let mutated = false; const updated = lines.map((line) => { - // Match bullet points (-, *, +) or numbered lists (e.g. 1., 2., 3)) + // Match bullet points (-, *, +) or numbered lists, for example: 1., 2., 3. or 1), 2), 3). const match = line.match(/^(\s*)([-*+]|\d+[.)])\s+(.*)$/); if (!match) { return line; diff --git a/.github/scripts/verifier_issue_formatter.js b/.github/scripts/verifier_issue_formatter.js index 68967c4ac..627611f63 100644 --- a/.github/scripts/verifier_issue_formatter.js +++ b/.github/scripts/verifier_issue_formatter.js @@ -20,6 +20,25 @@ function isPlaceholderContent(text) { normalized === ''; } +/** + * Check if a "blocking gap" is actually about missing source info rather than + * an actual verification failure. These should not trigger follow-up issues + * since there's nothing actionable to fix - the source simply lacked criteria. + * + * @param {string} text - Gap text to check + * @returns {boolean} True if gap is about missing source info + */ +function isMissingInfoGap(text) { + const normalized = String(text || '').toLowerCase().trim(); + // Match verifier complaints about missing acceptance criteria or testability + // Note: /i flag removed since text is already lowercased + return /provide\s+explicit\s+acceptance\s+criteria/.test(normalized) || + /acceptance\s+criteria.*missing/.test(normalized) || + /no\s+(testable|verifiable)\s+(criteria|requirements)/.test(normalized) || + /unable\s+to\s+verify.*missing/.test(normalized) || + /cannot\s+verify.*no\s+criteria/.test(normalized); +} + /** * Check if an item looks like a markdown section header that was incorrectly * captured as a list item (e.g., "## Related" or "### Notes"). @@ -550,11 +569,13 @@ function formatFollowUpIssue({ : '[Follow-up] Verifier failure - unmet acceptance criteria'; // Determine if this issue has substantive content worth creating - // Skip if we have no real tasks/criteria (just defaults) and no verifier gaps + // Skip if we have no real tasks/criteria (just defaults) and no verifier gaps. + // Also skip if all gaps are about missing source info (not actual verification failures). + const substantiveGaps = findings.gaps.filter(g => !isMissingInfoGap(g)); const hasSubstantiveContent = Boolean( (newTasks.length > 0 && !newTasks.every(t => isPlaceholderContent(t))) || (refinedUnmetCriteria.length > 0 && !refinedUnmetCriteria.every(c => isPlaceholderContent(c))) || - findings.gaps.length > 0 || + substantiveGaps.length > 0 || findings.unmetCriteria.length > 0 ); @@ -623,10 +644,11 @@ function formatSimpleFollowUpIssue({ ? `Verifier failure for PR #${prNumber}` : 'Verifier failure on merged commit'; - // Simple format always has substantive content (verifier output) - // since we only use it when we have actual verifier output to display + // Simple format: check substantive content. + // Filter out gaps that are about missing source info (not actual failures). + const substantiveGaps = findings.gaps.filter(g => !isMissingInfoGap(g)); const hasSubstantiveContent = Boolean( - findings.gaps.length > 0 || + substantiveGaps.length > 0 || findings.unmetCriteria.length > 0 || (verifierOutput && verifierOutput.trim().length > 0) ); @@ -649,6 +671,7 @@ module.exports = { buildChecklist, // Helper functions exported for testing isPlaceholderContent, + isMissingInfoGap, looksLikeSectionHeader, looksLikeReferenceLink, stripCheckboxesFromScope, diff --git a/.github/workflows/agents-guard.yml b/.github/workflows/agents-guard.yml index 628c3121e..7a3e22585 100644 --- a/.github/workflows/agents-guard.yml +++ b/.github/workflows/agents-guard.yml @@ -311,8 +311,8 @@ jobs: const headers = error?.response?.headers || {}; const remainingRaw = headers['x-ratelimit-remaining'] ?? headers['X-RateLimit-Remaining']; const resetRaw = headers['x-ratelimit-reset'] ?? headers['X-RateLimit-Reset']; - const remaining = typeof remainingRaw === 'string' ? Number(remainingRaw) : Number(remainingRaw); - const reset = typeof resetRaw === 'string' ? Number(resetRaw) : Number(resetRaw); + const remaining = Number(remainingRaw); + const reset = Number(resetRaw); const isRateLimit = status === 403 && Number.isFinite(remaining) && remaining <= 0; const retryable = isRateLimit || status === 429 || !status || (status >= 500 && status < 600); if (!retryable || attempt === maxAttempts) { @@ -439,8 +439,8 @@ jobs: const headers = error?.response?.headers || {}; const remainingRaw = headers['x-ratelimit-remaining'] ?? headers['X-RateLimit-Remaining']; const resetRaw = headers['x-ratelimit-reset'] ?? headers['X-RateLimit-Reset']; - const remaining = typeof remainingRaw === 'string' ? Number(remainingRaw) : Number(remainingRaw); - const reset = typeof resetRaw === 'string' ? Number(resetRaw) : Number(resetRaw); + const remaining = Number(remainingRaw); + const reset = Number(resetRaw); const isRateLimit = status === 403 && Number.isFinite(remaining) && remaining <= 0; const retryable = isRateLimit || status === 429 || !status || (status >= 500 && status < 600); if (!retryable || attempt === maxAttempts) { diff --git a/.github/workflows/health-44-gate-branch-protection.yml b/.github/workflows/health-44-gate-branch-protection.yml index 7f0ed228d..3e4a244ac 100644 --- a/.github/workflows/health-44-gate-branch-protection.yml +++ b/.github/workflows/health-44-gate-branch-protection.yml @@ -41,8 +41,8 @@ jobs: const headers = error?.response?.headers || {}; const remainingRaw = headers['x-ratelimit-remaining'] ?? headers['X-RateLimit-Remaining']; const resetRaw = headers['x-ratelimit-reset'] ?? headers['X-RateLimit-Reset']; - const remaining = typeof remainingRaw === 'string' ? Number(remainingRaw) : Number(remainingRaw); - const reset = typeof resetRaw === 'string' ? Number(resetRaw) : Number(resetRaw); + const remaining = Number(remainingRaw); + const reset = Number(resetRaw); const isRateLimit = status === 403 && Number.isFinite(remaining) && remaining <= 0; const retryable = isRateLimit || status === 429 || !status || (status >= 500 && status < 600); if (!retryable || attempt === maxAttempts) { diff --git a/templates/consumer-repo/.github/workflows/agents-guard.yml b/templates/consumer-repo/.github/workflows/agents-guard.yml index 628c3121e..7a3e22585 100644 --- a/templates/consumer-repo/.github/workflows/agents-guard.yml +++ b/templates/consumer-repo/.github/workflows/agents-guard.yml @@ -311,8 +311,8 @@ jobs: const headers = error?.response?.headers || {}; const remainingRaw = headers['x-ratelimit-remaining'] ?? headers['X-RateLimit-Remaining']; const resetRaw = headers['x-ratelimit-reset'] ?? headers['X-RateLimit-Reset']; - const remaining = typeof remainingRaw === 'string' ? Number(remainingRaw) : Number(remainingRaw); - const reset = typeof resetRaw === 'string' ? Number(resetRaw) : Number(resetRaw); + const remaining = Number(remainingRaw); + const reset = Number(resetRaw); const isRateLimit = status === 403 && Number.isFinite(remaining) && remaining <= 0; const retryable = isRateLimit || status === 429 || !status || (status >= 500 && status < 600); if (!retryable || attempt === maxAttempts) { @@ -439,8 +439,8 @@ jobs: const headers = error?.response?.headers || {}; const remainingRaw = headers['x-ratelimit-remaining'] ?? headers['X-RateLimit-Remaining']; const resetRaw = headers['x-ratelimit-reset'] ?? headers['X-RateLimit-Reset']; - const remaining = typeof remainingRaw === 'string' ? Number(remainingRaw) : Number(remainingRaw); - const reset = typeof resetRaw === 'string' ? Number(resetRaw) : Number(resetRaw); + const remaining = Number(remainingRaw); + const reset = Number(resetRaw); const isRateLimit = status === 403 && Number.isFinite(remaining) && remaining <= 0; const retryable = isRateLimit || status === 429 || !status || (status >= 500 && status < 600); if (!retryable || attempt === maxAttempts) { diff --git a/templates/consumer-repo/tools/resolve_mypy_pin.py b/templates/consumer-repo/tools/resolve_mypy_pin.py index 4585d9c19..5c547ec38 100755 --- a/templates/consumer-repo/tools/resolve_mypy_pin.py +++ b/templates/consumer-repo/tools/resolve_mypy_pin.py @@ -30,7 +30,17 @@ def get_mypy_python_version() -> str | None: content = pyproject_path.read_text() data = tomlkit.parse(content) - return data.get("tool", {}).get("mypy", {}).get("python_version") + tool_raw = data.get("tool") + # Use dict() to normalize tomlkit types and satisfy mypy + # tomlkit returns Table objects that act like dicts but mypy doesn't know this + tool: dict[str, object] = dict(tool_raw) if hasattr(tool_raw, "get") else {} # type: ignore[arg-type,call-overload] + mypy_raw = tool.get("mypy") + mypy: dict[str, object] = dict(mypy_raw) if hasattr(mypy_raw, "get") else {} # type: ignore[arg-type,call-overload] + version = mypy.get("python_version") + # Validate type before conversion - TOML can parse various types + if isinstance(version, (str, int, float)): + return str(version) + return None except ImportError: pass diff --git a/tools/resolve_mypy_pin.py b/tools/resolve_mypy_pin.py index 8d53d7e4a..6e2786189 100755 --- a/tools/resolve_mypy_pin.py +++ b/tools/resolve_mypy_pin.py @@ -30,12 +30,12 @@ def get_mypy_python_version() -> str | None: content = pyproject_path.read_text() data = tomlkit.parse(content) - tool = data.get("tool") - if not hasattr(tool, "get"): - tool = {} - mypy = tool.get("mypy") - if not hasattr(mypy, "get"): - mypy = {} + tool_raw = data.get("tool") + # Use dict() to normalize tomlkit types and satisfy mypy + # tomlkit returns Table objects that act like dicts but mypy doesn't know this + tool: dict[str, object] = dict(tool_raw) if hasattr(tool_raw, "get") else {} # type: ignore[arg-type,call-overload] + mypy_raw = tool.get("mypy") + mypy: dict[str, object] = dict(mypy_raw) if hasattr(mypy_raw, "get") else {} # type: ignore[arg-type,call-overload] version = mypy.get("python_version") # Validate type before conversion - TOML can parse various types if isinstance(version, (str, int, float)):