OSAC-2141: replace draft PRs with GitHub validation labels

[adalton]

Summary

• Stop using draft PRs as a quality signal for validation failures and non-zero exit codes
• Apply configurable GitHub PR labels (ai-validation-failed, ai-nonzero-exit) that communicate the issue without delaying CI or reviewer engagement
• Always create regular PRs and always transition Jira tickets to "in review"
• Apply/clear validation labels when code is pushed; leave unchanged when no changes produced
• Add PRValidationLabels config following the existing empty-string-disables pattern

Test plan

• All 16 test packages pass with -race -count=1
• New-ticket: non-zero exit applies ai-nonzero-exit label, creates regular PR, transitions to in-review
• New-ticket: validation failed applies ai-validation-failed label (takes precedence over nonzero exit)
• New-ticket: repo config pr.draft: true still creates draft PR, still transitions to in-review
• Feedback: validation label applied when changes pushed with failure
• Feedback: validation labels cleared when changes pushed with success
• Feedback: labels unchanged when no changes pushed
• Config: PRValidationLabels round-trips through YAML/mapstructure
• Config: empty values when not configured (no defaults applied)
• Resolver: PRValidationLabels passes through from ProjectConfig to ProjectSettings
• Unit tests for validationLabel() and validationPassed() (12 cases covering precedence)
• Unit tests for setPRValidationLabel and clearPRValidationLabels (7 cases)

Assisted-by: Claude noreply@anthropic.com

Packages affected

• executor/: Implements configurable PR validation labeling (validation_failed / nonzero_exit) using mutually exclusive label management, applied/cleared across both “new ticket” and “feedback” flows (while honoring pr.draft: true config for draft PR creation).
• models/: Adds PRValidationLabels to project/Jira configuration schema and project settings.
• projectresolver/: Propagates per-project PRValidationLabels into resolved project settings.
• jobmanager/: Updates documentation/comments clarifying that draft PRs are created only when repo config enables pr.draft: true (validation failures no longer trigger drafts).
• services/: GitHub integration adds/implements AddPRLabel and RemovePRLabel operations for PR labels.
• scanner/: Extends PR labeler stubs/tests to support add/remove PR label behavior (used by scanner’s labeler integration, e.g., idle-label handling).
• config/ docs: Updates config.example.yaml and AGENTS.md to document the new pr_validation_labels configuration and behavior.

Execution pipeline

• Continues to create regular PRs by default; drafts are created only when repository configuration explicitly sets pr.draft: true.
• Applies mutually exclusive PR validation labels based on AI outcomes:
  • Applies ai-validation-failed when validation is explicitly failed.
  • Applies ai-nonzero-exit when the container exits non-zero and validation was not explicitly reported as failed.
• On subsequent successful validations, clears configured validation labels; when no changes are produced, leaves validation labels unchanged.
• Multi-repo flow applies/clears labels per repository that received commits, while keeping the ticket lifecycle transition behavior consistent (“In Review” still occurs under the new model).

Configuration and deployment

• Adds configurable per-project jira.pr_validation_labels (using the empty-string-disables pattern) documented in config.example.yaml and AGENTS.md.
• No infrastructure/container/workspace/crash-recovery changes; no deployment changes indicated.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: e014dde7-da03-4727-a955-cbb412716213

📥 Commits

Reviewing files that changed from the base of the PR and between 0ea2791 and cf62b8a.

📒 Files selected for processing (2)

• executor/labels_test.go
• executor/pipeline.go

---

Walkthrough

Adds configurable GitHub PR validation labels, propagates them through project settings, and applies or clears them in new-ticket and feedback pipelines based on validation results and process exit codes. Draft creation now follows repository configuration directly.

Changes

PR validation label workflow

Layer / File(s)	Summary
Configuration and project settings models/config.go, models/project_settings.go, projectresolver/..., config.example.yaml, AGENTS.md, jobmanager/manager.go, models/*_test.go	Adds validation-label configuration, YAML loading, project-setting propagation, updated result semantics, and documentation.
PR label operations executor/executor.go, executor/labels.go, executor/executortest/stubs.go, executor/export_test.go	Adds GitHub label operations and best-effort mutually exclusive add/remove and clear behavior.
Pipeline validation flow executor/pipeline.go, executor/feedback.go	Applies validation labels across single- and multi-repository new-ticket and feedback flows, updates validation status calculation, and makes draft creation repository-configured.
Validation coverage executor/labels_test.go, executor/pipeline_test.go	Tests label selection, cleanup, exit-code and explicit validation outcomes, no-change behavior, draft handling, and multi-repository labeling.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• flightctl/jira-ai-issue-solver#36: Modifies an overlapping feedback execution path.
• flightctl/jira-ai-issue-solver#42: Changes the multi-repository commit SHA flow used by feedback labeling.
• flightctl/jira-ai-issue-solver#44: Modifies overlapping feedback execution paths.

Suggested labels: executor, config

Suggested reviewers: amir-yogev-gh

Sequence Diagram(s)

sequenceDiagram
  participant Pipeline
  participant GitService
  participant GitHubPR
  Pipeline->>Pipeline: Evaluate validation result and exit code
  Pipeline->>GitService: Add target label or clear configured labels
  GitService->>GitHubPR: Update pull request labels

Loading

🚥 Pre-merge checks | ✅ 11 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 26.92% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Unchecked-Errors	⚠️ Warning	pipeline_test.go adds several _, _ = p.Execute(...) ignores with no justification comments; the only prod blank-id ignore I found is commented as safe.	Capture the error and assert nil, or add a short comment at each ignore explaining why the test can safely discard it.

✅ Passed checks (11 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: replacing draft PRs with GitHub validation labels.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
No-Hardcoded-Secrets	✅ Passed	No hardcoded secrets were introduced; the new literals are label names/placeholders, and the added config/type code contains no real credentials or base64 secret blobs.
No-Weak-Crypto	✅ Passed	No weak-crypto primitives, custom crypto, or secret/token comparisons were introduced in the only changed code paths.
No-Injection-Vectors	✅ Passed	No SQL/shell/eval/yaml/pickle/HTML injection primitives were added; label changes use GitHub API calls only.
Container-Privileges	✅ Passed	PASS: The PR only changes docs/config/code; no container/K8s manifests were modified, and no privileged:true/hostPID/hostNetwork/hostIPC/SYS_ADMIN/allowPrivilegeEscalation:true settings appear.
No-Sensitive-Data-In-Logs	✅ Passed	New validation-label logs only include owner/repo/pr/label; no passwords, tokens, emails, session IDs, hostnames, or customer data are logged in the added code.
Resource-Leaks	✅ Passed	No new leaked files, HTTP bodies, DB/network connections, or shutdownless goroutines were introduced in the touched code.
Ai-Attribution	✅ Passed	PR includes acceptable AI trailer 'Assisted-by: Claude'; no Co-Authored-By AI attribution found.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 8

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@executor/feedback.go`:
- Around line 496-506: Restrict the label updates in the loop around
validationLabel and repoInfos to repositories present in repoSHAs, skipping
unchanged entries before calling setPRValidationLabel or
clearPRValidationLabels. Preserve the existing label behavior for changed
repositories, and add a regression test covering repoInfos with only one
repository included in repoSHAs.

In `@executor/labels_test.go`:
- Around line 528-542: The “empty target only removes others” test only checks
the removal count, so duplicate removals can pass without verifying both
configured labels were removed. Update the assertions in the test around
SetPRValidationLabel, including the corresponding case near the referenced
second location, to compare removed against the exact set of distinct validation
labels while preserving the existing added-label assertion.
- Around line 563-569: Strengthen the best-effort error tests in
executor/labels_test.go at lines 563-569 and 611-620 by using an observed zap
logger and asserting the expected warning entries. Cover both add and removal
failures in the “errors are swallowed” subtest, and verify the removal-failure
warning includes error context in the second subtest; ensure each test has
assertions for the observable logs.

In `@executor/labels.go`:
- Around line 137-147: Update the removal log in the label cleanup flow and the
addition log around targetLabel to include structured owner, repo, and pr_number
fields alongside the existing label and error fields. Use the existing PR
context variables owner, repo, and prNumber, while preserving the current log
levels and messages.

In `@executor/pipeline_test.go`:
- Around line 3194-3197: Update the test around p.Execute to capture both its
result and error, then assert the expected successful no-change outcome before
checking labelTouched. Do not discard the Execute error; ensure unrelated early
failures cause the test to fail rather than producing a false positive.

In `@executor/pipeline.go`:
- Around line 981-983: Propagate the first multi-repo PR’s draft state into
JobResult: ensure repoPR carries the PR draft flag when PRs are created, then
assign result.Draft from prs[0].draft alongside PRURL and PRNumber. Preserve the
existing validation flow.

In `@models/config_test.go`:
- Around line 1888-1972: Extend the configuration tests around LoadConfig and
PRValidationLabels with a partial-disable case where validation_failed is
explicitly empty while nonzero_exit has a custom value, and assert both decoded
values. Also cover the inverse case if required by the existing per-label
behavior, preserving the current omitted-label and fully populated cases.
- Line 1850: Update the temporary-file setup and cleanup in the affected tests
to check and report errors from file Close calls, and replace deferred os.Remove
calls with t.Cleanup callbacks that handle removal failures through the test’s
error-reporting mechanism. Apply this consistently to the temporary key paths
around the existing test setup blocks, without discarding any returned errors.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: a2334811-c7da-4689-ae5c-07d179f66448

📥 Commits

Reviewing files that changed from the base of the PR and between 03933ad and 0c43274.

📒 Files selected for processing (16)

• AGENTS.md
• config.example.yaml
• executor/executor.go
• executor/executortest/stubs.go
• executor/export_test.go
• executor/feedback.go
• executor/labels.go
• executor/labels_test.go
• executor/pipeline.go
• executor/pipeline_test.go
• jobmanager/manager.go
• models/config.go
• models/config_test.go
• models/project_settings.go
• projectresolver/resolver.go
• projectresolver/resolver_test.go