feat: flowspec doctor, status, GPG signing, template registry, event system, constitution tests#1236
feat: flowspec doctor, status, GPG signing, template registry, event system, constitution tests#1236jpoley wants to merge 7 commits into
Conversation
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>
🟢 Security Scan Results (Advisory)
No critical or high severity issues found. How to remediate# View detailed scan results
gh run download 23973485541 -n security-scan-results-a4d70dbca0e699cd8e77b525975083151a66e94a
# Triage findings with AI assistance
specify security triage
# Generate fix suggestions
specify security fix
# Apply fixes and re-scan
flowspec security scanSee the Security tab for detailed findings. |
There was a problem hiding this comment.
Pull request overview
This PR adds two user-facing capabilities to the Flowspec CLI: a flowspec doctor health-check command and agent GPG commit-signing support (library + CLI), along with tests and documentation.
Changes:
- Introduce
flowspec doctorchecks (version, Python, tools, workflow config, agent naming, constitution) with Rich-formatted output. - Add GPG signing key management (
generate,delete/rotate, git local config) and aflowspec gpg {setup,status,rotate}Typer subcommand. - Add docs and unit tests covering the new doctor/signing behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
src/flowspec_cli/doctor.py |
Implements doctor checks, result model, and formatted display. |
tests/test_doctor.py |
Unit tests for doctor checks and orchestration. |
src/flowspec_cli/signing.py |
Core GPG + git signing helpers with keyring integration. |
src/flowspec_cli/gpg_cli.py |
Typer CLI wrapper for setup/status/rotate commands. |
tests/test_signing.py |
Unit tests for signing/keyring/git/GPG helper behavior. |
docs/guides/gpg-signing.md |
End-user guide for setup, verification, and rotation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from pathlib import Path | ||
| from unittest.mock import MagicMock, patch |
There was a problem hiding this comment.
Unused imports (Path, MagicMock) will trigger Ruff F401 in CI. Remove them or use them in the tests.
| from pathlib import Path | |
| from unittest.mock import MagicMock, patch | |
| from unittest.mock import patch |
| from unittest.mock import MagicMock, patch | ||
| import yaml |
There was a problem hiding this comment.
Unused imports (sys, Path) will trigger Ruff F401 in CI. They aren’t needed here because patching flowspec_cli.doctor.sys.version_info doesn’t require importing sys in the test module.
| from unittest.mock import MagicMock, patch | |
| import yaml |
| # Check if key already exists | ||
| if key_exists(): | ||
| fingerprint = get_key_fingerprint() | ||
| if fingerprint: | ||
| return fingerprint | ||
| # Key record exists but fingerprint missing - regenerate |
There was a problem hiding this comment.
key_exists() is implemented as get_key_fingerprint() is not None, so the branch that handles “key record exists but fingerprint missing” is unreachable. Consider collapsing this to a single fingerprint = get_key_fingerprint() check (or redefining key_exists() to actually verify the key exists in the GPG keychain).
| # Check if key already exists | |
| if key_exists(): | |
| fingerprint = get_key_fingerprint() | |
| if fingerprint: | |
| return fingerprint | |
| # Key record exists but fingerprint missing - regenerate | |
| # Check if a stored fingerprint already exists | |
| fingerprint = get_key_fingerprint() | |
| if fingerprint: | |
| return fingerprint |
| ["--batch", "--generate-key"], | ||
| input_data=key_params | ||
| ) | ||
|
|
||
| if returncode != 0: | ||
| raise GPGKeyGenerationError(f"Failed to generate GPG key: {stderr}") | ||
|
|
||
| # Extract fingerprint from the newly created key | ||
| stdout, stderr, returncode = _run_gpg_command([ | ||
| "--list-keys", | ||
| "--with-colons", | ||
| GPG_KEY_EMAIL | ||
| ]) | ||
|
|
||
| if returncode != 0: | ||
| raise GPGKeyGenerationError(f"Failed to list GPG keys: {stderr}") | ||
|
|
||
| # Parse fingerprint from colon-separated output | ||
| # Format: fpr:::::::::FINGERPRINT: | ||
| fingerprint = None | ||
| for line in stdout.splitlines(): | ||
| if line.startswith("fpr:"): | ||
| parts = line.split(":") | ||
| if len(parts) >= 10: | ||
| fingerprint = parts[9] |
There was a problem hiding this comment.
Fingerprint detection is based on gpg --list-keys ... {GPG_KEY_EMAIL} and then taking the first fpr: line. If the user already has any key with the same email/UID (or multiple matching keys), this can select the wrong fingerprint and store/configure an unrelated key. Prefer deriving the fingerprint from the key-creation output (e.g., GnuPG status output) or ensure the lookup uniquely identifies the newly-created key (unique UID, select newest by creation time, etc.).
| ["--batch", "--generate-key"], | |
| input_data=key_params | |
| ) | |
| if returncode != 0: | |
| raise GPGKeyGenerationError(f"Failed to generate GPG key: {stderr}") | |
| # Extract fingerprint from the newly created key | |
| stdout, stderr, returncode = _run_gpg_command([ | |
| "--list-keys", | |
| "--with-colons", | |
| GPG_KEY_EMAIL | |
| ]) | |
| if returncode != 0: | |
| raise GPGKeyGenerationError(f"Failed to list GPG keys: {stderr}") | |
| # Parse fingerprint from colon-separated output | |
| # Format: fpr:::::::::FINGERPRINT: | |
| fingerprint = None | |
| for line in stdout.splitlines(): | |
| if line.startswith("fpr:"): | |
| parts = line.split(":") | |
| if len(parts) >= 10: | |
| fingerprint = parts[9] | |
| ["--batch", "--status-fd", "1", "--generate-key"], | |
| input_data=key_params | |
| ) | |
| if returncode != 0: | |
| raise GPGKeyGenerationError(f"Failed to generate GPG key: {stderr}") | |
| # Extract fingerprint from the key-creation status output. | |
| # Expected format: [GNUPG:] KEY_CREATED <type> <fingerprint> | |
| fingerprint = None | |
| for line in stdout.splitlines(): | |
| if line.startswith("[GNUPG:] KEY_CREATED "): | |
| parts = line.split() | |
| if len(parts) >= 4: | |
| fingerprint = parts[3] |
| fingerprint = get_key_fingerprint() | ||
| if not fingerprint: | ||
| raise GPGError("No agent GPG key found. Run generate_agent_key() first.") | ||
|
|
||
| cwd = project_root if project_root else Path.cwd() | ||
|
|
||
| # Check if we're in a git repository | ||
| _, _, returncode = _run_git_command(["rev-parse", "--git-dir"], cwd=cwd) | ||
| if returncode != 0: | ||
| raise GPGConfigurationError(f"Not a git repository: {cwd}") |
There was a problem hiding this comment.
configure_git_signing() trusts the fingerprint stored in keyring without verifying the corresponding secret key is still present in the GPG keychain. If the key was deleted externally, this will configure git to sign with a non-existent key and failures will only surface later during git commit. Consider validating via gpg --list-secret-keys --with-colons <fingerprint> (and raising a clear error / regenerating) before writing git config.
| >>> from flowspec_cli.doctor import run_doctor | ||
| >>> results = run_doctor(fix=False) |
There was a problem hiding this comment.
The module docstring example implies run_doctor() returns an empty list when all checks pass (if not issues:), but run_doctor() currently always returns the full list of CheckResult (including PASS). Either adjust the example/wording (e.g., filter to FAIL/WARN) or change run_doctor() to return only non-PASS results to match the example.
| >>> from flowspec_cli.doctor import run_doctor | |
| >>> results = run_doctor(fix=False) | |
| >>> from flowspec_cli.doctor import CheckStatus, run_doctor | |
| >>> results = run_doctor(fix=False) | |
| >>> issues = [result for result in results if result.status != CheckStatus.PASS] |
| status=CheckStatus.FAIL, | ||
| message="Could not determine flowspec version", |
There was a problem hiding this comment.
Treating an undetectable flowspec version as FAIL may be overly strict (e.g., running from a source checkout or environments where package metadata isn’t available). Also, the tests expect the message to potentially be "unknown", but the current failure message is different. Consider downgrading this to WARN with message="unknown" (or similar) to keep doctor useful without hard-failing on metadata lookup issues.
| status=CheckStatus.FAIL, | |
| message="Could not determine flowspec version", | |
| status=CheckStatus.WARN, | |
| message="unknown", |
| # ✓ GPG key generated | ||
| # ✓ Git configured for signing | ||
| # | ||
| # Fingerprint: A1B2C3D4E5F6G7H8I9J0K1L2M3N4O5P6Q7R8S9T0 |
There was a problem hiding this comment.
The example “Fingerprint” values include non-hex characters (e.g., I, J, K, …). Real GPG fingerprints are hex (0-9, A-F), so using a hex-only example avoids confusion when users compare against gpg --fingerprint output.
| # Fingerprint: A1B2C3D4E5F6G7H8I9J0K1L2M3N4O5P6Q7R8S9T0 | |
| # Fingerprint: A1B2C3D4E5F60718293A4B5C6D7E8F9012345678 |
…ification, unused imports
|
need to close fix all and make a new PR |
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>
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>
…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>
…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>
Closes #1221 #1222 #1224 #1225 #1226 #1223
Combined PR for all 6 features. Addresses all Copilot feedback from previous rounds.