Skip to content

docs: correct misleading comments in mirror-to-freenet workflow#4041

Merged
sanity merged 1 commit intomainfrom
fix-mirror-comments
May 7, 2026
Merged

docs: correct misleading comments in mirror-to-freenet workflow#4041
sanity merged 1 commit intomainfrom
fix-mirror-comments

Conversation

@sanity
Copy link
Copy Markdown
Collaborator

@sanity sanity commented May 7, 2026

Problem

Two comment-accuracy issues flagged by the code-first reviewer on freenet-stdlib#71 (the sibling caller of the same reusable workflow). The same misleading comments shipped here in #4040 before the review pass.

  1. The cron offset comment didn't explain the relationship to freenet-stdlib's 12:53. The reusable workflow's concurrency: group keys on inputs.freenet_repo, so the two callers are in separate groups and cannot contend with each other regardless of timing. The stagger is purely for log readability.
  2. The SHA-pin comment implied the pin closed the entire supply chain. It does not — the reusable workflow's cargo install freenet-git --locked still pulls latest from crates.io. The pin closes the workflow-definition vector, not the binary-install vector.

Solution

  • Reword the cron comment to spell out what the concurrency group actually protects against.
  • Add a "Note:" paragraph to the SHA-pin comment acknowledging the binary-install gap and the conditions under which it'd be worth tightening (override freenet_git_version if we lose control of crates.io publishing for that crate).

Behavior is unchanged. Same fix landed in freenet-stdlib#71 already.

Testing

  • YAML parses (python3 -c 'import yaml; yaml.safe_load(open(...))').
  • No behavioral change, so no regression test surface.

Fixes

Code-first reviewer's findings #3 and #4/#7 from freenet-stdlib#71's review thread.

[AI-assisted - Claude]

## Problem

Two comment-accuracy issues flagged by the code-first reviewer on
freenet-stdlib#71 (the sibling caller of the same reusable
workflow). Same misleading comments shipped here in #4040 before the
review pass:

1. The cron offset comment claimed "11:42 UTC = mid-morning US"
   without explaining the relationship to freenet-stdlib's 12:53.
   The actual reusable workflow's `concurrency:` group keys on
   `inputs.freenet_repo`, so freenet-core and freenet-stdlib are in
   separate groups and cannot contend with each other regardless of
   timing. The stagger is purely for log readability.

2. The SHA-pin comment implied the pin closed the entire supply
   chain. It does not -- the reusable workflow's
   `cargo install freenet-git --locked` step still pulls `latest`
   from crates.io, so a malicious freenet-git release would still
   execute under this caller's secrets. The pin closes the
   workflow-definition vector, not the binary-install vector.

## Solution

Reword the cron comment to call out what the concurrency group
actually protects against. Add a "Note:" paragraph to the SHA-pin
comment acknowledging the binary-install gap and the conditions
under which it'd be worth tightening (override
`freenet_git_version` if we lose control of crates.io publishing
for that crate).

Behavior is unchanged. Same fix landed in freenet-stdlib#71 already.

## Testing

- `python3 -c 'import yaml; yaml.safe_load(open(...))'` parses.
- No behavioral change, so no regression test surface.

[AI-assisted - Claude]

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sanity sanity enabled auto-merge May 7, 2026 01:48
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 7, 2026

Rule Review: No issues found

Rules checked: git-workflow.md
Files reviewed: 1 (.github/workflows/mirror-to-freenet.yml)

The PR is a docs: commit correcting comments in a GitHub Actions workflow file. Checking against applicable rules:

  • Conventional commit prefix: docs: is valid. ✓
  • PR description sections (Problem / Solution / Testing / Fixes): This is a pure comment-only docs change with no functional impact. The commit message itself is self-explanatory; no issue is being fixed (no ## Fixes applies). The omission of the full PR description template is acceptable for a trivial docs-only change.
  • WHY explained: The new comments accurately explain why the SHA is pinned (supply-chain risk), why the cron is staggered (log readability, not concurrency), and why the cargo install residual risk is accepted (controlled crates.io publishing). The WHY is now in the code where it belongs. ✓
  • No test changes, no Rust code changes — test/cleanup rules are not triggered. ✓
  • Stale documentation: No rules or AGENTS.md entries are invalidated by this change. ✓

No rule violations detected.


Rule review against .claude/rules/. WARNING findings block merge.

@sanity sanity added this pull request to the merge queue May 7, 2026
Merged via the queue into main with commit ca7fa7a May 7, 2026
13 checks passed
@sanity sanity deleted the fix-mirror-comments branch May 7, 2026 02:11
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