Skip to content

fix: spec-author 钉源 ciw_ref 输入(ADR-0043 模式,W0-C4 #133) - #48

Merged
randypanding merged 1 commit into
mainfrom
fix-spec-author-ciwref
Aug 21, 2026
Merged

fix: spec-author 钉源 ciw_ref 输入(ADR-0043 模式,W0-C4 #133)#48
randypanding merged 1 commit into
mainfrom
fix-spec-author-ciwref

Conversation

@randypanding

Copy link
Copy Markdown
Contributor

github.action_ref 在 caller 上下文实测解析错误(指向 checkout action 的 SHA → not our ref)。改用本仓 check.yml 已有的 ciw-ref 钉源模式(ADR-0043/P2-9):caller 把 uses: pin 的 40 位 SHA 作为 ciw_ref 传入,被调方 fail-closed 校验后按该 SHA checkout 本仓。

Copilot AI lite review requested due to automatic review settings August 21, 2026 07:14
@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@randypanding, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Limit details: You’ve used all 10 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2f6876a5-01c9-4f9c-b2c9-53d5befbef02

📥 Commits

Reviewing files that changed from the base of the PR and between 086e3d4 and 39f1c32.

📒 Files selected for processing (1)
  • .github/workflows/spec-author.yml

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix spec-author reusable workflow pinning by requiring ciw_ref (ADR-0043)

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Require caller-provided ciw_ref (40-char SHA) for reusable workflow pinning.
• Fail-closed if ciw_ref is missing/invalid to prevent running unintended code.
• Checkout CI-Workflows repo at inputs.ciw_ref instead of unreliable github.action_ref.
Diagram

graph TD
  A["Caller workflow"] --> B["spec-author.yml (reusable)"] --> C{{"Validate ciw_ref"}} --> D["actions/checkout"] --> E[("CI-Workflows repo")]
  C -->|"invalid"| F["Fail closed"]

  subgraph Legend
    direction LR
    _step["Step"] ~~~ _dec{{"Decision"}} ~~~ _repo[("Repo")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep using github.action_ref
  • ➕ No additional input plumbing from caller
  • ➖ Empirically unreliable in caller context (can resolve to another action’s SHA)
  • ➖ Weakens pinning guarantees; risk of running unintended code
2. Derive ref from github.workflow_ref / github.sha
  • ➕ Avoids new explicit input
  • ➖ Does not necessarily equal the caller’s uses:@ for the reusable workflow
  • ➖ Can reintroduce ambiguity around which ref is actually pinned/executed
3. Hardcode a branch/tag ref (e.g., main)
  • ➕ Simplest configuration
  • ➖ Breaks supply-chain pinning guarantees
  • ➖ Allows behavior to change without caller approval

Recommendation: The chosen approach (explicit ciw_ref passed by the caller and validated fail-closed) is the most robust for ADR-0043-style self-pinning: it makes the pin explicit, verifiable, and independent of GitHub context variables that can mis-resolve in reusable workflows.

Files changed (1) +11 / -1

Bug fix (1) +11 / -1
spec-author.ymlRequire ciw_ref input and use it for pinned checkout +11/-1

Require ciw_ref input and use it for pinned checkout

• Adds a required ciw_ref input (40-hex SHA) for both workflow_call and workflow_dispatch. Introduces a fail-closed validation step ensuring ciw_ref matches the caller’s pinned uses SHA, then switches checkout to use inputs.ciw_ref instead of github.action_ref.

.github/workflows/spec-author.yml

@randypanding
randypanding merged commit 1cae5c8 into main Aug 21, 2026
12 checks passed
@randypanding
randypanding deleted the fix-spec-author-ciwref branch August 21, 2026 07:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates spec-author to use a caller-provided, validated ciw_ref SHA for repository checkout.

Changes:

  • Adds ciw_ref inputs for reusable and manual runs.
  • Validates the 40-character SHA fail-closed.
  • Checks out the repository at the validated SHA.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 49 to 50
# 必须显式拉本仓(CI-Workflows)且 ref=github.action_ref(调用方钉定的
# SHA,自引用钉点——被调方永远执行自己被钉定的版本,#143 实测教训)
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. ciw_ref 命名不一致 🐞 Bug ☼ Reliability
Description
该工作流把输入命名为 ciw_ref,但注释明确引用了 check.yml 的 ciw-ref 模式;调用方若按 ADR/check.yml 习惯传 ciw-ref 将不会映射到
inputs.ciw_ref,导致校验失败并阻断 workflow。
Code

.github/workflows/spec-author.yml[R14-17]

+      # 钉源自证(ADR-0043 同款,check.yml ciw-ref 模式):必须与 caller 的 uses: pin
+      # 同值(40 位 SHA)——空或非 40 位 = fail-closed 红。github.action_ref 在
+      # caller 上下文不可靠(实测解析成 checkout action 的 SHA)
+      ciw_ref: { type: string, required: true, default: "" }
Relevance

●●● Strong

Recent repository precedent explicitly uses hyphenated ciw-ref, supporting correction of the
inconsistent caller-facing input name.

PR-#32

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
spec-author.yml 新增输入为 ciw_ref 且在步骤中读取 ${{ inputs.ciw_ref }};但同仓内的 check.yml 既有模式使用的是
ciw-ref。由于 workflow_call 输入名严格匹配,调用方若传 ciw-ref 将不会赋值到 ciw_ref,从而触发新加的 fail-closed 校验。

.github/workflows/spec-author.yml[14-17]
.github/workflows/spec-author.yml[51-55]
.github/workflows/check.yml[11-15]
.github/workflows/check.yml[57-63]

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

### Issue description
The reusable workflow input is named `ciw_ref`, but the repo’s established convention (e.g., `check.yml`) and comments reference `ciw-ref`. Callers that follow the existing pattern may pass `ciw-ref`, which will not populate `inputs.ciw_ref`, causing the new fail-closed guard to trip.

### Issue Context
This workflow is invoked cross-repo (caller/callee). Input name mismatches are a common source of hard-to-debug failures.

### Fix Focus Areas
- .github/workflows/spec-author.yml[14-17]
- .github/workflows/spec-author.yml[51-60]

### Suggested fix
Pick one naming scheme and make it consistent end-to-end:
- Preferred for consistency with existing ADR/check workflow: rename the input key to `ciw-ref` (and update all references to `${{ inputs.ciw-ref }}` and error messages).
- Or keep `ciw_ref`, but update comments to stop mentioning `ciw-ref` and ensure all callers are updated accordingly.

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



Remediation recommended

2. SHA 大写被拒 🐞 Bug ≡ Correctness
Description
钉源校验只允许 [0-9a-f]{40},会把合法但包含大写 A-F 的 SHA(或上游工具输出大写)误判为无效并直接失败。结果是调用方即使传入了有效 commit id,也会在校验步骤
fail-closed。
Code

.github/workflows/spec-author.yml[R54-55]

+        run: |
+          [[ "$CIW_REF" =~ ^[0-9a-f]{40}$ ]] || { echo "::error::ciw_ref 缺失或非 40 位 SHA(caller 必须把 uses: pin 的 SHA 同时作为 ciw_ref 传入——ADR-0043 钉源模式)"; exit 1; }
Relevance

●● Moderate

The concern is technically plausible, but repository history provides no accepted precedent for
uppercase SHA normalization.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR 新增的校验步骤使用了仅匹配小写的正则,因此对包含大写字母的 40 位十六进制 SHA 会直接报错退出;Git 文档将对象名描述为 40 位十六进制字符串,十六进制表示存在大小写两种形式。

.github/workflows/spec-author.yml[51-60]
🌐 Git revision/object name is described as a full SHA-1 object name: a 40-byte hexadecimal string.
🌐 Git discussion notes mixed-case SHA-1 hex must be accepted; lowercase should be output but parsers should accept A-F as well as a-f.

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

### Issue description
The workflow validates `inputs.ciw_ref` with `^[0-9a-f]{40}$`, rejecting uppercase hex digits. Git object IDs are hex strings and may be represented with A–F; rejecting them can cause valid calls to fail.

### Issue Context
This is a fail-closed guard for the ADR-0043 pinning mode. It should validate “40 hex chars” rather than “40 lowercase hex chars”, or normalize to lowercase before validating.

### Fix Focus Areas
- .github/workflows/spec-author.yml[51-60]

### Suggested fix
- Change the regex to accept both cases: `^[0-9a-fA-F]{40}$`, OR
- Normalize `CIW_REF=$(echo "$CIW_REF" | tr '[:upper:]' '[:lower:]')` then validate with lowercase regex.

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



Informational

3. required 与 default 冲突 🐞 Bug ⚙ Maintainability
Description
workflow_call 中同时设置 required: truedefault: ""
容易造成误解且默认值语义无效;很多校验器会把该组合视为配置错误,且调用方仍必须显式传参才能通过 workflow_call 校验。
Code

.github/workflows/spec-author.yml[R14-17]

+      # 钉源自证(ADR-0043 同款,check.yml ciw-ref 模式):必须与 caller 的 uses: pin
+      # 同值(40 位 SHA)——空或非 40 位 = fail-closed 红。github.action_ref 在
+      # caller 上下文不可靠(实测解析成 checkout action 的 SHA)
+      ciw_ref: { type: string, required: true, default: "" }
Relevance

●●● Strong

Removing an ineffective default from a required workflow_call input is a deterministic configuration
cleanup aligned with validation semantics.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
spec-author.yml 为 workflow_callciw_ref 同时声明 required 与 default;社区/工具链已有明确案例指出该组合会导致“required
input not provided”并被 lint 视为错误配置。

.github/workflows/spec-author.yml[14-17]
🌐 actionlint adds a lint for workflow_call inputs where required: true and default are both set, noting the call still fails with 'Input ... is required, but not provided' when omitted.
🌐 GitHub docs show required: true inputs must be passed from the caller workflow via with, otherwise the call fails validation.

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

### Issue description
`on.workflow_call.inputs.ciw_ref` is declared with both `required: true` and `default: ""`. This is a contradictory configuration: a required input is still required (the default does not help), and tooling may flag it.

### Issue Context
The workflow already performs a fail-closed runtime validation step. The declaration should be unambiguous to callers and tooling.

### Fix Focus Areas
- .github/workflows/spec-author.yml[14-17]

### Suggested fix
Choose one:
- Keep `required: true` and remove `default`, OR
- Keep `default: ""` and set `required: false` (rely on the existing fail-closed step).

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


Grey Divider

Context sources
✅ Web pages:
  +12 more
Review mode: ⚖️ Balanced: This changes reusable-workflow inputs and fail-closed SHA validation plus checkout behavior, so it has meaningful CI supply-chain and execution risk; the localized diff does not warrant extended redundancy.
ⓘ  2 issues published inline · 3 in summary

Grey Divider

Tip of the day
💡 Did you know, you can tweak Display preferences with a live preview to see your comment before it ships

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment on lines +54 to +55
run: |
[[ "$CIW_REF" =~ ^[0-9a-f]{40}$ ]] || { echo "::error::ciw_ref 缺失或非 40 位 SHA(caller 必须把 uses: pin 的 SHA 同时作为 ciw_ref 传入——ADR-0043 钉源模式)"; exit 1; }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Sha 大写被拒 🐞 Bug ≡ Correctness

钉源校验只允许 [0-9a-f]{40},会把合法但包含大写 A-F 的 SHA(或上游工具输出大写)误判为无效并直接失败。结果是调用方即使传入了有效 commit id,也会在校验步骤
fail-closed。
Agent Prompt
### Issue description
The workflow validates `inputs.ciw_ref` with `^[0-9a-f]{40}$`, rejecting uppercase hex digits. Git object IDs are hex strings and may be represented with A–F; rejecting them can cause valid calls to fail.

### Issue Context
This is a fail-closed guard for the ADR-0043 pinning mode. It should validate “40 hex chars” rather than “40 lowercase hex chars”, or normalize to lowercase before validating.

### Fix Focus Areas
- .github/workflows/spec-author.yml[51-60]

### Suggested fix
- Change the regex to accept both cases: `^[0-9a-fA-F]{40}$`, OR
- Normalize `CIW_REF=$(echo "$CIW_REF" | tr '[:upper:]' '[:lower:]')` then validate with lowercase regex.

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

Comment on lines +14 to +17
# 钉源自证(ADR-0043 同款,check.yml ciw-ref 模式):必须与 caller 的 uses: pin
# 同值(40 位 SHA)——空或非 40 位 = fail-closed 红。github.action_ref 在
# caller 上下文不可靠(实测解析成 checkout action 的 SHA)
ciw_ref: { type: string, required: true, default: "" }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

2. Ciw_ref 命名不一致 🐞 Bug ☼ Reliability

该工作流把输入命名为 ciw_ref,但注释明确引用了 check.yml 的 ciw-ref 模式;调用方若按 ADR/check.yml 习惯传 ciw-ref 将不会映射到
inputs.ciw_ref,导致校验失败并阻断 workflow。
Agent Prompt
### Issue description
The reusable workflow input is named `ciw_ref`, but the repo’s established convention (e.g., `check.yml`) and comments reference `ciw-ref`. Callers that follow the existing pattern may pass `ciw-ref`, which will not populate `inputs.ciw_ref`, causing the new fail-closed guard to trip.

### Issue Context
This workflow is invoked cross-repo (caller/callee). Input name mismatches are a common source of hard-to-debug failures.

### Fix Focus Areas
- .github/workflows/spec-author.yml[14-17]
- .github/workflows/spec-author.yml[51-60]

### Suggested fix
Pick one naming scheme and make it consistent end-to-end:
- Preferred for consistency with existing ADR/check workflow: rename the input key to `ciw-ref` (and update all references to `${{ inputs.ciw-ref }}` and error messages).
- Or keep `ciw_ref`, but update comments to stop mentioning `ciw-ref` and ensure all callers are updated accordingly.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants