j4k-oss/fta-check
Watch
2
0
Fork
You've already forked fta-check
 0
Code Issues Pull requests Releases 5 Packages 1 Activity Actions

chore: align with j4k-align 17.17.1 #22

Merged
jercik merged 1 commit from chore/align-17-17-1 into main 2026-11-04 07:12:53 +00:00
Conversation 3 Commits 1 Files changed 5 +118 -11
jercik commented 2026-11-04 06:48:04 +00:00
Owner

Adds j4k-align 17.17's baked-Node check before every actions/setup-node step in the Forgejo workflows. A job now fails with a named error when its image doesn't already have the Node version from .node-version, instead of letting setup-node download it from GitHub.

The same repair updates the other managed files: the commit-msg workflow parses PR fields with readable multi-line code and has reworded fork-PR comments, scripts/check-msg.sh lists the accepted types when it rejects a header, and a few workflow comments are corrected.

Adds j4k-align 17.17's baked-Node check before every `actions/setup-node` step in the Forgejo workflows. A job now fails with a named error when its image doesn't already have the Node version from `.node-version`, instead of letting setup-node download it from GitHub. The same repair updates the other managed files: the commit-msg workflow parses PR fields with readable multi-line code and has reworded fork-PR comments, `scripts/check-msg.sh` lists the accepted types when it rejects a header, and a few workflow comments are corrected.
chore: align with j4k-align 17.17.1
All checks were successful
Checks / reuse (pull_request) Has been skipped
Details
commit-msg / same-repo-pr (pull_request) Has been skipped
Details
commit-msg / commitlint (pull_request) Successful in 8s
Details
Checks / quality-checks (pull_request) Successful in 48s
Details
Checks / qc-result (pull_request) Has been skipped
Details
Review / Review (pull_request_target) Successful in 18m0s
Details
bcd1d52981
forgejo-actions commented 2026-11-04 07:06:06 +00:00

Review 01M6JN1DVB26W7TV2K2KK61BT6 — head bcd1d529816b87564a076e794d3c2135b43adf11

Review — j4k-oss/fta-check @ e1861c6bb5

Scope: diff against base tree d4b4389ee4dc
Review finished: 6 succeeded, 0 failed, 4 skipped by dispatch
Triage: settled
Facts: current review-wide projection

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v3",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v5",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (2)

low — The baked-Node step selects between two inputs at runtime, but only one is ever set

  • claim: 01M6JNP5MEY1ZMQBJSP54KCQH3
  • anchor: .forgejo/workflows/checks.yml (snippet)
if [ -n "${REQUIRED_NODE_VERSION:-}" ] && [ -z "${REQUIRED_NODE_VERSION_FILE:-}" ]; then
  • lens: aposd-plain · arm: default
  • verdicts: 2 valid / 0 invalid / 0 uncertain
    • pass 01M6JNPN03Y5WANAH4SPD415KK · valid: The anchored selection line is grounded (exact) in checks.yml. The reviewer gives a concrete trace: every copy sets REQUIRED_NODE_VERSION_FILE='.node-version' in the step env, and a tree-wide grep finds REQUIRED_NODE_VERSION only at read sites. The batch offers no counterevidence, and Forgejo does not inject repository variables into a step's env unless the step maps them. So the inline-pin branch and the 'needs exactly one' else branch cannot run in this repository, and every reader of the step still has to parse them. That is Nonobvious Code and Special-General Mixture in the shipped file, and the reviewer quotes AGENTS.md: 'No parameter or option no caller passes'. The defect is in the file whether or not it is rendered. Remedy gap: the batch suggests these workflows may be rendered by j4k-align (see the related repetition claim). If so, a local edit to version="" would be overwritten, and the fix belongs in the template as a render-time choice, which the reviewer also proposes. Low severity is appropriate.
    • pass 01M6JNPN03Y5WANAH4SPD415KK · valid: Resubmitted because shell expansion garbled the quoted code in my previous verdict for this claim. The judgment is unchanged. The anchored selection line is grounded (exact) in checks.yml. The reviewer gives a concrete trace: every copy sets REQUIRED_NODE_VERSION_FILE='.node-version' in the step env, and a tree-wide grep finds REQUIRED_NODE_VERSION only at read sites. The batch offers no counterevidence, and Forgejo does not inject repository variables into a step's env unless the step maps them. So the inline-pin branch and the 'needs exactly one' else branch cannot run in this repository, and every reader of the step still has to parse them. That is Nonobvious Code and Special-General Mixture in the shipped file, and the reviewer quotes AGENTS.md: 'No parameter or option no caller passes'. The defect is in the file whether or not it is rendered. Remedy gap: the batch suggests these workflows may be rendered by j4k-align (see the related repetition claim). If so, a local edit to version="$(cat -- .node-version)" would be overwritten, and the fix belongs in the template as a render-time choice, which the reviewer also proposes. Low severity is appropriate.
  • disposition: none

Every reader of the step has to work through a three-way input selection, including a diagnostic path, that cannot run in this repository. Ten of the step's roughly 25 script lines exist to choose between inputs when the choice is fixed. The result is Special-General Mixture and Nonobvious Code: the reader is left wondering who sets REQUIRED_NODE_VERSION and when.

Each of the three copies sets REQUIRED_NODE_VERSION_FILE: ".node-version" in the step's own env. A repo-wide grep finds REQUIRED_NODE_VERSION only at its read sites, with no assignment in any step, job or workflow env. So the first branch (pin="this workflow's Node pin") and the else branch (needs exactly one of REQUIRED_NODE_VERSION and REQUIRED_NODE_VERSION_FILE) are unreachable unless the runner injects that variable. AGENTS.md rules out this shape: "No parameter or option no caller passes".

Correction: reduce the selection to version="$(cat -- .node-version)" and pin=.node-version. The next step's node-version-file: ".node-version" already names the same file, and the exact-version diagnostic keeps naming the pin, so no reader-facing meaning is lost. If other repositories need an inline pin, the generator should make that choice at render time rather than shipping both paths in each rendered file.

I read the step in checks.yml, dedupe-check.yml and release.yml, and grepped the whole tree for assignments. This is static reasoning; nothing was run.

Proof gap: the wording "this workflow's Node pin" suggests a shared j4k-align template serving repositories with inline pins, but that template is outside this tree. A template that must emit both forms at runtime, with a reason render-time selection cannot work, would refute this claim. A runner-level REQUIRED_NODE_VERSION would also make the else branch fire, and nothing in the tree shows one.

low — The baked-Node probe hard-codes setup-node's private cache layout and assumes x64

  • claim: 01M6JNP63DG3B7TX7S84FDYKX6
  • anchor: .forgejo/workflows/checks.yml (snippet)
if [ ! -f "${RUNNER_TOOL_CACHE:-/opt/hostedtoolcache}/node/$version/x64.complete" ]; then
  • lens: aposd-plain · arm: default
  • verdicts: 2 valid / 0 invalid / 0 uncertain
    • pass 01M6JNPN03Y5WANAH4SPD415KK · valid: The grounded excerpt shows the probe checks for '/opt/hostedtoolcache/node//x64.complete'. That hard-codes the architecture and the tool cache's '<tool>/<version>/<arch>.complete' marker layout, which is a decision owned by setup-node and @actions/tool-cache, not by this workflow: the Information Leakage red flag. The reviewer adds that the same jobs already key their pnpm cache on runner.arch, so the literal 'x64' is inconsistent with how the job treats architecture elsewhere. The impact is overstated, and I would keep it at low. Every job runs on ubuntu-latest, and nothing shows a non-x64 host, so the misdiagnosis is latent. Any probe of a baked tool cache must mirror this layout; the image baking presumably uses the same convention, so the reviewer's correction documents the coupling rather than removing it. The concrete improvement that holds is deriving the architecture from runner.arch and stating in a comment which convention the probe mirrors. If these files are rendered by j4k-align, as other batch evidence suggests, that change belongs in the template.
    • pass 01M6JNPN03Y5WANAH4SPD415KK · valid: Resubmitted because shell expansion garbled the quoted path in my previous verdict for this claim. The judgment is unchanged. The grounded excerpt shows the probe checks for '${RUNNER_TOOL_CACHE:-/opt/hostedtoolcache}/node/$version/x64.complete'. That hard-codes the architecture and the tool cache's '<tool>/<version>/<arch>.complete' marker layout, which is a decision owned by setup-node and @actions/tool-cache, not by this workflow: the Information Leakage red flag. The reviewer adds that the same jobs already key their pnpm cache on runner.arch, so the literal 'x64' is inconsistent with how the job treats architecture elsewhere. The impact is overstated, and I would keep it at low. Every job runs on ubuntu-latest, and nothing shows a non-x64 host, so the misdiagnosis is latent. Any probe of a baked tool cache must mirror this layout; the image baking presumably uses the same convention, so the reviewer's correction documents the coupling rather than removing it. The concrete improvement that holds is deriving the architecture from runner.arch and stating in a comment which convention the probe mirrors. If these files are rendered by j4k-align, as other batch evidence suggests, that change belongs in the template.
  • disposition: none

On any runner that is not x64, a job image with the pinned Node baked in would still fail this check, and the diagnostic would wrongly say "This job image does not bake Node" and send the maintainer to j4k-align --fix or node_ci_node_versions. The same false failure would hit every job if a later actions/setup-node@v6 release stored its tools differently, because the tag floats within v6. This is Information Leakage: the workflow encodes a storage decision that belongs to the action in the next step.

The probe tests for the file ${RUNNER_TOOL_CACHE:-/opt/hostedtoolcache}/node/$version/x64.complete. That path carries three assumptions: the node/<version>/<arch>.complete marker layout, the default cache root, and a literal x64. The same jobs already treat architecture as variable, since their pnpm cache key is node-cache-${{ runner.os }}-${{ runner.arch }}-pnpm-....

Correction: state the host's architecture rather than a literal, by mapping runner.arch or uname -m to the tool cache's name, and keep the layout knowledge in one definition with a comment naming the tool-cache convention it mirrors and why it does. The comment serves the maintainer who must decide whether a failure means a missing bake or a changed cache layout. src/lib/resolve-fta-binary.ts is a precedent: its comment says it mirrors getBinaryPath() from fta-cli 3.x.

I read the step and the neighbouring Setup Node.js and Restore pnpm store steps in all three workflows. Every job here declares runs-on: ubuntu-latest, and nothing in the tree shows an arm64 runner, so the architecture failure is conditional on one existing; today's impact is latent. The setup-node and @actions/tool-cache source is not in this tree and there is no network here, so the marker layout comes from my knowledge of that library and is unverified.

A runner inventory confirming x64-only hosts would remove the architecture half of this claim. The leakage half would stand unless setup-node documents this layout as a stable interface, which would refute it.

Other claims

  • grounding-pending (0)
  • ungrounded (0)
  • rejected (0)
  • duplicate-of (0)
  • unadjudicated (1)
    • 01M6JNP55KP0B28NFXQ6B0WBTQ medium — The baked-Node policy script is pasted as 29 identical lines into three workflows

Coverage

Coverage pass: 01M6JN1DVEVHP582E8DCZ12SYH
Accounting: complete (all slots terminal; see slot results for failures)
Slot health: healthy

lens part arm unit status runs loss
general-bug whole default no-claims 1 no
writing-quality whole default no-claims 1 no
restated-sets whole default no-claims 1 no
unix-cli whole default no-claims 1 no
module-design whole default no-claims 1 no
aposd-plain whole default claims-emitted 1 no
  • test-trimming — skipped-by-dispatch: no tests changed
  • project-docs — skipped-by-dispatch: no project documentation changed
  • use-effect — skipped-by-dispatch: no React code changed
  • react — skipped-by-dispatch: no React code changed
<!-- review:summary --> **Review** `01M6JN1DVB26W7TV2K2KK61BT6` — head `bcd1d529816b87564a076e794d3c2135b43adf11` # Review — j4k-oss/fta-check @ e1861c6bb51d Scope: diff against base tree `d4b4389ee4dc` Review finished: 6 succeeded, 0 failed, 4 skipped by dispatch Triage: settled Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v5", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (2) ### low — The baked-Node step selects between two inputs at runtime, but only one is ever set - claim: `01M6JNP5MEY1ZMQBJSP54KCQH3` - anchor: `.forgejo/workflows/checks.yml` (snippet) ``` if [ -n "${REQUIRED_NODE_VERSION:-}" ] && [ -z "${REQUIRED_NODE_VERSION_FILE:-}" ]; then ``` - lens: aposd-plain · arm: default - verdicts: 2 valid / 0 invalid / 0 uncertain - pass `01M6JNPN03Y5WANAH4SPD415KK` · valid: The anchored selection line is grounded (exact) in checks.yml. The reviewer gives a concrete trace: every copy sets REQUIRED_NODE_VERSION_FILE='.node-version' in the step env, and a tree-wide grep finds REQUIRED_NODE_VERSION only at read sites. The batch offers no counterevidence, and Forgejo does not inject repository variables into a step's env unless the step maps them. So the inline-pin branch and the 'needs exactly one' else branch cannot run in this repository, and every reader of the step still has to parse them. That is Nonobvious Code and Special-General Mixture in the shipped file, and the reviewer quotes AGENTS.md: 'No parameter or option no caller passes'. The defect is in the file whether or not it is rendered. Remedy gap: the batch suggests these workflows may be rendered by j4k-align (see the related repetition claim). If so, a local edit to version="" would be overwritten, and the fix belongs in the template as a render-time choice, which the reviewer also proposes. Low severity is appropriate. - pass `01M6JNPN03Y5WANAH4SPD415KK` · valid: Resubmitted because shell expansion garbled the quoted code in my previous verdict for this claim. The judgment is unchanged. The anchored selection line is grounded (exact) in checks.yml. The reviewer gives a concrete trace: every copy sets REQUIRED_NODE_VERSION_FILE='.node-version' in the step env, and a tree-wide grep finds REQUIRED_NODE_VERSION only at read sites. The batch offers no counterevidence, and Forgejo does not inject repository variables into a step's env unless the step maps them. So the inline-pin branch and the 'needs exactly one' else branch cannot run in this repository, and every reader of the step still has to parse them. That is Nonobvious Code and Special-General Mixture in the shipped file, and the reviewer quotes AGENTS.md: 'No parameter or option no caller passes'. The defect is in the file whether or not it is rendered. Remedy gap: the batch suggests these workflows may be rendered by j4k-align (see the related repetition claim). If so, a local edit to version="$(cat -- .node-version)" would be overwritten, and the fix belongs in the template as a render-time choice, which the reviewer also proposes. Low severity is appropriate. - disposition: none > Every reader of the step has to work through a three-way input selection, including a diagnostic path, that cannot run in this repository. Ten of the step's roughly 25 script lines exist to choose between inputs when the choice is fixed. The result is Special-General Mixture and Nonobvious Code: the reader is left wondering who sets `REQUIRED_NODE_VERSION` and when. > > Each of the three copies sets `REQUIRED_NODE_VERSION_FILE: ".node-version"` in the step's own `env`. A repo-wide grep finds `REQUIRED_NODE_VERSION` only at its read sites, with no assignment in any step, job or workflow `env`. So the first branch (`pin="this workflow's Node pin"`) and the `else` branch (`needs exactly one of REQUIRED_NODE_VERSION and REQUIRED_NODE_VERSION_FILE`) are unreachable unless the runner injects that variable. `AGENTS.md` rules out this shape: "No parameter or option no caller passes". > > Correction: reduce the selection to `version="$(cat -- .node-version)"` and `pin=.node-version`. The next step's `node-version-file: ".node-version"` already names the same file, and the exact-version diagnostic keeps naming the pin, so no reader-facing meaning is lost. If other repositories need an inline pin, the generator should make that choice at render time rather than shipping both paths in each rendered file. > > I read the step in `checks.yml`, `dedupe-check.yml` and `release.yml`, and grepped the whole tree for assignments. This is static reasoning; nothing was run. > > Proof gap: the wording "this workflow's Node pin" suggests a shared `j4k-align` template serving repositories with inline pins, but that template is outside this tree. A template that must emit both forms at runtime, with a reason render-time selection cannot work, would refute this claim. A runner-level `REQUIRED_NODE_VERSION` would also make the `else` branch fire, and nothing in the tree shows one. ### low — The baked-Node probe hard-codes setup-node's private cache layout and assumes x64 - claim: `01M6JNP63DG3B7TX7S84FDYKX6` - anchor: `.forgejo/workflows/checks.yml` (snippet) ``` if [ ! -f "${RUNNER_TOOL_CACHE:-/opt/hostedtoolcache}/node/$version/x64.complete" ]; then ``` - lens: aposd-plain · arm: default - verdicts: 2 valid / 0 invalid / 0 uncertain - pass `01M6JNPN03Y5WANAH4SPD415KK` · valid: The grounded excerpt shows the probe checks for '/opt/hostedtoolcache/node//x64.complete'. That hard-codes the architecture and the tool cache's '&lt;tool>/&lt;version>/&lt;arch>.complete' marker layout, which is a decision owned by setup-node and @actions/tool-cache, not by this workflow: the Information Leakage red flag. The reviewer adds that the same jobs already key their pnpm cache on runner.arch, so the literal 'x64' is inconsistent with how the job treats architecture elsewhere. The impact is overstated, and I would keep it at low. Every job runs on ubuntu-latest, and nothing shows a non-x64 host, so the misdiagnosis is latent. Any probe of a baked tool cache must mirror this layout; the image baking presumably uses the same convention, so the reviewer's correction documents the coupling rather than removing it. The concrete improvement that holds is deriving the architecture from runner.arch and stating in a comment which convention the probe mirrors. If these files are rendered by j4k-align, as other batch evidence suggests, that change belongs in the template. - pass `01M6JNPN03Y5WANAH4SPD415KK` · valid: Resubmitted because shell expansion garbled the quoted path in my previous verdict for this claim. The judgment is unchanged. The grounded excerpt shows the probe checks for '${RUNNER_TOOL_CACHE:-/opt/hostedtoolcache}/node/$version/x64.complete'. That hard-codes the architecture and the tool cache's '&lt;tool>/&lt;version>/&lt;arch>.complete' marker layout, which is a decision owned by setup-node and @actions/tool-cache, not by this workflow: the Information Leakage red flag. The reviewer adds that the same jobs already key their pnpm cache on runner.arch, so the literal 'x64' is inconsistent with how the job treats architecture elsewhere. The impact is overstated, and I would keep it at low. Every job runs on ubuntu-latest, and nothing shows a non-x64 host, so the misdiagnosis is latent. Any probe of a baked tool cache must mirror this layout; the image baking presumably uses the same convention, so the reviewer's correction documents the coupling rather than removing it. The concrete improvement that holds is deriving the architecture from runner.arch and stating in a comment which convention the probe mirrors. If these files are rendered by j4k-align, as other batch evidence suggests, that change belongs in the template. - disposition: none > On any runner that is not x64, a job image with the pinned Node baked in would still fail this check, and the diagnostic would wrongly say "This job image does not bake Node" and send the maintainer to `j4k-align --fix` or `node_ci_node_versions`. The same false failure would hit every job if a later `actions/setup-node@v6` release stored its tools differently, because the tag floats within v6. This is Information Leakage: the workflow encodes a storage decision that belongs to the action in the next step. > > The probe tests for the file `${RUNNER_TOOL_CACHE:-/opt/hostedtoolcache}/node/$version/x64.complete`. That path carries three assumptions: the <code>node/&lt;version&gt;/&lt;arch&gt;.complete</code> marker layout, the default cache root, and a literal `x64`. The same jobs already treat architecture as variable, since their pnpm cache key is `node-cache-${{ runner.os }}-${{ runner.arch }}-pnpm-...`. > > Correction: state the host's architecture rather than a literal, by mapping `runner.arch` or `uname -m` to the tool cache's name, and keep the layout knowledge in one definition with a comment naming the tool-cache convention it mirrors and why it does. The comment serves the maintainer who must decide whether a failure means a missing bake or a changed cache layout. `src/lib/resolve-fta-binary.ts` is a precedent: its comment says it mirrors `getBinaryPath()` from fta-cli 3.x. > > I read the step and the neighbouring `Setup Node.js` and `Restore pnpm store` steps in all three workflows. Every job here declares `runs-on: ubuntu-latest`, and nothing in the tree shows an arm64 runner, so the architecture failure is conditional on one existing; today's impact is latent. The setup-node and `@actions/tool-cache` source is not in this tree and there is no network here, so the marker layout comes from my knowledge of that library and is unverified. > > A runner inventory confirming x64-only hosts would remove the architecture half of this claim. The leakage half would stand unless setup-node documents this layout as a stable interface, which would refute it. ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (0) - unadjudicated (1) - `01M6JNP55KP0B28NFXQ6B0WBTQ` medium — The baked-Node policy script is pasted as 29 identical lines into three workflows ## Coverage Coverage pass: 01M6JN1DVEVHP582E8DCZ12SYH Accounting: complete (all slots terminal; see slot results for failures) Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | no-claims | 1 | no | | writing-quality | whole | default | no-claims | 1 | no | | restated-sets | whole | default | no-claims | 1 | no | | unix-cli | whole | default | no-claims | 1 | no | | module-design | whole | default | no-claims | 1 | no | | aposd-plain | whole | default | claims-emitted | 1 | no | - test-trimming — skipped-by-dispatch: no tests changed - project-docs — skipped-by-dispatch: no project documentation changed - use-effect — skipped-by-dispatch: no React code changed - react — skipped-by-dispatch: no React code changed
🚀 1
forgejo-actions reviewed 2026-11-04 07:06:07 +00:00
.forgejo/workflows/checks.yml
@ -184,0 +186,4 @@
REQUIRED_NODE_VERSION_FILE: ".node-version"
run: |
set -eu
if [ -n "${REQUIRED_NODE_VERSION:-}" ] && [ -z "${REQUIRED_NODE_VERSION_FILE:-}" ]; then
forgejo-actions commented 2026-11-04 07:06:07 +00:00

low — The baked-Node step selects between two inputs at runtime, but only one is ever set

Every reader of the step has to work through a three-way input selection, including a diagnostic path, that cannot run in this repository. Ten of the step's roughly 25 script lines exist to choose between inputs when the choice is fixed. The result is Special-General Mixture and Nonobvious Code: the reader is left wondering who sets REQUIRED_NODE_VERSION and when.

Each of the three copies sets REQUIRED_NODE_VERSION_FILE: ".node-version" in the step's own env. A repo-wide grep finds REQUIRED_NODE_VERSION only at its read sites, with no assignment in any step, job or workflow env. So the first branch (pin="this workflow's Node pin") and the else branch (needs exactly one of REQUIRED_NODE_VERSION and REQUIRED_NODE_VERSION_FILE) are unreachable unless the runner injects that variable. AGENTS.md rules out this shape: "No parameter or option no caller passes".

Correction: reduce the selection to version="$(cat -- .node-version)" and pin=.node-version. The next step's node-version-file: ".node-version" already names the same file, and the exact-version diagnostic keeps naming the pin, so no reader-facing meaning is lost. If other repositories need an inline pin, the generator should make that choice at render time rather than shipping both paths in each rendered file.

I read the step in checks.yml, dedupe-check.yml and release.yml, and grepped the whole tree for assignments. This is static reasoning; nothing was run.

Proof gap: the wording "this workflow's Node pin" suggests a shared j4k-align template serving repositories with inline pins, but that template is outside this tree. A template that must emit both forms at runtime, with a reason render-time selection cannot work, would refute this claim. A runner-level REQUIRED_NODE_VERSION would also make the else branch fire, and nothing in the tree shows one.

lens aposd-plain · arm default · tally 2 valid / 0 invalid / 0 uncertain
claim 01M6JNP5MEY1ZMQBJSP54KCQH3 of review 01M6JN1DVB26W7TV2K2KK61BT6

<!-- review:claim:01M6JNP5MEY1ZMQBJSP54KCQH3 --> **low** — The baked-Node step selects between two inputs at runtime, but only one is ever set > Every reader of the step has to work through a three-way input selection, including a diagnostic path, that cannot run in this repository. Ten of the step's roughly 25 script lines exist to choose between inputs when the choice is fixed. The result is Special-General Mixture and Nonobvious Code: the reader is left wondering who sets `REQUIRED_NODE_VERSION` and when. > > Each of the three copies sets `REQUIRED_NODE_VERSION_FILE: ".node-version"` in the step's own `env`. A repo-wide grep finds `REQUIRED_NODE_VERSION` only at its read sites, with no assignment in any step, job or workflow `env`. So the first branch (`pin="this workflow's Node pin"`) and the `else` branch (`needs exactly one of REQUIRED_NODE_VERSION and REQUIRED_NODE_VERSION_FILE`) are unreachable unless the runner injects that variable. `AGENTS.md` rules out this shape: "No parameter or option no caller passes". > > Correction: reduce the selection to `version="$(cat -- .node-version)"` and `pin=.node-version`. The next step's `node-version-file: ".node-version"` already names the same file, and the exact-version diagnostic keeps naming the pin, so no reader-facing meaning is lost. If other repositories need an inline pin, the generator should make that choice at render time rather than shipping both paths in each rendered file. > > I read the step in `checks.yml`, `dedupe-check.yml` and `release.yml`, and grepped the whole tree for assignments. This is static reasoning; nothing was run. > > Proof gap: the wording "this workflow's Node pin" suggests a shared `j4k-align` template serving repositories with inline pins, but that template is outside this tree. A template that must emit both forms at runtime, with a reason render-time selection cannot work, would refute this claim. A runner-level `REQUIRED_NODE_VERSION` would also make the `else` branch fire, and nothing in the tree shows one. lens `aposd-plain` · arm `default` · tally 2 valid / 0 invalid / 0 uncertain claim `01M6JNP5MEY1ZMQBJSP54KCQH3` of review `01M6JN1DVB26W7TV2K2KK61BT6`
👎 1
jercik commented 2026-11-04 07:08:09 +00:00
Author
Owner

This repeats a finding already rejected in j4k/media-preservation#42 (comment). This step is generated by j4k-align and isn't an interface of this repository.

  • The REQUIRED_NODE_VERSION mode has a caller. The step is one shared script (src/template-scripts/baked-node-check-script.ts in @j4k/align@17.17.1), and align's credential-contract-forgejo.yml.hbs renders it with REQUIRED_NODE_VERSION: "{{nodeExecutionVersion}}". That template's tests cover the branch and its error.
  • Removing the branch here would be drift. Align's workflows audit would report it, and the next j4k-align --fix would restore it.
<!-- gh-feedback:reply-to:230740 --> This repeats a finding already rejected in https://code.j4k.dev/j4k/media-preservation/pulls/42#issuecomment-230702. This step is generated by j4k-align and isn't an interface of this repository. - The `REQUIRED_NODE_VERSION` mode has a caller. The step is one shared script (`src/template-scripts/baked-node-check-script.ts` in `@j4k/align@17.17.1`), and align's `credential-contract-forgejo.yml.hbs` renders it with `REQUIRED_NODE_VERSION: "{{nodeExecutionVersion}}"`. That template's tests cover the branch and its error. - Removing the branch here would be drift. Align's `workflows` audit would report it, and the next `j4k-align --fix` would restore it.
jercik marked this conversation as resolved
.forgejo/workflows/checks.yml
@ -184,0 +205,4 @@
echo "::error::$pin does not name an exact Node version (X.Y.Z), so setup-node would resolve it from GitHub."
exit 1
fi
if [ ! -f "${RUNNER_TOOL_CACHE:-/opt/hostedtoolcache}/node/$version/x64.complete" ]; then
forgejo-actions commented 2026-11-04 07:06:07 +00:00

low — The baked-Node probe hard-codes setup-node's private cache layout and assumes x64

On any runner that is not x64, a job image with the pinned Node baked in would still fail this check, and the diagnostic would wrongly say "This job image does not bake Node" and send the maintainer to j4k-align --fix or node_ci_node_versions. The same false failure would hit every job if a later actions/setup-node@v6 release stored its tools differently, because the tag floats within v6. This is Information Leakage: the workflow encodes a storage decision that belongs to the action in the next step.

The probe tests for the file ${RUNNER_TOOL_CACHE:-/opt/hostedtoolcache}/node/$version/x64.complete. That path carries three assumptions: the node/<version>/<arch>.complete marker layout, the default cache root, and a literal x64. The same jobs already treat architecture as variable, since their pnpm cache key is node-cache-${{ runner.os }}-${{ runner.arch }}-pnpm-....

Correction: state the host's architecture rather than a literal, by mapping runner.arch or uname -m to the tool cache's name, and keep the layout knowledge in one definition with a comment naming the tool-cache convention it mirrors and why it does. The comment serves the maintainer who must decide whether a failure means a missing bake or a changed cache layout. src/lib/resolve-fta-binary.ts is a precedent: its comment says it mirrors getBinaryPath() from fta-cli 3.x.

I read the step and the neighbouring Setup Node.js and Restore pnpm store steps in all three workflows. Every job here declares runs-on: ubuntu-latest, and nothing in the tree shows an arm64 runner, so the architecture failure is conditional on one existing; today's impact is latent. The setup-node and @actions/tool-cache source is not in this tree and there is no network here, so the marker layout comes from my knowledge of that library and is unverified.

A runner inventory confirming x64-only hosts would remove the architecture half of this claim. The leakage half would stand unless setup-node documents this layout as a stable interface, which would refute it.

lens aposd-plain · arm default · tally 2 valid / 0 invalid / 0 uncertain
claim 01M6JNP63DG3B7TX7S84FDYKX6 of review 01M6JN1DVB26W7TV2K2KK61BT6

<!-- review:claim:01M6JNP63DG3B7TX7S84FDYKX6 --> **low** — The baked-Node probe hard-codes setup-node's private cache layout and assumes x64 > On any runner that is not x64, a job image with the pinned Node baked in would still fail this check, and the diagnostic would wrongly say "This job image does not bake Node" and send the maintainer to `j4k-align --fix` or `node_ci_node_versions`. The same false failure would hit every job if a later `actions/setup-node@v6` release stored its tools differently, because the tag floats within v6. This is Information Leakage: the workflow encodes a storage decision that belongs to the action in the next step. > > The probe tests for the file `${RUNNER_TOOL_CACHE:-/opt/hostedtoolcache}/node/$version/x64.complete`. That path carries three assumptions: the <code>node/&lt;version&gt;/&lt;arch&gt;.complete</code> marker layout, the default cache root, and a literal `x64`. The same jobs already treat architecture as variable, since their pnpm cache key is `node-cache-${{ runner.os }}-${{ runner.arch }}-pnpm-...`. > > Correction: state the host's architecture rather than a literal, by mapping `runner.arch` or `uname -m` to the tool cache's name, and keep the layout knowledge in one definition with a comment naming the tool-cache convention it mirrors and why it does. The comment serves the maintainer who must decide whether a failure means a missing bake or a changed cache layout. `src/lib/resolve-fta-binary.ts` is a precedent: its comment says it mirrors `getBinaryPath()` from fta-cli 3.x. > > I read the step and the neighbouring `Setup Node.js` and `Restore pnpm store` steps in all three workflows. Every job here declares `runs-on: ubuntu-latest`, and nothing in the tree shows an arm64 runner, so the architecture failure is conditional on one existing; today's impact is latent. The setup-node and `@actions/tool-cache` source is not in this tree and there is no network here, so the marker layout comes from my knowledge of that library and is unverified. > > A runner inventory confirming x64-only hosts would remove the architecture half of this claim. The leakage half would stand unless setup-node documents this layout as a stable interface, which would refute it. lens `aposd-plain` · arm `default` · tally 2 valid / 0 invalid / 0 uncertain claim `01M6JNP63DG3B7TX7S84FDYKX6` of review `01M6JN1DVB26W7TV2K2KK61BT6`
🚀 1
jercik commented 2026-11-04 07:12:15 +00:00
Author
Owner

Tracked in j4k/align#1076, which documents the node/<version>/<arch>.complete tool-cache marker and the fixed x64 assumption in j4k-align's shared check. This step is generated from that source, so the change belongs there, not in this repository.

The architecture half is latent today. The ubuntu-latest job image (code.j4k.dev/j4k-oss/node-ci:26-bookworm) is a single linux/amd64 manifest according to skopeo inspect, so every job that runs this check runs on x64.

<!-- gh-feedback:reply-to:230741 --> Tracked in https://code.j4k.dev/j4k/align/pulls/1076, which documents the `node/<version>/<arch>.complete` tool-cache marker and the fixed `x64` assumption in j4k-align's shared check. This step is generated from that source, so the change belongs there, not in this repository. The architecture half is latent today. The `ubuntu-latest` job image (`code.j4k.dev/j4k-oss/node-ci:26-bookworm`) is a single `linux/amd64` manifest according to `skopeo inspect`, so every job that runs this check runs on x64.
jercik marked this conversation as resolved
jercik commented 2026-11-04 07:08:14 +00:00
Author
Owner

Replying to comment #230736

Unadjudicated claim 01M6JNP55KP0B28NFXQ6B0WBTQ (medium), "The baked-Node policy script is pasted as 29 identical lines into three workflows": not a defect. The same finding was rejected with evidence in j4k/relief-ui#21 (comment).

The three copies are rendered from one j4k-align source, src/template-scripts/baked-node-check-script.ts (@j4k/align@17.17.1), through {{templateScript "baked-node-check"}}, so a change to the check is made once. Align's workflows audit reports any hand-edited copy as drifted and --fix rewrites it. Its src/workflow-baked-node.test.ts also ties each step's .node-version pin to the following setup-node step.

> Replying to comment #230736 Unadjudicated claim `01M6JNP55KP0B28NFXQ6B0WBTQ` (medium), "The baked-Node policy script is pasted as 29 identical lines into three workflows": not a defect. The same finding was rejected with evidence in https://code.j4k.dev/j4k/relief-ui/pulls/21#issuecomment-230731. The three copies are rendered from one j4k-align source, `src/template-scripts/baked-node-check-script.ts` (`@j4k/align@17.17.1`), through `{{templateScript "baked-node-check"}}`, so a change to the check is made once. Align's `workflows` audit reports any hand-edited copy as drifted and `--fix` rewrites it. Its `src/workflow-baked-node.test.ts` also ties each step's `.node-version` pin to the following setup-node step.
jercik merged commit e86f40362d into main 2026-11-04 07:12:53 +00:00
jercik deleted branch chore/align-17-17-1 2026-11-04 07:12:53 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
j4k-oss/fta-check!22
No description provided.