Skip to content

fix: simplify tree output defaults - #173

Merged
danielewood merged 5 commits into
mainfrom
fix/post-merge-trust-anchor-followups
Mar 11, 2026
Merged

fix: simplify tree output defaults#173
danielewood merged 5 commits into
mainfrom
fix/post-merge-trust-anchor-followups

Conversation

@danielewood

Copy link
Copy Markdown
Collaborator

Summary

  • make certkit tree default to a command-focused text view and add --flags/--inherited opt-ins for text-mode flag detail
  • keep tree --json as the detailed machine-readable surface and update docs/tests accordingly
  • restore duplicate-SKI validation to use the store's existing latest-cert fallback

Validation

  • pre-commit run --all-files
  • go test ./cmd/certkit -run TestTreeCommand -count=1
  • go test ./internal/certstore -run TestRunValidation -count=1

Copilot AI review requested due to automatic review settings March 11, 2026 18:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts certkit tree to default to a command-focused text view (with opt-in text flag detail), and updates validation to handle duplicate-SKI renewal sets by using the store’s existing “latest NotAfter” selection.

Changes:

  • Update certkit tree text output defaults; add --flags / --inherited for opt-in flag detail while keeping --json as the detailed machine-readable output.
  • Change RunValidation to use MemStore.GetCert (latest-expiring cert per SKI) instead of erroring on duplicate SKIs; update/extend tests accordingly.
  • Refresh docs/changelog/architecture notes to match the new tree behavior and validation semantics.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/certstore/validate.go Switch SKI lookup to GetCert (latest NotAfter) to avoid duplicate-SKI ambiguity errors
internal/certstore/validate_test.go Update regression coverage to assert the later renewal cert is validated
cmd/certkit/tree.go Add --flags/--inherited opt-ins; default text output becomes commands-only
cmd/certkit/readonly_commands_test.go Include new tree flag globals in snapshot/restore to keep tests isolated
cmd/certkit/cli_semantics_test.go Update tree command semantics tests for new default/opt-in behavior
README.md Adjust CLI command summary to match new tree default
EXAMPLES.md Document opt-in flag detail for tree text output
CHANGELOG.md Record user-visible behavior change under Unreleased
.claude/docs/architecture.md Update CLI architecture doc for new tree behavior
RALPH.md Add entries documenting the fixed issues and rationale

You can also share your feedback on Copilot code review. Take the survey.

Comment thread cmd/certkit/tree.go Outdated
Comment thread cmd/certkit/cli_semantics_test.go
@danielewood
danielewood force-pushed the fix/post-merge-trust-anchor-followups branch from 6f516bc to 3eb10bf Compare March 11, 2026 18:45
Copilot AI review requested due to automatic review settings March 11, 2026 18:51

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.


You can also share your feedback on Copilot code review. Take the survey.

Comment thread internal/certstore/validate_test.go
Comment thread README.md Outdated
@danielewood
danielewood merged commit d219442 into main Mar 11, 2026
15 checks passed
@danielewood
danielewood deleted the fix/post-merge-trust-anchor-followups branch March 11, 2026 23:39
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.

2 participants