fix(attest): treat repo-root at its default as unset, and unify explicit-flag detection

A config or env value for --repo-root marked the flag Changed even when it
equaled the "." default, because bindFlags applies it with Flags().Set()
regardless of value. That made KOSLI_REPO_ROOT=. or a config-file repo-root:
"." hard-fail a CI-defaulted commit instead of warning, contradicting the
help text. repoRootExplicit now also requires the value to differ from ".".

begin trail kept its own commitExplicit/repoRootExplicit booleans because it
doesn't go through addAttestationFlags. commitInfoRequest now takes the flag
set directly and derives both from it, so there is one place, not two, that
knows how "explicit" is decided.

Adds coverage for the KOSLI_COMMIT env route (explicit, hard-fails) and the
KOSLI_REPO_ROOT=. case (still warns), and converts a map-based test loop to a
slice for deterministic failure output.

Addresses review feedback on #1202.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014HZK6c41JmmUMAB71ZsDpX

Author: claude

cmd/kosli/attestation.go

@@ -64,10 +64,6 @@ type CommonAttestationOptions struct {
 	commitRequiredFor string
 }
 
-func (o *CommonAttestationOptions) flagChanged(name string) bool {
-	return o.flags != nil && o.flags.Changed(name)
-}
-
 func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestationPayload) error {
 	var err error
 
@@ -91,12 +87,11 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation
 
 	if o.commitSHA != "" {
 		payload.Commit, err = commitInfoRequest{
-			repoRoot:         o.srcRepoRoot,
-			sha:              o.commitSHA,
-			redacted:         o.redactedCommitInfo,
-			commitExplicit:   o.flagChanged("commit"),
-			repoRootExplicit: o.flagChanged("repo-root"),
-			requiredFor:      o.commitRequiredFor,
+			repoRoot:    o.srcRepoRoot,
+			sha:         o.commitSHA,
+			redacted:    o.redactedCommitInfo,
+			flags:       o.flags,
+			requiredFor: o.commitRequiredFor,
 		}.resolve()
 		if err != nil {
 			return err
@@ -132,13 +127,32 @@ func (o *CommonAttestationOptions) run(args []string, payload *CommonAttestation
 }
 
 type commitInfoRequest struct {
-	repoRoot string
-	sha      string
-	redacted []string
-	// commitExplicit is false when --commit was defaulted from the CI environment.
-	commitExplicit   bool
-	repoRootExplicit bool
-	requiredFor      string
+	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
@@ -147,26 +161,23 @@ type commitInfoRequest struct {
 // (kosli-dev/server#6094). An unresolvable commit, as in a shallow clone,
 // deliberately takes the same route.
 func (r commitInfoRequest) resolve() (*gitview.BasicCommitInfo, error) {
-	gv, err := gitview.New(r.repoRoot)
+	commitInfo, err := r.lookup()
 	if err == nil {
-		var commitInfo *gitview.CommitInfo
-		commitInfo, err = gv.GetCommitInfoFromCommitSHA(r.sha, false, r.redacted)
-		if err == nil {
-			return &commitInfo.BasicCommitInfo, nil
-		}
+		return &commitInfo.BasicCommitInfo, nil
 	}
 
-	origin := "--commit " + r.sha
-	if !r.commitExplicit {
-		origin += " (defaulted from the CI environment)"
+	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", origin, 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", origin, err)
+		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. 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.", origin, 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
 }
 

cmd/kosli/attestationCommitInfo_test.go

@@ -87,6 +87,29 @@ func (suite *AttestationCommitInfoTestSuite) TestCIDefaultedCommitNotInRepositor
 	})
 }
 
+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")} {
@@ -137,15 +160,15 @@ func (suite *AttestationCommitInfoTestSuite) TestExplicitCommitIsAttached() {
 func (suite *AttestationCommitInfoTestSuite) TestCommandsNeedingTheCommitFail() {
 	suite.T().Chdir(suite.T().TempDir())
 	suite.inCI(suite.headHash, func() {
-		for cmd, need := range map[string]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",
+		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(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(), "The commit is required to "+need, cmd)
-			suite.NotContains(out, "[warning] proceeding without commit info", cmd)
+			_, 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)
 		}
 	})
 }

cmd/kosli/beginTrail.go

@@ -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.`
@@ -50,8 +51,9 @@ type beginTrailOptions struct {
 	repoURL              string
 	repoProvider         string
 	repoNameExplicit     bool
-	commitExplicit       bool
-	repoRootExplicit     bool
+	// flags lets commitInfoRequest tell a passed --commit/--repo-root from a
+	// defaulted one.
+	flags *pflag.FlagSet
 }
 
 type TrailPayload struct {
@@ -87,8 +89,7 @@ func newBeginTrailCmd(out io.Writer) *cobra.Command {
 		},
 		RunE: func(cmd *cobra.Command, args []string) error {
 			o.repoNameExplicit = cmd.Flags().Changed("repository")
-			o.commitExplicit = cmd.Flags().Changed("commit")
-			o.repoRootExplicit = cmd.Flags().Changed("repo-root")
+			o.flags = cmd.Flags()
 			return o.run(args)
 		},
 	}
@@ -133,11 +134,10 @@ func (o *beginTrailOptions) run(args []string) error {
 
 	if o.commitSHA != "" {
 		o.payload.Commit, err = commitInfoRequest{
-			repoRoot:         o.srcRepoRoot,
-			sha:              o.commitSHA,
-			redacted:         o.redactedCommitInfo,
-			commitExplicit:   o.commitExplicit,
-			repoRootExplicit: o.repoRootExplicit,
+			repoRoot: o.srcRepoRoot,
+			sha:      o.commitSHA,
+			redacted: o.redactedCommitInfo,
+			flags:    o.flags,
 		}.resolve()
 		if err != nil {
 			return err