Skip to content

Add blog-hub-notify Action: file hub sync issues when posts publish - #42

Merged
mickdarling merged 8 commits into
developfrom
feature/blog-hub-notify
Aug 1, 2026
Merged

Add blog-hub-notify Action: file hub sync issues when posts publish#42
mickdarling merged 8 commits into
developfrom
feature/blog-hub-notify

Conversation

@mickdarling

Copy link
Copy Markdown
Member

Closes #41. Spoke side of DollhouseMCP/DollhouseResearch-website#39 (revised event-driven design, no auto-PR).

What it does

On push to main touching _blog_posts/**:

  1. First-parent diff finds added/modified posts (deletions ignored)
  2. Extracts title, date, description from front matter (commit date as fallback)
  3. Files one issue per post on DollhouseMCP/DollhouseResearch-website, labeled blog-hub-sync (label already created), containing a ready-to-paste _data/blog_hub.yml entry and the canonical /blog/<slug>/ URL
  4. Dedupes: skips any slug that already has an open sync issue

The same-session manual rule in AGENTS.md remains the fast path; this is the backstop that removes staleness risk.

Setup needed after merge (Mick)

Create secret BLOG_HUB_NOTIFY_TOKEN on this repo: fine-grained PAT, resource owner DollhouseMCP, repository access only DollhouseResearch-website, permissions Issues: Read and write. Until it exists the workflow no-ops with a step-summary notice (VisiDelta-style) instead of failing the push.

Verification

  • YAML parses (PyYAML), embedded script passes bash -n
  • Actions pinned to full SHAs per repo convention
  • Trigger only fires on main + _blog_posts/** paths, so PR branches never run it

🤖 Generated with Claude Code

https://claude.ai/code/session_018y4M47JPR3dawoDUpCTbtd

Closes #41. Implements the spoke side of the revised design in
DollhouseResearch-website#39: on push to main touching _blog_posts/,
extract each post's front matter and file a blog-hub-sync issue on the
hub repo with a ready-to-paste blog_hub.yml entry. No auto-PR by
design; the issue is the signal for an editorial pass. No-ops with a
step-summary notice when BLOG_HUB_NOTIFY_TOKEN is unset, and dedupes
against existing open sync issues per slug.

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

@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: 2f85cd8c54

ℹ️ 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/blog-hub-notify.yml Outdated
Comment on lines +94 to +96
- title: "${title}"
date: ${date}
summary: ${description}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Quote and escape the generated YAML scalars

When a description contains YAML-significant text such as : , stripping its front-matter quotes and emitting it as a plain scalar produces an invalid ready-to-paste entry. This already occurs for _blog_posts/dollhousemcp-console-tour.md, whose description causes mapping values are not allowed in this context; titles containing double quotes can similarly break the quoted title. Serialize or correctly quote and escape these values before embedding them.

Useful? React with 👍 / 👎.

Comment thread .github/workflows/blog-hub-notify.yml Outdated

# First-parent diff so a merge from develop reports what actually
# landed on main. Added + modified posts only; deletions ignored.
mapfile -t files < <(git diff --name-only --diff-filter=AM HEAD^ HEAD -- '_blog_posts/*.md' || true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Include renamed posts in the notification diff

When a published post file is renamed, Git classifies the change as R, so --diff-filter=AM returns neither the old nor the new path and the workflow exits without filing an issue. The hub can consequently retain the old dead canonical URL; include renamed destinations in the diff, or disable rename detection so the destination is treated as an addition while deletions remain ignored.

Useful? React with 👍 / 👎.

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review: blog-hub-notify Action

Reviewed the new workflow end-to-end against the repo existing action-pinning conventions and against the actual front matter of the posts in _blog_posts/. Overall this is a solid, low-risk addition (push-only trigger, least-privilege PAT, graceful no-op path) - a few correctness issues in the front-matter extraction are worth fixing before it processes a real post, plus one reliability nit in the dedup logic.

Findings

1. description extraction silently drops content for posts that use excerpt instead (moderate)
The description lookup only matches a literal description: key. One of the seven existing posts, _blog_posts/the-15-minute-mystery-ai-agents-chase-ghosts-in-ci.md, has no description field at all, only excerpt:. If that post (or another like it) is ever modified again, the generated issue will have a blank summary line instead of falling back to excerpt. Worth adding a fallback: try description, then fall back to excerpt if empty.

2. summary: line is emitted unquoted in the generated snippet, unlike title (moderate)

- title: "TITLE"
  date: DATE
  summary: DESCRIPTION

title is wrapped in quotes but summary is not. Any description containing a colon+space, a leading dash/hash/asterisk, or other YAML-significant character will produce an invalid ready-to-paste snippet, which undercuts the main value proposition of the issue body. None of the current descriptions happen to trip this, but it is a matter of when, not if. Suggest quoting the summary value the same way title is.

3. Neither title nor summary/description escapes embedded double quotes (minor)
If a post title or description itself contains a double-quote character, the front-matter extraction strips the wrapping quotes on read but the re-emitted quoted value does not escape internal quotes, so the pasted YAML breaks. Lower likelihood than #2, but same root cause, worth a shared escape step if you are touching this code for #1/#2.

4. Dedup relies on GitHub fuzzy, eventually-consistent search (minor-moderate)
The dedup check uses gh issue list --search with a quoted slug scoped to in:title. Slugs are hyphenated (e.g. fixing-mcp-server-disconnected-claude-desktop), and GitHub search tokenization around hyphens is not guaranteed to keep the quoted phrase intact, so this could both false-positive (skip filing when it should not) and false-negative (file a duplicate) depending on how the index tokenizes a given slug. Search results also lag actual issue creation, so two pushes close together could double-file before the first issue is indexed. Since the check already filters by the blog-hub-sync label, an exact-match comparison (list open issues with that label as JSON titles, and compare the exact expected title string in bash) would be more deterministic than relying on search-phrase semantics.

5. The _blog_posts/*.md glob used for the diff is not recursive (minor, future-proofing)
The trigger path filter (_blog_posts/**) is recursive, but the git diff pathspec used to find changed posts only matches files directly in _blog_posts/. Fine today since all posts are flat, but if posts are ever organized into subfolders (e.g. by year), the workflow would still fire on the push but silently skip the nested files in the diff. Cheap to future-proof now with a recursive glob.

What is good here

  • Action pinning: the actions/checkout SHA used here matches the exact SHA already used in jekyll-build.yml, website-quality.yml, and visidelta-preview.yml, consistent with repo convention rather than a fabricated or unverified hash.
  • Least privilege: top-level permissions: contents: read, with the cross-repo gh issue calls scoped to a separate fine-grained PAT (BLOG_HUB_NOTIFY_TOKEN) limited to Issues on just the hub repo, rather than broadening the default GITHUB_TOKEN.
  • Graceful degradation: missing PAT produces a step-summary notice and a clean exit instead of failing the push, so it will not block deploys before the secret is configured.
  • Trigger scoping: push to main plus _blog_posts/** paths only, so PR branches never run it, and since it is push-triggered (not pull_request), it is not attacker-controllable by external contributors without main-branch write access.
  • set -euo pipefail with an explicit fallback around the git diff process substitution reasonably handles the no-parent-commit edge case without masking other failures.

Test coverage

No automated test exercises the embedded bash, which is understandable for a small ops workflow, and the PR description notes bash -n and YAML parsing were checked manually. That said, finding #1 above (the excerpt-vs-description posts) would have been caught immediately by running the front-matter extraction against the repo actual 7 existing posts before merging, which is cheap to do locally.


Generated with Claude Code

- Fall back to excerpt when a post has no description (one existing
  post relies on it)
- Quote and escape the summary and title in the generated snippet;
  several live summaries contain colons that would break unquoted YAML
- Dedup by exact title against open blog-hub-sync issues instead of
  GitHub search (tokenization + index lag made search unreliable)
- Recursive diff pathspec so posts in future subfolders are picked up

Verified: front-matter extraction run against all 7 existing posts;
YAML parses; bash -n clean.

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

@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: d2ff480ab2

ℹ️ 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/blog-hub-notify.yml Outdated
# First-parent diff so a merge from develop reports what actually
# landed on main. Added + modified posts only; deletions ignored.
# Recursive under _blog_posts/ so posts in future subfolders count.
mapfile -t files < <(git diff --name-only --diff-filter=AM HEAD^ HEAD -- '_blog_posts/' | grep -E '\.md$' || true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Diff the complete push range

When one push advances main by multiple commits, such as a rebase-merged PR, this examines only the final commit and misses posts added or modified by earlier commits in the same push, even though those changes caused the workflow to trigger. Diff github.event.before through GITHUB_SHA and fetch enough history so every published post in the push is reported.

Useful? React with 👍 / 👎.

Comment thread .github/workflows/blog-hub-notify.yml Outdated
infm && index($0, key ": ")==1 {
sub(key ": ", ""); print; exit
}
' "$1" | sed -E 's/^"(.*)"$/\1/; s/^'"'"'(.*)'"'"'$/\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.

P2 Badge Parse front matter before re-encoding it

For valid quoted YAML containing escapes, removing only the outer quotes leaves the YAML encoding in the value and corrupts the generated entry. For example, title: "Agent \"Profiles\"" is emitted by the new yaml_dq path as a title containing literal backslashes, while a single-quoted contraction such as 'DollhouseMCP''s' retains the doubled apostrophe. This behavior of the newly added escaping path is fresh evidence beyond the prior review comment; parse the front matter as YAML before serializing its values.

Useful? React with 👍 / 👎.

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review: blog-hub-notify.yml

Solid design overall — least-privilege PAT scoped to Issues on just the hub repo, contents: read on the default token, trigger correctly scoped to push on main (never runs on PR branches), graceful no-op with a step-summary notice when the secret is absent, and the exact-title dedup approach is a sensible fix for the GitHub-search-unreliability issue called out in the second commit. The actions/checkout SHA also checked out fine — it matches the same pin already used in jekyll-build.yml/website-quality.yml/visidelta-preview.yml, so it's consistent with repo convention rather than a one-off typo.

One correctness bug and a couple of minor robustness points:

Correctness: git diff HEAD^ HEAD only sees the last commit of a push, not the whole push

.github/workflows/blog-hub-notify.yml:52

mapfile -t files < <(git diff --name-only --diff-filter=AM HEAD^ HEAD -- '_blog_posts/' | grep -E '\.md$' || true)

HEAD^ HEAD diffs the tip commit against its immediate parent. For a normal PR merged with "Create a merge commit" (which this repo appears to use — e.g. 744c934 Merge pull request #35 from ...), that's correct: HEAD^ is the pre-merge state on main, so the first-parent diff captures every commit the PR brought in, matching the comment's intent.

But it breaks for any push that lands multiple non-merge commits on main in one push — e.g. a maintainer pushing two local commits at once, or a "Rebase and merge" PR strategy if that's ever enabled. If commit A (earlier in the push) adds _blog_posts/new-post.md and commit B (the tip) touches something unrelated, the workflow still fires (the trigger's paths filter looks at the whole push), but git diff HEAD^ HEAD only compares B against A — new-post.md doesn't differ between those two, so files comes back empty, "No added or modified posts in this push" is logged, and the post is silently and permanently missed (there's no later run that will ever re-diff that range).

The standard fix is to diff ${{ github.event.before }} against ${{ github.sha }} instead of HEAD^/HEAD, guarding for before being the all-zero SHA (new branch) and giving checkout enough fetch-depth to have before available locally.

Minor: || true also swallows a genuine git diff failure, not just "no .md matches"

Same line (52). The || true is presumably there to stop grep finding zero .md files from tripping pipefail under set -e — but it also silently absorbs a failing git diff itself (e.g. HEAD^ being unresolvable, or insufficient history from fetch-depth: 2 in some edge case), producing the same "no posts" output as the legitimate empty case. Worth splitting the "no .md files matched" case from "the diff command itself failed" so a real failure doesn't look identical to a normal no-op run in the logs.

Minor: no concurrency guard → possible duplicate issues on rapid successive pushes

The dedup logic fetches open_titles once per run (line 76) and then checks it in-memory per post. If two pushes touching _blog_posts/ land close together, both runs can fetch the open-issue list before either has created its issue, and both will file a blog-hub-sync issue for the same slug. Low-severity (duplicate issue, not data loss), but a concurrency: group: blog-hub-notify (without cancel-in-progress, so runs queue rather than skip) would close the window cheaply if it's worth the complexity.

Test coverage

The PR body's manual verification (front-matter extraction against all 7 existing posts, bash -n, YAML parse) is a reasonable substitute given how hard this is to unit test as embedded YAML bash. No blocking ask here, but if you want easier iteration later, a workflow_dispatch input to re-run the "file issues" step against an arbitrary commit range would make it possible to dry-run against real history without waiting for a qualifying push.

Nothing here blocks merging if the multi-commit-push case is an accepted edge case for now — just flagging it since it's exactly the staleness scenario this Action exists to backstop.

- Diff github.event.before..sha instead of HEAD^..HEAD so multi-commit
  pushes can't silently skip a post (zero-SHA guarded via first parent)
- Let a failing git diff fail the step; only zero .md matches is a no-op
- Queue concurrent runs (concurrency group, no cancel) to close the
  double-file window on rapid pushes
- Add workflow_dispatch with an explicit base/head range for dry runs

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

@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: dc987c14c5

ℹ️ 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/blog-hub-notify.yml Outdated
Comment on lines +29 to +31
concurrency:
group: blog-hub-notify
cancel-in-progress: false

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve every queued push notification

When three or more pushes arrive while the first run is active, GitHub Actions retains only one pending run per concurrency group and replaces the existing pending run with the newest one even when cancel-in-progress is false. Because each run processes only its own github.event.before..github.sha range, the replacement run does not cover changes unique to the canceled middle push, so a post from that push can remain permanently unreported. Use a mechanism that coalesces ranges or otherwise persists every push rather than relying on this concurrency group as a queue.

Useful? React with 👍 / 👎.

Comment thread .github/workflows/blog-hub-notify.yml Outdated
Comment on lines +150 to +153
gh issue create -R "$HUB_REPO" \
--title "$issue_title" \
--label blog-hub-sync \
--body-file "$body_file"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Make the advertised dry run side-effect free

When a maintainer invokes workflow_dispatch expecting the documented dry run, the workflow still executes gh issue create for every matching post and files real issues in the hub repository. Either skip issue creation and print the proposed bodies for dispatch runs, or stop advertising this input and dispatch path as a dry run so manual verification cannot create unintended issues.

Useful? React with 👍 / 👎.

Comment thread .github/workflows/blog-hub-notify.yml Outdated
Comment on lines +115 to +117
title="$(fm_field "$f" title)"
date="$(fm_field "$f" date)"
description="$(fm_field "$f" description)"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Read dispatched metadata from the requested head

When workflow_dispatch supplies a head_sha other than the checked-out workflow ref, the diff correctly selects paths from that requested head but these calls read each file from the checkout working tree instead. A post changed again after the requested head therefore produces metadata from the wrong revision, while a post absent at the checkout tip makes awk fail; extract the front matter from $head or explicitly check out the requested head before processing.

Useful? React with 👍 / 👎.

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review: blog-hub-notify.yml

Solid, well-thought-out workflow — the concurrency guard, exact-title dedup, first-parent-fallback for new branches, and the "no-op with a step-summary notice instead of failing" behavior when the PAT is missing are all good defensive choices. A few notes:

Nitpick / doc drift

  • The top-of-file comment and the workflow's own generated issue body reference "the same-session manual rule (see AGENTS.md)" / "the hub publishing rule in the DollhouseResearch-website README," but I couldn't find that rule documented anywhere in this repo's AGENTS.md. Worth double-checking the rule actually lives where the comment says, so a future reader isn't sent chasing a reference that doesn't exist here.
  • The workflow_dispatch inputs are described as being "for the dry-run diff range," but there's no actual dry-run gate in the script — triggering it manually (with the PAT present) will file real issues on the hub repo, same as a push would. Consider either renaming the input description or adding a real DRY_RUN flag that skips the gh issue create call and just prints what it would file.

Edge cases (low risk, worth being aware of)

  • fm_field() only handles single-line scalar front matter (key: value). It doesn't handle YAML block scalars (description: >- / description: |) — if a future post used that style, sub(key ": ", "") would still match and capture the block indicator (>-) as the literal description text, silently producing a garbled summary: line in the generated issue rather than failing loudly. Current posts all use quoted single-line scalars (checked a few in _blog_posts/), so this isn't a live bug today, just a latent footgun if the front matter style ever changes.
  • base="$(git rev-parse "${head}^")" (first-push-to-branch fallback) assumes head has a parent; a true root-commit push would fail here. Extremely unlikely for _blog_posts/** given the repo already has history, so not worth guarding against.
  • If a post file is renamed (not just added/modified), --diff-filter=AM without rename detection (-M) will only pick up the new path as an "A", which is actually the desired outcome here (files the issue for the new slug), so this is fine, just noting the assumption.

Security

  • Nothing concerning. permissions: contents: read at the workflow level is correctly least-privileged since the actual cross-repo write happens via the separate fine-grained BLOG_HUB_NOTIFY_TOKEN PAT, not the default GITHUB_TOKEN. The token presence is checked without ever echoing its value. Trigger is push-to-main-only plus manually-gated workflow_dispatch, so there's no pwn-request-via-fork-PR surface. I double-checked the heredoc at lines 131–148 that builds the issue body: it looks like an unquoted-heredoc injection risk at first glance (variables interpolated directly, literal backticks in the markdown), but all literal backticks are correctly escaped (```) and shell parameter expansion doesn't recursively re-interpret the values being substituted, so this is safe as written.
  • The pinned actions/checkout SHA matches the same SHA already used in every other workflow in this repo (jekyll-build.yml, website-quality.yml, visidelta-preview.yml), so it's consistent with existing convention rather than a new risk.

Test coverage

  • PR description notes YAML parses and bash -n passes, which is reasonable for a workflow-only change. Given the branching complexity (push vs. workflow_dispatch, zero-sha fallback, token-present/absent), it'd be worth doing one live workflow_dispatch dry run against a real base/head range before merging, if that hasn't happened already, just to confirm the diff range and dedup logic behave as expected end-to-end.

Overall: no blockers, nice attention to the double-file race and graceful degradation. The two doc/behavior nits above are the only things I'd want resolved before merge, and even those are minor.

Round-3 review: the dispatch inputs claimed dry-run semantics but filed
real issues. dry_run input (default true) now prints what would be
filed; pass dry_run=false to file for real from a manual range.

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

Copy link
Copy Markdown
Member Author

Review round 3 addressed: workflow_dispatch now defaults to a true dry run (dry_run=false files for real). On the AGENTS.md reference the reviewer flagged: the publishing-rule gotcha lands in #39 (feature/blog-hub-spoke-intro), so the reference resolves once both PRs are on develop — merge in either order, they're complementary. Block-scalar front matter noted as a latent limitation; current posts all use single-line scalars.

@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: 88d3f1a9c3

ℹ️ 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/blog-hub-notify.yml Outdated

# Let a genuinely failing diff (bad ref, shallow history) fail the
# step loudly; only the "no .md files matched" case is a normal no-op.
diff_out="$(git diff --name-only --diff-filter=AM "$base" "$head" -- '_blog_posts/')"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Skip unpublished posts before notifying the hub

When a main-branch change adds or edits a document with published: false, this path-only diff includes it even though the production pipeline runs Jekyll without --unpublished, so Jekyll does not emit the page. The workflow consequently files an issue claiming the post was published and supplies a canonical URL that returns 404; parse the front matter and skip documents that are not publishable before creating issues.

Useful? React with 👍 / 👎.

@claude

claude Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review: blog-hub-notify.yml

Went through the full script logic (front-matter parsing, diff-range selection, dedup, dry-run gating) and cross-checked it against the actual repo state (_config.yml permalink, an existing post's front matter, other workflows' actions/checkout pin). Overall this is solid, well-thought-through work — the four rounds of self-review already caught the issues I'd normally flag (first-parent diff range, YAML quoting, exact-title dedup, true dry-run semantics). A few smaller things worth a look:

Code quality / best practices

  • Verified, not a bug: the heredoc body (cat > "$body_file" <<EOF ... EOF) is deliberately left-aligned to the script's own baseline indentation (10 spaces) rather than the surrounding for/if block's indentation. That's required so the plain <<EOF terminator survives YAML's uniform-indent stripping — a future editor who "fixes" that indentation to match the loop body would silently break the heredoc (the terminator would stop matching and cat would swallow the rest of the script as body text). Worth a one-line comment calling that out explicitly, since it's non-obvious and easy to "clean up" by accident.
  • Front-matter extraction is hand-rolled awk/sed rather than a real YAML parser. That's consistent with this repo's shell-only, no-dependency convention (per AGENTS.md), and I confirmed it correctly handles the real front matter shapes in _blog_posts/ (quoted titles with colons, unquoted dates, the one post using excerpt as fallback). Just flagging as a known limitation: a multi-line/block-scalar description: would silently truncate to its first line rather than erroring.

Potential bugs

  • set -euo pipefail + no per-item error containment in the for f in "${files[@]}" loop means a single gh issue create failure (rate limit, transient API error) aborts the whole step — any remaining posts in the same push won't get filed and there's no automatic retry. Given this is a low-volume, infrequent workflow that's probably an acceptable tradeoff (matches the "let failures fail loudly" philosophy from the commit history), but worth confirming that's the intended behavior rather than an oversight vs. e.g. continuing the loop and summarizing failures at the end.
  • Minor: the date fallback (git log -1 --format=%cs -- "$f") reads from the checked-out HEAD, not explicitly from $head. For push events these are the same ref so it's fine; for a workflow_dispatch dry run where head_sha differs from the branch tip that was actually checked out, the fallback date could reflect the wrong point in time. Only affects posts missing a front-matter date (none currently), and only the dry-run preview text, so low impact.
  • Edge case: github.event.before for a force-push isn't guaranteed to be an ancestor of the new tip, which could make git diff base..head pick up unrelated history and re-notify for already-synced posts. Probably moot if main is protected against force-push, but worth a mental note.

Performance

No concerns — single gh issue list call reused across all files in the push rather than re-querying per file, and the diff is scoped to _blog_posts/** via both the trigger paths filter and the diff pathspec.

Security

  • actions/checkout is pinned to a full commit SHA (34e114876b...) matching the SHA already used in jekyll-build.yml, website-quality.yml, and visidelta-preview.yml — good, consistent with repo convention (I couldn't independently verify the SHA against upstream from this sandbox since outbound network calls were blocked here, but its reuse across three other existing workflows in this repo is strong evidence it's already been vetted).
  • Least privilege looks right: top-level permissions: contents: read, and the cross-repo PAT (BLOG_HUB_NOTIFY_TOKEN) is scoped to a single external repo with Issues read/write only — no broader GITHUB_TOKEN permissions requested.
  • No injection risk found in the templated issue body: title/description are interpolated into an unquoted heredoc, but heredoc parameter expansion substitutes values once and does not re-evaluate any $()/backticks embedded in the substituted text, so untrusted-ish front-matter content (this repo's own posts, so not really an attacker-controlled input, but still) can't break out into command execution.
  • Token gating is handled gracefully — missing BLOG_HUB_NOTIFY_TOKEN no-ops with a GITHUB_STEP_SUMMARY notice instead of failing the push, matching the "VisiDelta-style" pattern mentioned in the PR description.

Test coverage

No automated tests, but that matches this repo's established convention (AGENTS.md: "No unit tests — this is a static site"). The manual verification described in the PR body (front-matter extraction against all 7 existing posts, YAML parse, bash -n) is a reasonable bar given that. If this workflow grows more logic over time, a lightweight actionlint/shellcheck CI step on .github/workflows/** might be worth adding at some point, but that's a separate, non-blocking improvement.

Nice work iterating through the review rounds already visible in the commit history — this looks ready to merge pending the BLOG_HUB_NOTIFY_TOKEN secret setup called out in the PR description.

- Parse front matter with PyYAML and re-serialize the snippet with
  yaml.safe_dump: kills the escape/quoting corruption class (embedded
  quotes, colons, block scalars) the shell munging couldn't handle
- Skip posts with published: false (Jekyll won't emit them; the old
  path filed issues with 404 canonical URLs)
- Read post content from the head revision via git show, not the
  checkout tree — dispatch runs against older ranges now read the
  right bytes
- Add --no-renames so a renamed post surfaces as an Add at its new
  path instead of vanishing from --diff-filter=AM
- Remove the concurrency group: GitHub replaces queued runs, so
  queueing could drop a middle push's diff range permanently; a rare
  duplicate issue is cheaper than a permanently missed post

Verified: YAML parses, bash -n clean, embedded python ast-parses, and
the parser round-trips all 7 existing posts with correct quoting.

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

@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: e1d2f98bd6

ℹ️ 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".


# One fetch of open sync issues; exact-title dedup below instead of
# relying on GitHub search tokenization/index lag.
open_titles="$(gh issue list -R "$HUB_REPO" --label blog-hub-sync --state open --limit 200 --json title --jq '.[].title')"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Distinguish updates from first-time publication

When an already indexed post is edited after its original sync issue has been closed, the modified path is processed again, but deduplication checks only open issues and the generated body instructs the editor to insert a new entry at the top. Following that instruction creates a duplicate hub entry for routine post corrections; check the hub's existing entries or otherwise distinguish first publication from updates.

Useful? React with 👍 / 👎.

Comment thread .github/workflows/blog-hub-notify.yml Outdated

for f in "${files[@]}"; do
slug="$(basename "$f" .md)"
url="${SITE_BASE}/${slug}/"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Honor front-matter permalinks in generated URLs

When a collection document overrides its route with a front-matter permalink, Jekyll publishes it at that override, but this always constructs /blog/<filename>/. The resulting issue and ready-to-paste hub entry then contain a canonical URL that can return 404; derive the effective URL from the parsed front matter rather than always using the basename.

Useful? React with 👍 / 👎.

mickdarling and others added 3 commits August 1, 2026 11:44
- Diff with --name-status: A files a "Index new" issue as before, M
  files a "Sync updated" issue instructing editors to update the
  existing hub entry rather than insert a duplicate
- Dedup checks both issue flavors so an edit while the original issue
  is still open files nothing
- Honor front-matter permalink overrides when deriving the canonical
  URL; default remains /blog/<basename>/ from the collection config

Verified: yaml/bash/ast clean; functional tests for new mode, updated
mode, permalink override, and published: false skip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018y4M47JPR3dawoDUpCTbtd
Sonar S8544 (unpinned dependency) and S8541 (sdist setup-script
execution): the pip fallback now installs pyyaml==6.0.2 with
--only-binary :all:. The fallback still only fires if the runner
image ever drops its preinstalled PyYAML.

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

sonarqubecloud Bot commented Aug 1, 2026

Copy link
Copy Markdown

@mickdarling
mickdarling merged commit 898cd02 into develop Aug 1, 2026
2 checks passed
@mickdarling
mickdarling deleted the feature/blog-hub-notify branch August 1, 2026 15:50
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