chore: replace peaceiris/actions-gh-pages and dorny/paths-filter with native git/shell#109
chore: replace peaceiris/actions-gh-pages and dorny/paths-filter with native git/shell#109Copilot wants to merge 12 commits into
Conversation
… native git/shell Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/a76f3f0b-859f-486a-b72d-f2c7c9b48477 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds two repo-local composite actions (gh-pages-deploy, paths-filter), a reusable detect-changes workflow, supporting scripts and tests, and updates multiple workflows to use the new actions and gate jobs via the detect-changes workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant Workflow as "Workflow (caller)"
participant Action as "Composite Action\n(gh-pages-deploy)"
participant Script as "deploy.sh"
participant Git as "Local Git\n(worktree)"
participant Remote as "Git Remote\n(origin)"
Workflow->>Action: invoke with inputs/env
Action->>Script: execute deploy.sh (INPUT_* env)
Script->>Git: fetch/check publish branch, create or switch worktree/orphan
Script->>Git: compute destination path, clear if needed
Script->>Git: rsync publish_dir → worktree (apply excludes/destination_dir)
Script->>Git: stage changes, commit if changes
Script->>Remote: push publish_branch (token-embedded URL)
Script->>Git: git worktree remove --force (cleanup)
Script-->>Workflow: exit status / logs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
I pity the fool who doesn't read this! 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…d shell tests Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/4f60af0f-87e2-4078-a371-9a2ec9f09868 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
…ecov coverage Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/0047cdb7-30cf-42d6-b536-50e08c386248 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
… actions Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/0047cdb7-30cf-42d6-b536-50e08c386248 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #109 +/- ##
==========================================
Coverage 100.00% 100.00%
==========================================
Files 1 2 +1
Lines 263 382 +119
Branches 113 159 +46
==========================================
+ Hits 263 382 +119
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/LEARNINGS.md (1)
105-118:⚠️ Potential issue | 🟡 MinorReplace the
#—placeholders in the categorised learnings table.I pity the fool who leaves the source PR blank when the PR number is already known. A5, A6, S4, and S5 lose traceability in the table future agents are supposed to scan first. As per coding guidelines, "Follow the learning pattern in
docs/LEARNINGS.md— add entries with category headers, concise descriptions, links to PRs, and update the Categorised Learnings tables by topic".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LEARNINGS.md` around lines 105 - 118, The table rows for A5, A6, S4 and S5 in docs/LEARNINGS.md currently have "#—" placeholders in the "Source PR" column; replace each placeholder with the actual PR identifier (e.g. "PR `#1234`" or a GitHub PR link) that introduced the learning so the entries are traceable, keeping the existing table format and category headers; specifically update the rows for A5 (Composite action outputs), A6 (git worktree cleanup + rsync mtime/size note), S4 (actions vs third-party actions recommendation) and S5 (pass GitHub context via env), ensuring each Source PR cell contains the PR number or link consistent with other table entries and follow the documented learning pattern..github/workflows/unit-tests.yml (1)
57-64:⚠️ Potential issue | 🟠 MajorDon't let the status job hide a broken
changesjob.I pity the fool who reports green when change detection already blew up. Right now this step only checks
needs.test.result, so a failure in the reusable workflow or local paths-filter action turnstestintoskippedand this status still exits 0.Suggested fix
- if [[ "${{ needs.test.result }}" == "failure" ]]; then + if [[ "${{ needs.changes.result }}" == "failure" || "${{ needs.test.result }}" == "failure" ]]; then echo "Unit tests failed" exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/unit-tests.yml around lines 57 - 64, The report step currently only inspects needs.test.result and can report success when the reusable 'changes' job (or paths-filter) failed; update the "Report status" step to check the result of both dependent jobs (e.g., needs.test.result and needs.changes.result) and exit non‑zero if any existing dependency's result is not "success" (treat "failure" or other non-success values as failure), ensuring the workflow fails when the changes job or test job fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/gh-pages-deploy/deploy.sh:
- Around line 72-95: The current logic only clears stale files when DEPLOY is to
a subdirectory (DESTINATION_DIR set) and leaves root deploys intact; change it
so when KEEP_FILES != "true" we remove stale files for root deploys too while
preserving previews/; do this by removing the DESTINATION_DIR check and either
(a) always using rsync with --delete when KEEP_FILES != "true" and adding a
protect exclude "--exclude=previews/" into the RSYNC_EXCLUDES array, or (b) if
keeping the find-based removal, run the find ... -delete unconditionally when
KEEP_FILES != "true" but skip deleting the previews/ directory (e.g., exclude
"${DEST}/previews" from deletion). Update the rsync invocation (rsync -a ...) to
include the --delete flag when KEEP_FILES != "true" and ensure RSYNC_EXCLUDES
contains "--exclude=previews/" so previews/ is preserved; keep existing symbols:
KEEP_FILES, DESTINATION_DIR, DEST, RSYNC_EXCLUDES, EXCLUDE_ASSETS, PUBLISH_DIR,
WORKTREE_DIR, and the .nojekyll touch logic unchanged.
In @.github/workflows/detect-changes.yml:
- Around line 70-76: Update the "filters" code patterns in detect-changes.yml's
filters block so CI-only changes are detected: add glob entries for YAML
workflow files (*.yml, *.yaml), shell scripts (*.sh), and the
actions/reusable-workflow locations (e.g., add patterns matching the actions
directory and workflow files) alongside the existing
JS/TS/CSS/tests/package*.json globs so changes to CI workflows and action
scripts no longer resolve to code=false.
In @.github/workflows/preview.yml:
- Around line 7-10: The preview workflow's concurrency group uses
"pages-preview-${{ github.event.number }}" which doesn't coordinate with the
production deploy's concurrency group ("pages"), allowing simultaneous pushes to
gh-pages; update the concurrency.group value in this file from
"pages-preview-${{ github.event.number }}" to "pages" so all workflows that push
gh-pages share the same lock (leave cancel-in-progress behavior as appropriate),
and if you still need per-PR cancellation preserve that logic at the job level
or elsewhere that does not perform the branch push.
In `@docs/LEARNINGS.md`:
- Around line 150-155: Update the PR log entry in docs/LEARNINGS.md: replace the
placeholder "PR #—" with "PR `#109`", change the title to "chore: replace
peaceiris/actions-gh-pages and dorny/paths-filter with native git/shell", and
correct the learning pointer from "(→ A1, S5)" to "(→ A5, A6)"; ensure the entry
follows the repository learning pattern by adding appropriate category headers,
a concise description, and a link to PR `#109`, and then update the Categorised
Learnings tables by topic to include this entry (verify related files referenced
like .github/actions/paths-filter/action.yml and
.github/actions/gh-pages-deploy/deploy.sh are noted if required).
In `@tests/gh-pages-deploy.test.sh`:
- Around line 104-106: The subshell that starts with "(" runs cd "${TMPWORK}"
then env; if the cd fails the script continues in the wrong directory — update
the subshell's cd invocation (the line containing cd "${TMPWORK}" inside the "("
... ")" block) to fail fast by appending a short-circuit exit (i.e., change the
cd command to include "|| exit 1") so the subshell stops immediately when the
temp worktree can't be entered.
---
Outside diff comments:
In @.github/workflows/unit-tests.yml:
- Around line 57-64: The report step currently only inspects needs.test.result
and can report success when the reusable 'changes' job (or paths-filter) failed;
update the "Report status" step to check the result of both dependent jobs
(e.g., needs.test.result and needs.changes.result) and exit non‑zero if any
existing dependency's result is not "success" (treat "failure" or other
non-success values as failure), ensuring the workflow fails when the changes job
or test job fails.
In `@docs/LEARNINGS.md`:
- Around line 105-118: The table rows for A5, A6, S4 and S5 in docs/LEARNINGS.md
currently have "#—" placeholders in the "Source PR" column; replace each
placeholder with the actual PR identifier (e.g. "PR `#1234`" or a GitHub PR link)
that introduced the learning so the entries are traceable, keeping the existing
table format and category headers; specifically update the rows for A5
(Composite action outputs), A6 (git worktree cleanup + rsync mtime/size note),
S4 (actions vs third-party actions recommendation) and S5 (pass GitHub context
via env), ensuring each Source PR cell contains the PR number or link consistent
with other table entries and follow the documented learning pattern.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 15760f86-87a2-4049-b9f2-c4674fe3c0d9
📒 Files selected for processing (16)
.github/actions/gh-pages-deploy/action.yml.github/actions/gh-pages-deploy/deploy.sh.github/actions/paths-filter/action.yml.github/actions/paths-filter/filter.js.github/scripts/detect-changes.sh.github/workflows/deploy.yml.github/workflows/detect-changes.yml.github/workflows/e2e-tests.yml.github/workflows/preview-cleanup.yml.github/workflows/preview.yml.github/workflows/unit-tests.ymldocs/LEARNINGS.mdpackage.jsontests/detect-changes.test.shtests/gh-pages-deploy.test.shtests/paths-filter.test.js
…es in deploy.yml Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/6069e328-f7f9-49c0-8432-e49f35b9902a Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
…ntation Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/6069e328-f7f9-49c0-8432-e49f35b9902a Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/LEARNINGS.md (1)
117-118:⚠️ Potential issue | 🟡 MinorI pity the fool who left placeholder PR numbers in the Security learnings table!
S4 and S5 both say "PR #—" when they oughta say PR
#109! Don't be a fool - fix those placeholders!💪 Proposed fix
-| S4 | Prefer `actions/` (GitHub's official org) over third-party organisations for GitHub Actions steps. `peaceiris/actions-gh-pages` can be replaced with native `git worktree` + `rsync` shell commands; `dorny/paths-filter` can be replaced with a `git diff --name-only` shell step. | #— | -| S5 | Always pass GitHub context values to shell scripts via `env:` vars (e.g. `GH_SHA: ${{ github.sha }}`), never by interpolating `${{ }}` directly inside `run:`. Inline interpolation allows expression injection if an attacker controls the context value. | #— | +| S4 | Prefer `actions/` (GitHub's official org) over third-party organisations for GitHub Actions steps. `peaceiris/actions-gh-pages` can be replaced with native `git worktree` + `rsync` shell commands; `dorny/paths-filter` can be replaced with a `git diff --name-only` shell step. | `#109` | +| S5 | Always pass GitHub context values to shell scripts via `env:` vars (e.g. `GH_SHA: ${{ github.sha }}`), never by interpolating `${{ }}` directly inside `run:`. Inline interpolation allows expression injection if an attacker controls the context value. | `#109` |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LEARNINGS.md` around lines 117 - 118, Update the Security learnings table entries labeled "S4" and "S5" to replace the placeholder PR reference "PR #—" with the correct PR number "PR `#109`"; specifically edit the table cells containing the S4 and S5 rows so the rightmost column reads "PR `#109`" (or uses inline code formatting `#109` if consistent with the table style).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/LEARNINGS.md`:
- Around line 105-106: Update the PR reference placeholders in the LEARNINGS
table entries A5 and A6 to the actual PR number: replace "PR #—" with "PR `#109`"
for both rows so the table correctly cites PR `#109` instead of the placeholder;
target the two table cells containing A5 and A6 text in docs/LEARNINGS.md.
- Around line 150-182: Replace the four placeholder "PR #—" tokens in the four
PR log entries with the correct PR number "#109" (search for the four
occurrences of the literal string "PR #—" in the LEARNINGS.md PR entries) and
update the incorrect learning pointer "(→ A1, S5)" to "(→ A5, A6)" (look for
that pointer on the PR entry describing composite actions). Also ensure each
updated PR entry follows the project's learning entry pattern (category headers,
concise description, link to PR `#109`) and update the Categorised Learnings
tables to include these corrected entries under the appropriate topics.
---
Duplicate comments:
In `@docs/LEARNINGS.md`:
- Around line 117-118: Update the Security learnings table entries labeled "S4"
and "S5" to replace the placeholder PR reference "PR #—" with the correct PR
number "PR `#109`"; specifically edit the table cells containing the S4 and S5
rows so the rightmost column reads "PR `#109`" (or uses inline code formatting
`#109` if consistent with the table style).
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 56736e0f-ee11-44e3-8bf4-aa020acd57b1
📒 Files selected for processing (5)
.github/actions/paths-filter/filter.js.github/workflows/deploy.ymldocs/LEARNINGS.mdpackage.jsontests/paths-filter.test.js
✅ Files skipped from review due to trivial changes (1)
- .github/actions/paths-filter/filter.js
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/deploy.yml
- package.json
- tests/paths-filter.test.js
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/actions/gh-pages-deploy/deploy.sh (1)
75-83:⚠️ Potential issue | 🟠 MajorDon’t let root deploys keep ghost files alive, fool.
Line [78] only clears stale content for subdirectory deploys. Root deploys can keep deleted assets indefinitely, which leaves old production files publicly served.
Proposed fix
-# When keep_files is false and deploying to a subdirectory, clear stale content -# so deleted files don't linger. Root deployments always preserve other content -# (e.g. the previews/ directory) regardless of keep_files. -if [[ "${KEEP_FILES}" != "true" && -n "${DESTINATION_DIR}" ]]; then - # Suppress errors (|| true): a non-zero exit here means DEST is already empty - # or a file is already gone — both are harmless. Genuine failures (e.g. - # permission errors on the runner) will surface at the rsync or git-add step. - find "${DEST}" -mindepth 1 -delete 2>/dev/null || true -fi +# When keep_files is false, remove stale files before sync. +# Preserve previews/ for root deploys. +if [[ "${KEEP_FILES}" != "true" ]]; then + if [[ -z "${DESTINATION_DIR}" ]]; then + RSYNC_EXCLUDES+=("--exclude=previews/") + RSYNC_DELETE=(--delete) + else + find "${DEST}" -mindepth 1 -delete 2>/dev/null || true + fi +fi @@ -rsync -a ${RSYNC_EXCLUDES[@]+"${RSYNC_EXCLUDES[@]}"} "${PUBLISH_DIR%/}/" "${DEST}/" +rsync -a ${RSYNC_DELETE[@]+"${RSYNC_DELETE[@]}"} \ + ${RSYNC_EXCLUDES[@]+"${RSYNC_EXCLUDES[@]}"} \ + "${PUBLISH_DIR%/}/" "${DEST}/"Also applies to: 101-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/gh-pages-deploy/deploy.sh around lines 75 - 83, The conditional currently only clears stale files when KEEP_FILES != "true" and DESTINATION_DIR is set, leaving root deployments to accumulate stale files; change the logic so that whenever KEEP_FILES != "true" the script runs the find "${DEST}" -mindepth 1 -delete 2>/dev/null || true cleanup regardless of DESTINATION_DIR. Update the block using KEEP_FILES/DESTINATION_DIR/DEST (and the duplicate cleanup block later that mirrors this logic) so root deploys also purge deleted assets, keeping the same error suppression (|| true) behavior.
🧹 Nitpick comments (2)
.github/workflows/preview-cleanup.yml (1)
23-25: Line 23 says “positive integer,” but the regex still allows0.If you want strict alignment, use
^[1-9][0-9]*$(or update the comment to “non-negative integer”).Suggested tweak
- # Validate PR_NUMBER is a positive integer to guard against malformed + # Validate PR_NUMBER is a positive integer to guard against malformed # context values that could produce an unexpected directory path. - if [[ ! "${PR_NUMBER}" =~ ^[0-9]+$ ]]; then + if [[ ! "${PR_NUMBER}" =~ ^[1-9][0-9]*$ ]]; then echo "Unexpected PR_NUMBER value: '${PR_NUMBER}' — aborting cleanup" >&2 exit 1 fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/preview-cleanup.yml around lines 23 - 25, The comment and regex disagree: the check for PR_NUMBER currently allows zero but says “positive integer”; update the validation in the PR_NUMBER check so it enforces a strictly positive integer by replacing the regex with one that disallows 0 (e.g., use the pattern ^[1-9][0-9]*$) or, alternatively, keep the current regex and change the comment to say “non-negative integer”; locate the PR_NUMBER validation block in the preview-cleanup.yml workflow (the if [[ ! "${PR_NUMBER}" =~ ... ]]) and apply the chosen fix so the comment and validation match..github/actions/gh-pages-deploy/deploy.sh (1)
32-33: Set an EXIT trap for worktree cleanup, fool.Line [117] cleanup runs only on the happy path. With
set -e, failures before the end can leave a registered worktree behind and trip later invocations in the same job.Proposed hardening
WORKTREE_DIR="$(mktemp -d)" +REPO_ROOT="$(git rev-parse --show-toplevel)" + +cleanup() { + set +e + cd "${REPO_ROOT}" || return + git worktree remove --force "${WORKTREE_DIR}" 2>/dev/null || rm -rf "${WORKTREE_DIR}" +} +trap cleanup EXIT @@ -# Unregister and remove the worktree to avoid conflicts when the action is -# called multiple times within the same job (e.g. a preview workflow that -# deploys the site and then uploads screenshots). -cd - -git worktree remove --force "${WORKTREE_DIR}" 2>/dev/null || rm -rf "${WORKTREE_DIR}" +# Cleanup is handled by trap on all exit paths.Also applies to: 113-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/actions/gh-pages-deploy/deploy.sh around lines 32 - 33, The script creates a temporary WORKTREE_DIR (variable WORKTREE_DIR="$(mktemp -d)") but only removes it on the happy path (cleanup at lines ~113-117); add a trap 'EXIT' that always removes the worktree if set and non-empty (and unregisters it if necessary) so failures under set -e cannot leave the worktree behind; update the cleanup logic referenced around the existing cleanup function/name to be idempotent and referenced by the trap, e.g., define a cleanup function that checks WORKTREE_DIR and runs git worktree remove/unregister and rm -rf, then register it with 'trap cleanup EXIT'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/paths-filter/filter.js:
- Around line 272-273: After calling parseFilters(filtersYaml) (the filtersMap
variable), add a fail-fast guard that detects an empty or malformed
INPUT_FILTERS parse result (e.g., filtersMap is null/undefined or
Object.keys(filtersMap).length === 0) and immediately fails the action with a
clear error message (using core.setFailed or throwing) so the script doesn't
continue and emit only changes={} without per-filter outputs; place this check
right after the parseFilters(filtersYaml) line to ensure parse failures are
explicit and per-filter outputs are not silently omitted.
- Around line 231-245: The current spawnSync calls that run 'git ls-files' and
'git diff --name-only' use r.stdout.trim().split('\n') which strips
leading/trailing whitespace from valid filenames; update both spawnSync
invocations to pass the -z flag (i.e., ['ls-files','-z'] and
['diff','-z','--name-only', base, sha]) and replace the
r.stdout.trim().split('\n').filter(Boolean) handling with
r.stdout.split('\0').filter(Boolean) so filenames are NUL-delimited and
whitespace is preserved.
---
Duplicate comments:
In @.github/actions/gh-pages-deploy/deploy.sh:
- Around line 75-83: The conditional currently only clears stale files when
KEEP_FILES != "true" and DESTINATION_DIR is set, leaving root deployments to
accumulate stale files; change the logic so that whenever KEEP_FILES != "true"
the script runs the find "${DEST}" -mindepth 1 -delete 2>/dev/null || true
cleanup regardless of DESTINATION_DIR. Update the block using
KEEP_FILES/DESTINATION_DIR/DEST (and the duplicate cleanup block later that
mirrors this logic) so root deploys also purge deleted assets, keeping the same
error suppression (|| true) behavior.
---
Nitpick comments:
In @.github/actions/gh-pages-deploy/deploy.sh:
- Around line 32-33: The script creates a temporary WORKTREE_DIR (variable
WORKTREE_DIR="$(mktemp -d)") but only removes it on the happy path (cleanup at
lines ~113-117); add a trap 'EXIT' that always removes the worktree if set and
non-empty (and unregisters it if necessary) so failures under set -e cannot
leave the worktree behind; update the cleanup logic referenced around the
existing cleanup function/name to be idempotent and referenced by the trap,
e.g., define a cleanup function that checks WORKTREE_DIR and runs git worktree
remove/unregister and rm -rf, then register it with 'trap cleanup EXIT'.
In @.github/workflows/preview-cleanup.yml:
- Around line 23-25: The comment and regex disagree: the check for PR_NUMBER
currently allows zero but says “positive integer”; update the validation in the
PR_NUMBER check so it enforces a strictly positive integer by replacing the
regex with one that disallows 0 (e.g., use the pattern ^[1-9][0-9]*$) or,
alternatively, keep the current regex and change the comment to say
“non-negative integer”; locate the PR_NUMBER validation block in the
preview-cleanup.yml workflow (the if [[ ! "${PR_NUMBER}" =~ ... ]]) and apply
the chosen fix so the comment and validation match.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b4cd099-4148-4314-9ac7-7da209f8d873
📒 Files selected for processing (4)
.github/actions/gh-pages-deploy/deploy.sh.github/actions/paths-filter/filter.js.github/workflows/detect-changes.yml.github/workflows/preview-cleanup.yml
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/detect-changes.yml
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ⛔ CodeRabbit does not have permission to push changes to this repository. Please ensure CodeRabbit has Contents: Read and write permission. |
Fixed 1 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/LEARNINGS.md`:
- Around line 117-118: Update the Security & Supply Chain table entries S4 and
S5 in docs/LEARNINGS.md by replacing the placeholder PR reference "#—" with the
correct PR number "#109" for both rows (the lines containing "S4 | Prefer
`actions/`..." and "S5 | Always pass GitHub context..."); ensure the format
matches the other table entries (category header, concise description, link/PR
column) so the table remains consistent with the Architecture table changes
already made.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c46dc931-26a0-4ca5-8b3e-2eaaa9684381
📒 Files selected for processing (1)
docs/LEARNINGS.md
Signed-off-by: RB <7775707+nitrocode@users.noreply.github.com>
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/LEARNINGS.md (1)
118-119:⚠️ Potential issue | 🟡 MinorReplace remaining
#—placeholders with the actual PR reference.I pity the fool who leaves unresolved placeholders in the Source PR column — update both rows to
#109so these learnings are traceable and consistent with the rest of this update.💪 Proposed fix
-| S4 | Prefer `actions/` (GitHub's official org) over third-party organisations for GitHub Actions steps. `peaceiris/actions-gh-pages` can be replaced with native `git worktree` + `rsync` shell commands; `dorny/paths-filter` can be replaced with a `git diff --name-only` shell step. | #— | -| S5 | Always pass GitHub context values to shell scripts via `env:` vars (e.g. `GH_SHA: ${{ github.sha }}`), never by interpolating `${{ }}` directly inside `run:`. Inline interpolation allows expression injection if an attacker controls the context value. | #— | +| S4 | Prefer `actions/` (GitHub's official org) over third-party organisations for GitHub Actions steps. `peaceiris/actions-gh-pages` can be replaced with native `git worktree` + `rsync` shell commands; `dorny/paths-filter` can be replaced with a `git diff --name-only` shell step. | `#109` | +| S5 | Always pass GitHub context values to shell scripts via `env:` vars (e.g. `GH_SHA: ${{ github.sha }}`), never by interpolating `${{ }}` directly inside `run:`. Inline interpolation allows expression injection if an attacker controls the context value. | `#109` |As per coding guidelines, "Follow the learning pattern in
docs/LEARNINGS.md— add entries with category headers, concise descriptions, links to PRs, and update the Categorised Learnings tables by topic".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/LEARNINGS.md` around lines 118 - 119, Update the two table rows for learning items S4 and S5 in docs/LEARNINGS.md by replacing the placeholder Source PR column values `#—` with the actual PR reference `#109`; locate the rows identified by the symbols "S4" and "S5" and set their third-column value to `#109` so the entries are traceable and consistent with the rest of the document.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/LEARNINGS.md`:
- Line 151: Replace the PR log heading string "PR `#109` refactor: replace
third-party actions with native git worktree and diff" in docs/LEARNINGS.md with
the actual Conventional Commit title: "chore: replace peaceiris/actions-gh-pages
and dorny/paths-filter with native git/shell" so the entry exactly matches the
PR metadata and aids traceability.
---
Duplicate comments:
In `@docs/LEARNINGS.md`:
- Around line 118-119: Update the two table rows for learning items S4 and S5 in
docs/LEARNINGS.md by replacing the placeholder Source PR column values `#—` with
the actual PR reference `#109`; locate the rows identified by the symbols "S4"
and "S5" and set their third-column value to `#109` so the entries are traceable
and consistent with the rest of the document.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8eebc9c6-960f-45ec-9bb5-a9b014b2acbf
📒 Files selected for processing (1)
docs/LEARNINGS.md
Signed-off-by: RB <7775707+nitrocode@users.noreply.github.com>
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ⛔ CodeRabbit does not have permission to push changes to this repository. Please ensure CodeRabbit has Contents: Read and write permission. |
|
@coderabbitai apply |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI need to check what specific suggestion you're asking me to apply, fool! Let me look at the PR review comments right quick! 🏁 Script executed: Length of output: 80737
Applying all unresolved CodeRabbit review comments on PR
|
Agent-Logs-Url: https://github.com/nitrocode/token-deathclock/sessions/5186c4fa-4576-41cf-a87e-2a817e718f97 Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>


deploy.sh: Add EXIT trap for guaranteed worktree cleanup; addrsync --deletewith--exclude=.git(protects worktree ref) +--exclude=previews/(protects preview dirs) for root deploys; move.nojekyllcreation to AFTER rsync (prevents--deletefrom removing it)detect-changes.yml: Add.github/actions/**/*.sh/yml/yaml,.github/scripts/**/*.sh,.github/workflows/**/*.yml/yamlglob patterns to filterpreview.yml: Change concurrency group to"pages"(shared with deploy.yml) +cancel-in-progress: falseto prevent cancelling in-flight production deploysunit-tests.yml: Check bothneeds.test.resultANDneeds.changes.resultin status steptests/gh-pages-deploy.test.sh: Add|| exit 1tocd "${TMPWORK}"; add root-deploy stale-file removal and previews-preservation tests (24 tests total, all pass)filter.js: Switch to NUL-delimited-zgit output (git ls-files -z,git diff --name-only -z, split on\0); add fail-fast guard for empty filtersMaptests/paths-filter.test.js: Update allgetChangedFiles/maintests for NUL output +-zflag; replace empty-filters test with fail-fast guard test; resetprocess.exitCodein beforeEach (448 tests, all pass)docs/LEARNINGS.md: Add PR log entry