Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cmd/entire/cli/integration_test/trail_resume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ func TestTrailResume_UsesCheckpointSessionsWhenLocalStateIsMissing(t *testing.T)
checkpointID,
"Create hello method",
"Create goodbye method",
"entire trail resume 321 --project gh/entireio --repo gh/entireio/cli --branch feature/test-branch --session " + firstSession.ID,
"entire trail resume 321 --project gh/entireio --repo gh/entireio/cli --branch feature/test-branch --session " + secondSession.ID,
"entire trail finding 321 --project gh/entireio --repo gh/entireio/cli --branch feature/test-branch --json",
"entire trail resume 321 --project gh/entireio --branch feature/test-branch --session " + firstSession.ID,
"entire trail resume 321 --project gh/entireio --branch feature/test-branch --session " + secondSession.ID,
"entire trail finding 321 --project gh/entireio --branch feature/test-branch --json",
} {
if !strings.Contains(contextOutput, want) {
t.Fatalf("trail resume --no-resume output missing %q:\n%s", want, contextOutput)
Expand Down
35 changes: 35 additions & 0 deletions cmd/entire/cli/trail_legacy_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,3 +138,38 @@ func TestTrailCheckoutPreservesBranchSelectionByMode(t *testing.T) {
})
}
}

func TestLegacyResumeIgnoresDeprecatedRepoFlag(t *testing.T) {
// Serial: isolates CWD and replaces the repository client constructor.
setupTrailBranchSelectionTest(t)
setupWorkingRepoClient(t, func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/api/v1/trails/gh/acme/widget/7":
assert.NoError(t, json.NewEncoder(w).Encode(api.TrailResource{ID: projectTrailTestChange, Number: 7, Branch: "feature/work"}))
case "/api/v1/trails/gh/acme/widget/7/reviews/comments":
assert.NoError(t, json.NewEncoder(w).Encode(map[string]any{"comments": []any{}}))
default:
t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
http.NotFound(w, r)
}
})
cmd := newTrailResumeCmd(legacyTrailMode)
require.True(t, cmd.Flags().Lookup("repo").Hidden, "--repo must be hidden")
var out, errOut bytes.Buffer
cmd.SetOut(&out)
cmd.SetErr(&errOut)
// A repository other than this checkout's must not stop resume.
cmd.SetArgs([]string{"7", "--repo", "other/elsewhere", "--branch", "feature/work", "--no-resume", "--json"})
require.NoError(t, cmd.ExecuteContext(t.Context()))
var resumed trailResumeContext
require.NoError(t, json.Unmarshal(out.Bytes(), &resumed))
for _, command := range resumed.Commands {
require.NotContains(t, command, "--repo")
}
var raw struct {
Trail map[string]any `json:"trail"`
}
require.NoError(t, json.Unmarshal(out.Bytes(), &raw))
require.NotContains(t, raw.Trail, "repo", "resume context no longer reports a repository")
require.Contains(t, errOut.String(), "--repo has been deprecated")
}
6 changes: 3 additions & 3 deletions cmd/entire/cli/trail_mode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,13 +141,13 @@ func TestLegacyTrailRequestsDoNotResolveProject(t *testing.T) {

func TestProjectResumeCommandsKeepOptIn(t *testing.T) {
t.Parallel()
ctx := buildTrailResumeContextForRepoWithSkipped(api.TrailResource{
ctx := buildTrailResumeContext(api.TrailResource{
ID: projectTrailTestChange, Number: 7, Branch: "feature/work",
Parent: &api.TrailParentReference{ID: projectTrailTestID, Number: 42, Host: "gh", Project: "acme"},
}, nil, "", 0, trailResumeFindingsContext{}, "gh/acme/widget")
}, nil, "", 0, trailResumeFindingsContext{})
for _, command := range ctx.Commands {
require.Contains(t, command, "ENTIRE_PROJECT_TRAILS=1 entire trail")
require.Contains(t, command, "42 --project gh/acme --repo gh/acme/widget --branch feature/work")
require.Contains(t, command, "42 --project gh/acme --branch feature/work")
}
}

Expand Down
81 changes: 18 additions & 63 deletions cmd/entire/cli/trail_resume_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const (

type trailResumeOptions struct {
Selector string
ExpectedRepo string
ExpectedBranch string
SessionID string
CheckpointID string
Expand All @@ -56,7 +55,6 @@ type trailResumeTrailContext struct {
ID string `json:"id,omitempty"`
Number int `json:"number,omitempty"`
Title string `json:"title,omitempty"`
Repo string `json:"repo,omitempty"`
Project string `json:"project,omitempty"`
Branch string `json:"branch"`
Base string `json:"base,omitempty"`
Expand All @@ -65,12 +63,6 @@ type trailResumeTrailContext struct {
URL string `json:"url,omitempty"`
}

type trailResumeRepository struct {
Forge string
Owner string
Repo string
}

type trailResumeSessionContext struct {
SessionID string `json:"session_id"`
Agent string `json:"agent,omitempty"`
Expand Down Expand Up @@ -149,17 +141,19 @@ start. Non-interactive runs show the same context and print resume commands for
the latest checkpoint on the trail branch. Use --session or --checkpoint to
resume an exact session or checkpoint.

Use --repo to assert the GitHub repository for copied commands, and --branch
with a trail number or id to assert the branch you expect the trail to be
attached to. If either assertion does not match the current checkout or trail,
resume stops before checking anything out.`,
Use --branch with a trail number or id to assert the branch you expect the
trail to be attached to. If it does not match, resume stops before checking
anything out.`,
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
selector, err := parseOptionalTrailSelector(args, opts.Selector)
if err != nil {
return err
}
opts.Selector = selector
if cmd.Flags().Changed("repo") {
fmt.Fprintln(cmd.ErrOrStderr(), "Flag --repo has been deprecated, it has no effect and will be removed")
}
if err := validateTrailResumeOptions(opts); err != nil {
return err
}
Expand All @@ -171,14 +165,23 @@ resume stops before checking anything out.`,
cmd.Flags().StringVar(&opts.Selector, "trail", "", mode.help(
"Trail to resume (number, id, or branch; defaults to the current branch's trail)",
"Project trail number, ID, or <repo>/<number> (defaults to the current branch's parent)"))
cmd.Flags().StringVar(&opts.ExpectedRepo, "repo", "", "Expected GitHub repository (owner/name); fails if the current checkout points elsewhere")
cmd.Flags().StringVar(&opts.ExpectedBranch, "branch", "", "Expected trail branch; fails if the trail is attached to a different branch")
cmd.Flags().StringVar(&opts.SessionID, "session", "", "Resume a specific known local session on the trail branch")
cmd.Flags().StringVar(&opts.CheckpointID, "checkpoint", "", "Resume a specific checkpoint on the trail branch")
cmd.Flags().BoolVarP(&opts.Force, "force", "f", false, "Skip prompts and overwrite existing session logs from checkpoints")
cmd.Flags().BoolVar(&opts.JSON, "json", false, "Output trail resume context as JSON")
cmd.Flags().BoolVar(&opts.NoResume, "no-resume", false, "Show trail resume context without restoring or resuming a session")

// --repo once asserted the checkout's repository. It is accepted and
// ignored so previously copied resume commands keep working. The notice
// goes to stderr itself: cobra's MarkDeprecated writes to the output
// writer when one is set, which would corrupt --json.
var deprecatedRepo string
cmd.Flags().StringVar(&deprecatedRepo, "repo", "", "Deprecated; has no effect")
if err := cmd.Flags().MarkHidden("repo"); err != nil {
panic(fmt.Sprintf("hide repo flag: %v", err))
}

return cmd
}

Expand All @@ -192,9 +195,6 @@ func validateTrailResumeOptions(opts trailResumeOptions) error {
if opts.NoResume && (strings.TrimSpace(opts.SessionID) != "" || strings.TrimSpace(opts.CheckpointID) != "") {
return errors.New("cannot combine --no-resume with --session or --checkpoint")
}
if _, err := parseTrailResumeRepoFlag(opts.ExpectedRepo); err != nil {
return fmt.Errorf("validate --repo: %w", err)
}
if checkpointID := strings.TrimSpace(opts.CheckpointID); checkpointID != "" {
if err := id.Validate(checkpointID); err != nil {
return fmt.Errorf("validate --checkpoint: %w", err)
Expand All @@ -208,17 +208,6 @@ func runTrailResume(cmd *cobra.Command, mode *trailMode, opts trailResumeOptions
// home an agent reads from its own settings is invisible to the environment.
agent.EnableHomeProbes()
ctx := cmd.Context()
forge, owner, repo, err := resolveTrailRemote(ctx)
if err != nil {
return err
}
expectedRepo, err := parseTrailResumeRepoFlag(opts.ExpectedRepo)
if err != nil {
return fmt.Errorf("validate --repo: %w", err)
}
if err := validateTrailResumeExpectedRepo(trailResumeRepository{Forge: forge, Owner: owner, Repo: repo}, expectedRepo); err != nil {
return err
}
selected, err := mode.workingContext(cmd, opts.Selector, opts.ExpectedBranch, true)
if err != nil {
return err
Expand All @@ -245,12 +234,10 @@ func runTrailResume(cmd *cobra.Command, mode *trailMode, opts trailResumeOptions
}

display := *found
repoRef := owner + "/" + repo
if selected.Target != nil {
display.Title, display.Status = selected.Parent.Title, selected.Parent.Status
repoRef = forge + "/" + repoRef
}
resumeCtx := buildTrailResumeContextForRepoWithSkipped(display, sessions, sessionsUnavailable, sessionsSkipped, findings, repoRef)
resumeCtx := buildTrailResumeContext(display, sessions, sessionsUnavailable, sessionsSkipped, findings)
if opts.JSON {
return encodeTrailResumeContextJSON(cmd.OutOrStdout(), resumeCtx)
}
Expand Down Expand Up @@ -279,34 +266,6 @@ func runTrailResume(cmd *cobra.Command, mode *trailMode, opts trailResumeOptions
return resumeTrailLatest(ctx, cmd, branch, opts.Force, "")
}

func parseTrailResumeRepoFlag(value string) (trailResumeRepository, error) {
value = strings.TrimSpace(value)
if value == "" {
return trailResumeRepository{}, nil
}
if strings.HasPrefix(value, "gh/") || strings.HasPrefix(value, "et/") {
forge, owner, repo, err := parseTrailRepoArg(value)
return trailResumeRepository{Forge: forge, Owner: owner, Repo: repo}, err
}
owner, repo, err := parseGitHubURL(value)
if err != nil {
return trailResumeRepository{}, err
}
return trailResumeRepository{Forge: "gh", Owner: owner, Repo: repo}, nil
}

func validateTrailResumeExpectedRepo(current, expected trailResumeRepository) error {
if strings.TrimSpace(expected.Repo) == "" {
return nil
}
if current.Forge == expected.Forge &&
strings.EqualFold(current.Owner, expected.Owner) &&
strings.EqualFold(current.Repo, expected.Repo) {
return nil
}
return fmt.Errorf("this command targets repository %s/%s, but the current checkout is %s/%s", expected.Owner, expected.Repo, current.Owner, current.Repo)
}

func validateTrailResumeExpectedBranch(found *api.TrailResource, expectedBranch string) error {
expectedBranch = strings.TrimSpace(expectedBranch)
if expectedBranch == "" {
Expand Down Expand Up @@ -657,12 +616,11 @@ func trailResumeTopFindingOptions() trailReviewListOptions {
}
}

func buildTrailResumeContextForRepoWithSkipped(found api.TrailResource, sessions []trailResumeSessionContext, sessionsUnavailable string, sessionsSkipped int, findings trailResumeFindingsContext, repoFullName string) trailResumeContext {
func buildTrailResumeContext(found api.TrailResource, sessions []trailResumeSessionContext, sessionsUnavailable string, sessionsSkipped int, findings trailResumeFindingsContext) trailResumeContext {
trailCtx := trailResumeTrailContext{
ID: found.ID,
Number: found.Number,
Title: strings.TrimSpace(found.Title),
Repo: strings.TrimSpace(repoFullName),
Branch: strings.TrimSpace(found.Branch),
Base: strings.TrimSpace(found.Base),
Status: strings.TrimSpace(found.Status),
Expand Down Expand Up @@ -711,9 +669,6 @@ func buildTrailResumeCommands(ctx trailResumeContext) []string {
arg += " --project " + shellArg(project)
}
findingArg := arg
if repo := strings.TrimSpace(ctx.Trail.Repo); repo != "" {
arg += " --repo " + shellArg(repo)
}
if branch := strings.TrimSpace(ctx.Trail.Branch); branch != "" {
arg += " --branch " + shellArg(branch)
}
Expand Down
60 changes: 2 additions & 58 deletions cmd/entire/cli/trail_resume_cmd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,6 @@ func TestValidateTrailResumeOptions(t *testing.T) {
name: "json no resume accepted",
opts: trailResumeOptions{JSON: true, NoResume: true},
},
{
name: "invalid repo assertion",
opts: trailResumeOptions{ExpectedRepo: "not a repo"},
wantErr: "validate --repo",
},
{
name: "repo assertion accepted",
opts: trailResumeOptions{ExpectedRepo: "entireio/cli"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -93,29 +84,6 @@ func TestValidateTrailResumeOptions(t *testing.T) {
}
}

func TestValidateTrailResumeExpectedRepo(t *testing.T) {
t.Parallel()

current := trailResumeRepository{Forge: "gh", Owner: "EntireIO", Repo: "CLI"}
expected := trailResumeRepository{Forge: "gh", Owner: "entireio", Repo: "cli"}
if err := validateTrailResumeExpectedRepo(current, expected); err != nil {
t.Fatalf("validateTrailResumeExpectedRepo() matching repo = %v, want nil", err)
}
if err := validateTrailResumeExpectedRepo(current, trailResumeRepository{}); err != nil {
t.Fatalf("validateTrailResumeExpectedRepo() empty expected repo = %v, want nil", err)
}

err := validateTrailResumeExpectedRepo(current, trailResumeRepository{Forge: "gh", Owner: "entireio", Repo: "entire.io"})
if err == nil {
t.Fatal("validateTrailResumeExpectedRepo() mismatch = nil, want error")
}
for _, want := range []string{"targets repository entireio/entire.io", "current checkout is EntireIO/CLI"} {
if !strings.Contains(err.Error(), want) {
t.Fatalf("error = %q, want it to mention %q", err, want)
}
}
}

func TestValidateTrailResumeExpectedBranch(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -166,7 +134,7 @@ func TestBuildTrailResumeContextSortsCheckpointSessions(t *testing.T) {

now := time.Date(2026, 6, 23, 12, 0, 0, 0, time.UTC)
const newSessionID = "new-session"
ctx := buildTrailResumeContextForRepoWithSkipped(api.TrailResource{
ctx := buildTrailResumeContext(api.TrailResource{
ID: "trl_1",
Number: 575,
Title: "Add trail resume",
Expand All @@ -188,7 +156,7 @@ func TestBuildTrailResumeContextSortsCheckpointSessions(t *testing.T) {
LastActive: now,
CheckpointID: "aaaaaaaaaaaa",
},
}, "", 0, trailResumeFindingsContext{}, "")
}, "", 0, trailResumeFindingsContext{})

if len(ctx.Sessions) != 2 {
t.Fatalf("sessions len = %d, want 2: %#v", len(ctx.Sessions), ctx.Sessions)
Expand Down Expand Up @@ -219,30 +187,6 @@ func TestBuildTrailResumeContextSortsCheckpointSessions(t *testing.T) {
}
}

func TestBuildTrailResumeContextWithRepoIncludesRepoInResumeCommands(t *testing.T) {
t.Parallel()

ctx := buildTrailResumeContextForRepoWithSkipped(api.TrailResource{
ID: "trl_1",
Number: 575,
Title: "Add trail resume",
Branch: "feature/trail-resume",
}, nil, "", 0, trailResumeFindingsContext{}, "gh/entireio/cli")

wantCommands := []string{
"entire trail finding 575 --json",
"entire trail resume 575 --repo gh/entireio/cli --branch feature/trail-resume",
}
if len(ctx.Commands) != len(wantCommands) {
t.Fatalf("commands len = %d, want %d: %#v", len(ctx.Commands), len(wantCommands), ctx.Commands)
}
for i, want := range wantCommands {
if ctx.Commands[i] != want {
t.Fatalf("commands[%d] = %q, want %q", i, ctx.Commands[i], want)
}
}
}

func newTrailResumeCheckpointTestRepo(t *testing.T) (string, *git.Repository, *git.Worktree) {
t.Helper()

Expand Down
4 changes: 2 additions & 2 deletions cmd/entire/cli/trail_working_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ func (t *trailWorkingContext) description() string {
}

// resolveProjectTrailWorkingContext is projectTrailMode.workingContext.
// localOnly is used by checkout/resume: --repo may assert a local repository,
// but must never cause these commands to check out a foreign branch here.
// localOnly is used by checkout/resume, which act on the local clone: checkout
// rejects --repo and resume ignores it, so neither targets a foreign repository.
func resolveProjectTrailWorkingContext(cmd *cobra.Command, selector, branch string, localOnly bool) (_ *trailWorkingContext, err error) {
if change, ok := parseProjectTrailChangeSelector(selector); ok {
if strings.TrimSpace(branch) != "" {
Expand Down
Loading