Skip to content

feat: doctor, GPG signing, and all Copilot fixes from #1236#1238

Closed
jpoley wants to merge 9 commits into
mainfrom
fix/pr-1236-copilot-fixes
Closed

feat: doctor, GPG signing, and all Copilot fixes from #1236#1238
jpoley wants to merge 9 commits into
mainfrom
fix/pr-1236-copilot-fixes

Conversation

@jpoley

@jpoley jpoley commented Apr 5, 2026

Copy link
Copy Markdown
Owner

Summary

Rebased from closed PR #1236 (feat/all-issues-combined) with all Copilot review findings resolved across both rounds of review (PR #1236 + #1237).

Copilot findings fixed (18 total across both review rounds)

signing.py:

  • Use --status-fd for reliable GPG fingerprint extraction from key generation (not --list-keys)
  • Verify secret key by exact fingerprint (not email) in configure_git_signing()
  • Simplify unreachable key_exists() branch in generate_agent_key()
  • Add secret key presence check before configuring git
  • Fix unused stderr variable in get_key_info() (Ruff F841)

doctor.py:

  • Validate flow.*.agent.md pattern explicitly in check_agent_files()
  • Downgrade unknown version from FAIL to WARN
  • Fix module docstring example to filter non-PASS results
  • Update docstring to reflect library function (not CLI command)

init.py (CLI wiring):

  • Register gpg_app sub-app with main Typer CLI (flowspec gpg)
  • Register doctor command with main CLI (flowspec doctor)

tests:

  • Remove unused imports (Path, MagicMock, sys) from test files
  • Mock httpx.get in doctor tests to prevent network dependency / flaky CI
  • Fix agent naming tests to validate flow.*.agent.md convention
  • Update signing test mocks for --status-fd and fingerprint-based verification

docs/guides/gpg-signing.md:

  • Use valid hex-only fingerprint examples
  • Remove false telemetry integration claims

Closes #1221 #1222 #1223 #1224 #1225 #1226

Test plan

  • uv run ruff check . — all checks passed
  • uv run ruff format --check . — all files formatted
  • uv run pytest tests/ -x -q — 3536 passed, 42 skipped, 0 failures

🤖 Generated with Claude Code

jpoley and others added 9 commits April 3, 2026 11:23
Implements flowspec doctor command that checks environment setup and
surfaces configuration problems with clear fix instructions.

Checks performed:
- flowspec CLI version
- Python version compatibility (requires 3.11+)
- Required tools installed (backlog.md, beads)
- Workflow configuration present and valid
- Agent files using correct naming convention (dot notation)
- Constitution file present

Features:
- Color-coded output (✅ pass, ⚠️ warn, ❌ fail)
- Suggested fix commands for each issue
- --verbose flag for detailed diagnostics
- --fix flag stub for future auto-fix functionality

Closes #1221

🤖 Generated with Claude Code

Co-Authored-By: Claude <noreply@anthropic.com>
Implement GPG commit signing for Flowspec agents:

1. Created src/flowspec_cli/signing.py:
   - generate_agent_key(): Generate 4096-bit RSA GPG key for agents
   - configure_git_signing(): Set user.signingkey and commit.gpgsign=true
   - get_key_fingerprint(): Return stored fingerprint from keyring
   - key_exists(): Check if key already exists
   - delete_agent_key(): Support key rotation

2. Added CLI subcommands via src/flowspec_cli/gpg_cli.py:
   - flowspec gpg setup: Generate key and configure git signing
   - flowspec gpg status: Show key and signing configuration status
   - flowspec gpg rotate: Delete old key and generate new one

3. Integrated fingerprint into telemetry output:
   - flowspec telemetry status shows GPG fingerprint when signing is active
   - Provides audit trail of agent commit signatures

4. Documentation in docs/guides/gpg-signing.md:
   - Key management best practices
   - Key rotation workflow and schedule recommendations
   - Troubleshooting guide
   - GitHub/GitLab integration instructions

5. Comprehensive test coverage in tests/test_signing.py:
   - Key generation, configuration, retrieval, and deletion
   - Error handling and edge cases
   - Git command integration tests

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

Co-Authored-By: Claude <noreply@anthropic.com>
Extract fingerprint from key-creation status output ([GNUPG:] KEY_CREATED)
instead of post-hoc --list-keys lookup, which could match the wrong key
if multiple keys share the same email. Addresses remaining Copilot
feedback from PR #1236.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: jason poley <jason.poley@gmail.com>
- signing: verify secret key by fingerprint (not email) in configure_git_signing
- signing: fix unused stderr variable in get_key_info (Ruff F841)
- doctor: validate flow.*.agent.md pattern explicitly in check_agent_files
- doctor: update module docstring to reflect library function
- tests: mock httpx.get in doctor tests to prevent network dependency
- tests: fix agent naming tests to validate flow.*.agent.md convention
- cli: wire gpg_app and doctor command into main Typer app
- docs: remove false telemetry integration claims from GPG guide

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: jason poley <jason.poley@gmail.com>
Copilot AI review requested due to automatic review settings April 5, 2026 22:49
@github-actions github-actions Bot added documentation Improvements or additions to documentation config github source tests labels Apr 5, 2026
@github-actions

github-actions Bot commented Apr 5, 2026

Copy link
Copy Markdown
Contributor

🟢 Security Scan Results (Advisory)

Note: Security scanning is advisory and does not block PR merges.

  • Total Findings: 11
  • Critical: 0
  • High: 0

No critical or high severity issues found.

How to remediate
# View detailed scan results
gh run download 24012232890 -n security-scan-results-1d7a940a0e6f5a2724a055db32b3df4a295f9a83

# Triage findings with AI assistance
specify security triage

# Generate fix suggestions
specify security fix

# Apply fixes and re-scan
flowspec security scan

See the Security tab for detailed findings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds end-user “doctor” diagnostics and agent GPG commit-signing management to the Flowspec CLI, including CLI wiring, tests, and user documentation.

Changes:

  • Introduces a new flowspec gpg sub-app (setup/status/rotate) backed by a GPG key management module.
  • Introduces a new flowspec doctor command with environment/workflow/agent-file checks and formatted output.
  • Adds comprehensive tests and a new guide for GPG signing usage and troubleshooting.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/flowspec_cli/signing.py Implements GPG key generation/storage, git signing configuration, status checks, and key deletion.
src/flowspec_cli/gpg_cli.py Adds Typer subcommands to manage agent signing lifecycle and display status.
src/flowspec_cli/doctor.py Implements health checks (version, python, tools, workflow config, agents, constitution) and rendering.
src/flowspec_cli/__init__.py Wires flowspec gpg sub-app and flowspec doctor command into the main CLI.
tests/test_signing.py Adds unit tests covering signing key generation, git config, status, deletion, and subprocess error handling.
tests/test_doctor.py Adds unit tests for doctor checks and avoids network dependency via mocking.
docs/guides/gpg-signing.md Documents setup/status/rotation, verification, and troubleshooting for agent commit signing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +284 to +286
# Expected pattern: flow.*.agent.md
if not (name.startswith("flow.") and name.endswith(".agent.md")):
non_compliant_files.append(str(agent_file.relative_to(project_root)))

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check_agent_files() currently treats any filename that starts with flow. and ends with .agent.md as compliant. That will incorrectly PASS names like flow.agent.md or flow..agent.md, which do not actually match the documented flow.*.agent.md convention. Consider validating the full structure (e.g., split on . and require ['flow', <non-empty>, 'agent', 'md'] or an equivalent regex).

Copilot uses AI. Check for mistakes.
Comment on lines +139 to +143
if line.startswith("[GNUPG:] KEY_CREATED "):
parts = line.split()
if len(parts) >= 4:
fingerprint = parts[3]
break

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generate_agent_key() stores whatever token follows KEY_CREATED as the fingerprint without validating format/length. Since the rest of the module treats this value as an exact fingerprint (e.g., secret-key verification compares against fpr records), it would be safer to validate that the extracted value is a 40-character hex fingerprint (and normalize case) before persisting it to keyring, otherwise later operations can fail in confusing ways.

Copilot uses AI. Check for mistakes.
Comment on lines +178 to +180
1. Export the agent's public key:
```bash
gpg --armor --export agent@flowspec.local > agent-key.asc

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guide exports the public key by email (gpg --export agent@flowspec.local). After key rotation, multiple keys can share this UID, and --export may include the wrong (or multiple) keys. Recommend exporting by the exact fingerprint shown by flowspec gpg status instead to avoid uploading the wrong key to GitHub.

Suggested change
1. Export the agent's public key:
```bash
gpg --armor --export agent@flowspec.local > agent-key.asc
1. Export the agent's public key using the exact fingerprint shown by `flowspec gpg status`:
```bash
# Replace A1B2C3D4E5F6A7B8C9D0E1F2A3B4C5D6E7F8A9B0 with the fingerprint from `flowspec gpg status`
gpg --armor --export A1B2C3D4E5F6A7B8C9D0E1F2A3B4C5D6E7F8A9B0 > agent-key.asc

Copilot uses AI. Check for mistakes.
Comment on lines +337 to +344
# Export ASCII-armored public key
gpg --armor --export agent@flowspec.local > agent-public-key.asc

# Export binary public key
gpg --export agent@flowspec.local > agent-public-key.gpg

# Show fingerprint
gpg --fingerprint agent@flowspec.local

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, the “Export Public Key for Distribution” examples use the email as the selector. To avoid ambiguity when multiple agent keys exist, prefer using the specific fingerprint (or at least show how to obtain it first) for gpg --export / gpg --fingerprint commands.

Suggested change
# Export ASCII-armored public key
gpg --armor --export agent@flowspec.local > agent-public-key.asc
# Export binary public key
gpg --export agent@flowspec.local > agent-public-key.gpg
# Show fingerprint
gpg --fingerprint agent@flowspec.local
# First, find the exact fingerprint for the agent key
gpg --fingerprint agent@flowspec.local
# Use the specific fingerprint from the output below in export commands
# Example fingerprint: A1B2C3D4E5F6A7B8C9D0E1F2A3B4C5D6E7F8A9B0
# Export ASCII-armored public key
gpg --armor --export A1B2C3D4E5F6A7B8C9D0E1F2A3B4C5D6E7F8A9B0 > agent-public-key.asc
# Export binary public key
gpg --export A1B2C3D4E5F6A7B8C9D0E1F2A3B4C5D6E7F8A9B0 > agent-public-key.gpg
# Show fingerprint for that exact key
gpg --fingerprint A1B2C3D4E5F6A7B8C9D0E1F2A3B4C5D6E7F8A9B0

Copilot uses AI. Check for mistakes.
Comment on lines +350 to +352
# Delete secret and public key
gpg --delete-secret-and-public-key agent@flowspec.local

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual key deletion by email (gpg --delete-secret-and-public-key agent@flowspec.local) can be ambiguous if multiple keys exist with that UID (e.g., after rotations). Prefer instructing deletion by fingerprint to ensure the intended key is removed.

Suggested change
# Delete secret and public key
gpg --delete-secret-and-public-key agent@flowspec.local
# Look up the key fingerprint for this UID
gpg --fingerprint agent@flowspec.local
# Delete secret and public key by fingerprint
gpg --delete-secret-and-public-key A1B2C3D4E5F6A7B8C9D0E1F2A3B4C5D6E7F8A9B0

Copilot uses AI. Check for mistakes.
jpoley added a commit that referenced this pull request Apr 5, 2026
Add doctor health-check command and GPG agent signing module with
CLI wiring, tests, and documentation. All Copilot review findings
from PRs #1236, #1237, and #1238 are resolved.

Doctor checks: CLI version, Python version, tool availability,
workflow config, agent file naming (flow.*.agent.md), constitution.

GPG signing: key generation via --status-fd, fingerprint validation
(40-char hex), secret key verification by fingerprint, git config,
key rotation, and keyring storage.

Closes #1221 #1222 #1223 #1224 #1225 #1226

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: jason poley <jason.poley@gmail.com>
@jpoley jpoley closed this Apr 5, 2026
jpoley added a commit that referenced this pull request Apr 5, 2026
…ixes

Add `flowspec doctor` health-check command and `flowspec gpg` agent
signing module, fully addressing all Copilot findings from PRs #1236,
#1237, #1238, and #1239 plus a thorough independent security review.

Key design decisions in signing module:
- Fingerprint normalization centralized in get_key_fingerprint()
  (strip, uppercase, 40-char hex validation, logging on failure)
- Key generation uses --status-fd for reliable fingerprint extraction
- Secret key verification uses exact fingerprint match (case-insensitive)
- All test mocks use realistic 40-char hex fingerprints via TEST_FP constant

Doctor module improvements:
- httpx imported at module level (coding standard compliance)
- Strict flow.<phase>.agent.md validation (rejects flow.agent.md)
- UTF-8 encoding on all file I/O
- Version check distinguishes "confirmed current" from "could not check"

Documentation quality:
- All GPG commands use fingerprint (not email) to avoid ambiguity
- Broken related-doc links replaced with anchor links
- Security note about ~/.gnupg file permissions added
- Manual deletion warns about keyring cleanup

Closes #1221 #1222 #1223 #1224 #1225 #1226

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: jason poley <jason.poley@gmail.com>
jpoley added a commit that referenced this pull request Apr 18, 2026
…ixes

Add `flowspec doctor` health-check command and `flowspec gpg` agent
signing module, fully addressing all Copilot findings from PRs #1236,
#1237, #1238, and #1239 plus a thorough independent security review.

Key design decisions in signing module:
- Fingerprint normalization centralized in get_key_fingerprint()
  (strip, uppercase, 40-char hex validation, logging on failure)
- Key generation uses --status-fd for reliable fingerprint extraction
- Secret key verification uses exact fingerprint match (case-insensitive)
- All test mocks use realistic 40-char hex fingerprints via TEST_FP constant

Doctor module improvements:
- httpx imported at module level (coding standard compliance)
- Strict flow.<phase>.agent.md validation (rejects flow.agent.md)
- UTF-8 encoding on all file I/O
- Version check distinguishes "confirmed current" from "could not check"

Documentation quality:
- All GPG commands use fingerprint (not email) to avoid ambiguity
- Broken related-doc links replaced with anchor links
- Security note about ~/.gnupg file permissions added
- Manual deletion warns about keyring cleanup

Closes #1221 #1222 #1223 #1224 #1225 #1226

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: jason poley <jason.poley@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

config documentation Improvements or additions to documentation github source tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: flowspec doctor — setup health check and diagnostics

2 participants