Skip to content

docs(auto-add-to-project): add known callers list and concurrency rationale note#21

Merged
lml2468 merged 1 commit into
mainfrom
docs/auto-add-project-followup-notes
May 16, 2026
Merged

docs(auto-add-to-project): add known callers list and concurrency rationale note#21
lml2468 merged 1 commit into
mainfrom
docs/auto-add-project-followup-notes

Conversation

@lml2468
Copy link
Copy Markdown
Contributor

@lml2468 lml2468 commented May 16, 2026

Background

Follow-up to PR #4 (merged). Reviewer @yujiawei left two non-blocking suggestions in the review that are worth implementing now that all caller PRs have landed:

Changes

Note 4 — Known callers list
Added a # Known callers: section after the usage example, listing all 8 repos that currently call this reusable workflow. Future maintainers can now find consumers without grepping the entire org.

Note 3 — No concurrency: rationale
Added a comment explaining why concurrency: is intentionally absent: actions/add-to-project is idempotent on already-added items, so concurrent runs are harmless.

What's NOT changed

  • Note 1 (caller pinning): All callers already reference @v1, not @main
  • Note 2 (pull_request_target security): SECURITY comment already exists in the file ✅

cc @yujiawei

@lml2468 lml2468 requested a review from yujiawei May 16, 2026 10:07
Copy link
Copy Markdown

@yujiawei yujiawei left a comment

Choose a reason for hiding this comment

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

Code Review — PR #21 (.github)

Summary

Docs-only follow-up to PR #4: adds a Known callers: list (8 repos) and a short rationale for why concurrency: is intentionally absent from auto-add-to-project.yml. +13 / -0, single file, comments only — no executable change.

Verdict: Approve. The factual claims check out and the documentation lives in the right place. A couple of optional nits below.

Verification

Known callers list — accurate

I cross-checked the 8 listed repos against org:Mininglamp-OSS path:.github/workflows/auto-add-to-project.yml and confirmed each caller actually pins uses: Mininglamp-OSS/.github/.github/workflows/auto-add-to-project.yml@v1:

Repo Listed uses: confirmed
octo-web @v1
octo-adapters @v1
octo-matter @v1
octo-smart-summary @v1
octo-admin @v1
octo-lib @v1
octo-deployment @v1
octo-server @v1

No callers are missing from the list, and no listed caller is absent from the org. The @v1 pin assertion in the PR body ("All callers already reference @v1") is also accurate.

Concurrency rationale — accurate

The note claims actions/add-to-project is idempotent on already-added items. This matches the action's behavior: it issues a GraphQL addProjectV2ItemById mutation, which is no-op server-side for items already on the project board. Two concurrent runs for the same issue/PR cannot produce duplicates. Omitting concurrency: is therefore a deliberate, defensible choice rather than an oversight — worth documenting.

The triggers (issues: [opened], pull_request_target: [opened]) further bound the concern: each issue/PR can only fire opened once, so the realistic concurrent-run scenario is narrow to begin with.

Placement

Both notes sit in the leading comment block right after the usage example and before name:. Adjacent to the existing "Why a workflow instead of GitHub's native Auto-add rule" and "Direct-trigger requirements" sections — consistent with the file's documentation style.

Suggestions (non-blocking)

  1. Caller-list drift risk. This list will silently rot the moment someone adds or removes a caller without updating .github. Two lightweight options for a future PR:

    • Add a # Last verified: 2026-05-16 line so readers know the freshness window.
    • Or, longer term, have a scheduled workflow in this repo run the same gh api search/code query and open an issue when the list drifts. Not for this PR.
  2. Sort order. Listing the 8 callers alphabetically (octo-adapters, octo-admin, octo-deployment, octo-lib, octo-matter, octo-server, octo-smart-summary, octo-web) would make future "is X a caller?" lookups slightly faster. Pure nit.

  3. Concurrency note wording. "idempotent on already-added items" is correct but slightly understates it — the GraphQL mutation itself is idempotent regardless of prior state, which is why concurrent runs are safe even in the (impossible-for-opened) case of two simultaneous first-adds. If you ever loosen the trigger types, this distinction matters. Optional rephrase: … because actions/add-to-project's underlying addProjectV2ItemById mutation is idempotent — duplicates are not created even under concurrent first-adds.

Risk assessment

  • Behavior change: none (comments only).
  • CI impact: none.
  • Security: none — the SECURITY block above pull_request_target: is untouched.
  • Reversibility: trivial.

Ship it.

Copy link
Copy Markdown

@yujiawei yujiawei left a comment

Choose a reason for hiding this comment

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

Code Review — PR #21 (.github)

Summary

Docs-only follow-up to PR #4: adds a Known callers: list (8 repos) and a short rationale for why concurrency: is intentionally absent from auto-add-to-project.yml. +13 / -0, single file, comments only — no executable change.

Verdict: Approve. The factual claims check out and the documentation lives in the right place. A couple of optional nits below.

Verification

Known callers list — accurate

I cross-checked the 8 listed repos against org:Mininglamp-OSS path:.github/workflows/auto-add-to-project.yml and confirmed each caller actually pins uses: Mininglamp-OSS/.github/.github/workflows/auto-add-to-project.yml@v1:

Repo Listed uses: confirmed
octo-web @v1
octo-adapters @v1
octo-matter @v1
octo-smart-summary @v1
octo-admin @v1
octo-lib @v1
octo-deployment @v1
octo-server @v1

No callers are missing from the list, and no listed caller is absent from the org. The @v1 pin assertion in the PR body ("All callers already reference @v1") is also accurate.

Concurrency rationale — accurate

The note claims actions/add-to-project is idempotent on already-added items. This matches the action's behavior: it issues a GraphQL addProjectV2ItemById mutation, which is no-op server-side for items already on the project board. Two concurrent runs for the same issue/PR cannot produce duplicates. Omitting concurrency: is therefore a deliberate, defensible choice rather than an oversight — worth documenting.

The triggers (issues: [opened], pull_request_target: [opened]) further bound the concern: each issue/PR can only fire opened once, so the realistic concurrent-run scenario is narrow to begin with.

Placement

Both notes sit in the leading comment block right after the usage example and before name:. Adjacent to the existing "Why a workflow instead of GitHub's native Auto-add rule" and "Direct-trigger requirements" sections — consistent with the file's documentation style.

Suggestions (non-blocking)

  1. Caller-list drift risk. This list will silently rot the moment someone adds or removes a caller without updating .github. Two lightweight options for a future PR:

    • Add a # Last verified: 2026-05-16 line so readers know the freshness window.
    • Or, longer term, have a scheduled workflow in this repo run the same gh api search/code query and open an issue when the list drifts. Not for this PR.
  2. Sort order. Listing the 8 callers alphabetically (octo-adapters, octo-admin, octo-deployment, octo-lib, octo-matter, octo-server, octo-smart-summary, octo-web) would make future "is X a caller?" lookups slightly faster. Pure nit.

  3. Concurrency note wording. "idempotent on already-added items" is correct but slightly understates it — the GraphQL mutation itself is idempotent regardless of prior state, which is why concurrent runs are safe even in the (impossible-for-opened) case of two simultaneous first-adds. If you ever loosen the trigger types, this distinction matters. Optional rephrase: … because actions/add-to-project's underlying addProjectV2ItemById mutation is idempotent — duplicates are not created even under concurrent first-adds.

Risk assessment

  • Behavior change: none (comments only).
  • CI impact: none.
  • Security: none — the SECURITY block above pull_request_target: is untouched.
  • Reversibility: trivial.

Ship it.

@lml2468 lml2468 merged commit 68fca11 into main May 16, 2026
1 of 2 checks passed
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