Skip to content

refactor(detect): remove step summary output from the detection job - #792

Merged
davidslater merged 2 commits into
mainfrom
ace/01KZEGM3KB63V4DK6KYFTGH07C
Aug 7, 2026
Merged

davidslater merged 2 commits into
mainfrom
ace/01KZEGM3KB63V4DK6KYFTGH07C

Conversation

@davidslater

Copy link
Copy Markdown
Collaborator

Created by GitHub Ace · View Session

What

The detection job no longer writes anything to the GitHub Actions step summary.

Removed:

  • pkg/stepsummary — the artifact inventory Markdown table
  • pkg/detector/summary.go — the rendered-prompt block (FormatPromptSummary) and the verdict block (FormatVerdictSummary, AppendStepSummary)
  • The --step-summary flag (and its GITHUB_STEP_SUMMARY default) on both threat-detect and threat-detect conclude, plus the related path-collision checks
  • ThreatMarker and the <!-- gh-aw-threat-* --> marker constants, which were only ever rendered into the verdict summary

Kept

  • The JSONL run log (--log-file) still records the full recursive artifact inventory, prompt metadata, and verdict.
  • conclude still writes its self-contained job-log diagnostics, including the ThreatHeadline that distinguishes a tooling failure (agent_failure/parse_error) from a real security finding.
  • Result-file vs $GITHUB_OUTPUT/$GITHUB_ENV collision rejection in conclude, and --log-file vs --output collision rejection in the detection run.

Docs & spec

  • specs/threat-detection-spec.md: TD-20c rewritten to state the detector MUST NOT write to the step summary; TD-20g and TD-20h removed; TD-20i reduced to the job-log headline contract.
  • specs/usage-spec.md (U-09), README.md, DEVGUIDE.md, CLAUDE.md updated accordingly.

Verification

make fmt lint build test passes. gosec findings are unchanged from the pre-existing baseline (only reduced by the deleted files).

The detector no longer writes to GITHUB_STEP_SUMMARY. Drop the artifact
inventory table, the rendered-prompt block, and the conclude verdict block
along with the --step-summary flags on both commands. Observability now lives
solely in the JSONL run log and the conclude job-log diagnostics.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 7, 2026 16:34

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

Removes GitHub Actions step-summary output while retaining JSONL observability and job-log diagnostics.

Changes:

  • Removes step-summary flags, writers, markers, and tests.
  • Preserves run-log inventory and conclusion diagnostics.
  • Updates specifications and documentation.
Show a summary per file
File Description
specs/usage-spec.md Documents JSONL-only inventory.
specs/threat-detection-spec.md Revises summary-output requirements.
README.md Removes step-summary usage.
DEVGUIDE.md Updates inventory guidance.
CLAUDE.md Updates repository guidance.
cmd/threat-detect/main.go Removes detection summary output.
cmd/threat-detect/main_test.go Removes summary tests.
cmd/threat-detect/logfile_test.go Removes summary assertions.
cmd/threat-detect/conclude.go Removes verdict-summary output.
cmd/threat-detect/conclude_test.go Updates conclusion tests.
pkg/detector/reason.go Removes summary markers.
pkg/detector/reason_test.go Removes marker tests.
pkg/detector/summary.go Deletes summary rendering.
pkg/detector/summary_test.go Deletes renderer tests.
pkg/stepsummary/stepsummary.go Deletes inventory summary writer.
pkg/stepsummary/stepsummary_test.go Deletes writer tests.

Review details

Tip

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

  • Files reviewed: 16/16 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread specs/threat-detection-spec.md Outdated
Comment on lines +279 to +281
artifact inventory defined by TD-17b, the rendered prompt, and the conclusion
verdict are surfaced through the run log (TD-20a) and the `conclude`
diagnostics (TD-20d) only.
The rendered prompt is no longer surfaced anywhere; the run log records only
prompt metadata. Say so explicitly instead of implying the full prompt is
available via the run log or conclude diagnostics.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>

Copy link
Copy Markdown
Collaborator Author

This states that the rendered prompt remains surfaced through the run log or conclude, but prompt_built records only prompt metadata (main.go:344-355) and conclude reports only the verdict. With FormatPromptSummary deleted, the rendered prompt is no longer surfaced anywhere, so the normative text should say that explicitly rather than promising unavailable diagnostics.

Good catch — fixed in e50029e. TD-20c now reads:

TD-20c: The detector MUST NOT write to the GitHub Actions step summary. The artifact inventory defined by TD-17b is surfaced through the run log (TD-20a) only, and the conclusion verdict through the run log and the conclude diagnostics (TD-20d). The rendered prompt itself is not surfaced: the run log records only its metadata (byte count, resolved workflow name/description, custom-prompt provenance, scaffolding detection).

@davidslater
davidslater merged commit b6c3ebd into main Aug 7, 2026
8 checks passed
@davidslater
davidslater deleted the ace/01KZEGM3KB63V4DK6KYFTGH07C branch August 7, 2026 17:17
davidslater added a commit that referenced this pull request Aug 7, 2026
main removed all step-summary output from the detector (#792), so the preflight
block written to GITHUB_STEP_SUMMARY is dropped along with
engine.FormatPreflightSummary and its test. Preflight now surfaces solely
through stderr and the engine_preflight run-log event, matching TD-20c; TD-20j
and the README/CLAUDE.md notes are reworded accordingly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
davidslater added a commit to github/gh-aw that referenced this pull request Aug 7, 2026
…r external detector

The upstream threat-detect binary removed the --step-summary flag entirely
in v0.4.5 (github/gh-aw-threat-detection#792): it no longer writes any
step-summary output. Our compiler was still:

  - passing --step-summary <path> to threat-detect (now a stale, unused arg)
  - resetting/touching ThreatDetectionStepSummaryPath before execution on the
    external-detector path (dead code — nothing writes to it anymore)
  - emitting an "Append detection step summary" host-side step to copy that
    file into $GITHUB_STEP_SUMMARY (always a no-op now, since the file is
    never populated)

Removed all three. The step-summary reset/touch is now scoped to the inline
detection path only, where the engine's own execution step still overrides
GITHUB_STEP_SUMMARY to write there. Updated the isolation test to assert
these are absent on the external-detector path, and updated docs.

Co-authored-by: David Slater <12449447+davidslater@users.noreply.github.com>
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.

2 participants