Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 47 additions & 8 deletions .github/workflows/run_sampler.yml
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,11 @@ jobs:

- name: Install silverfin-cli
run: |
# Unpinned, matching every other production workflow's convention (none pin to a
# branch/SHA/tag) — relies on run-sampler + --compact + result-URL surfacing being on
# main (silverfin-cli PR #261, "agustin-bso-sampler-easy-results", merged 2026-07-15).
npm install https://github.com/silverfin/silverfin-cli.git
# Pinned to a commit on silverfin-cli's sampler-compact-diff-v2 branch, since
# --add-diffs-folder (used below) is not yet on main. Switch back to installing
# unpinned main, matching every other production workflow's convention, once that
# branch's PR merges.
npm install https://github.com/silverfin/silverfin-cli.git#5accd6b
VERSION=$(node ./node_modules/silverfin-cli/bin/cli.js -V)
echo "CLI version: ${VERSION}"

Expand Down Expand Up @@ -281,21 +282,41 @@ jobs:

- name: Download results.zip for the workflow artifact
id: download
if: steps.sampler.outputs.report_url != ''
if: always() && steps.sampler.outputs.report_url != ''
env:
REPORT_URL: ${{ steps.sampler.outputs.report_url }}
run: |
# The CLI's --compact path downloads to a temp dir and deletes it after printing the
# diff, so results.zip never lands on disk on its own — fetch it again here for the
# artifact (Q12: full zip kept for rendering-only regressions the compact diff can't see).
# Note: the presigned REPORT_URL itself is short-lived (measured at ~5 minutes past
# run completion, not the long-lived link it was originally assumed to be — see
# silverfin-cli's CI_AUTH_SAMPLER_PLAN.md §10.4). A failure here means the human-facing
# PR comment link below falls back to that presigned URL, which may already be dead.
if curl -sL --fail --max-time 300 -o results.zip "${REPORT_URL}"; then
echo "downloaded=true" >> "$GITHUB_OUTPUT"
else
echo "::warning::Could not download results.zip from the report URL for the artifact upload (the report link itself is still valid)."
echo "::warning::Could not download results.zip from the report URL for the artifact upload."
echo "downloaded=false" >> "$GITHUB_OUTPUT"
fi

- name: Add diffs/ folder for the entries the compact diff flagged
id: add_diffs
# Pure local re-analysis of the zip already on disk - no network call, no partner API
# (silverfin-cli's --from-zip path). Best-effort: a failure here shouldn't cost the
# reviewer the plain results.zip artifact they'd otherwise get.
if: always() && steps.download.outputs.downloaded == 'true'
continue-on-error: true
run: |
if node ./node_modules/silverfin-cli/bin/cli.js run-sampler --from-zip results.zip --add-diffs-folder; then
echo "diffs_added=true" >> "$GITHUB_OUTPUT"
else
echo "::warning::Failed to add diffs/ folder to results.zip — the uploaded artifact is the plain sampler output for this run."
echo "diffs_added=false" >> "$GITHUB_OUTPUT"
fi

Comment thread
michieldegezelle marked this conversation as resolved.
- name: Upload results.zip artifact
id: upload
if: always() && steps.download.outputs.downloaded == 'true'
uses: actions/upload-artifact@v4
with:
Expand All @@ -305,14 +326,16 @@ jobs:

- name: Post result comment on the PR
if: always() && inputs.pull_request_number != ''
uses: actions/github-script@v7
uses: actions/github-script@v8
env:
PARTNER: ${{ inputs.partner }}
HANDLES: ${{ inputs.handles }}
ACCOUNT_TEMPLATES: ${{ inputs.account_templates }}
FIRM_IDS: ${{ inputs.firm_ids }}
SAMPLER_OK: ${{ steps.sampler.outputs.sampler_ok }}
REPORT_URL: ${{ steps.sampler.outputs.report_url }}
ARTIFACT_URL: ${{ steps.upload.outputs.artifact-url }}
DIFFS_ADDED: ${{ steps.add_diffs.outputs.diffs_added }}
COMPACT: ${{ steps.sampler.outputs.compact }}
PR_NUMBER: ${{ inputs.pull_request_number }}
RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
Expand All @@ -321,6 +344,13 @@ jobs:
const marker = `<!-- silverfin-sampler-result-${process.env.PARTNER} -->`;
const ok = process.env.SAMPLER_OK === "true";
const reportUrl = process.env.REPORT_URL || "";
// The backend's presigned reportUrl expires ~5 minutes after the run completes
// (CI_AUTH_SAMPLER_PLAN.md §10.4) — by the time a human reads this comment it's
// always dead. Prefer the GitHub Actions artifact link, which stays valid for the
// full 7-day retention window; fall back to the presigned URL only if the artifact
// upload itself didn't happen (e.g. the post-run download failed).
const artifactUrl = process.env.ARTIFACT_URL || "";
const diffsAdded = process.env.DIFFS_ADDED || "";
const compact = (process.env.COMPACT || "").trim();
const runUrl = process.env.RUN_URL;

Expand All @@ -334,7 +364,16 @@ jobs:
if (process.env.HANDLES) lines.push(`- Reconciliation handles: \`${process.env.HANDLES}\``);
if (process.env.ACCOUNT_TEMPLATES) lines.push(`- Account templates: \`${process.env.ACCOUNT_TEMPLATES}\``);
lines.push(`- Firm(s): \`${process.env.FIRM_IDS}\``);
if (reportUrl) lines.push(`- **[📊 Open full sampler report](${reportUrl})** (downloads \`results.zip\`)`);
if (artifactUrl) {
lines.push(`- **[📊 Open full sampler report](${artifactUrl})** (GitHub sign-in required; downloads \`results.zip\`, kept 7 days)`);
if (diffsAdded === "false") {
lines.push(` - ⚠️ Could not add the \`diffs/\` folder to this artifact — see the [workflow run](${runUrl}) logs. The plain \`results.zip\` is still there.`);
}
} else if (reportUrl) {
lines.push(`- **[📊 Open full sampler report](${reportUrl})** (downloads \`results.zip\` — this direct link expires ~5 minutes after the run, may already be gone)`);
} else {
lines.push(`- ⚠️ No sampler report link available for this run — see the [workflow run](${runUrl}) logs.`);
}
lines.push(`- Workflow run: ${runUrl}`);
lines.push("");
if (compact) {
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,8 @@ _Steps:_
* Loads the partner's credentials from the `PARTNER_CONFIG_JSON` secret and captures the token on disk before the run.
* Runs `run-sampler`, retrying on a cross-repo "already in progress" 422 (the backend allows only one sampler run per partner at a time; retries for up to 90 minutes).
* Captures the token again after the run and writes it back to `PARTNER_CONFIG_JSON_<partner>` via `gh secret set` only if it rotated (401 refresh mid-run).
* Downloads `results.zip` and uploads it as a short-lived (7-day) workflow artifact, for the rare rendering-only regression the compact diff can't see.
* Posts (or updates) a result comment on the PR with the compact diff and, when a report URL was produced, a link to the full report — otherwise the `results.zip` artifact is the fallback.
* Downloads `results.zip`, best-effort adds a `diffs/` folder of before/after `view.html` for the entries the compact diff flagged, and uploads it as a 7-day workflow artifact.
* Posts (or updates) a result comment on the PR with the compact diff and a link to the workflow artifact (kept 7 days; GitHub sign-in required) as the primary way to open the full report; falls back to the presigned report URL (short-lived, ~5 min) only if the artifact upload did not happen.
* Fails the job if the sampler run did not complete successfully.

_Authentication note:_
Expand Down
Loading