[JacobStephens2]

Summary

On a repository with GitHub issues turned off, a Security run with fixing allowed spent a security-fix-publishing Session, then failed at gh issue list … has disabled issues. The cause: nothing checked the repository could hold a Security fix Ticket before publication tried to list and create issues.

When fixing is allowed, the run now reads has_issues before it selects a pending fix:

 run_through
   ready_issue gate
   security_records → failed_fix gate
+  if fixing: outside.has_issues()
+    false → fixing = false, reason = "Security fixing skipped: <repo> has issues turned off, …"
+    error → pre-audit failure (never taken for "off")
   next_fix (only if still fixing)
   unchanged-Base gate
+  step(reason)
   processes::security::run  (audit → reproduce; fix-findings skipped)
+  Ended.fixing_skipped = reason → appended to the ending line and Run notification

• Effect on the Security run: the run audits and records findings as a report-only run does, then ends findings recorded (exit 0). It starts no publishing Session and creates no issue or fix link. The reproduced finding no longer suppresses the audit. On an unchanged Base, a repeat run is the usual quiet skip.
• Custom process path:
  • fix-findings reads has_issues only once a finding awaits a fix. If issues are off, the Step succeeds and says why.
  • The security-audit/reproduce-findings "awaits an allowed fix" skip no longer fires when issues are off.
• Adapter: GitHub::has_issues reads gh api repos/{repo}. A missing or non-bool has_issues is an InvalidResponse error.
• Test fake: tests/fakes/gh.rs gains a has_issues state, default on. When it is false, gh issue list and gh issue create fail as gh does.
• Docs:
  • docs/guide.md and the regenerated manual.
  • site/index.html, site/how-it-works/, site/architecture/.
  • An ADR-0015 amendment.

Evidence

Execution evidence

• Before (source at ca8065f4, with the new fake and test):
  cargo test --test security_run a_repository_with_issues_off_audits_and_skips_fixing_without_a_publishing_session

  thirdshift: security-fix-publishing: session started
  thirdshift: security-fix-publishing: session ended after 0s
  thirdshift: gh issue list failed: the 'acme/widgets' repository has disabled issues
    left: Some(1)
   right: Some(0)
  test result: FAILED. 0 passed; 1 failed
  

  cargo test --test process issues_are_turned_off: both new Custom-path tests FAILED.
• After: the same command passes.

  thirdshift: Security fixing skipped: acme/widgets has issues turned off, so no Security fix Ticket can be filed
  …
  thirdshift: Security audit recorded 0 new finding(s), 1 already recorded; Security fixing skipped: acme/widgets has issues turned off, so no Security fix Ticket can be filed
  test result: ok. 1 passed; 0 failed
  

  The notification subject is Security run: findings recorded, and its body carries the skip line.
• Further regressions:
  • findings_a_new_audit_reproduces_are_not_fixed_when_issues_are_off (single and Spec-sized).
  • malformed_issues_metadata_fails_before_the_audit.
  • In security::tests: issues_turned_off_* (single and Spec, unchanged Base) and an_unreadable_issues_setting_fails_before_work_and_report_only_runs_skip_it.
  • In tests/process: fixing_is_skipped_without_a_session_when_issues_are_turned_off, issues_turned_off_with_nothing_to_fix_reports_no_skip, malformed_issues_metadata_fails_the_fix_step_before_any_session and a_record_awaiting_a_fix_skips_no_audit_when_issues_are_turned_off.
• Full run: scripts/affected-tests main → PASS npm test --prefix site, PASS cargo test (922s).
• Pre-existing flake: tests/process findings::fixing_dispatches_each_reproduced_finding_and_keeps_unreproduced_findings_private failed in an earlier full run. It also fails 5 of 6 times with this branch's src/ and tests/fakes/ reverted, so the cause is upstream, not this change. When two fixes publish within one second, the second publishing session's whole-second window counts the first fix's issue Keeping the PR mergeable: merge the Base branch and conflict Repair #8: "created public issue(s) outside the staged path: Keeping the PR mergeable: merge the Base branch and conflict Repair #8". No open issue tracks it.

Visual evidence

None needed: the site edits are one-sentence text changes to hand-written pages, with no layout change.

Merge Danger

Door: two-way

Blast Radius: narrow

Only Security runs and Security finding Steps with fixing allowed are affected. They now make one extra gh api repos/{repo} read; if that read fails, the run fails before its audit, where before it would have gone on.

Unaddressed findings

Standards

• 1. The issues-off rule is checked at three sites, reading the setting at different points (partly addressed).
  • fix-findings now reads only when a fix awaits, matching the audit Step (src/processes/findings.rs, inside while let Some(..) = records.take_next_fix()).
  • The Security run still reads up front. After an audit it can go on to fix a finding reproduced in that run, so it must decide fixing before processes::security::run.
  • The guide documents this ("first reads the repository's has_issues setting").
• 3. fixing_skipped: Option<String> is a primitive. The one reason now comes from a single constructor, security::issues_turned_off, which also owns the prefix (finding 2). A new type would add no behaviour.
• 4. Duplicates the repos/{repo} read in advisories.rs. The two reads are separate and read different fields. Sharing them would put metadata-error coupling into private-record storage selection, which is outside A Security run on a repository with issues turned off fails at Security fix publishing #2094.
• 6. Magic index and a two-behaviour unit test. Kept as is: these are judgement calls on test readability, and both tests still fail on a single, named behaviour.
• 7. Prompt-text coupling in a fake agent. This follows existing practice in tests/security_run.rs (lines 2838, 4828, 5482).

Spec

• 3. A failed audit's notification omits the skip reason. The proposed test was run as written: cargo test --bin thirdshift security::tests::a_failed_audit_still_says_why_fixing_was_skipped does not compile (no method named audit_error, no variant … SecurityRun found for enum command::Ending).
  • Declined on design grounds: a failed Run's ending line is its failure cause (Account::of_failure).
  • The skip reason still appears on stderr before the audit starts, and in the Command log.

Review coverage

Both reviewers read every changed file. site/docs/index.html is generated output, excluded by both reviewers on npm run docs:check --prefix site exiting 0. The review ran at 3ff6b2f; the later fix commits address its findings.

Closes #2094

Built with claude 2.1.314 · opus · default effort

Security review outcome

No unaddressed introduced findings.