Skip to content

ci: scope engineering-practice skill updates - #590

Merged
devantler merged 2 commits into
mainfrom
codex/engineering-practices-sync-588-r51
Oct 23, 2026
Merged

devantler merged 2 commits into
mainfrom
codex/engineering-practices-sync-588-r51

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Agentic Engineer

Why

Updating engineering guidance currently also refreshes unrelated skill bundles, making focused maintenance harder to control.

What

Adds a focused engineering-practices update option while preserving the existing scheduled behavior and security boundaries. Corrects the links to maintained automation guidance.

Part of #588

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Validation at 1743be46c0fa96e5e2eaf05472bc15bf72987671:

  • The focused scope regression initially rejected workflow_dispatch:engineering-practices; the fixed resolver now selects only that plugin's skill directory and the existing canonical per-skill branch prefix. Scheduled/default all behavior is unchanged. Unknown scopes, injection and scoped schedules remain refused.
  • The permissions regression initially exposed broad workflow-level write permissions. Moving the same effective write permissions to the updater job makes the boundary explicit; the selector and version jobs retain their existing read scopes. No App grant, credential, installed cache or bundled skill file was changed.
  • Local resolver tests, ShellCheck, actionlint, offline zizmor, all manifest/provenance checks and instruction-size validation passed. The complete native pre-PR CI 39888572699 also passed at this exact head.
  • The published body passes the current template contract. Native PR checks and current-head review remain required. The normal scheduled updater independently opened chore(deps): update agent skills (plugins/engineering-practices/skills/ways-of-working) #589; that generated ways-of-working update is not replaced by hand edits here. After this scope change merges, its bounded entrypoint still needs native dispatch/readback before Allow engineering-practice-only skill synchronization #588 is complete.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: d3598894-9c7e-4438-bd8a-1c09494726ee
📥 Commits

Reviewing files that changed from the base of the PR and between 49e1b8a and 1743be4.

📒 Files selected for processing: 5
  • .github/workflows/update-agent-skills.yaml
  • AGENTS.md
  • docs/scripts.md
  • scripts/skill-update-scope.sh
  • scripts/skill-update-scope.test.sh

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🧰 Additional context used
📚 Code guidelines: 1
docs/scripts.md — auto-discovered
📓 Path-based instructions: 1
Applicability reference: AGENTS.md: # devantler-tech/agent-plugins | Guide | Read it before | |---|---| | [docs/scripts.md](docs/scripts.md) | adding, finding or changing a script under `scripts/` | Source excerpt: | Task | Start here | |--...

📄 CodeRabbit inference engine (docs/scripts.md)

Files:

  • scripts/skill-update-scope.sh
  • scripts/skill-update-scope.test.sh
🪛 LanguageTool
docs/scripts.md

[uncategorized] ~11-~11: The official name of this software platform is spelled with a capital “H”.
Context: ...d plugin | Dispatch Update Agent Skills wit...

(GITHUB)

🪛 zizmor (1.30.1)
.github/workflows/update-agent-skills.yaml

[warning] 3-15: insufficient job-level concurrency limits (concurrency-limits): workflow is missing concurrency setting

(concurrency-limits)

[warning] 43-43: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment

(undocumented-permissions)

🔇 Additional comments: 5
.github/workflows/update-agent-skills.yaml: 1

13-13: LGTM!

Also applies to: 17-17, 42-44

scripts/skill-update-scope.sh: 1

9-9: LGTM!

scripts/skill-update-scope.test.sh: 1

15-21: LGTM!

Also applies to: 27-31, 35-43

AGENTS.md: 1

131-132: LGTM!

Also applies to: 146-149

docs/scripts.md: 1

11-11: LGTM!

📝 Walkthrough

The manual update workflow adds engineering-practices as a selectable scope and assigns write permissions to the update job. The scope resolver maps that selection to the engineering-practices skills directory and update branch. Tests cover the new selection, scope validation, dispatch options, and permissions. Documentation adds the selection to the update guidance and updates the automation links.

Possibly related issues

  • Issue 588: The changes add the engineering-practices selection and correct the automation links described in the issue.
  • Issue 240: The changes update this repository’s automation links, but do not address other retirement requirements.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to 1743b

The scoped workflow update is mergeable with no established correctness, security, or availability risk.

Pre-merge checks: Passed 5
Check name Status Explanation
Description check Passed The description explains the need for focused engineering-practices updates, preserved scheduled behavior, security boundaries, and corrected automation links. It is directly related to the changeset.
Title check Passed The title clearly identifies the main change: adding scoped engineering-practices skill updates through CI.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. (3 skipped: 3 unsupported.)

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

The review summary at 1743be46c0fa96e5e2eaf05472bc15bf72987671 was finding-free but its ancillary scanner reported missing caller concurrency and unexplained permission grants. Both are addressed in df8f35aab4342fec1ac9d9636a797fabe1d645d2.

All selections can touch the same generated per-skill branches. One repository-bound, non-cancelling workflow queue now covers the selector, update and digest/version/release-note follow-ups. Its distinct group cannot deadlock with the pinned reusable updater, which declares no concurrency group. The existing effective updater permissions are unchanged and now each explains its purpose.

The new policy control first failed with the intended missing-queue diagnostic, then passed with the complete scope/dispatch/refusal suite. Full manifest, provenance, upstream-license and instruction discovery validation, ShellCheck, offline zizmor and diff checks passed. No bundled skill or installed cache was edited. Fresh CI/review at the new head remain required; the earlier green is not carried forward.

Local actionlint 1.7.12 reports one unsupported-key diagnostic for queue; it has not implemented GitHub's current documented multi-run queue syntax. No check is disabled or suppression added. Native acceptance of this workflow and its scoped dispatch remain required; this local diagnostic is not reported as an actionlint pass. Queueing keeps up to GitHub's 100 waiting-run limit and never claims unlimited delivery.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Review hand-off at df8f35aab4342fec1ac9d9636a797fabe1d645d2: the complete current-head CodeRabbit artifact guard reports ADMIT. That review route is unavailable, so progression continues to Codex. The earlier finding-free review covered only 1743be46c0fa96e5e2eaf05472bc15bf72987671; its ancillary findings were addressed in this head and are not treated as a fresh green. Native CI is green; current-head substantive review remains required.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-23T07:55:55.827597Z df8f35a Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: df8f35aab4

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

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Agentic Engineer

Ready at df8f35aab4342fec1ac9d9636a797fabe1d645d2:

  • Full native PR CI is green. Codex completed current-head review without findings (receipt); the complete artifact guard confirms that result. The previous ancillary concurrency and permission findings are fixed here. LanguageTool's capitalization suggestion concerns the literal .github path, which must remain exact rather than become a different path.
  • As an operator, I ran the real bounded engineering-practices dispatch at this exact head: 39922034057. Its selector, updater, two canonical per-skill proposals and both digest/version/release-note follow-ups all succeeded. Only the engineering-practices skill directory was selected. This is observed execution, not inferred behavior or a substituted hand edit.
  • The regression suite covers scope rejection, unchanged scheduled defaults, job-level permissions and the shared non-cancelling queue. Local manifest/provenance and security checks pass; the documented old actionlint syntax limitation is not counted as a pass. The live body meets the repository template contract.

The generated ways-of-working proposal still requires its own exact-head review, CI and merge. Issue #588 remains open until the merged main entrypoint and downstream delivery are verified. Promotion does not waive any fresh checks triggered by that event.

@devantler
devantler marked this pull request as ready for review October 23, 2026 08:08
@devantler
devantler merged commit c8a67b0 into main Oct 23, 2026
30 checks passed
@github-project-automation github-project-automation Bot moved this from 🫴 Ready to ✅ Done in 🌊 Project Board Oct 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

1 participant