only run post upon success

[poulet42]

currently if a job gets cancelled before the install goes to completion, post script still runs, uploading a partial cache forever. Can we just run post action upon success?

Summary by CodeRabbit

• Bug Fixes

  • Post-job processing now runs only after a successful job.

• Style

  • Updated configuration string formatting without changing behavior.

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[coderabbitai]

Warning

Review limit reached

Next included review available in 4 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1691f276-c22f-4c08-9002-33ffbb71fd21

📥 Commits

Reviewing files that changed from the base of the PR and between 0033bde and dd3f6e2.

📒 Files selected for processing (2)

• README.md
• action.yml

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a462b80e-5e20-4984-a526-f0c3182c47fc

📥 Commits

Reviewing files that changed from the base of the PR and between 1dad155 and 0033bde.

📒 Files selected for processing (1)

• action.yml

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details

🔇 Additional comments (1)

action.yml (1)

44-52: LGTM!

Also applies to: 66-73, 90-90

---

📝 Walkthrough

Walkthrough

The action metadata changes several YAML string quotes without changing values. It also adds post-if: success() so the post step runs only when the job succeeds.

Changes

Action metadata

Layer / File(s)	Summary
Input metadata and post-step guard action.yml	The input defaults and deprecation message use double-quoted YAML strings. The post step now runs only when the job succeeds.

Estimated code review effort: 2 (Simple) | ~5 minutes

Merge Risk: ⚪ Minimal · up to 0033b

The change limits the post action to successful jobs and leaves existing input values unchanged; no actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: zkochan, stanzilla

Poem

A rabbit checked the action's chart
The quotes changed, but not the heart
When success shines across the run
The post-step hops and gets things done
Soft YAML paws leave no trace apart

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: the post action runs only after successful job completion.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

_{Reviews (3): Last reviewed commit: "docs: record that the cache is saved onl..." | Re-trigger Greptile}

Dismissed because a newer commit was pushed; Greptile will re-review the current head.

[zkochan]

Thanks for digging into this — the problem is real and your diagnosis is right. post-if defaults to always(), so a job cancelled mid-pnpm install writes a short store, and because cache keys are immutable, later runs at the same lockfile restore it, finish the install locally, and then skip the save because the key already matched. It stays stuck until the lockfile changes.

Where I've landed differently is on post-if: success() being the right lever.

The store itself is never left in a bad state — it's content-addressable and pnpm re-fetches whatever is missing — so this is purely "the cache is less useful than it could be", bounded to one lockfile on one branch.

Against that, success() also stops the save on every failed job, and two of those are cases where saving is what you want:

• Install succeeds, tests fail. The store is complete and perfectly cacheable, but nothing gets saved. That's the normal shape while iterating on a branch, so it would mean a full re-download on every red run.
• Install fails on a bad lockfile. It will fail again on that same lockfile, so the partial store it leaves is actually useful — each retry restores most of it and fails fast instead of re-downloading first.

It also can't help entries that are already stuck, since the key can't be rewritten.

The root cause is the key scheme rather than the post condition: we never refresh an entry we restored from, and at an unchanged lockfile there is no key to write a better store to. The fix I'd want is to make the primary key unique per run (append github.run_id) and let the existing restore-keys prefix do the matching — then every complete run publishes a fresher entry and a short one gets superseded instead of pinned. That's a bigger change: it moves what cache-hit means and adds entries against the 10 GB repo quota, so I'd rather do it deliberately than as a side effect here.

So I'm closing this one — but it was a good catch, and the underlying issue is on my list. If you'd like to take a run at the run-id keying, I'm happy to review it.