Skip to content

Align no-op tracker expiry with maintenance workflow generation#37965

Merged
pelikhan merged 5 commits into
mainfrom
copilot/fix-no-op-runs-expiry-issue
Jun 9, 2026
Merged

Align no-op tracker expiry with maintenance workflow generation#37965
pelikhan merged 5 commits into
mainfrom
copilot/fix-no-op-runs-expiry-issue

Conversation

Copilot AI commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

No-op tracker issues are stamped at runtime with a 30-day expiry, but maintenance workflow generation previously ignored that path unless an explicit expires: existed elsewhere. This change makes compile-time expiry detection account for safe-outputs.noop.report-as-issue (including default-enabled behavior), so stamped no-op expirations are actually enforced.

  • Expiry source parity (runtime ↔ compile-time)

    • Updated scanWorkflowsForExpires to treat no-op issue reporting as an expiry source.
    • When no-op reporting is enabled (default or explicit true), it contributes a 30-day expiry window (24 * 30 hours), matching the runtime footer behavior.
  • No-op report-as-issue semantics

    • Added isNoOpReportAsIssueEnabled(*string) bool to centralize the enablement rule:
      • nil => enabled (default)
      • "false" (case-insensitive, trimmed) => disabled
      • other values => enabled
  • Focused maintenance workflow coverage

    • Extended maintenance workflow tests to cover:
      • no-op default (unset report-as-issue) => maintenance workflow generated
      • explicit report-as-issue: true => maintenance workflow generated
      • explicit report-as-issue: false => maintenance workflow not generated
const defaultNoOpIssueExpirationHours = 24 * 30

if workflowData.SafeOutputs.NoOp != nil &&
   isNoOpReportAsIssueEnabled(workflowData.SafeOutputs.NoOp.ReportAsIssue) {
    hasExpires = true
    if minExpires == 0 || defaultNoOpIssueExpirationHours < minExpires {
        minExpires = defaultNoOpIssueExpirationHours
    }
}

Copilot AI and others added 3 commits June 8, 2026 23:13
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix no-op runs issue expiration enforcement Align no-op tracker expiry with maintenance workflow generation Jun 8, 2026
Copilot AI requested a review from pelikhan June 8, 2026 23:18
@pelikhan pelikhan marked this pull request as ready for review June 8, 2026 23:25
Copilot AI review requested due to automatic review settings June 8, 2026 23:25
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer failed during the skills-based review.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

⚠️ PR Code Quality Reviewer failed during code quality review.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #37965 does not have the 'implementation' label and has only 61 new lines of code in business logic directories (≤100 threshold).

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel completed test quality analysis.

Copilot AI left a comment

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.

Pull request overview

This PR aligns maintenance workflow generation with the runtime behavior of no-op tracker issue expiry by treating safe-outputs.noop.report-as-issue (including its default-enabled behavior) as an expiry source during compile-time scanning.

Changes:

  • Added centralized isNoOpReportAsIssueEnabled(*string) bool logic and a 30-day default no-op issue expiry constant (in hours).
  • Updated scanWorkflowsForExpires to flag expiry when no-op issue reporting is enabled and to factor its 30-day window into minExpires.
  • Expanded maintenance workflow tests to cover no-op default/true/false behavior for workflow generation.
Show a summary per file
File Description
pkg/workflow/maintenance_workflow.go Detects enabled no-op report-as-issue as an expiry source (30-day window) when deciding whether to generate maintenance workflows.
pkg/workflow/maintenance_workflow_test.go Adds test cases verifying maintenance workflow generation behavior for default/explicit no-op report-as-issue settings.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@pelikhan

pelikhan commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

@copilot run pr-finisher skill

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100 — Excellent

Analyzed 3 test(s) (new table rows in TestGenerateMaintenanceWorkflow_WithExpires): 3 design, 0 implementation, 0 guideline violations.

📊 Metrics & Test Classification (3 tests analyzed)
Metric Value
New/modified tests analyzed 3
✅ Design tests (behavioral contracts) 3 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 2 (67%)
Duplicate test clusters 0
Test inflation detected Yes — 43 test lines / 18 prod lines ≈ 2.4:1 (threshold 2:1)
🚨 Coding-guideline violations 0

Test Classification Details

Test File Classification Issues Detected
noop report-as-issue default pkg/workflow/maintenance_workflow_test.go:148 ✅ Design Happy path — no edge case
noop report-as-issue false pkg/workflow/maintenance_workflow_test.go:161 ✅ Design Disabled path (edge case ✅)
noop report-as-issue true pkg/workflow/maintenance_workflow_test.go:176 ✅ Design Explicit-enabled path (edge case ✅)

Language Support

Tests analyzed:

  • 🐹 Go (*_test.go): 3 tests — unit (//go:build !integration)
  • 🟨 JavaScript (*.test.cjs, *.test.js): 0 tests

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). Build tag present ✅, no mock libraries ✅.

i️ Minor note: test/production line-addition ratio is ≈ 2.4:1 (43 test lines vs 18 production lines), which triggers the inflation flag and costs 10 points. This is largely due to Go struct-literal verbosity for table rows — not a real quality concern here, but worth noting if the table continues to grow.

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

  • Assert on observable outputs, return values, or state changes
  • Cover error paths and boundary conditions
  • Would catch a behavioral regression if deleted
  • Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

  • Assert on internal function calls (mocking internals)
  • Only test the happy path with typical inputs
  • Break during legitimate refactoring even when behavior is correct
  • Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §27173437921

🧪 Test quality analysis by Test Quality Sentinel · 301.2 AIC · ⌖ 49.3 AIC ·

@github-actions github-actions Bot left a comment

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.

✅ Test Quality Sentinel: 80/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). All 3 new table rows verify observable behavioral contracts; no mock libraries; build tag present.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

Copilot AI commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@copilot run pr-finisher skill

Done in 4d06484.

I ran the pr-finisher pass, fixed the actionable failure (TestCompileSpecificFiles_DeletesMaintenanceWorkflow) by updating the no-expires fixture to set safe-outputs.noop: false, and re-ran local validation: make fmt, make lint, make test-unit, make test, make recompile (all passing).

Since agent pushes don’t retrigger CI, checks on this new HEAD need a maintainer re-trigger.

@pelikhan pelikhan merged commit a53fea0 into main Jun 9, 2026
29 checks passed
@pelikhan pelikhan deleted the copilot/fix-no-op-runs-expiry-issue branch June 9, 2026 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No-op runs issue expiry is stamped at runtime but never enforced without an explicit expires: field

3 participants