Skip to content

Fix ssh-signing.md allowed_signers Snippet to Match host-setup.md - #974

Merged
ptr727 merged 1 commit into
developfrom
issue-713-ssh-signing-allowed-signers
Aug 24, 2026
Merged

Fix ssh-signing.md allowed_signers Snippet to Match host-setup.md#974
ptr727 merged 1 commit into
developfrom
issue-713-ssh-signing-allowed-signers

Conversation

@ptr727

@ptr727 ptr727 commented Aug 24, 2026

Copy link
Copy Markdown
Owner

Applies the same fix PR #708 made in docs/host-setup.md to docs/ssh-signing.md, which carried the identical bugs:

  1. Missing --global on git config user.email, which silently writes the wrong principal into allowed_signers when run from inside an existing repo checkout with a local override.
  2. A backslash line-continuation across two lines, collapsed into one, matching the fixed shape in docs/host-setup.md.

Also fixes the identical inline reference to the same snippet in the Troubleshooting section.

Fixes #713

🤖 Generated with Claude Code

Applies the same fix PR #708 made in docs/host-setup.md: add --global to
git config user.email so a repo-local override can't write the wrong
principal into allowed_signers, and collapse the backslash-continued
two-line echo into one so there is no continuation to break. Also fixes
the identical inline reference in the Troubleshooting section.

Fixes #713
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 2 minutes.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 65a5e7b0-2115-44a7-b21e-d389a1d763d8

📥 Commits

Reviewing files that changed from the base of the PR and between 11c7787 and 1e96655.

📒 Files selected for processing (1)
  • docs/ssh-signing.md

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

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Align ssh-signing.md allowed_signers snippet with host-setup.md

📝 Documentation 🐞 Bug fix 🕐 Less than 10 minutes

Grey Divider

AI Description

• Fix allowed_signers snippet to read user.email from global git config.
• Remove fragile line-continuation so the echo appends correctly as a single line.
• Update troubleshooting snippet reference to match the corrected command.
Diagram

graph TD
  U((Developer)) --> D["docs/ssh-signing.md"] --> C["Run allowed_signers snippet"] --> A["~/.config/git/allowed_signers"] --> G["Git SSH signing"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Deduplicate snippet via shared include/partial
  • ➕ Prevents host-setup and ssh-signing docs from drifting again
  • ➕ Enables single-point fixes for future corrections
  • ➖ Requires docs tooling/support for includes (or a build step)
  • ➖ Adds indirection when reading docs on GitHub
2. Link to host-setup snippet instead of duplicating it
  • ➕ Simple; avoids repeated command text across docs
  • ➕ Keeps the authoritative snippet in one place
  • ➖ Readers must context-switch to another doc
  • ➖ May be less discoverable for users who only read ssh-signing.md
3. Add a docs check to catch `git config user.email` (missing --global) in snippets
  • ➕ Automated guardrail against reintroducing the same mistake
  • ➕ Scales to other common snippet footguns
  • ➖ Extra CI/lint maintenance
  • ➖ Potential false positives in unrelated documentation contexts

Recommendation: The PR’s direct fix is appropriate and low-risk. As a follow-up, consider deduplicating or linking the snippet to reduce future drift; adding a lightweight docs lint for known-bad patterns could further prevent recurrence.

Files changed (1) +2 / -3

Documentation (1) +2 / -3
ssh-signing.mdFix allowed_signers snippet to use global user.email and single-line echo +2/-3

Fix allowed_signers snippet to use global user.email and single-line echo

• Updates the setup snippet to use 'git config --global user.email' to avoid repo-local overrides producing an incorrect principal. Collapses the backslash-continued 'echo' into a single line, and updates the troubleshooting reference to match.

docs/ssh-signing.md

@ptr727
ptr727 merged commit 80fcac2 into develop Aug 24, 2026
8 checks passed
@ptr727
ptr727 deleted the issue-713-ssh-signing-allowed-signers branch August 24, 2026 19:45
ptr727 added a commit that referenced this pull request Aug 24, 2026
PR #975 (the develop -> main promotion PR) surfaced this against #974's
change: the
"signing failed: no allowed signers" troubleshooting line paraphrased
the setup
snippet as an unquoted, truncated `echo ... >> ...` form. Copied
literally, a shell
strips the quotes around `"git"` (writing `namespaces=git` instead of
`namespaces="git"`), and the truncated destination path isn't a real
command.

Both CodeRabbit and Qodo flagged this independently on PR #975. Fix: use
the exact,
complete command from the setup section (matching line 35) instead of
paraphrasing it.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Documentation**
- Corrected the SSH signing troubleshooting command to use proper
quoting and the complete allowed signers file path.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
ptr727 added a commit that referenced this pull request Aug 24, 2026
Promotes develop to main.

Includes:
- #974: Fix ssh-signing.md allowed_signers snippet to match
host-setup.md (fixes #713)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Documentation**
* Updated SSH signing setup and troubleshooting instructions to use the
email address configured globally in Git.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
@ptr727 ptr727 mentioned this pull request Aug 25, 2026
ptr727 added a commit that referenced this pull request Aug 25, 2026
Promotes #981 (issue #947) to `main`.

## What

- `merge-and-release/SKILL.md` step 7's clean-tree preflight now scopes
its
  `git status --porcelain --untracked-files=all --ignored` check to
`-- .agents/skills/ .claude-plugin/` (all three carried copies), instead
of
  the whole checkout, matching `skills_install.py`'s own `source_ref()`
watched-path list. An ordinary build cache or lockfile elsewhere in the
  tree no longer blocks the refresh.
- `skills_install.py`'s install summary prints two separate lines, one
per
install target's own outcome, instead of folding both into one sentence.

## Review

PR #981 went through two review rounds:

- qodo flagged the PR title's lowercase "to" as a title-case violation.
Declined: `to` is an allowed lowercase title-case bind word per the
fleet's
  own convention, and this exact false positive is already documented in
  `docs/pr-reviewer-evaluation.md`.
- CodeRabbit correctly caught that the first pass under-scoped the
preflight
  to `.agents/skills/` alone, missing `.claude-plugin/`, which
`skills_install.py` also reads (`claude plugin marketplace add` installs
from it). Fixed, and CodeRabbit's re-review on the fixed head came back
  clean ("No actionable comments were generated in the recent review.").

Copilot's review account is still quota-exhausted repo-wide (confirmed
again
this session, consistent with recent PRs #974-980), so PR #981 merged to
`develop` on qodo + CodeRabbit coverage alone, both clean on the final
head,
CI green (8/8), `mergeStateStatus: CLEAN`.

Closes #947


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Refined Skills refresh checks to include tracked, untracked, and
ignored files in all relevant installation paths.
  * Unrelated ignored files no longer block Skills refreshes.

* **Improvements**
* Installation results now separately report global Skills setup and
marketplace registration for clearer status visibility.

* **Documentation**
* Updated Skills refresh guidance to clarify which installation paths
are checked and how ignored files are handled.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
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