Skip to content

feat(gate): adr-required 索引世界拒引 lifecycle=archived 的 ADR(W1-C1 #164 交付 3,ADR-0053) - #207

Merged
randypanding merged 4 commits into
mainfrom
w1c1-archived-filter
Aug 21, 2026
Merged

feat(gate): adr-required 索引世界拒引 lifecycle=archived 的 ADR(W1-C1 #164 交付 3,ADR-0053)#207
randypanding merged 4 commits into
mainfrom
w1c1-archived-filter

Conversation

@randypanding

@randypanding randypanding commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

动机

W1-C1(#164 交付 3)收口:adr-required 索引世界(#174 落地)当前只校验"编号在 INDEX entries
∧ archive 正本可达"。#164 要求 lifecycle=archived 的 ADR 不可被新 PR 引用为新决策依据——
archived(编号 0017/0030 一次性破玻璃回填)是历史事件记录,不是可引用的现行决策。

Card: #164

变更(.github/workflows/gate.yml,仅索引世界路径)

  • INDEX 解析(python3)拆两路输出:adr_map.txt(active/superseded,带 archive_path)+
    adr_archived.txt(lifecycle=archived 的编号)
  • per-ref 校验先查 archived 表:命中即 ::error::…生命周期=archived(历史回填,不可作为新 决策依据) → MISSING=1(引用现行/superseding ADR 或补新 ADR)
  • 幽灵 ADR 语义不变(不在两表 → 原错误);legacy 世界路径零改动;files API 截断 fail-closed 不变
  • OK 日志追加"archived 不可引"注记

本地演练(对 live INDEX.yaml @ cd5b2b16,新逻辑精确复算)

引用 判定
ADR-0053(active) valid → archive 正本可达
编号 0019(superseded) valid(可引,仅 archived 被拒)
编号 0011(双档) valid(两条 archive_path 任一可达即过)
编号 0017(archived) REJECT(新错误路径)
编号 9999(幽灵) REJECT(防幽灵语义不回归)

有效映射 57 = 59 entries − 2 archived;archived 集合 {0017, 0030}。

在线验证(本 PR 自身即测试载体)

本 PR 改 .github/workflows/(C1 路径)且引用 ADR-0053(active,已迁 archive)——
其 adr-required 运行即索引世界正向实测:INDEX@main 命中 + archive raw 200。
#164 AC-2 正向证据;负向证据见随后一次性验证 PR。)

风险

Copilot AI lite review requested due to automatic review settings August 21, 2026 14:37
@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: 6 minutes

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: 7f7fdb36-b954-4876-9f1f-f9836e59f0e5

📥 Commits

Reviewing files that changed from the base of the PR and between ccf176a and 7a3ab0a.

📒 Files selected for processing (2)
  • .github/workflows/gate.yml
  • scripts/adr_index_map.py

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Gate adr-required: reject lifecycle=archived ADR references in index world

✨ Enhancement ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Parse INDEX.yaml into active/superseded map plus an archived-number denylist.
• Fail adr-required when PR references an archived ADR number (policy: historical backfill only).
• Keep legacy-world behavior, ghost-ADR failure semantics, and fail-closed network handling
 unchanged.
Diagram

graph TD
  A["PR text (title/body)"] --> B["adr-required job"] --> C["Fetch INDEX.yaml"] --> D["Parse INDEX (python)"] --> E["adr_map.txt (active/superseded)"] --> F["Validate ADR refs"] --> G["HEAD raw (archive)"] --> H["Pass/Fail gate"]
  D --> I["adr_archived.txt (denylist)"] --> F
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Inline lifecycle filtering during lookup (no extra file)
  • ➕ Fewer intermediate artifacts (/tmp/adr_archived.txt)
  • ➕ Slightly simpler control flow (single source of truth)
  • ➖ Harder to keep bash lookup logic simple (needs multi-column parsing + lifecycle checks)
  • ➖ Less debuggable than two explicit outputs
2. Maintain a repo-side denylist (static archived ADR numbers)
  • ➕ Removes dependency on INDEX lifecycle correctness at runtime
  • ➕ Very fast/simple gate logic
  • ➖ Duplicates source of truth (INDEX already carries lifecycle)
  • ➖ Higher maintenance burden and risk of drift

Recommendation: Current approach is a good fit: it keeps INDEX as the source of truth, makes the policy explicit via a dedicated archived denylist, and preserves existing semantics (ghost ADR failure, fail-closed behavior, legacy world untouched). The extra /tmp file is acceptable for clarity and auditability in a gate script.

Files changed (1) +21 / -7

Other (1) +21 / -7
gate.ymlReject archived ADR references in adr-required index-world path +21/-7

Reject archived ADR references in adr-required index-world path

• Extends the index-world adr-required logic to split INDEX parsing into an allow-map (active/superseded) and a denylist of lifecycle=archived numbers. During per-reference validation, archived numbers now emit an error and fail the gate before attempting archive reachability checks; success logging notes the new archived restriction.

.github/workflows/gate.yml

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

This PR updates the adr-required gate (index-world path) to reject references to ADRs whose lifecycle=archived, enforcing W1-C1 #164 delivery 3 / ADR-0053: archived ADRs are historical backfill and must not be used as new decision basis in new PRs.

Changes:

  • Split INDEX parsing output into an “active/superseded mapping” (/tmp/adr_map.txt) and an “archived set” (/tmp/adr_archived.txt).
  • Add an early check in index-world validation to fail the gate if a referenced ADR number is in the archived set.
  • Update logs/comments to reflect “archived not allowed” in the success path.

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

Comment thread .github/workflows/gate.yml Outdated
p = e.get("archive_path")
if not p:
continue
num = f"{int(e[\"number\"]):04d}"
# 让本步骤的存在性清单与 org-gate v1.4.2 按文件名校验零改动兼容),
# 正本在 archive 仓 adr/。被引 ADR-NNNN 必须在 INDEX entries 且
# archive_path 非空,并 HEAD 校验 archive 正本可达(raw 200)。
# archive_path 非空,并 HEAD 校验 archive 正本可达(raw 200);
@qodo-code-review

qodo-code-review Bot commented Aug 21, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Remediation recommended

1. Archived list skips no-path ✓ Resolved 🐞 Bug ≡ Correctness
Description
In INDEX.yaml parsing, entries are skipped when archive_path is empty before checking lifecycle, so
an entry with lifecycle=archived but empty archive_path will not be recorded in adr_archived.txt and
will later be treated as a “not in INDEX entries” ghost ADR instead of an archived ADR rejection.
Code

.github/workflows/gate.yml[R181-184]

+                    p = e.get("archive_path")
+                    if not p:
+                        continue
+                    num = f"{int(e[\"number\"]):04d}"
Relevance

●●● Strong

Deterministic correctness bug in new gate logic; team history favors fixing ADR existence/validation
edge cases.

PR-#19
PR-#61

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new parsing logic skips entries without archive_path before checking lifecycle, so those
archived numbers never reach adr_archived.txt. The subsequent validation depends on adr_archived.txt
to trigger the archived-specific rejection; if the number is missing there, it falls through to the
“not in INDEX entries” path.

.github/workflows/gate.yml[172-188]
.github/workflows/gate.yml[209-220]

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 INDEX.yaml parsing logic writes `adr_archived.txt` only for entries that have a non-empty `archive_path`, because it `continue`s on empty `archive_path` before evaluating `lifecycle`. This can misclassify archived entries that lack `archive_path` as “ghost ADRs” and bypass the intended archived-specific error path.

### Issue Context
The per-ref validation explicitly prioritizes checking `adr_archived.txt` to emit an archived-specific error. That intent is undermined if archived entries aren’t always added to the archived table.

### Fix Focus Areas
- .github/workflows/gate.yml[176-188]
- .github/workflows/gate.yml[211-216]

### Suggested change
In the Python loop:
- Compute `num` first.
- If `lifecycle == "archived"`, write `num` to `adr_archived.txt` regardless of `archive_path`.
- Only put the entry into `adr_map.txt` when `archive_path` is present and lifecycle is not archived.

This preserves the current rejection behavior but ensures the correct error classification/message for archived ADRs even when `archive_path` is missing.

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


Grey Divider

Context sources
✅ Compliance rules (platform): 17 rules
Review mode: ⚖️ Balanced: 这是修改 CI gate 运行时校验逻辑的行为性变更,涉及 ADR 引用准入与 fail-closed 语义;虽集中于单一路径,但需一次完整审查以核验解析、匹配和兼容性。

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 thread .github/workflows/gate.yml Outdated
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