Skip to content
4 changes: 3 additions & 1 deletion .claude/skills/new-command/references/archetype-attest.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,9 @@ Canonical example: `cmd/kosli/attestCustom.go` — read it in full and adapt.
- `RequireFlags(cmd, []string{"flow", "trail", "name", ...})` for type-specific required flags.

**`RunE`**
- Capture `o.repoURLExplicit = cmd.Flags().Changed("repo-url")` before delegating.
- Capture `o.repoURLExplicit = cmd.Flags().Changed("repo-url")` and `o.repoNameExplicit = cmd.Flags().Changed("repository")` before delegating.
- Do not capture `--commit` or `--repo-root`: `addAttestationFlags` keeps the flag set on the options and `CommonAttestationOptions.run` reads `Changed` from it.
- If the command cannot work without the commit (as `attest pullrequest *` and `attest jira` cannot), set `o.commitRequiredFor = "<what needs it>"` in `run` before delegating.

**`run` method**
- Build the URL: `url.JoinPath(global.Host, "api/v2/attestations", global.Org, o.flowName, "trail", o.trailName, "<type-slug>")`.
Expand Down
1 change: 1 addition & 0 deletions cmd/kosli/attestJira.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ func (o *attestJiraOptions) run(args []string) error {
return err
}

o.commitRequiredFor = "search for Jira issue keys"
err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload)
if err != nil {
return err
Expand Down
78 changes: 71 additions & 7 deletions cmd/kosli/attestation.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/kosli-dev/cli/internal/gitview"
"github.com/kosli-dev/cli/internal/requests"
"github.com/spf13/pflag"
)

const commitDescription = `You can optionally associate the attestation to a git commit using ^--commit^ (requires access to a git repo).
Expand Down Expand Up @@ -56,6 +57,11 @@ type CommonAttestationOptions struct {
repoProvider string
repoURLExplicit bool
repoNameExplicit bool
// flags lets run tell a passed flag from a defaulted one.
flags *pflag.FlagSet
// commitRequiredFor completes "the commit is required to ..."; empty when
// commit info is optional.
commitRequiredFor string
}

func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestationPayload) error {
Expand All @@ -80,15 +86,18 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation
}

if o.commitSHA != "" {
gv, err := gitview.New(o.srcRepoRoot)
payload.Commit, err = commitInfoRequest{
repoRoot: o.srcRepoRoot,
sha: o.commitSHA,
redacted: o.redactedCommitInfo,
flags: o.flags,
requiredFor: o.commitRequiredFor,
}.resolve()
if err != nil {
return fmt.Errorf("failed to get commit info. %s", err)
return err
}
commitInfo, err := gv.GetCommitInfoFromCommitSHA(o.commitSHA, false, o.redactedCommitInfo)
if err != nil {
return fmt.Errorf("failed to get commit info. %s", err)
}
payload.Commit = &commitInfo.BasicCommitInfo
} else if o.commitRequiredFor != "" {
return fmt.Errorf("no commit info is available, and the commit is required to %s. Pass --commit", o.commitRequiredFor)
}

payload.GitRepoInfo, err = getGitRepoInfoFromEnvironment()
Expand Down Expand Up @@ -117,6 +126,61 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation
return err
}

type commitInfoRequest struct {
repoRoot string
sha string
redacted []string
flags *pflag.FlagSet
requiredFor string
}

// lookup reads the commit info from the repository at repoRoot, or returns
// the error from opening it or resolving sha within it.
func (r commitInfoRequest) lookup() (*gitview.CommitInfo, error) {
gv, err := gitview.New(r.repoRoot)
if err != nil {
return nil, err
}
return gv.GetCommitInfoFromCommitSHA(r.sha, false, r.redacted)
}

func (r commitInfoRequest) commitExplicit() bool {
return r.flags != nil && r.flags.Changed("commit")
}

// repoRootExplicit is true only when --repo-root carries a value other than
// its "." default: bindFlags marks a config or env value as Changed even when
// it equals the default, and "." itself asks for nothing.
func (r commitInfoRequest) repoRootExplicit() bool {
return r.flags != nil && r.flags.Changed("repo-root") && r.repoRoot != "."
}

// resolve returns nil, nil when the lookup fails but nothing was asked for
// explicitly and the command can do without the commit: a CI-defaulted
// --commit must not fail a job with no checked-out repository
// (kosli-dev/server#6094). An unresolvable commit, as in a shallow clone,
// deliberately takes the same route.
func (r commitInfoRequest) resolve() (*gitview.BasicCommitInfo, error) {
commitInfo, err := r.lookup()
if err == nil {
return &commitInfo.BasicCommitInfo, nil
}

describedCommit := "--commit " + r.sha
if !r.commitExplicit() {
describedCommit += " (defaulted from the CI environment)"
}
switch {
case r.requiredFor != "":
return nil, fmt.Errorf("failed to get commit info for %s: %s. The commit is required to %s, so point --repo-root at a repository containing it", describedCommit, err, r.requiredFor)
case r.commitExplicit() || r.repoRootExplicit():
return nil, fmt.Errorf("failed to get commit info for %s: %s. Point --repo-root at a repository containing it", describedCommit, err)
}
logger.Warn("proceeding without commit info: %s could not be read: %s.", describedCommit, err)
logger.Warn("Kosli binds an attestation reported before its artifact through this commit, so point --repo-root at a repository containing it if that binding is needed.")
return nil, nil
}

// mergeGitRepoInfo applies flag overrides onto base (which may be nil) and
// returns nil if ID, Name, or URL is still empty after merging, so that the
// field is omitted from the JSON payload.
Expand Down
191 changes: 191 additions & 0 deletions cmd/kosli/attestationCommitInfo_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
package main

import (
"os"
"testing"

"github.com/go-git/go-git/v5"
"github.com/stretchr/testify/suite"
)

// AttestationCommitInfoTestSuite covers how a failed commit lookup is reported
// (kosli-dev/server#6094, kosli-dev/server#5615). The CI default exists only
// while KOSLI_TESTS is unset, because DefaultValue returns "" under it, so inCI
// unsets it around the command run.
type AttestationCommitInfoTestSuite struct {
suite.Suite
headHash string
defaultKosliArguments string
}

const (
commitInfoTestFingerprint = "7509e5bda0c762d2bac7f90d758b5b2263fa01ccbc542ab5e3df163be08e6ca9"
// A well-formed SHA that is not in this repository, as in a shallow clone.
absentSHA = "0d4c1e1b7f5c2a9e8b3d6f0a1c4e7b2d5a8f3c60"
)

func (suite *AttestationCommitInfoTestSuite) SetupTest() {
repo, err := git.PlainOpen("../..")
suite.Require().NoError(err)
head, err := repo.Head()
suite.Require().NoError(err)
suite.headHash = head.Hash().String()

global = &GlobalOpts{
ApiToken: "DRY_RUN",
Org: "test-org",
Host: "http://localhost:8001",
DryRun: true,
}
suite.defaultKosliArguments = " --dry-run --host http://localhost:8001 --org test-org --api-token DRY_RUN"
}

// inCI runs f with the CI defaults live, as in a GitHub Actions job whose
// GITHUB_SHA is sha.
func (suite *AttestationCommitInfoTestSuite) inCI(sha string, f func()) {
if value, set := os.LookupEnv("KOSLI_TESTS"); set {
suite.Require().NoError(os.Unsetenv("KOSLI_TESTS"))
defer func() { suite.Require().NoError(os.Setenv("KOSLI_TESTS", value)) }()
}
suite.T().Setenv("GITHUB_RUN_NUMBER", "1")
suite.T().Setenv("GITHUB_SHA", sha)
f()
}

func (suite *AttestationCommitInfoTestSuite) attestGeneric(extraFlags string) string {
return "attest generic --fingerprint " + commitInfoTestFingerprint + " --name foo --flow f --trail t " + extraFlags + suite.defaultKosliArguments
}

func (suite *AttestationCommitInfoTestSuite) beginTrail(extraFlags string) string {
return "begin trail t --flow f " + extraFlags + suite.defaultKosliArguments
}

func (suite *AttestationCommitInfoTestSuite) TestCIDefaultedCommitWithoutRepositoryWarns() {
// --repo-root defaults to ".", so run where there is no repository at all.
suite.T().Chdir(suite.T().TempDir())
suite.inCI(suite.headHash, func() {
for _, cmd := range []string{suite.attestGeneric(""), suite.beginTrail("")} {
_, out, _, _, err := executeCommandC(cmd)
suite.Require().NoError(err, cmd)
suite.Contains(out, "[warning] proceeding without commit info", cmd)
suite.Contains(out, "--commit "+suite.headHash+" (defaulted from the CI environment)", cmd)
suite.Contains(out, "repository does not exist", cmd)
suite.Contains(out, "THIS IS A DRY-RUN", cmd)
suite.NotContains(out, "git_commit_info", cmd)
}
})
}

func (suite *AttestationCommitInfoTestSuite) TestCIDefaultedCommitNotInRepositoryWarns() {
suite.T().Chdir("../..")
suite.inCI(absentSHA, func() {
_, out, _, _, err := executeCommandC(suite.attestGeneric(""))
suite.Require().NoError(err)
suite.Contains(out, "[warning] proceeding without commit info")
suite.Contains(out, "--commit "+absentSHA+" (defaulted from the CI environment)")
suite.NotContains(out, "git_commit_info")
})
}

func (suite *AttestationCommitInfoTestSuite) TestCommitFromEnvVarIsExplicit() {
suite.T().Chdir(suite.T().TempDir())
suite.T().Setenv("KOSLI_COMMIT", suite.headHash)
_, out, _, _, err := executeCommandC(suite.attestGeneric(""))
suite.Require().Error(err)
suite.Contains(err.Error(), "failed to get commit info for --commit "+suite.headHash+":")
suite.NotContains(err.Error(), "defaulted from the CI environment")
suite.NotContains(out, "[warning] proceeding without commit info")
}

// A --repo-root set via the environment or config to its own "." default
// must not count as explicit: bindFlags marks the flag Changed regardless of
// whether the applied value differs from the default.
func (suite *AttestationCommitInfoTestSuite) TestRepoRootFromEnvVarAtDefaultValueStillWarns() {
suite.T().Chdir(suite.T().TempDir())
suite.T().Setenv("KOSLI_REPO_ROOT", ".")
suite.inCI(suite.headHash, func() {
_, out, _, _, err := executeCommandC(suite.attestGeneric(""))
suite.Require().NoError(err)
suite.Contains(out, "[warning] proceeding without commit info")
})
}

func (suite *AttestationCommitInfoTestSuite) TestCIDefaultedCommitWithExplicitRepoRootFails() {
suite.inCI(suite.headHash, func() {
for _, cmd := range []string{suite.attestGeneric("--repo-root testdata"), suite.beginTrail("--repo-root testdata")} {
_, out, _, _, err := executeCommandC(cmd)
suite.Require().Error(err, cmd)
suite.Contains(err.Error(), "failed to get commit info for --commit "+suite.headHash+" (defaulted from the CI environment)", cmd)
suite.Contains(err.Error(), "repository does not exist", cmd)
suite.Contains(err.Error(), "Point --repo-root at a repository containing it", cmd)
suite.NotContains(out, "[warning] proceeding without commit info", cmd)
}
})
}

func (suite *AttestationCommitInfoTestSuite) TestExplicitCommitWithoutRepositoryFails() {
tests := []cmdTestCase{
{
wantError: true,
name: "attest generic: an explicit --commit fails when --repo-root has no repository",
cmd: suite.attestGeneric("--commit " + suite.headHash + " --repo-root testdata"),
goldenRegex: "Error: failed to get commit info for --commit " + suite.headHash + ": .*repository does not exist\\. Point --repo-root at a repository containing it\n",
},
{
wantError: true,
name: "begin trail: an explicit --commit fails when --repo-root has no repository",
cmd: suite.beginTrail("--commit " + suite.headHash + " --repo-root testdata"),
goldenRegex: "Error: failed to get commit info for --commit " + suite.headHash + ": .*repository does not exist\\. Point --repo-root at a repository containing it\n",
},
}
runTestCmd(suite.T(), tests)
}

func (suite *AttestationCommitInfoTestSuite) TestExplicitCommitIsAttached() {
tests := []cmdTestCase{
{
name: "attest generic: an explicit --commit is resolved and sent",
cmd: suite.attestGeneric("--commit " + suite.headHash + " --repo-root ../.."),
goldenRegex: `(?s)"git_commit_info": \{.*"sha1": "` + suite.headHash + `"`,
},
{
name: "begin trail: an explicit --commit is resolved and sent",
cmd: suite.beginTrail("--commit " + suite.headHash + " --repo-root ../.."),
goldenRegex: `(?s)"git_commit_info": \{.*"sha1": "` + suite.headHash + `"`,
},
}
runTestCmd(suite.T(), tests)
}

func (suite *AttestationCommitInfoTestSuite) TestCommandsNeedingTheCommitFail() {
suite.T().Chdir(suite.T().TempDir())
suite.inCI(suite.headHash, func() {
for _, tc := range []struct{ cmd, need string }{
{"attest pullrequest github --name foo --flow f --trail t --github-token tok --github-org o --repository r" + suite.defaultKosliArguments, "find pull requests"},
{"attest jira --name foo --flow f --trail t --jira-base-url https://x.atlassian.net --jira-username u --jira-api-token tok" + suite.defaultKosliArguments, "search for Jira issue keys"},
} {
_, out, _, _, err := executeCommandC(tc.cmd)
suite.Require().Error(err, tc.cmd)
suite.Contains(err.Error(), "failed to get commit info for --commit "+suite.headHash+" (defaulted from the CI environment)", tc.cmd)
suite.Contains(err.Error(), "The commit is required to "+tc.need, tc.cmd)
suite.NotContains(out, "[warning] proceeding without commit info", tc.cmd)
}
})
}
Comment thread
claude[bot] marked this conversation as resolved.

// Unreachable from the CLI while RequireFlags keeps --commit non-empty; guards
// the nil dereference that would follow if that changed.
func (suite *AttestationCommitInfoTestSuite) TestCommandsNeedingTheCommitFailWithoutOne() {
o := &CommonAttestationOptions{
fingerprintOptions: &fingerprintOptions{},
attestationNameTemplate: "foo",
commitRequiredFor: "find pull requests",
}
err := o.run([]string{}, &CommonAttestationPayload{})
suite.Require().Error(err)
suite.Contains(err.Error(), "the commit is required to find pull requests")
}

func TestAttestationCommitInfoTestSuite(t *testing.T) {
suite.Run(t, new(AttestationCommitInfoTestSuite))
}
17 changes: 11 additions & 6 deletions cmd/kosli/beginTrail.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/kosli-dev/cli/internal/gitview"
"github.com/kosli-dev/cli/internal/requests"
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)

const beginTrailShortDesc = `Begin or update a Kosli flow trail.`
Expand Down Expand Up @@ -50,6 +51,9 @@ type beginTrailOptions struct {
repoURL string
repoProvider string
repoNameExplicit bool
// flags lets commitInfoRequest tell a passed --commit/--repo-root from a
// defaulted one.
flags *pflag.FlagSet
}

type TrailPayload struct {
Expand Down Expand Up @@ -85,6 +89,7 @@ func newBeginTrailCmd(out io.Writer) *cobra.Command {
},
RunE: func(cmd *cobra.Command, args []string) error {
o.repoNameExplicit = cmd.Flags().Changed("repository")
o.flags = cmd.Flags()
return o.run(args)
},
}
Expand Down Expand Up @@ -128,15 +133,15 @@ func (o *beginTrailOptions) run(args []string) error {
}

if o.commitSHA != "" {
gv, err := gitview.New(o.srcRepoRoot)
o.payload.Commit, err = commitInfoRequest{
repoRoot: o.srcRepoRoot,
sha: o.commitSHA,
redacted: o.redactedCommitInfo,
flags: o.flags,
}.resolve()
if err != nil {
return err
}
commitInfo, err := gv.GetCommitInfoFromCommitSHA(o.commitSHA, false, o.redactedCommitInfo)
if err != nil {
return err
}
o.payload.Commit = &commitInfo.BasicCommitInfo
}

base, err := getGitRepoInfoFromEnvironment()
Expand Down
1 change: 1 addition & 0 deletions cmd/kosli/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func addListFlags(cmd *cobra.Command, o *listOptions, customPageLimit ...int) {
}

func addAttestationFlags(cmd *cobra.Command, o *CommonAttestationOptions, payload *CommonAttestationPayload, ci string) {
o.flags = cmd.Flags()
commitFlagDesc := attestationCommitFlag
if _, ok := cmd.Annotations["pr"]; ok {
commitFlagDesc = "the git merge commit to be checked for associated pull requests."
Expand Down
1 change: 1 addition & 0 deletions cmd/kosli/pullrequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func (o *attestPROptions) run(args []string) error {
return err
}

o.commitRequiredFor = "find pull requests"
err = o.CommonAttestationOptions.run(args, o.payload.CommonAttestationPayload)
if err != nil {
return err
Expand Down
Loading
Loading