Skip to content

refactor(ci): standardize rolling-issue dedup on embedded HTML markers - #109

Merged
kyle-sexton merged 9 commits into
mainfrom
refactor/rolling-issue-dedup-standardize
Jul 16, 2026
Merged

refactor(ci): standardize rolling-issue dedup on embedded HTML markers#109
kyle-sexton merged 9 commits into
mainfrom
refactor/rolling-issue-dedup-standardize

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes decision #56 (Decisions Log: https://claude.ai/code/artifact/232ecdce-8316-4880-8c0a-dc3c7dcf3a63).

Corrected premise: the decision assumed 3 rolling-maintenance-issue workflows, each with a different dedup mechanism. Empirically there are 4: link-check.yml, queue-monitor-liveness.yml, and tool-version-drift-check.yml all used the same title-search dedup (gh issue list --search 'in:title "..."'), while only pulumi-version-drift-check.yml already used an embedded-HTML-marker. The intent (converge on the marker mechanism) still holds — it's a 3-file port to the 4th file's existing pattern, not "each different."

Why markers over title search

Title search breaks on a retitled issue (the search string no longer matches) and can false-positive-match an unrelated issue that happens to share title text. A marker embedded in the issue body is exact and survives a retitle — the same reasoning pulumi-version-drift-check.yml already documents for its own mechanism.

Implementation

Ported all 3 to marker-based lookup, inlined per file (not a shared composite action). I could not confirm from GitHub's docs, and found no existing precedent in this repo, that a reusable workflow's uses: ./.github/actions/... reference resolves against its own repo when called cross-repo — rather than build on unverified behavior for something that gates real issue-tracking, I inlined the lookup logic directly in each file, matching pulumi-version-drift-check.yml's own single-script style.

  • link-check.yml: marker prepended to lychee's generated report file before it's passed to create-issue-from-file.
  • queue-monitor-liveness.yml, tool-version-drift-check.yml: marker added as the first line of their existing hand-built issue-body heredocs.

Each workflow's own marker is scoped to it (<!-- ci-workflows:<workflow-name>:v1:active -->), consistent with pulumi-version-drift-check.yml's naming.

Search is scoped state=open, matching the original title-search's scoping. pulumi-version-drift-check.yml can safely search state=all only because it swaps its marker to a :resolved sentinel on close; none of these 3 do that, so state=all would keep matching a closed issue's stale :active marker on every later run.

Verification

  • zizmor on all 3 modified files: no findings (3 suppressed, matching repo baseline).
  • actionlint: clean on link-check.yml and queue-monitor-liveness.yml. tool-version-drift-check.yml hangs locally in this environment — confirmed pre-existing, reproduces identically against the unmodified file on main, unrelated to this change. Repo's own hosted CI actionlint will validate it on this PR.
  • No behavior change to what each workflow actually checks — only the tracking-issue lookup mechanism changed.

Closes decision #56 (naming-ci-workflows-dedup-mechanism-standardization,
Decisions Log: https://claude.ai/code/artifact/232ecdce-8316-4880-8c0a-dc3c7dcf3a63).

Corrected premise: there are 4 rolling-maintenance-issue workflows in this
repo, not 3 — link-check.yml, queue-monitor-liveness.yml, and
tool-version-drift-check.yml all used the same title-search dedup
(gh issue list --search 'in:title "..."'), while only
pulumi-version-drift-check.yml already used an embedded-HTML-marker.
Title search breaks on a retitled issue and can match an unrelated issue
that happens to share title text; a marker embedded in the body is exact
and survives a retitle.

Ported the 3 title-search workflows to pulumi-version-drift-check.yml's
marker mechanism (inlined per file, not a shared composite action — a
reusable workflow's local-action resolution across repos is unverified
against GitHub's docs and this repo has no existing precedent for it, so
inlining is the empirically-safe choice; it also matches
pulumi-version-drift-check.yml's own single-script style).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 0s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4f7cb9559f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/link-check.yml Outdated
state=all matched the marker on closed issues too, since none of these
three workflows swap the marker to a :resolved sentinel on close (unlike
the pulumi-version-drift-check.yml reference, which does swap and is the
only workflow that safely searches state=all). Without a swap, a closed
issue keeps matching state=all forever: the next failure either targets a
closed issue-number (unverified whether create-issue-from-file reopens
it) or opens a second issue, which then trips the script's own
count > 1 guard. state=open matches the original title-search scoping
these workflows had before the marker migration and needs no reopen
assumption.
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4a7c6b6cc0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/link-check.yml Outdated
Comment thread .github/workflows/link-check.yml
Comment thread .github/workflows/tool-version-drift-check.yml Outdated
link-check.test.cjs still sliced the lookup-to-update range as a single
"no if:" block and asserted the pre-migration `steps.tracking.outputs.number`
key; the new "Embed the marker in the lychee report" step sits between lookup
and update and is conditional, and the lookup step now emits
`issue-number`. Narrow the "no if:" assertion to just the lookup step and
match the renamed output key throughout.

network-timeout-policy.test.cjs still asserted the pre-migration
`existing="$(gh_read issue list` shape for tool-version-drift-check.yml;
match the marker-based `matches="$(gh_read api --paginate` lookup instead.
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb6c0390f0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/link-check.yml Outdated
link-check.yml resolved its rolling tracking issue with a raw, unbounded
gh api call, while its sibling tool-version-drift-check.yml routes the
identical lookup through a gh_read() timeout+retry wrapper. That left one
of the two rolling-issue workflows outside the network-timeout policy in a
change set whose whole purpose is to standardize the mechanism.

Give link-check the same bounded gh_read wrapper, and add a
network-timeout-policy test asserting its lookup routes through gh_read so
the gap can't silently reopen.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012A6iDcXrxwNXEjgTM3RC5J
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 0s —— View job


I'll analyze this and get back to you.

link-check.yml is a reusable workflow: a caller can invoke it more than
once with different inputs.issue-title/args to track separate reports
(e.g. docs links vs. site links). The marker this PR standardized on was
a fixed constant, so every invocation searched for the same marker -- a
healthy invocation could close another invocation's still-failing
tracking issue, and two failing invocations would overwrite the same
issue with different titles/bodies.

Scope the marker with a hash of inputs.issue-title, the same key the
pre-marker title-search lookup used, so distinct configured reports keep
distinct tracking issues. Compute it once in the lookup step and pass it
to the embed step via a step output so both stay byte-identical.
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 0s —— View job


I'll analyze this and get back to you.

Address the unresolved P2 review finding from PR #109 ("Preserve existing
title-based tracking issues").

The marker-based lookup this branch introduced (4f7cb95) only finds issues
whose body already contains the scoped marker. A still-open issue created by
the previous title-search version of this workflow has no marker, so the
lookup returns empty and create-issue-from-file opens a second rolling issue
instead of updating (and eventually closing) the pre-existing one -- the
exact single-issue invariant this migration exists to preserve.

Fetch the open-issues page once into open_issues and reuse it for both
searches: the marker search first, and, only when it finds nothing, a
one-time title-exact fallback mirroring the pre-marker lookup this workflow
used before the marker migration. Once a fallback-adopted issue is updated,
its body carries the marker (the following "Embed the marker" step already
stamps every report body), so the fallback naturally stops applying to it
on the next run.

Updates network-timeout-policy.test.cjs's link-check assertion for the
renamed open_issues variable this refactor introduces; the bounded gh_read
routing it verifies is unchanged.

The second half of this same reviewer finding ("Ignore closed marker
matches before updating issues") was already fixed earlier on this branch
by 4a7c6b6 (state=all -> state=open), which independently closes that gap;
its review thread just was never marked resolved.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012A6iDcXrxwNXEjgTM3RC5J
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 0s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d4a6088e5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/queue-monitor-liveness.yml Outdated
link-check.yml's marker-search-with-title-fallback fix (2d4a608)
addressed only the workflow named in review. queue-monitor-liveness.yml
and tool-version-drift-check.yml carry the identical pre-marker/marker
rolling-issue pattern and the same gap: an issue created before the
marker existed is invisible to a marker-only search. Apply the same
fallback to both, and update the shared contract test's regex for the
renamed open_issues variable.
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 3m 24s —— View job


Code Review

The architecture is sound and the Codex concerns from earlier rounds have all been addressed. I found one correctness inconsistency in the network-call policy and a missing test to cover it.


One Real Finding

queue-monitor-liveness.yml find-issue step uses unbounded gh api

queue-monitor-liveness.yml line 163:

open_issues="$(gh api --paginate --slurp "repos/${GH_REPO}/issues?state=open&per_page=100")"

This is a plain, unbound gh api call. The gh_read helper defined inside the check step (lines 62–77) is not available here — each step runs in a fresh shell. The job-level timeout-minutes: 5 provides a coarse cap but no per-call retry.

Compare the other two workflows modified in this PR:

The network-timeout-policy.test.cjs test at line 62 explicitly asserts the link-check tracking lookup goes through gh_read; a matching assertion for queue-monitor-liveness.yml is missing and would have caught this.

Fix this →


Everything Else Looks Good

Codex P2 — state=all / closed issue reuse: Not applicable. All three workflows use state=open. Closed issues are never returned by the marker search, so create-issue-from-file creates a fresh issue on the next failure. The PR description explains why state=all is only safe for pulumi-version-drift-check.yml (which swaps to a :resolved sentinel on close). ✓

Codex P2 — migration fallback for pre-marker issues: Present and correct in all three workflows. The title-search fallback runs only when the marker search returns zero results and hits open issues, meaning it will not adopt a closed pre-marker issue. The create-issue-from-file update step then embeds the marker in the body, so the fallback only triggers once per repo. ✓

Marker scoping in link-check: The sha256sum | cut -c1-12 keying on ISSUE_TITLE is the right approach for a reusable workflow that can be invoked multiple times with different report titles. The marker is output as steps.tracking.outputs.marker and re-used in the embed step — it cannot drift between the two steps. ✓

state=open vs. state=all: Using state=open is correct for workflows that don't swap to a :resolved sentinel. A closed-and-manually-resolved tracking issue stays closed; the next failure opens a fresh one. ✓

network-timeout-policy.test.cjstool-version-drift-check.yml assertion (line 55–59): The regex /Find existing tracking issue[\s\S]*?set -euo pipefail[\s\S]*?open_issues="\$\(gh_read api --paginate/u matches the actual content. ✓

link-check.test.cjs — insert of "Embed the marker" step between find and update: The test slices lookupStep up to the next \n - name: boundary (which is now "Embed the marker"), so the if:-absence assertion still covers only the find step. The updateStep is correctly sliced from the update step to the close step. ✓

SHA pins: All uses: pins carry a full 40-char SHA with a version comment. ✓

Security posture: No new secrets, no broadened token usage, no pull_request_target exposure. ✓


Summary

One consistency bug: queue-monitor-liveness.yml's find-issue step needs the gh_read wrapper re-declared inside it (as the other two do), plus a matching assertion in network-timeout-policy.test.cjs. Everything else is well-structured and the previous Codex findings have all been resolved.

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8cf08aa28d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/link-check.yml
The pre-marker version of this workflow required `--label link-check`
when matching a tracking issue by title. The marker refactor (4f7cb95)
dropped that constraint: both the marker match and the title fallback
now search every open issue in the repository, so an issue whose body
or title happens to match (coincidentally, or by construction in a repo
where issue creation isn't maintainer-only) can be adopted and then
overwritten or closed by this automation. Filter the source fetch by
the label the create step already applies, restoring the original
constraint for both lookup paths.
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 2m 41s —— View job


Code Review

  • Gather context and understand request
  • Read changed files and current diff
  • Verify prior finding (queue-monitor-liveness.yml gh_read wrapper)
  • Review new commit (label constraint restoration in link-check.yml)

New Commit (af9290e) — Label Constraint Restoration

This commit correctly addresses Codex's last P2 comment. Adding labels=link-check to the issues API query (link-check.yml line 120) restores the pre-marker ownership constraint: only issues carrying the link-check label — which the create step has always applied — can be adopted or closed by this automation. Because open_issues is now pre-filtered by the label, the title fallback (line 137) is implicitly scoped to the same set with no extra network call needed. The inline comment explains the rationale clearly. No issues with this commit.


Still-Open Finding from Prior Round

The prior review's one real finding has not been addressed by this commit.

queue-monitor-liveness.yml find-issue step uses bare gh api

queue-monitor-liveness.yml line 163:

open_issues="$(gh api --paginate --slurp "repos/${GH_REPO}/issues?state=open&per_page=100")"

The two sibling workflows both define gh_read() inside their find-issue steps and route the call through it:

queue-monitor-liveness.yml has neither the helper definition nor the bounded call. The timeout-minutes: 5 job cap is a coarse guard only; a stalled GitHub API call can block for the full 5 minutes before the job is killed, vs. the 60s per-call ceiling the gh_read wrapper enforces in the other two workflows. The network-timeout-policy.test.cjs test explicitly asserts the gh_read api --paginate pattern for both sibling workflows (lines 57–59, 62–72) but has no matching assertion for queue-monitor-liveness.yml, which would have caught this.

Fix this →


Everything Else Confirmed

  • tool-version-drift-check.yml: gh_read() defined and used correctly (lines 349–390). ✓
  • link-check.yml test coverage (link-check.test.cjs): both test cases (rolling-issue lifecycle and per-report marker scoping) still assert the correct step names, if: placement, and output references after the "Embed the marker" step was inserted between find and update. ✓
  • network-timeout-policy.test.cjs: tool-version-drift-check.yml and link-check.yml assertions pass against the current files. ✓
  • state=open scoping: correct for all three workflows (none swap to a :resolved sentinel on close). ✓
  • Migration title fallbacks: present and correct in all three workflows; the open_issues variable is reused without a second network call. ✓
  • SHA pins: all uses: carry a full 40-char SHA with version comment. ✓
  • Security posture: no new secrets, no broadened token grant, no pull_request_target. ✓

Summary

One finding carries over from the previous round: queue-monitor-liveness.yml line 163 still calls gh api directly without the bounded gh_read wrapper that the two sibling workflows both define locally. The latest commit (af9290e) is correct and closes the label-constraint gap in link-check.yml. Everything else is in good shape.

@kyle-sexton
kyle-sexton merged commit b1f25e8 into main Jul 16, 2026
33 checks passed
@kyle-sexton
kyle-sexton deleted the refactor/rolling-issue-dedup-standardize branch July 16, 2026 04:52
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.

1 participant