Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 114 additions & 0 deletions .github/scripts/__tests__/verifier-issue-formatter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const {
extractCheckedItems,
buildChecklist,
isPlaceholderContent,
isMissingInfoGap,
looksLikeSectionHeader,
looksLikeReferenceLink,
stripCheckboxesFromScope,
Expand Down Expand Up @@ -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'));
Expand Down Expand Up @@ -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);
});
});
});

Expand Down Expand Up @@ -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);
});
});
});
});
4 changes: 2 additions & 2 deletions .github/scripts/agents_pr_meta_update_body.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion .github/scripts/keepalive_loop.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
33 changes: 28 additions & 5 deletions .github/scripts/verifier_issue_formatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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").
Expand Down Expand Up @@ -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
);

Expand Down Expand Up @@ -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)
);
Expand All @@ -649,6 +671,7 @@ module.exports = {
buildChecklist,
// Helper functions exported for testing
isPlaceholderContent,
isMissingInfoGap,
looksLikeSectionHeader,
looksLikeReferenceLink,
stripCheckboxesFromScope,
Expand Down
8 changes: 4 additions & 4 deletions .github/workflows/agents-guard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/health-44-gate-branch-protection.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions templates/consumer-repo/.github/workflows/agents-guard.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
12 changes: 11 additions & 1 deletion templates/consumer-repo/tools/resolve_mypy_pin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 6 additions & 6 deletions tools/resolve_mypy_pin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)):
Expand Down
Loading