feat(forge): resolve the spec-to-pr engine from a semver range

[behnam-deriv]

Summary

This is a live engine bump for every caller that doesn't pass engine_ref. deriv-blox, derivatives-trader, deriv-prop-trading and deriv-api-v2 all use @master without it. On merge they move from 26484fc (v0.6.1) to v0.6.7 (a4f4c23), which is 114 commits and includes feat: patches (on-demand token minting, fast-lane delta spec, deferral picker, OpenSpec auto-repair). After that, each new v0.6.x tag rolls out to them with no PR in this repo. That's the intended policy: "newest 0.6.x", not "bug fixes only".

Forge no longer needs a manual engine_ref bump for every spec-to-pr release. engine_ref now accepts:

• a caret range — new default ^0.6.7. At run time this resolves to the newest matching vX.Y.Z release tag, using npm caret rules. While spec-to-pr is on 0.x the minor is the breaking boundary, so ^0.6.7 means >=0.6.7 <0.7.0 (this matches spec-to-pr's README). From 1.x on, it means "same major".
• an exact tag — e.g. v0.6.7.
• a 40-char SHA — to freeze. Existing callers that pass a SHA are unaffected.

The resolved tag is peeled to its commit SHA. The workflow logs it (engine_ref ^0.6.7 -> v0.6.7 (a4f4c23…)) and fetches that commit.

Security trade-off: Forge's default no longer pins one fixed commit. It follows spec-to-pr's vX.Y.Z tags, which aren't protected there (spec-to-pr has branch rulesets only), so anyone with write access to that repo can create or move a tag and the next default run executes it. A tag can be moved, a commit cannot: pass a 40-char SHA to freeze. Branches, pre-release tags and anything that isn't a range, tag or SHA are rejected. The resolved tag and SHA are logged and added to the issue comment Forge posts.

Opting into a breaking release (e.g. 0.7.0) is a one-line change of the default to ^0.7.0.

Changes

• .github/workflows/forge.yml: new engine_ref semantics and default; resolver in the "Fetch the spec-to-pr engine" step.
• tests/forge-contract.sh: the engine_ref default may now be a caret range or a SHA. Added a check that branch names are rejected.

Testing

• Ran the resolver against spec-to-pr's real tags:
  • ^0.6.7 and ^0.6.0 → v0.6.7
  • ^0.5.0 → v0.5.0
  • v0.6.1 → 26484fc (the previous pin)
  • ^0.7.0, ^1.0.0 and master each fail with a clear error
  • a SHA is passed through as-is
• bash tests/forge-contract.sh passes.
• Not yet exercised in a real Actions run.

🤖 Generated with Claude Code

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

Package	Version	Score	Details

Scanned Manifest Files

[behnam-deriv]

Thanks for the review. Addressed in dc3ef93:

🟡 1. Floating on movable tags — accepted as-is, no change. The trade-off is intentional and is in the PR description. Following release tags is the point of this PR. Callers who need a fixed version can still pass a SHA in engine_ref, and every run logs the exact tag and commit it used. One correction to the suggested wording: spec-to-pr currently has branch rulesets only (its master requires a reviewed PR) and no v* tag ruleset. So I haven't added text claiming tags are protected.

🟡 2. No automated test for the resolver — fixed. Added tests/forge-resolver.sh, and CI runs it via lint-actions.yml. I didn't use the suggested snippet: it extracts only the TAG=$(…) line and wraps it in a second TAG=$(eval …), so TAG would never be set correctly. The test instead extracts the whole resolver block from forge.yml verbatim and runs it against a local fixture repo. It covers:

• caret semantics on 0.x, on ≥1, and on 0.0.x
• pre-release exclusion
• numeric ordering (0.6.10 > 0.6.9)
• annotated vs lightweight tag peeling
• SHA passthrough
• a branch named like an in-range release
• malformed refs (^0.6, ~0.6.7, 0.6.7, …)

I checked that it fails if the caret filter is broken (e.g. z >= p → z > p).

🟢 3. Missing empty-SHA guard on the range path — fixed. It now uses the same ::error:: guard as the exact-tag path.

🟢 4. Comment wrap in the contract test header — fixed.

🤖 Generated with Claude Code

[sparsh-deriv]

Checked out dc3ef93 in an isolated worktree and ran bash tests/forge-resolver.sh and bash tests/forge-contract.sh. Both passed. CI is green.

The resolver itself is solid: caret, exact tag, or SHA; npm 0.x caret rules; peel to a commit; fetch that SHA. Rejecting branches is stricter than master, which fetched ENGINE_REF as-is.

Four callers use @master and none pass engine_ref: deriv-blox, derivatives-trader, deriv-prop-trading, deriv-api-v2. Merge moves all of them from 26484fc (v0.6.1) to v0.6.7 (a4f4c23, 114 commits later). After that they follow new v0.6.x tags. deriv-api-v2's old in-repo Forge already resolved release tags, so the direction fits. The PR summary should say this is a live engine bump for those callers.

The input description still talks about a "reviewed release flow". spec-to-pr has branch rulesets only (PR restrictions plus org push protection). There is no v* tag ruleset. You already said that on the PR. The workflow text should match.

Soften the description and name the bump and I will approve.

[github-actions]

Model: kimi-k3

🤖 Kimi PR Review Complete

🔄 Follow-up Review

Summary

3 of 4 issues from the previous review have been resolved. The remaining item is the known supply-chain posture point: default: "^0.6.7" still floats on mutable git tags in deriv-com/spec-to-pr. The new commit does meaningfully improve this area — the engine_ref description now explicitly discloses that tags are not protected there and tells callers to pass a 40-char SHA to freeze, which resolves the "hidden dependency" half of the previous suggestion. The other half (an actual tag-protection ruleset on spec-to-pr) is confirmed by that same comment to be absent, and remains an out-of-repo step. The new changes in this increment (engine tag/SHA surfaced on the issue comment, anchored contract-test regex) are sound — I checked the failure-before-fetch and SHA-input paths and found no regressions, and the previously fixed items (resolver test, empty-SHA guard, comment wrap) remain intact.

Recommendation: APPROVE — the one remaining item is a MEDIUM whose substance is an out-of-repo hardening step (tag ruleset on deriv-com/spec-to-pr); the in-repo half of that suggestion (documenting the posture where the input is declared) was done in this commit, and nothing in the diff blocks merge.

---

🔴 Critical Issues (BLOCK MERGE)

None.

🟠 High Priority Issues

None.

🟡 Medium Priority Issues

🟡 1. engine_ref default now floats on movable git tags — add tag protection on spec-to-pr — .github/workflows/forge.yml:108-121

Details

Severity	File	Lines
MEDIUM	.github/workflows/forge.yml	108-121

❌ Problematic Code:

          A range or tag follows whatever vX.Y.Z tags exist on spec-to-pr at run
          time. Tags are not protected there (its rulesets cover branches only),
          so anyone with write access to that repo can create or move one, and
          the next run executes it with this job's write token in scope. A tag
          can be moved, a commit cannot: pass a 40-char SHA to freeze.
...
        # Policy: every caller that doesn't pass engine_ref runs the newest
        # v0.6.x tag, and each new v0.6.x tag rolls out to them with no PR here.
        default: "^0.6.7"

📋 Issue: Still present from the previous review. With ^0.6.7 as the default, anyone who can create, move, or delete v0.6.x tags in deriv-com/spec-to-pr changes the code that executes with a write token in every caller repo — git tags are mutable, and git ls-remote resolution picks up a force-moved tag silently on the next run. Progress since the last review: the engine_ref description now explicitly discloses exactly this ("Tags are not protected there (its rulesets cover branches only)… pass a 40-char SHA to freeze"), so the risk is no longer a hidden dependency — that half of the previous suggestion is done, and done honestly. What remains is the substance: the tag-protection ruleset itself, which the new comment confirms does not exist (spec-to-pr's rulesets cover branches only).

⚠️ Impact: A compromised or careless push to spec-to-pr (tag re-pointed to arbitrary commits) executes that code in every Forge consumer repo with a write token in scope, in every caller's CI, without any PR or review gate. The new issue-comment reporting (Engine: spec-to-pr v0.6.x <sha>) makes the resolved ref visible after the fact, but does not prevent a moved tag from being picked up.

✅ Fix: No code change required in this repo — pair this merge with a tag-protection ruleset on deriv-com/spec-to-pr:

• GitHub repo Settings → Rules → Rulesets → target refs/tags/v*
• Restrict creations and updates to the release role/app only; block deletions and force-pushes

💡 Explanation: The range default is only as immutable as the tags it resolves against. A ruleset that forbids updating/deleting v* tags restores the "a release cannot be re-pointed after the fact" property the SHA pin used to give, while keeping the no-manual-bump workflow this PR wants. The workflow's own description now states the exact gap, so the ruleset is the single remaining step.

🟢 Low Priority Issues

None.

Summary Table

Priority	Count	Categories
🔴 Critical	0	—
🟠 High	0	—
🟡 Medium	1	Supply-chain posture (movable tags)
🟢 Low	0	—

---

Recommendations

1. Before or alongside merging, add a ruleset on deriv-com/spec-to-pr that restricts creation of v* tags to the release flow and forbids tag updates/deletions — this closes the one mutability gap the caret default introduces. The engine_ref description now documents the gap explicitly, so nothing in-repo is left to do for this item.
2. Nothing else outstanding. The new engine reporting (id: engine + engine_tag/engine_sha outputs appended to the issue comment) was checked for regressions: ENGINE_TAG is assigned in all three resolver branches, the report step's ${ENGINE_SHA:-} / ${ENGINE_TAG:+$ENGINE_TAG } guards correctly handle both the failed-before-fetch case and the tagless SHA-input case, and the anchored contract-test regex (^ +default: "(\^X.Y.Z|[0-9a-f]{40})"$) matches the current default and is strictly stronger than what it replaced. tests/forge-resolver.sh still extracts and executes the resolver block verbatim — both anchors survived the new commit.

The verified fixes were confirmed against both diffs: the empty-SHA guard remains at .github/workflows/forge.yml:343, the resolver test and its CI wiring are untouched by this increment and intact, and the contract-test header no longer has the dangling wrap.

---

Auto Fix Claude Reviews

Action	Open Dashboard

[sparsh-deriv]

ad128de covers the four points from the last review.

The description now says tags are unprotected and a SHA is the freeze. The default comment and the PR body name the live bump for the four @master callers and drop "non-breaking". The fetch step exports engine_tag/engine_sha after checkout, and report appends them to the issue. The contract grep is anchored.

I re-ran bash tests/forge-resolver.sh and bash tests/forge-contract.sh at ad128de. Both passed. CI is green.

Writing the outputs after checkout means the issue names what actually landed. A failed fetch stays silent.