diff --git a/internal/scriptrun/env.go b/internal/scriptrun/env.go new file mode 100644 index 000000000..fb8659dfd --- /dev/null +++ b/internal/scriptrun/env.go @@ -0,0 +1,41 @@ +package scriptrun + +// Operation names the high-level gs command driving a script call. +// Used as the value of GS_OPERATION in the script's environment so +// scripts can branch on which gs subcommand invoked them. +// +// See doc/src/guide/scripts.md for the full table. +type Operation string + +// Shared GS_OPERATION values. Keep in sync with doc/src/guide/scripts.md. +const ( + OpCommitCreate Operation = "commit-create" + OpCommitAmend Operation = "commit-amend" + OpBranchCreate Operation = "branch-create" + OpBranchSubmit Operation = "branch-submit" + OpBranchSquash Operation = "branch-squash" + OpBranchRestack Operation = "branch-restack" + OpUpstackRestack Operation = "upstack-restack" + OpDownstackRestack Operation = "downstack-restack" + OpStackRestack Operation = "stack-restack" + OpRepoRestack Operation = "repo-restack" + OpIntegrationRebuild Operation = "integration-rebuild" +) + +// EnvFor returns the shared minimum environment that every script +// receives: GS_OPERATION (required), GS_BRANCH and GS_BASE (omitted +// when empty). Feature-specific extras layer on top of this slice via +// the existing RunRequest.Env mechanism. +// +// The returned slice is safe to append to. +func EnvFor(op Operation, branch, base string) []string { + env := make([]string, 0, 3) + env = append(env, "GS_OPERATION="+string(op)) + if branch != "" { + env = append(env, "GS_BRANCH="+branch) + } + if base != "" { + env = append(env, "GS_BASE="+base) + } + return env +} diff --git a/internal/scriptrun/env_test.go b/internal/scriptrun/env_test.go new file mode 100644 index 000000000..81bbd2b8c --- /dev/null +++ b/internal/scriptrun/env_test.go @@ -0,0 +1,60 @@ +package scriptrun_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.abhg.dev/gs/internal/scriptrun" +) + +func TestEnvFor(t *testing.T) { + tests := []struct { + name string + op scriptrun.Operation + branch string + base string + want []string + }{ + { + name: "OperationOnly", + op: scriptrun.OpIntegrationRebuild, + want: []string{"GS_OPERATION=integration-rebuild"}, + }, + { + name: "BranchPresent", + op: scriptrun.OpBranchRestack, + branch: "feat1", + want: []string{ + "GS_OPERATION=branch-restack", + "GS_BRANCH=feat1", + }, + }, + { + name: "AllThree", + op: scriptrun.OpCommitCreate, + branch: "feat1", + base: "main", + want: []string{ + "GS_OPERATION=commit-create", + "GS_BRANCH=feat1", + "GS_BASE=main", + }, + }, + { + name: "BaseWithoutBranchOmitsBoth", + op: scriptrun.OpStackRestack, + base: "main", + want: []string{ + "GS_OPERATION=stack-restack", + "GS_BASE=main", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := scriptrun.EnvFor(tt.op, tt.branch, tt.base) + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/internal/scriptrun/errors.go b/internal/scriptrun/errors.go new file mode 100644 index 000000000..d70358d89 --- /dev/null +++ b/internal/scriptrun/errors.go @@ -0,0 +1,26 @@ +package scriptrun + +import ( + "errors" + "fmt" +) + +// ErrEmptyOutput is returned by [ParseResponse] when the script +// produced no stdout. +var ErrEmptyOutput = errors.New("script produced no output") + +// InvalidOutputError wraps a JSON parse failure on script output. +// The original output is retained (truncated by callers as needed +// for logging) so handlers can surface diagnostic context. +type InvalidOutputError struct { + Err error + Output []byte +} + +func (e *InvalidOutputError) Error() string { + return fmt.Sprintf("invalid script output: %v", e.Err) +} + +func (e *InvalidOutputError) Unwrap() error { + return e.Err +} diff --git a/internal/scriptrun/resolver.go b/internal/scriptrun/resolver.go new file mode 100644 index 000000000..525ce9667 --- /dev/null +++ b/internal/scriptrun/resolver.go @@ -0,0 +1,58 @@ +package scriptrun + +import "encoding/json" + +// ResolveResponse is the JSON document a script emits on stdout. +// +// All three script-driven features (commit message generation, +// restack auto-resolve, integration auto-resolve) speak the same +// protocol. Each feature reads the subset of fields that applies. +// +// See doc/src/guide/scripts.md for the user-facing contract. +type ResolveResponse struct { + // Title is the first-line commit message or change-request title. + // Read by message-generation features; ignored by auto-resolvers. + Title string `json:"title,omitempty"` + + // Body is the multi-line body content for a commit or change + // request. Read by message-generation features. + Body string `json:"body,omitempty"` + + // Assumptions are notes the script wants surfaced in the gs log + // so the user can see what choices the script made. All features + // log these at info level. + Assumptions []string `json:"assumptions,omitempty"` + + // Questions, when non-empty, drives the interactive Q&A loop. + // git-spice prompts the user for each question and re-invokes the + // script with the answers persisted in the resolution file. + Questions []string `json:"questions,omitempty"` + + // UnresolvedFiles lists paths the script could not resolve. + // Read by auto-resolve features only; treated as "not done" -- + // the run is not terminal until this slice is empty. + UnresolvedFiles []string `json:"unresolved_files,omitempty"` +} + +// ParseResponse unmarshals raw script output into a ResolveResponse. +// Returns an error wrapping ErrInvalidOutput if the output is not a +// valid JSON document. +func ParseResponse(stdout []byte) (*ResolveResponse, error) { + var res ResolveResponse + if len(stdout) == 0 { + return nil, ErrEmptyOutput + } + if err := json.Unmarshal(stdout, &res); err != nil { + return nil, &InvalidOutputError{Err: err, Output: stdout} + } + return &res, nil +} + +// QAPair is a single question-answer record in a persistent +// resolution file. Resolution-file packages share this type so that +// answers gathered for one feature can be referenced from another (or +// at minimum so the on-disk schema is uniform). +type QAPair struct { + Question string `json:"question"` + Answer string `json:"answer"` +} diff --git a/internal/scriptrun/resolver_test.go b/internal/scriptrun/resolver_test.go new file mode 100644 index 000000000..bbce8f4a5 --- /dev/null +++ b/internal/scriptrun/resolver_test.go @@ -0,0 +1,78 @@ +package scriptrun_test + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.abhg.dev/gs/internal/scriptrun" +) + +func TestParseResponse(t *testing.T) { + tests := []struct { + name string + input string + want *scriptrun.ResolveResponse + }{ + { + name: "Empty object", + input: `{}`, + want: &scriptrun.ResolveResponse{}, + }, + { + name: "TitleOnly", + input: `{"title": "Fix the thing"}`, + want: &scriptrun.ResolveResponse{Title: "Fix the thing"}, + }, + { + name: "Full", + input: `{ + "title": "T", + "body": "B", + "assumptions": ["a1"], + "questions": ["q1"], + "unresolved_files": ["f1"] + }`, + want: &scriptrun.ResolveResponse{ + Title: "T", + Body: "B", + Assumptions: []string{"a1"}, + Questions: []string{"q1"}, + UnresolvedFiles: []string{"f1"}, + }, + }, + { + name: "ExtraFieldsIgnored", + input: `{"title": "T", "extra": 42}`, + want: &scriptrun.ResolveResponse{Title: "T"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := scriptrun.ParseResponse([]byte(tt.input)) + require.NoError(t, err) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestParseResponse_emptyInput(t *testing.T) { + _, err := scriptrun.ParseResponse(nil) + assert.ErrorIs(t, err, scriptrun.ErrEmptyOutput) +} + +func TestParseResponse_invalidJSON(t *testing.T) { + _, err := scriptrun.ParseResponse([]byte("not json")) + var invalid *scriptrun.InvalidOutputError + require.ErrorAs(t, err, &invalid) + assert.NotEmpty(t, invalid.Output) + assert.Contains(t, invalid.Error(), "invalid script output") +} + +func TestInvalidOutputError_unwraps(t *testing.T) { + inner := errors.New("boom") + e := &scriptrun.InvalidOutputError{Err: inner} + assert.ErrorIs(t, e, inner) +} diff --git a/internal/spice/config.go b/internal/spice/config.go index a249f993b..b6d8cc56a 100644 --- a/internal/spice/config.go +++ b/internal/spice/config.go @@ -236,6 +236,34 @@ func (c *Config) Shorthands() []string { return slices.Sorted(maps.Keys(c.shorthands)) } +// DefaultScriptResolveMaxIterations is the iteration cap used when +// spice.scriptResolve.maxIterations is unset or invalid. A script that +// keeps returning new questions hits this many iterations before +// git-spice gives up and falls back to manual resolution. +const DefaultScriptResolveMaxIterations = 10 + +// ScriptResolveMaxIterations returns the configured iteration cap for +// script-driven Q&A loops. Read from spice.scriptResolve.maxIterations +// and defaults to [DefaultScriptResolveMaxIterations]. Non-positive or +// non-numeric values fall back to the default and emit a warning. +func (c *Config) ScriptResolveMaxIterations() int { + key := git.ConfigKey("spice.scriptResolve.maxIterations").Canonical() + values, ok := c.items[key] + if !ok || len(values) == 0 { + return DefaultScriptResolveMaxIterations + } + last := strings.TrimSpace(values[len(values)-1]) + n, err := strconv.Atoi(last) + if err != nil || n <= 0 { + c.log.Warnf( + "invalid value for %v: %q (using default %d)", + key, last, DefaultScriptResolveMaxIterations, + ) + return DefaultScriptResolveMaxIterations + } + return n +} + // Validate checks if the configuration is valid for the given application. // This is a no-op, as we allow unknown configuration keys. func (*Config) Validate(*kong.Application) error { return nil } diff --git a/internal/spice/config_test.go b/internal/spice/config_test.go index 6e854ccac..70896020d 100644 --- a/internal/spice/config_test.go +++ b/internal/spice/config_test.go @@ -457,6 +457,82 @@ func TestConfig_ShellCommand(t *testing.T) { } } +func TestConfig_ScriptResolveMaxIterations(t *testing.T) { + tests := []struct { + name string + config string + want int + }{ + { + name: "Unset", + config: "", + want: spice.DefaultScriptResolveMaxIterations, + }, + { + name: "ValidOverride", + config: text.Dedent(` + [spice "scriptResolve"] + maxIterations = 25 + `), + want: 25, + }, + { + name: "InvalidFallsBackToDefault", + config: text.Dedent(` + [spice "scriptResolve"] + maxIterations = banana + `), + want: spice.DefaultScriptResolveMaxIterations, + }, + { + name: "ZeroFallsBackToDefault", + config: text.Dedent(` + [spice "scriptResolve"] + maxIterations = 0 + `), + want: spice.DefaultScriptResolveMaxIterations, + }, + { + name: "NegativeFallsBackToDefault", + config: text.Dedent(` + [spice "scriptResolve"] + maxIterations = -5 + `), + want: spice.DefaultScriptResolveMaxIterations, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + home := t.TempDir() + require.NoError(t, os.WriteFile( + filepath.Join(home, ".gitconfig"), + []byte(tt.config), + 0o600, + ), "write configuration file") + + ctx := t.Context() + gitCfg := git.NewConfig(git.ConfigOptions{ + Log: silogtest.New(t), + Dir: home, + Env: []string{ + "HOME=" + home, + "USER=testuser", + "GIT_CONFIG_NOSYSTEM=1", + }, + }) + spicecfg, err := spice.LoadConfig(ctx, gitCfg, spice.ConfigOptions{ + Log: silogtest.New(t), + }) + require.NoError(t, err, "load configuration") + + assert.Equal(t, tt.want, spicecfg.ScriptResolveMaxIterations()) + }) + } +} + func TestIntegrationConfig_gitConfigReferences(t *testing.T) { t.Setenv("HOME", "") t.Setenv("XDG_CONFIG_HOME", "") diff --git a/internal/spice/spicedir/spicedir.go b/internal/spice/spicedir/spicedir.go new file mode 100644 index 000000000..5d8e7e443 --- /dev/null +++ b/internal/spice/spicedir/spicedir.go @@ -0,0 +1,57 @@ +// Package spicedir owns the on-disk layout for git-spice's +// per-repository scratch directory: /.spice/. +// +// Today the directory holds persistent script-resolution files used by +// the message generator and the two auto-resolve features. Future +// features that need a per-repo cache (workspace snapshots, plan +// scratch, etc.) should put their data here instead of cluttering the +// repo root. +// +// Whether to track .spice/ in version control is the user's choice. +// spicedir does not modify .gitignore. +package spicedir + +import ( + "fmt" + "os" + "path/filepath" +) + +// DirName is the directory name relative to the repository root. +const DirName = ".spice" + +// Path returns the absolute path to the spice directory inside the +// given repository root. Path does not create the directory; call +// [EnsureDir] for that. +func Path(repoRoot string) string { + return filepath.Join(repoRoot, DirName) +} + +// EnsureDir creates the spice directory (and any missing parents) +// under the given repository root. Returns nil if it already exists. +func EnsureDir(repoRoot string) error { + dir := Path(repoRoot) + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("create spice dir: %w", err) + } + return nil +} + +// ResolutionPath returns the canonical path for a feature's +// resolution-history JSON file: /.spice/resolutions/.json. +// +// Callers should call [EnsureResolutionsDir] before writing to the +// returned path. +func ResolutionPath(repoRoot, feature string) string { + return filepath.Join(Path(repoRoot), "resolutions", feature+".json") +} + +// EnsureResolutionsDir creates /.spice/resolutions/ if it +// does not already exist. +func EnsureResolutionsDir(repoRoot string) error { + dir := filepath.Join(Path(repoRoot), "resolutions") + if err := os.MkdirAll(dir, 0o755); err != nil { + return fmt.Errorf("create resolutions dir: %w", err) + } + return nil +} diff --git a/internal/spice/spicedir/spicedir_test.go b/internal/spice/spicedir/spicedir_test.go new file mode 100644 index 000000000..4bdc13f0a --- /dev/null +++ b/internal/spice/spicedir/spicedir_test.go @@ -0,0 +1,44 @@ +package spicedir_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.abhg.dev/gs/internal/spice/spicedir" +) + +func TestPath(t *testing.T) { + got := spicedir.Path("/repo") + assert.Equal(t, filepath.Join("/repo", ".spice"), got) +} + +func TestResolutionPath(t *testing.T) { + got := spicedir.ResolutionPath("/repo", "restack") + assert.Equal(t, filepath.Join("/repo", ".spice", "resolutions", "restack.json"), got) +} + +func TestEnsureDir(t *testing.T) { + root := t.TempDir() + + require.NoError(t, spicedir.EnsureDir(root)) + + info, err := os.Stat(filepath.Join(root, ".spice")) + require.NoError(t, err) + assert.True(t, info.IsDir()) + + // Calling again is idempotent. + require.NoError(t, spicedir.EnsureDir(root)) +} + +func TestEnsureResolutionsDir(t *testing.T) { + root := t.TempDir() + + require.NoError(t, spicedir.EnsureResolutionsDir(root)) + + info, err := os.Stat(filepath.Join(root, ".spice", "resolutions")) + require.NoError(t, err) + assert.True(t, info.IsDir()) +}