feat(hack): add publishing tools for the blog - #640
Timur Tukaev (tym83) wants to merge 10 commits into
Conversation
Two tools over one implementation: publish_post creates a post, validate checks posts. Available as an MCP server over stdio and as a CI step, so a generated post and a hand-written one are held to identical rules. Writing markdown into the right directory was never the hard part. Keeping posts consistent with rules scattered across documentation, templates and habit is, and every check here exists because the mistake it prevents has been made in this repository already: an image filename among the topics, a term invented while writing, a post with images laid out as a plain file. Publishing validates before it writes and removes whatever it created if a later step fails, so a failed publish leaves nothing to clean up. Link checking has two modes. With a build in public/ links resolve against the paths the site actually serves and an unresolved link is an error. Without one, URLs are inferred from the content tree — only approximate, since Hugo derives them through permalinks, per-page aliases and version directories — so an unresolved link is a warning and the checker never fails on its own inference. Images are copied unchanged: resizing and AVIF/WebP conversion are Hugo's job, and it encodes AVIF natively as of 0.162. The Open Graph card is held to PNG or JPEG because social parsers render neither SVG, AVIF nor WebP as previews. Community links move to data/community-links.yaml so changing one does not mean editing published posts after the fact. No new runtime dependencies: the MCP protocol is implemented directly, and Pillow is used only to read image dimensions. 32 tests, each against a throwaway site in a temporary directory. Assisted-by: LLM Signed-off-by: tym83 <6355522@gmail.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
📝 WalkthroughWalkthroughChangesBlog publishing and validation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant MCPClient
participant MCPServer
participant Publisher
participant Validator
participant Git
MCPClient->>MCPServer: publish_post request
MCPServer->>Publisher: post arguments
Publisher->>Validator: validate generated post
Publisher->>Git: stage and commit post
Git-->>Publisher: commit metadata
Publisher-->>MCPServer: PublishResult
MCPServer-->>MCPClient: JSON-RPC result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
hack/mcp/test_mcp.py (1)
119-119: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename the loop variable
l.Ruff reports E741 for both lines. Use a descriptive name so the lint run stays clean.
♻️ Proposed fix
- lines = [l for l in out.splitlines() if l and not l.startswith("-")] + lines = [line for line in out.splitlines() if line and not line.startswith("-")]front = "\n".join( - l for l in VALID_FRONT.strip().splitlines() if not l.startswith("description") + line + for line in VALID_FRONT.strip().splitlines() + if not line.startswith("description") )Also applies to: 186-186
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/mcp/test_mcp.py` at line 119, Rename the list-comprehension loop variable l in both affected comprehensions to a descriptive name, and update its references within each expression so Ruff E741 is resolved without changing the filtering behavior.Source: Linters/SAST tools
hack/mcp/README.md (1)
33-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winState the test dependencies.
test_publish_bundle_with_imageimportsPIL, and all modules importyaml. Without PyYAML and Pillow the test run fails with an import error. The workflow installs both, but a local reader gets no hint here.📝 Proposed addition
```bash +python3 -m pip install PyYAML Pillow python3 hack/mcp/test_mcp.py</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@hack/mcp/README.mdaround lines 33 - 40, Add the required local test
dependency installation command to the “Running the tests” section before the
existing test command, specifying both PyYAML and Pillow so imports used by
test_mcp.py succeed.</details> <!-- cr-comment:v1:564bb30168152e4270206d82 --> </blockquote></details> <details> <summary>hack/mcp/frontmatter.py (1)</summary><blockquote> `38-50`: _🎯 Functional Correctness_ | _🔵 Trivial_ | _💤 Low value_ **Make the delimiter match line-anchored.** `split` accepts any text that starts with `---`, and it finds the first occurrence of `\n---` anywhere. A closing delimiter is only valid when it stands alone on its line. A YAML block scalar that contains a line starting with `---`, or an opening line such as `---title: x`, is parsed incorrectly. The current blog posts do not hit this, so this is a hardening nit. <details> <summary>♻️ Proposed stricter delimiter handling</summary> ```diff - if not text.startswith(DELIMITER): + if not re.match(rf"{DELIMITER}\s*\n", text): raise FrontMatterError("file does not start with '---'") - # Search for the closing delimiter on its own line. - rest = text[len(DELIMITER) :] - marker = f"\n{DELIMITER}" - end = rest.find(marker) - if end == -1: + # Search for the closing delimiter on its own line. + rest = text[len(DELIMITER) :] + closing = re.search(rf"\n{DELIMITER}[ \t]*(?:\n|$)", rest) + if closing is None: raise FrontMatterError("closing '---' not found") - fm = rest[:end] - body = rest[end + len(marker) :] + fm = rest[: closing.start()] + body = rest[closing.end() :] return fm.lstrip("\n"), body.lstrip("\n")This requires
import reat the top of the file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/mcp/frontmatter.py` around lines 38 - 50, Update the frontmatter splitter around the delimiter parsing to require the opening `---` and closing `---` markers to occupy complete lines, allowing only permitted line-ending whitespace as appropriate. Replace the broad prefix and substring searches with line-anchored matching, while preserving the existing `FrontMatterError` cases and returned frontmatter/body trimming behavior..mcp.json (1)
3-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse a path that does not depend on the MCP client working directory.
.mcp.jsonlauncheshack/mcp/server.pyas a relative argument. If the target client starts the process outside the repository root,python3can fail to find the server beforeserver.pyderives the repository root from__file__. Add an explicitcwdonly for clients that support it, or document the required working directory.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.mcp.json around lines 3 - 6, Update the cozystack-website MCP configuration to launch hack/mcp/server.py independently of the client’s working directory, using an explicit supported cwd or an equivalent absolute/repository-root-based path. Preserve the existing Python command and server entrypoint while ensuring startup succeeds when invoked outside the repository root.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/validate-content.yaml:
- Around line 52-53: Make the Validate content step in the workflow non-blocking
by adding continue-on-error: true, allowing existing post validation failures
without failing pull requests. Keep the python3 hack/mcp/server.py --check
command unchanged so the validation report still runs.
- Around line 19-24: Add a job-level permissions block to the validate job,
granting only read access required for repository checkout and workflow
execution, with all unspecified scopes disabled. Keep the existing runs-on,
environment, and steps unchanged.
In `@hack/mcp/core.py`:
- Around line 175-183: Update the image-copy loop to reject duplicate source
basenames before copying: detect when a name already exists in the current image
set and raise PublishError, and also reject any basename matching the generated
post filename (such as index.md) before writing. Preserve the existing copying
and copied tracking for valid, unique image names.
- Around line 198-207: Update the publish flow around `_commit` and the
exception cleanup to track the created branch and original git state, then pass
that information to the rollback helper. Ensure failure cleanup unstages paths,
checks out the original branch, and deletes any branch created by `_commit`,
while preserving the existing file rollback behavior and successful commit flow.
In `@hack/mcp/server.py`:
- Around line 198-202: Constrain the resolved path in the rel-handling branch
before calling validate.validate_post: after resolving (root / rel), verify it
remains within root and reject outside targets with the existing no-such-file
response. Preserve validation for paths inside the repository, including nested
paths, and ensure absolute paths and ../ traversal cannot escape root.
- Around line 328-331: Update the args.check branch and tool_validate contract
to return or expose structured validation reports alongside the rendered output,
then derive the exit status from those reports rather than searching output for
"FAIL". Ensure missing paths produce a nonzero status and warning-only results
remain successful, while preserving the existing printed text.
In `@hack/mcp/validate.py`:
- Around line 236-237: Update the index-page handling around the served.add call
to replace str(rel.parent).strip(".") with an explicit root-directory check: add
an empty served path only when rel.parent is the intended root, otherwise add
the parent path without removing leading dots from dot-directories such as
.well-known. Preserve dot-directory names exactly for link resolution.
---
Nitpick comments:
In @.mcp.json:
- Around line 3-6: Update the cozystack-website MCP configuration to launch
hack/mcp/server.py independently of the client’s working directory, using an
explicit supported cwd or an equivalent absolute/repository-root-based path.
Preserve the existing Python command and server entrypoint while ensuring
startup succeeds when invoked outside the repository root.
In `@hack/mcp/frontmatter.py`:
- Around line 38-50: Update the frontmatter splitter around the delimiter
parsing to require the opening `---` and closing `---` markers to occupy
complete lines, allowing only permitted line-ending whitespace as appropriate.
Replace the broad prefix and substring searches with line-anchored matching,
while preserving the existing `FrontMatterError` cases and returned
frontmatter/body trimming behavior.
In `@hack/mcp/README.md`:
- Around line 33-40: Add the required local test dependency installation command
to the “Running the tests” section before the existing test command, specifying
both PyYAML and Pillow so imports used by test_mcp.py succeed.
In `@hack/mcp/test_mcp.py`:
- Line 119: Rename the list-comprehension loop variable l in both affected
comprehensions to a descriptive name, and update its references within each
expression so Ruff E741 is resolved without changing the filtering behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a811be7a-81ff-4eb3-afe1-5456d67db7bd
📒 Files selected for processing (11)
.github/workflows/validate-content.yaml.gitignore.mcp.jsonREADME.mddata/community-links.yamlhack/mcp/README.mdhack/mcp/core.pyhack/mcp/frontmatter.pyhack/mcp/server.pyhack/mcp/test_mcp.pyhack/mcp/validate.py
| jobs: | ||
| validate: | ||
| runs-on: ubuntu-latest | ||
| env: | ||
| HUGO_VERSION: 0.164.0 | ||
| steps: |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Add a least-privilege permissions block.
The job only reads the repository. Without an explicit permissions key, the job inherits the repository or organization default, which can include write scopes for GITHUB_TOKEN. The job runs repository code, including npm install lifecycle scripts and ./hack/download_openapi.sh, so a broad token increases the impact of a malicious dependency.
🔒️ Proposed fix
jobs:
validate:
runs-on: ubuntu-latest
+ permissions:
+ contents: read
env:
HUGO_VERSION: 0.164.0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jobs: | |
| validate: | |
| runs-on: ubuntu-latest | |
| env: | |
| HUGO_VERSION: 0.164.0 | |
| steps: | |
| jobs: | |
| validate: | |
| runs-on: ubuntu-latest | |
| permissions: | |
| contents: read | |
| env: | |
| HUGO_VERSION: 0.164.0 | |
| steps: |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/validate-content.yaml around lines 19 - 24, Add a
job-level permissions block to the validate job, granting only read access
required for repository checkout and workflow execution, with all unspecified
scopes disabled. Keep the existing runs-on, environment, and steps unchanged.
| - name: Validate content | ||
| run: python3 hack/mcp/server.py --check |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
The strict validation step fails on the existing posts.
The PR description states that 18 published posts already have validation errors. This step exits non-zero, so every pull request that touches content/** or hack/mcp/** fails until those posts are corrected.
Choose one path before merge:
- Correct the 18 posts in this PR or in a preceding PR.
- Keep the step non-blocking for now, for example with
continue-on-error: true, and remove that once the content is clean.
I can generate the report of failing posts and the required front matter corrections, or open a tracking issue. Tell me which you prefer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/validate-content.yaml around lines 52 - 53, Make the
Validate content step in the workflow non-blocking by adding continue-on-error:
true, allowing existing post validation failures without failing pull requests.
Keep the python3 hack/mcp/server.py --check command unchanged so the validation
report still runs.
| copied = [] | ||
| for src in images: | ||
| source = Path(src).expanduser() | ||
| if not source.exists(): | ||
| raise PublishError(f"image not found: {source}") | ||
| dest = target.parent / source.name | ||
| shutil.copy2(source, dest) | ||
| created.append(dest) | ||
| copied.append(source.name) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Reject duplicate image basenames.
dest is derived from source.name. Two source paths with the same basename, for example /a/card.png and /b/card.png, copy over each other, and copied reports the name twice. A source named index.md overwrites the post that was just written.
Fail early on both cases.
♻️ Proposed guard
copied = []
for src in images:
source = Path(src).expanduser()
if not source.exists():
raise PublishError(f"image not found: {source}")
+ if source.name == "index.md" or source.name in copied:
+ raise PublishError(f"image name conflicts: {source.name}")
dest = target.parent / source.name📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| copied = [] | |
| for src in images: | |
| source = Path(src).expanduser() | |
| if not source.exists(): | |
| raise PublishError(f"image not found: {source}") | |
| dest = target.parent / source.name | |
| shutil.copy2(source, dest) | |
| created.append(dest) | |
| copied.append(source.name) | |
| copied = [] | |
| for src in images: | |
| source = Path(src).expanduser() | |
| if not source.exists(): | |
| raise PublishError(f"image not found: {source}") | |
| if source.name == "index.md" or source.name in copied: | |
| raise PublishError(f"image name conflicts: {source.name}") | |
| dest = target.parent / source.name | |
| shutil.copy2(source, dest) | |
| created.append(dest) | |
| copied.append(source.name) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/mcp/core.py` around lines 175 - 183, Update the image-copy loop to
reject duplicate source basenames before copying: detect when a name already
exists in the current image set and raise PublishError, and also reject any
basename matching the generated post filename (such as index.md) before writing.
Preserve the existing copying and copied tracking for valid, unique image names.
| if commit: | ||
| result.branch, result.commit = _commit( | ||
| root, target, target_dir, branch or f"blog/{slug}", title | ||
| ) | ||
|
|
||
| return result | ||
|
|
||
| except Exception: | ||
| _rollback(created) | ||
| raise |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Roll back the git state as well, not only the files.
The module docstring states that a failed publish leaves the repository exactly as it was. _commit can create a branch and stage paths before it fails, for example when git commit is rejected because user.email is not configured. The except block then removes the files, but the new branch stays checked out and the paths stay in the index. The user must clean up by hand, which the docstring and hack/mcp/README.md promise is unnecessary.
Track what _commit changed and undo it on failure.
🐛 Proposed direction
if commit:
- result.branch, result.commit = _commit(
- root, target, target_dir, branch or f"blog/{slug}", title
- )
+ original = _git(root, "rev-parse", "--abbrev-ref", "HEAD")
+ created_branch: str | None = None
+ try:
+ result.branch, result.commit = _commit(
+ root, target, target_dir, branch or f"blog/{slug}", title
+ )
+ except Exception:
+ _git_cleanup(root, original, created_branch, target_dir or target)
+ raise_commit must report the branch it created so the cleanup helper can unstage the paths, return to original, and delete that branch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/mcp/core.py` around lines 198 - 207, Update the publish flow around
`_commit` and the exception cleanup to track the created branch and original git
state, then pass that information to the rollback helper. Ensure failure cleanup
unstages paths, checks out the original branch, and deletes any branch created
by `_commit`, while preserving the existing file rollback behavior and
successful commit flow.
| if rel: | ||
| path = (root / rel).resolve() | ||
| if not path.exists(): | ||
| return f"{rel}: no such file" | ||
| reports = {rel: validate.validate_post(path, site)} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Constrain path to the repository root.
rel arrives from the MCP client. (root / rel).resolve() accepts ../ segments and absolute paths, so validate reads any file the process can read. Front matter values from that file are then echoed in the returned error messages.
Reject targets outside root.
🔒️ Proposed fix
if rel:
- path = (root / rel).resolve()
+ path = (root / rel).resolve()
+ try:
+ path.relative_to(root.resolve())
+ except ValueError:
+ return f"{rel}: path is outside the repository"
if not path.exists():
return f"{rel}: no such file"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if rel: | |
| path = (root / rel).resolve() | |
| if not path.exists(): | |
| return f"{rel}: no such file" | |
| reports = {rel: validate.validate_post(path, site)} | |
| if rel: | |
| path = (root / rel).resolve() | |
| try: | |
| path.relative_to(root.resolve()) | |
| except ValueError: | |
| return f"{rel}: path is outside the repository" | |
| if not path.exists(): | |
| return f"{rel}: no such file" | |
| reports = {rel: validate.validate_post(path, site)} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/mcp/server.py` around lines 198 - 202, Constrain the resolved path in
the rel-handling branch before calling validate.validate_post: after resolving
(root / rel), verify it remains within root and reject outside targets with the
existing no-such-file response. Preserve validation for paths inside the
repository, including nested paths, and ensure absolute paths and ../ traversal
cannot escape root.
| if args.check: | ||
| output = tool_validate(root, {"path": args.path} if args.path else {}) | ||
| print(output) | ||
| return 1 if "FAIL" in output else 0 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not derive the exit code from the substring FAIL.
Two problems follow from this check:
tool_validatereturns"<path>: no such file"for a missing--path. That output contains noFAIL, so--check --path <typo>exits 0 and the CI job passes without validating anything.- Any path or message that contains the text
FAILflips a warning-only run to a failure.
Compute the status from the reports instead of from the rendered text.
🐛 Proposed direction
if args.check:
- output = tool_validate(root, {"path": args.path} if args.path else {})
- print(output)
- return 1 if "FAIL" in output else 0
+ if args.path and not (root / args.path).exists():
+ print(f"{args.path}: no such file")
+ return 2
+ site = validate.Site(root)
+ if args.path:
+ reports = {args.path: validate.validate_post(root / args.path, site)}
+ else:
+ reports = validate.validate_tree(site)
+ print(tool_validate(root, {"path": args.path} if args.path else {}))
+ return 1 if any(r.errors for r in reports.values()) else 0A cleaner variant is to let tool_validate return the reports next to the text, so the reports are built once.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if args.check: | |
| output = tool_validate(root, {"path": args.path} if args.path else {}) | |
| print(output) | |
| return 1 if "FAIL" in output else 0 | |
| if args.check: | |
| if args.path and not (root / args.path).exists(): | |
| print(f"{args.path}: no such file") | |
| return 2 | |
| site = validate.Site(root) | |
| if args.path: | |
| reports = {args.path: validate.validate_post(root / args.path, site)} | |
| else: | |
| reports = validate.validate_tree(site) | |
| print(tool_validate(root, {"path": args.path} if args.path else {})) | |
| return 1 if any(r.errors for r in reports.values()) else 0 |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/mcp/server.py` around lines 328 - 331, Update the args.check branch and
tool_validate contract to return or expose structured validation reports
alongside the rendered output, then derive the exit status from those reports
rather than searching output for "FAIL". Ensure missing paths produce a nonzero
status and warning-only results remain successful, while preserving the existing
printed text.
| if rel.name == "index.html": | ||
| served.add(str(rel.parent).strip(".")) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Replace strip(".") with an explicit root check.
str(rel.parent).strip(".") strips dot characters from both ends of the whole path string. For public/index.html the intent is to add "", and that works. For a page under a dot-directory, such as public/.well-known/x/index.html, the served path becomes well-known/x. A link to /.well-known/x/ then fails resolution and the strict CI job reports an error.
🐛 Proposed fix
if rel.name == "index.html":
- served.add(str(rel.parent).strip("."))
+ parent = rel.parent
+ served.add("" if parent == Path(".") else str(parent))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if rel.name == "index.html": | |
| served.add(str(rel.parent).strip(".")) | |
| if rel.name == "index.html": | |
| parent = rel.parent | |
| served.add("" if parent == Path(".") else str(parent)) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/mcp/validate.py` around lines 236 - 237, Update the index-page handling
around the served.add call to replace str(rel.parent).strip(".") with an
explicit root-directory check: add an empty served path only when rel.parent is
the intended root, otherwise add the parent path without removing leading dots
from dot-directories such as .well-known. Preserve dot-directory names exactly
for link resolution.
yaml.safe_dump folds a long title across two lines. The fold survives into the templates, so og:title and the JSON-LD headline both came out with the break — and quoted twice on top of that. Front matter is now written directly: double-quoted scalars on one line, dates left unquoted so Hugo reads them as dates, and indented lists, which also matches how the hand-written posts look. Assisted-by: LLM Signed-off-by: tym83 <6355522@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
hack/mcp/frontmatter.py (1)
112-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the YAML document marker with an explicit suffix check.
rstrip("...")removes any trailing.characters. It does not remove the literal...suffix. Ruff reports B005 on this line. Use an explicit suffix check.Proposed fix
- return yaml.safe_dump(value, default_flow_style=True).strip().rstrip("...").strip() + rendered = yaml.safe_dump(value, default_flow_style=True).strip() + if rendered.endswith("..."): + rendered = rendered[:-3].rstrip() + return rendered🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hack/mcp/frontmatter.py` at line 112, Update the YAML serialization return expression in the frontmatter formatting function to remove the document marker only when the dumped value explicitly ends with the literal "..." suffix, replacing rstrip("...") with an explicit suffix check while preserving the existing trimming behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hack/mcp/frontmatter.py`:
- Around line 113-115: Update the string-scalar handling in _scalar to use a
YAML-compatible encoder that escapes control characters such as newline,
carriage return, and tab, while preserving correct handling of backslashes and
quotes. Ensure the emitted scalar remains valid for yaml.safe_load and
round-trips the original string value.
---
Nitpick comments:
In `@hack/mcp/frontmatter.py`:
- Line 112: Update the YAML serialization return expression in the frontmatter
formatting function to remove the document marker only when the dumped value
explicitly ends with the literal "..." suffix, replacing rstrip("...") with an
explicit suffix check while preserving the existing trimming behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c6152fa3-2630-448f-bbf0-8ea39f31885f
📒 Files selected for processing (2)
hack/mcp/frontmatter.pyhack/mcp/test_mcp.py
| text = str(value) | ||
| escaped = text.replace("\\", "\\\\").replace('"', '\\"') | ||
| return f'"{escaped}"' |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files matching frontmatter.py:\n'
fd -a 'frontmatter\.py$' . || true
if [ -f hack/mcp/frontmatter.py ]; then
printf '\nLine count:\n'
wc -l hack/mcp/frontmatter.py
printf '\nRelevant section:\n'
sed -n '1,180p' hack/mcp/frontmatter.py | cat -n
printf '\nSearch for _scalar and emit/load uses:\n'
rg -n '_scalar|emit|load|frontmatter|read|dump|scalar' hack/mcp/frontmatter.py
fi
printf '\nCheck YAML/JSON dependency presence hints:\n'
fd -a 'pyproject.toml|requirements.*|setup.*|Pipfile|uv.lock|poetry.lock' . | sed -n '1,40p'
rg -n '"pyyaml|PyYAML|pyyaml|jsonschema|mcp' pyproject.toml requirements*.txt setup.* Pipfile uv.lock poetry.lock 2>/dev/null || trueRepository: cozystack/website
Length of output: 7450
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
import yaml
samples = [
"line1\nline2",
"tab\there",
"carriage\rreturn",
"escape\\\quote",
"quote\"here",
"both\"&\\\n",
]
for value in samples:
escaped = value.replace("\\", "\\\\").replace('"', '\\"')
emitted = f'"{escaped}"'
loaded = yaml.safe_load(emitted)
json_emitted = json.dumps(value, ensure_ascii=False)
print(repr(value))
print(emitted)
print(repr(loaded))
print(loaded == value)
print(json_emitted)
print()
PYRepository: cozystack/website
Length of output: 272
🌐 Web query:
PyYAML safe_loader double quoted string control characters escape sequence documentation
💡 Result:
In PyYAML, the SafeLoader parses YAML according to standard YAML specifications [1]. Double-quoted scalars are the only scalar style that supports escape sequences, allowing you to include non-printable or special characters [1][2][3]. Escape Sequences Double-quoted strings in YAML support C-style escape sequences [2][4]. These sequences allow you to represent control characters and other non-printable characters [2][3]: - Hexadecimal Escapes: Use \x followed by 2 hex digits (e.g., \x0B for a vertical tab) [5][2][3]. - Unicode Escapes (16-bit): Use \u followed by 4 hex digits (e.g., \u0041 for 'A') [5][2][3]. - Unicode Escapes (32-bit): Use \U followed by 8 hex digits (e.g., \U00000041 for 'A') [5][2][3]. - Standard Predefined Escapes: Common sequences include \t (tab), \n (line feed), \r (carriage return), \" (double quote), and \\ (backslash) [5][2][3]. Usage Notes - Interpretation: Escape sequences are only interpreted within double-quoted scalars [2][3]. In other styles (plain, single-quoted, literal, or folded), backslashes do not have special meaning [2][3]. - Parsing Errors: If an invalid escape sequence is used—such as an incorrect number of hex digits or an unrecognized escape character—PyYAML will raise a ScannerError [6]. - Character Limitations: While double-quoted strings allow you to encode many characters, the YAML specification defines a set of non-printable characters that must be escaped [2][4]. Additionally, PyYAML's SafeLoader may raise a ReaderError if it encounters certain control characters in the input stream that are explicitly excluded by the YAML specification, regardless of whether they are escaped [7][8]. If you encounter issues with specific Unicode characters or emojis in older versions of PyYAML, upgrading to a newer version is often required to resolve scanner and reader bugs [7][8].
Citations:
- 1: https://pyyaml.org/wiki/PyYAMLDocumentation
- 2: https://yaml.org/spec/1.1/current.html
- 3: https://www.yaml.info/learn/quote
- 4: https://yaml.org/spec/history/2004-12-28/2004-12-28.pdf
- 5: https://www.yaml.info/learn/quote.html
- 6: https://docsearch.algolia.com/mcp/docs/repo/yaml/pyyaml
- 7: parser regression: yaml.reader.ReaderError: unacceptable character #x1f64b: special characters are not allowed yaml/pyyaml#250
- 8: https://stackoverflow.com/questions/44875403/loading-special-characters-with-pyyaml
Escape control characters in string scalars.
_scalar only escapes backslashes and double quotes, so embedded \n, \r, or \t characters are emitted literally. YAML double-quoted scalars must encode these as escape sequences, otherwise dump can create invalid/multiline front matter and load can read the value back unchanged.
Use a YAML/YAML-compatible encoder for string scalars, especially since the emitted output is parsed by yaml.safe_load.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hack/mcp/frontmatter.py` around lines 113 - 115, Update the string-scalar
handling in _scalar to use a YAML-compatible encoder that escapes control
characters such as newline, carriage return, and tab, while preserving correct
handling of backslashes and quotes. Ensure the emitted scalar remains valid for
yaml.safe_load and round-trips the original string value.
IvanHunters
left a comment
There was a problem hiding this comment.
Self-contained blog publishing/validation tooling, additive (+2075/-0, all new files), no new runtime deps, 34/34 tests pass and are non-degenerate. CI uses the safe pull_request trigger (fork code runs with a read-only token, no secrets — no privilege escalation). No blocking issues. Two minor points worth addressing before merge:
-
hack/mcp/core.py— incomplete git rollback. In_commit, when publishing frommainagit checkout -b <branch>happens first; if the commit then fails (e.g. a pre-commit hook),_rollbackrestores files on disk but leaves HEAD on the newly-created branch. This contradicts the PR's "a failed publish leaves nothing to clean up" claim. Suggest: remember the original branch and return to it (and delete the created branch) on the failure path, or create the branch only after a successful commit. -
hack/mcp/validate.py+core.py— taxonomy check is fail-open. Ifdata/taxonomy.yamlis missing/empty the term sets are empty and the check is silently skipped (warning in validate, silent return in publish). Since the workflow'spaths:includes that file, a PR that removes/renames it triggers the job but it degrades to warn-only and goes green with taxonomy effectively unvalidated. Suggest: in the--checkpath treat a missing taxonomy as an error, not a warning.
Minor hardening (optional, consistent with the rest of the repo otherwise): no permissions: block on the workflow (recommend contents: read); npm install vs npm ci; third-party actions pinned by tag rather than SHA.
## Summary Brings every blog post in line with the rules the publishing checks in #640 enforce: **83 posts, zero errors**, verified against a full production build. This is the content-side companion to #640. With it merged, the validation job there can stay strict from day one. ## Front matter Nine posts had no `author` and no `description`. Both feed the meta description and the JSON-LD `BlogPosting`, so an empty description leaves search results and structured data blank. The author of each was taken from the `**Author**:` line the post already carried in its own body — **not** from git history. That distinction matters here: all nine were committed by the same person, while the posts themselves were written by two different authors. Going by the committer would have credited five articles to the wrong person. Descriptions are written from the content of each post. ## Links 36 dead targets repaired: - The bulk pointed at `/docs/v1/`, a version directory that has never existed. Retargeted to `v1.6`. - Blog cross-links in the DIY series carried a day segment (`/blog/2024/04/05/...`), which the permalink pattern does not produce. - Two pages had moved: `virtualization/gpu-passthrough` is now `virtualization/gpu`, and `applications/virtual-machine` is now `virtualization/vm-instance`. - A link to the Kubefarm article was written as internal, although it lives on kubernetes.io — part 3 of the same series already links to it correctly. - `/community/` does not exist; that link now points at the community meeting calendar directly. Every replacement was verified to exist, either in the build or with a request against the live site. ## Directory names Five bundles disagreed with their own front matter about slug or date, and two carried no date prefix at all. The directories were renamed rather than the front matter edited, and that direction is deliberate: permalinks derive from `slug` and `date`, so renaming the directory leaves **every published URL untouched**. Verified for all five after the change. ## Verification `python3 hack/mcp/server.py --check` from #640, against a full build: 83 posts, no errors.
## Summary Two posts for the blog: - **Cozystack 1.6** — the release announcement: Talos Linux for tenant workers, tenant-controlled OIDC, the SecurityGroup API, hierarchical quotas, in-place etcd-operator adoption, and the upgrade notes. - **Blockstor** — the open-sourcing announcement for the LINSTOR-compatible storage control plane written from scratch in Go. Both were produced with the publishing tools from #640 rather than assembled by hand, so front matter, the page bundles, the Open Graph cards and the closing community sections are generated and were validated before anything was written. ## Verification Both posts pass the publishing checks with no errors and no warnings, against a full production build. Open Graph cards are 1200×630 PNG, so previews render in Telegram, Slack and LinkedIn. Rendered pages: - `/blog/2026/08/cozystack-1-6-talos-workers-tenant-sso-security-groups-hierarchical-quotas/` - `/blog/2026/08/blockstor-linstor-compatible-storage-for-kubernetes/` ## Note on the Blockstor text The Russian version of this announcement was already published on OpenNet in May. The English text here is not a translation of it: it covers what happened since, including the sixteen releases, the in-place LINSTOR migration tool, the green Cozystack end-to-end run, and an explicit call for contributors. ## Unrelated finding, worth a separate issue While checking the rendered output I noticed the JSON-LD `BlogPosting` block puts the title and description in doubly-quoted form: ``` "headline":"\"Cozystack 1.5: Gateway API, Default Backups, ...\"" ``` This is not introduced by these posts — it reproduces on the existing 1.5 and LINSTOR posts too. `layouts/partials/hooks/head-end.html` applies `jsonify` to values Hugo already returns quoted, so search engines and AI crawlers read a headline wrapped in literal quote characters. Happy to fix it in a separate PR if you agree it should go.
Bring the branch up to date with main so the publishing tools are validated against the current blog layout (date-prefixed bundles) and the .gitignore entries added on main are kept alongside the Python cache ignores. Assisted-by: LLM Signed-off-by: tym83 <6355522@gmail.com>
A title or description carrying a newline, tab or other control character was written raw inside the double quotes, so the generated front matter either spanned several lines or no longer parsed. Strings are now escaped the way YAML double-quoted scalars expect, and read back unchanged. The front matter splitter also accepted "---title: x" as an opening delimiter and any "\n---" prefix as the closing one; both delimiters now have to stand alone on their line. The document end marker on non-string scalars is removed as a suffix instead of with rstrip, which strips characters rather than the marker. Assisted-by: LLM Signed-off-by: tym83 <6355522@gmail.com>
When publishing from main, _commit created and checked out a new branch before committing. If the commit was then rejected (a pre-commit hook, missing identity), the file rollback removed the post but HEAD stayed on the new branch with the path still staged, contradicting the promise that a failed publish needs no manual cleanup. The index and the original branch are now restored, and the created branch deleted. Images are copied into the bundle by basename, so two sources with the same name silently overwrote each other, and an image named index.md overwrote the post. Such conflicts are now rejected before anything is written. Assisted-by: LLM Signed-off-by: tym83 <6355522@gmail.com>
The CI exit code was derived from the substring "FAIL" in the rendered output. A mistyped --path printed "no such file" and exited 0, so the job passed with nothing validated, while a warning-only post whose path contained "FAIL" would have failed it. The status now comes from the reports. A missing or empty data/taxonomy.yaml turned every taxonomy rule into a warning, so a change that removed or renamed the file went green with taxonomy unchecked. The check now treats that as an error. The validate tool takes its path from the MCP client; it resolved "../" segments and absolute paths and echoed front matter from any file the process could read. Paths are now confined to the repository. Assisted-by: LLM Signed-off-by: tym83 <6355522@gmail.com>
Built pages were registered with str(parent).strip("."), meant to turn
the root "." into "". It also stripped the leading dot of directories
such as .well-known, so the set of served paths held "well-known/x"
instead of ".well-known/x". Link resolution only kept working through
the index.html fallback; the root is now special-cased explicitly.
Assisted-by: LLM
Signed-off-by: tym83 <6355522@gmail.com>
The validation job only reads the repository but executes npm lifecycle scripts and downloaded tooling, so it should not inherit whatever token scopes the repository default grants. Dependencies are installed with npm ci, as in the deploy workflow, so the job uses the locked versions. The tool README now lists the Python packages the tests import, the cases in which the check exits non-zero, and how the relative server path in .mcp.json is meant to be launched. Assisted-by: LLM Signed-off-by: tym83 <6355522@gmail.com>
Ruff flagged ambiguous loop names, an unused import, unsorted imports, stale noqa markers and a few constructs with simpler equivalents in the publishing tools. Fixing them keeps a plain ruff run on hack/mcp quiet, so new findings stand out. The broad exception handlers that report any failure by design are marked as intentional, and the scripts with a shebang are now executable. Assisted-by: LLM Signed-off-by: tym83 <6355522@gmail.com>
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Hugo 0.164 starts PostCSS with Node's --permission flag, which Node 20 rejects, so the content check failed while building the site before it reached any of the checks. The deploy workflow and Netlify already use Node 22. Assisted-by: LLM Signed-off-by: tym83 <6355522@gmail.com>
461f4a4 to
6908a07
Compare
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Timur Tukaev (@tym83) NOT LGTM: the publish path can commit files that are not the post and can write outside the blog directory, and the whole-blog check misses one of the rules the README promises.
The tools give a post written by an agent and a post written by hand the same checks, and give the agent a safe way to lay out and commit a post. The scope looks right to me: it all runs locally or in a read-only CI job, nothing is sent anywhere, no tokens are involved, and a re-run refuses an existing target instead of overwriting it. The 45 tests pass locally and the check is green on the current blog.
Blocking:
_commitrunsgit commitwithout a pathspec, so anything already staged goes into thefeat(blog): ...commit next to the post. I staged an unrelated file, then rangit add -- post.mdandgit commit --dry-run, and both files were listed. Passing the path togit commit(or--only) limits it to the post.- An explicit
slugis put into the path as is. With an existing2026-10-01-existingdirectory,publish(..., slug="existing/../../../../../outside", commit=False)wroteoutside.mdone directory above the repository root. The derived slug is safe, so checking the explicit one against the same[a-z0-9-]shape is enough. - The README says
.htmlcontent is refused, butvalidate_treeonly collects*.md. A blog directory with one.htmlpost prints "No posts found" and--checkexits 0. The.htmlbranch invalidate_postis reachable only with--path. Collecting every file under the blog section (and a test for it) would make the rule real. - The PR body becomes the merge commit message, and some of it is no longer true. It says 34 tests, there are 45 now. It says PyYAML was already required, but no other script in
hack/imports it, so PyYAML and Pillow are both new dependencies, installed unpinned in the workflow. The "Merge order" section is done, since #642 merged on 2026-08-05. The paragraph about an earlier version of the link checker describes the history of the PR, not the code, and can go too.
Non-blocking:
- The blog permalink has no day, so a slug reused within one month maps to the same URL as an existing post. The
target.exists()check compares the dated file name only and lets it through. DOCS_VERSION_REmatchesv[\w.]+, so/docs/virtualization/reads as a pinned version. Three existing posts get a "pins docs version virtualization" warning from that.- The strict link check reads raw markdown.
render-link.htmlturns a link like/blog/2026-08-03-a-post.mdinto its permalink, but the checker reports it as not served, and it also checks links inside code fences. No current post hits this.
Summary
Two tools for the blog behind one implementation:
publish_postcreates a post,validatechecks posts. They are available both as an MCP server over stdio and as a CI step, so a generated post and a hand-written one are held to identical rules.Writing markdown into the right directory was never the hard part. Keeping posts consistent with rules that live scattered across documentation, templates and habit is — and every check here exists because the mistake it prevents has already been made in this repository: an image filename among the topics, a term invented while writing, a post with images laid out as a plain file.
What is added
hack/mcp/—validate.py(the rules),core.py(the single write path),server.py(MCP over stdio, and the CI runner via--check),frontmatter.py, tests, and a reference README.mcp.jsonregistering the server, so an MCP-capable client picks it up from a checkout with no separate installationdata/community-links.yamlso the closing section has one source of truth instead of being copied into every post.github/workflows/validate-content.yamlrunning the same checks on pull requestsNo new runtime dependencies. The MCP protocol is line-delimited JSON-RPC and is implemented directly rather than pulling in an SDK. PyYAML was already required; Pillow is used only to read image dimensions.
Design notes worth reviewing
Publishing validates before writing, and rolls back. If a later step fails, whatever was created is removed, so a failed publish never leaves debris to clean up by hand.
Link checking has two modes, and this is deliberate. With a build in
public/, links resolve against the paths the site actually serves and an unresolved link is an error. Without a build, URLs are inferred from the content tree — and that is only approximate, because Hugo derives them through permalinks, per-page aliases and version directories. Reproducing all of that faithfully is not worth it, so in the approximate mode an unresolved link is a warning: the checker never fails on its own guesswork. The CI job builds first, so it runs strict.I arrived at that split the hard way. An earlier version inferred URLs and confidently flagged working links as broken — it missed the blog permalink shape, files under
static/, per-page aliases and version-agnostic/docs/...links, all of which return 200 on the live site.Images are copied unchanged. Resizing and AVIF or WebP conversion belong to Hugo, which processes bundle resources natively and encodes AVIF as of 0.162. The Open Graph card is held to PNG or JPEG, because Telegram, LinkedIn and other parsers render neither SVG, AVIF nor WebP in
og:image— and that preview is the reason the card exists.Metadata is expected ready-made. Turning a draft into markdown and choosing taxonomy terms is the calling agent's job. This server lays the result out correctly and refuses what breaks the rules; it does not call out to anything.
Nothing here emits meta tags. The SEO and structured-data setup already lives in
layouts/partials/hooks/head-end.html, and duplicating it would only conflict. The job is to guarantee the quality of the fields those templates read.Out of scope on purpose:
content/*/docs/**(that belongs to the release pipeline) and translations (they have their own review gates).Verification
34 tests, each against a throwaway site in a temporary directory:
python3 hack/mcp/test_mcp.py.The tools have also been used for real: #641 publishes two posts produced with them.
Merge order
Merge #642 first. That PR repairs the existing content — front matter, dead links and bundle names — and brings the blog to zero errors. Until it lands, the validation job here would be red on pre-existing problems in 18 posts. With it merged, this job is green and can stay strict from the start.
No change is needed here either way; it is purely a question of order.