Skip to content

ci(#6116): always upload behaviour artifacts - #7056

Merged
ifireball merged 2 commits into
fullsend-ai:mainfrom
ifireball:codex/6116-always-upload-behaviour-artifacts
Sep 7, 2026
Merged

ci(#6116): always upload behaviour artifacts#7056
ifireball merged 2 commits into
fullsend-ai:mainfrom
ifireball:codex/6116-always-upload-behaviour-artifacts

Conversation

@ifireball

Copy link
Copy Markdown
Member

Summary

Verification

  • make lint

Closes #6116

Run base-branch redaction and artifact upload after successful as well as failed behaviour test runs. Keep upload gated on successful redaction.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Upload redacted behaviour artifacts for every test outcome

✨ Enhancement ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

AI Description

• Run trusted base-branch artifact redaction after every relevant behaviour test outcome.
• Upload successful and failed run artifacts only when redaction completes successfully.
Diagram

graph TD
  A["Relevant test run"] --> B["Base script checkout"] --> C["Artifact redaction"] --> D{"Redaction succeeds?"} -->|Yes| E["Artifact upload"]
  D -->|No| F["No upload"]
Loading
High-Level Assessment

The workflow-level always() conditions are the most direct approach. Retaining the relevance check and successful-redaction gate avoids unnecessary execution and preserves the existing security boundary before upload.

Files changed (1) +3 / -3

Other (1) +3 / -3
e2e.ymlRun artifact redaction and upload after every test outcome +3/-3

Run artifact redaction and upload after every test outcome

• Changes the trusted script checkout, redaction, and upload steps to run after successful or failed relevant behaviour tests. Artifact publication remains gated on redaction completing successfully.

.github/workflows/e2e.yml

@ifireball ifireball self-assigned this Sep 6, 2026
@qodo-code-review

qodo-code-review Bot commented Sep 6, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Informational

1. Contributors overlook passing-run logs ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The new always() conditions make redaction and upload run after successful behaviour tests, but
both the contributor guide and redaction script header still describe failure-only execution. Anyone
using those references to find or maintain debug output is told the old lifecycle and can wrongly
conclude that passing runs produce no artifact.
Code

.github/workflows/e2e.yml[348]

+        if: always() && steps.changes.outputs.relevant != 'false' && steps.redact.outcome == 'success'
Relevance

●●● Strong

Recent precedents accept correcting stale CI documentation and redaction-script behavior
descriptions after lifecycle changes.

PR-#6167
PR-#5665

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow's redaction and upload steps now use always(), whereas the contributor guide says
artifacts upload on failure and the script header says it is invoked after a job failure.

.github/workflows/e2e.yml[306-353]
docs/contributing/ci-workflows.md[185-191]
scripts/redact-behaviour-artifacts.sh[2-7]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Update documentation that still says behaviour artifacts are processed only after failures. The workflow now redacts and uploads artifacts after successful and failed relevant runs.

## Issue Context
Both the contributor guide and the redaction script header describe the previous failure-only lifecycle, while the workflow uses `always()`.

## Fix Focus Areas
- docs/contributing/ci-workflows.md[185-191]
- scripts/redact-behaviour-artifacts.sh[2-7]
- .github/workflows/e2e.yml[306-353]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 67 rules
Review mode: ⚖️ Balanced: This changes CI workflow control flow for test cleanup, secret redaction, and artifact uploads, so it has behavioral and security-sensitive implications despite the small diff.

Grey Divider

Tip of the day
💡 Did you know, you can copy the agent prompt from any finding and feed it to your IDE agent

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/e2e.yml
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 6, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:23 PM UTC · Completed 12:36 PM UTC

Commit: d3afaeb · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $2.76

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review fullsend-ai-review Bot added the risk/moderate PR risk: moderate label Sep 6, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 6, 2026

Copy link
Copy Markdown

Risk Assessment: moderate (2/5)

Details

Composite of 2.21 (0.50×2.0 + 0.30×2.71 + 0.20×2.0) rounds to 2/moderate; Tier 2 churn is marginally higher than the prior review (12 commits/30d and 9 authors vs. 7 and 5), and PROTECTED_PATH_COUNT rose to 2 (score 5), but the tiny diff size and non-production file scope keep the overall score unchanged at moderate.

Previous run

Risk Assessment: moderate (2/5)

Details

Single CI workflow file with a tiny diff drives low Tier 1 (1.75), but the file is a churn hotspot (7 commits in 30d, 26 fix/revert commits in 90d, 5 distinct authors) and touches a protected CI path, producing a moderate composite of 2.12 that rounds to 2.

@fullsend-ai-review

fullsend-ai-review Bot commented Sep 6, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] .github/workflows/e2e.yml, scripts/redact-behaviour-artifacts.sh — This PR modifies files under the protected paths .github/ and scripts/. The PR links to issue test(behaviour): post-run collection path and bulk agent artifact capture #6116 and the description explains the rationale (upload behaviour artifacts on every run, not only on failure). Human approval is always required for protected-path changes, regardless of context.
Previous run

Review

Findings

Medium

  • [protected-path] .github/workflows/e2e.yml — This PR modifies .github/workflows/e2e.yml, which is under the protected path .github/. The PR links to issue test(behaviour): post-run collection path and bulk agent artifact capture #6116 and the description explains the rationale (upload behaviour artifacts on every run, not only on failure). Human approval is always required for protected-path changes, regardless of context.

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Sep 6, 2026
Document that behaviour artifact redaction and upload run after successful and failed relevant test runs.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Assisted-by: Codex (gpt-5.6-sol)
@fullsend-ai-review

fullsend-ai-review Bot commented Sep 7, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 5:52 AM UTC · Ended 6:04 AM UTC

Commit: 5d4833d · View workflow run →

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown

Site preview

Preview: https://10fdec39-site.fullsend-ai.workers.dev

Commit: 5d4833d55805022f6ecc23fa72219a03ace0c839

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:52 AM UTC · Completed 6:03 AM UTC

Commit: 5d4833d · View workflow run →

Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.15

@ifireball
ifireball added this pull request to the merge queue Sep 7, 2026
Merged via the queue into fullsend-ai:main with commit d16cedf Sep 7, 2026
42 checks passed
@ifireball
ifireball deleted the codex/6116-always-upload-behaviour-artifacts branch September 7, 2026 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

requires-manual-review Review requires human judgment risk/moderate PR risk: moderate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(behaviour): post-run collection path and bulk agent artifact capture

2 participants