Skip to content

Revert command substitution workaround and add agent permission docs#147

Merged
sebastientaggart merged 2 commits into
devfrom
feature/145-fix-start-reprompt-and-deploy-tags
Apr 22, 2026
Merged

Revert command substitution workaround and add agent permission docs#147
sebastientaggart merged 2 commits into
devfrom
feature/145-fix-start-reprompt-and-deploy-tags

Conversation

@sebastientaggart

Copy link
Copy Markdown
Member

Revert the #137 two-step git rev-parse workaround in submit-for-review, restoring the simpler cd "$(git rev-parse --show-toplevel)" one-liner. Instead of working around agent-specific permission prompts in the skills, add an "Agent tips and tweaks" section to the docs that explains how to configure each agent's permission system (Claude Code permissions.allow, Cursor, Codex, Gemini).

This gets Code Cannon out of the business of agent-specific workarounds in skill instructions — users configure their agent once and all skills benefit.

Issue #145

@sebastientaggart

Copy link
Copy Markdown
Member Author

Review Summary

Verdict: APPROVE

Findings

  • [NOTE] The deny list in the new Claude Code permissions example uses Bash(git push --force *) — this won't match a bare git push --force (no trailing argument). Consider also adding Bash(git push --force) without the wildcard for completeness. Same applies to git reset --hard * and rm -rf *. Low impact since these are documentation suggestions users will adapt, not enforced config.

Otherwise the PR is clean:

  • The two-step git rev-parse / cd <repo-root> workaround is correctly collapsed back to the single cd "$(git rev-parse --show-toplevel)" one-liner across all five adapter files and the source skill template.
  • The sync hashes in the generated file footers have been updated, confirming sync.py was re-run.
  • The new "Agent tips and tweaks" docs section is well-structured, covers all four supported agents, and the deny rules demonstrate a safe-by-default posture.
  • No secrets, no dead code, no convention violations.

@sebastientaggart

Copy link
Copy Markdown
Member Author

Review Summary

Verdict: APPROVE

Findings

No issues found. Code looks correct and follows project conventions.

What was checked:

  • The two-step git rev-parse / cd <repo-root> pattern is correctly collapsed into a single cd "$(git rev-parse --show-toplevel)" one-liner across the source skill template (skills/submit-for-review.md) and all four adapter outputs (.agents/, .claude/, .cursor/, .gemini/). Both occurrences per file (pre-check and pre-merge) are updated consistently (10 total).
  • The deny rules in the new docs section (docs/index.md) include both bare-command and wildcard variants for git push --force and git reset --hard, addressing the previous review finding. rm -rf and sudo correctly use wildcard-only forms since the bare commands alone are harmless or non-existent.
  • Sync hashes in generated file footers are updated, consistent with the content changes.
  • No secrets, .env files, or unsafe patterns introduced.

@sebastientaggart
sebastientaggart merged commit 7ebf868 into dev Apr 22, 2026
6 checks passed
@sebastientaggart
sebastientaggart deleted the feature/145-fix-start-reprompt-and-deploy-tags branch April 22, 2026 23:43
@sebastientaggart sebastientaggart mentioned this pull request May 26, 2026
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