From 5a49bc9fe8e72d4cee59061742257452ae77672e Mon Sep 17 00:00:00 2001 From: Yuval Hayke Date: Thu, 20 Aug 2026 11:06:41 +0300 Subject: [PATCH 1/2] fix(linear): seed a ticket prompt only when the worktree is created handleSessionCreate ran ticketPromptFor on every session creation, which seeded the agent's first message from a Linear ticket in two ways: - reuse: linear.ExistingPrompt read back the prompt.txt already sitting in .fleet/ticket//. It never looked at the branch, so a checkout that once held a ticket re-asked the original task on every session added afterwards -- forever, including long after the checkout had moved on to master. - inference: a branch or worktree dir name matching - fetched and materialized the ticket, then seeded from it. A seeded first message is the gesture of starting a worktree from a ticket, not a property of the directory that worktree happens to be. So both branches go, and handleSessionCreate now passes msg.prompt through untouched. The worktree-creation path already set it explicitly (and ticketPromptFor early-returned on it), so `w` + a ticket and `fleet worktree --ticket` are unaffected; `a`/`n`/`A` always start empty. The negative pin (NegativelyPinned/pinNoTicket) existed only to stop the inference path re-asking Linear on every session start, so it goes with it rather than staying as a write-only file. pathTailAfterRepo stays -- palette_tickets.go still uses it. TestManualSessionCreationSeedsNoPrompt fails if handleSessionCreate reaches into internal/linear again, or if anything in app.go assigns to a .prompt field on the way to the launch. Cost: a worktree created outside fleet on a ticket-named branch no longer materializes its ticket. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WBQUYTfR7kzUjgozd58mr6 --- CLAUDE.md | 4 +- .../manual-session-no-ticket-prompt.md | 5 + internal/linear/linear_test.go | 32 ------- internal/linear/materialize.go | 60 ++---------- internal/ui/app.go | 27 +----- internal/ui/ticket.go | 91 +------------------ internal/ui/worktree_ticket_routing_test.go | 54 ++++++++++- 7 files changed, 70 insertions(+), 203 deletions(-) create mode 100644 changelog/unreleased/manual-session-no-ticket-prompt.md diff --git a/CLAUDE.md b/CLAUDE.md index f7eca295..e4e7bc5d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -151,12 +151,12 @@ Styles in `styles.go` are declared **bare** and constructed **only** in `ApplyPa - **`Available()` and `TeamKeys()` are called from the Update goroutine** and therefore touch no network and no keychain — two atomics and two small file reads. The keychain read happens once, in `warmLinear()` from `Init`. `Resolved()` is separate from `Available()` on purpose: before the warm finishes, "no credential" is ignorance, not a fact, and anything acting on the *absence* of one (the discovery tip) must wait. - Team keys come from `.fleet.json`/`.fleet.local.json` `{"linear":{"team":"BRZ"}}` (a local `team` **replaces** the committed one; `teams` lists append and dedupe — see the `.fleet.json` bullet below), falling back to `team_id` in a committed `.linear.toml`. That file belongs to the CLI fleet no longer uses, but reading one key out of a file someone already has costs nothing and makes this zero-touch for them; **`api_key` in the same file is never read** (`TestTeamKeysReadOnlyTeamID`). Gating is on the **set**, not one key — a workspace routinely has several teams and one repo may see branches from both. - One GraphQL round trip does everything (`issueFullQuery`): description, comments with author and timestamp, labels, assignee, priority, parent/children, attachments, **and the team's workflow states**, so the optional state write needs no second query. Measured at **87 complexity points** against a 10,000-per-query cap; rate limits are 2,500 req/hr and 3M complexity/hr against roughly 2 calls per worktree, so **no throttling machinery exists**. `issue(id: "BRZ-3182")` takes the shorthand identifier. Search is `searchIssues(term:)` — confirmed against the live schema, where `issueSearch(query:)` also exists and neither is deprecated — and is deliberately **unscoped by team**: the repo gate already decides *whether* to search, and someone typing prose wants matches, not a filter they didn't ask for. -- **Error classification cannot key on HTTP status** (`TestGraphQLErrorClassification`). Captured from the live API: an unknown issue returns **HTTP 200** with an `errors[]` entry whose own `extensions` carry `statusCode 400` and the message `Entity not found: Issue`; a bad token returns 401 with code `AUTHENTICATION_ERROR`. Reading the status alone would file "no such issue" as a generic failure and break the negative pin that stops fleet re-asking on every session start. +- **Error classification cannot key on HTTP status** (`TestGraphQLErrorClassification`). Captured from the live API: an unknown issue returns **HTTP 200** with an `errors[]` entry whose own `extensions` carry `statusCode 400` and the message `Entity not found: Issue`; a bad token returns 401 with code `AUTHENTICATION_ERROR`. Reading the status alone would file "no such issue" as a generic failure: `ticketStatusLine` swallows `ErrNotFound` (a branch naming no real issue is a resting state, not worth a line on the session you just started) and `fleet worktree --ticket` leans on it to reject a bad identifier *before* anything is created. - **Extensions are recovered, not trusted.** Linear's default alt text is literally `image.png` and its upload URLs carry no filename, so a real PNG would land unnamed and unextensioned — and an agent's file-read tool dispatches on extension, making a perfectly downloaded screenshot unreadable. `detectExt` sniffs magic bytes (`http.DetectContentType`), which also rejects a 401 HTML body that would otherwise sit beside real screenshots. Recovering the extension and rewriting the markdown links are a **matched pair**: fix one and the agent still sees nothing. `findImages` takes **only** `http(s)` targets (`TestFindImagesTakesOnlyRemoteLinks`) — a relative path or a `file:` link in a description is not something fleet has any business reading off disk and copying into a worktree. - Files land at `/.fleet/ticket//` — inside the worktree so the agent reads them with a relative path and **no permission prompt**, since a prompt on the session's first act renders as `-` waiting, the friction this feature removes. Git exclusion uses `git rev-parse --git-path info/exclude`, **never `--git-dir` + `info/exclude`**: `info` is on git's shared-path list, so a linked worktree's `--git-dir` gives a path git never reads — the entry would look installed and exclude nothing (`TestAddFleetExcludeFromLinkedWorktree` proves it with `check-ignore`). The entry is therefore repo-wide and written once, idempotently, and it is `.gitignore`'s opposite on purpose: `.gitignore` is tracked, so writing it would dirty a fresh worktree and risk committing customer screenshots. The exclude is written **before the first byte**, since a window where the files exist and the exclude does not is a window where `git add -A` sweeps a customer screenshot into a commit. - Branch names are `brz-3182-` (`BranchNameFor`), **not** Linear's own `branchName`, which carries an owner prefix (`alice/brz-3182-…`). Linear links a PR by finding the identifier anywhere in the name, so both link identically; this form matches the convention already on disk. - Inference is **team-gated** (`IdentifierFromBranch`). The regex is deliberately loose about the prefix because the *caller* gates on the repo's real team keys; ungated it reads `fix-123-thing` as `FIX-123` and `release-2024-cleanup` as `RELEASE-2024`, identifiers for teams that don't exist. The gate is what makes a non-ticket branch cost nothing. -- **Nothing polls.** Ticket work is event-driven and one-shot: at worktree creation, and at session creation when the branch names an issue and `.fleet/ticket//` is absent — the directory is the ledger, so it survives restarts and deleting it is the natural "refresh". `TestTicketWorkStaysOffTheWorkers` keeps it out of `refreshAllGitAndPR`, whose `workerStallThreshold` (90s) is already sized against ~70s of git + `gh` per repo. This is only affordable because there is no badge, hence no live state to keep fresh. +- **Nothing polls, and a prompt is seeded exactly once.** Ticket work is event-driven and one-shot: it happens at **worktree creation and nowhere else** (`w` + a ticket, `fleet worktree --ticket`), so a session added by hand with `a`/`n`/`A` starts empty like every other manually created session. `handleSessionCreate` therefore does no inference at all — it passes `msg.prompt` through untouched. fleet used to infer a ticket from the branch on *every* session creation, plus reuse the already-written `prompt.txt` when `.fleet/ticket//` was present. Both re-asked the original task on every session after the first, and the reuse branch never looked at the branch at all, so a worktree that once held a ticket kept seeding it after the checkout had moved on to `master`. A seeded first message is the *gesture* of starting a worktree from a ticket, not a property of the directory. `TestTicketWorkStaysOffTheWorkers` keeps it out of `refreshAllGitAndPR`, whose `workerStallThreshold` (90s) is already sized against ~70s of git + `gh` per repo. This is only affordable because there is no badge, hence no live state to keep fresh. - The one mutation resolves the team's started state by **type**, against a position-sorted list (`TestStartedStateResolvesByTypeAndPosition`), so it works on a team whose started state is called "In Dev" or "Doing". Position matters as much as type: a real team has several started states (In Progress at position 2, In Review at 1002) and the lowest is what a human means by "I'm starting this" — any other choice would move a fresh ticket straight to review. Fires **only on create-from-ticket** (config `linear_ticket_start`, default true), never when a later session opens in an existing worktree — by then a human may have moved the issue on, and dragging it backwards is the worst thing this could do. `meta.json` records `state_write` so it stays exactly-once. - The seeded prompt is a **short pointer that tells the agent not to start** (`TestSeedPromptTellsAgentNotToStart`), stated at the top and bottom because a first message describing a task reads as an instruction to perform it. Line 1 leads with the identifier before the title because it is three surfaces at once: the agent's instruction, the preview pane's prompt strip, and the input to `naming.GenerateTitle`, which cuts at ~50 runes. It rides `sessionCreateMsg.prompt` -> `Session.InitialPrompt`. - **Ticket suggestions live in the `w` dialog's existing New branch field, not a new field and not a mode** (`internal/ui/workspace_picker_ticket.go`). The field IS the literal option, so nothing duplicates it and only one thing ever claims Enter. Two rules make that hold: **exactly one highlight, and the caret lives with it** — arrowing onto a ticket blurs the input, typing returns both and keeps the keystroke (`isTypingKey`, borrowed from the snooze dialog, whose "the highlight is the promise" rule this follows); and **shape decides the default, never a mode** — text matching a team's identifier shape resolves *in place* (`LooksLikeIdentifier`), prose stays literal with tickets one down-arrow below. *In place* means the field **becomes the branch name** (`applyResolvedBranchName` → `BranchNameFor`) while the highlight stays put; it does **not** mean "only record the ticket". That half was missing at first, so typing `BRZ-3217` fetched the ticket, materialized it and named the session after it — and then created a git worktree literally called `BRZ-3217`, breaking the invariant `pickTicket`'s own comment states, that both ways of naming a ticket end up identical. The rewrite is gated on the field still holding **nothing but that identifier**: the generation counter drops a reply a later keystroke invalidated, but it cannot see a *current* reply for a shorter identifier you paused on en route (`BRZ-321` while typing `BRZ-3217`), which would otherwise drop `brz-321-` under the cursor and let the rest of the typing land on the end of it. The same check leaves a tail you edited by hand alone. The confirmation line reads **`✓ named from BRZ-3217 · `**, not a bare identifier: it renders in the same place the selectable ticket rows do, so on its own it read as a row you might still need to arrow onto when the naming had already happened — and it is the only thing on screen that explains why the field rewrote itself a moment earlier. The wording holds in both states `d.resolved` can be in, including a tail you typed yourself, because `onFieldChanged` drops the resolution the moment the text stops leading with the identifier. `✓` is U+2713, East-Asian-Neutral, so it is always one column — the same width check the priority gauge had to pass. The highlight never moves on its own; a picker that jumps its own selection is the ambiguity coming back through the window. `setSelection` is the single writer of `focus`/`ticketCursor`, enforced by `TestWorktreeSelectionMutatorIsTheOnlyWriter`, because a stray write skips the clamp and renders two selection markers. The footer names what Enter will do and changes as the highlight moves. diff --git a/changelog/unreleased/manual-session-no-ticket-prompt.md b/changelog/unreleased/manual-session-no-ticket-prompt.md new file mode 100644 index 00000000..a1acc3ce --- /dev/null +++ b/changelog/unreleased/manual-session-no-ticket-prompt.md @@ -0,0 +1,5 @@ +--- +type: fixed +--- + +**Fresh sessions start fresh** — A session you add by hand with `a`/`n`/`A` no longer inherits the Linear ticket prompt from the worktree it sits in. Only the session created alongside the worktree is briefed on the ticket. diff --git a/internal/linear/linear_test.go b/internal/linear/linear_test.go index dcab651e..5cab148d 100644 --- a/internal/linear/linear_test.go +++ b/internal/linear/linear_test.go @@ -443,38 +443,6 @@ func TestAuthHeaderFormDiffersByKind(t *testing.T) { } } -func TestExistingPromptIsTheReuseLedger(t *testing.T) { - wt := t.TempDir() - if _, ok := ExistingPrompt(wt); ok { - t.Error("empty worktree should have no prompt") - } - dir := TicketDir(wt, "BRZ-3182") - if err := os.MkdirAll(dir, 0755); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(dir, promptFile), []byte("seeded"), 0644); err != nil { - t.Fatal(err) - } - got, ok := ExistingPrompt(wt) - if !ok || got != "seeded" { - t.Errorf("ExistingPrompt = (%q, %v), want (seeded, true)", got, ok) - } -} - -func TestNegativePinStopsRefetch(t *testing.T) { - wt := t.TempDir() - if NegativelyPinned(wt, "FIX-123") { - t.Error("nothing pinned yet") - } - pinNoTicket(wt, "FIX-123") - if !NegativelyPinned(wt, "FIX-123") { - t.Error("a branch that resolved to no-such-issue must cost one subprocess ever, not one per session") - } - if NegativelyPinned(wt, "BRZ-1") { - t.Error("the pin must be identifier-specific") - } -} - // resetCredentialForTest drops the cached credential so a test can re-resolve. func resetCredentialForTest() { credState.mu.Lock() diff --git a/internal/linear/materialize.go b/internal/linear/materialize.go index b9b39f4d..62670d2a 100644 --- a/internal/linear/materialize.go +++ b/internal/linear/materialize.go @@ -3,7 +3,6 @@ package linear import ( "context" "encoding/json" - "errors" "fmt" "os" "path/filepath" @@ -20,13 +19,12 @@ import ( // JetBrains Fleet owns .fleet/ in project roots and a repo may legitimately // commit .fleet/settings.json. const ( - fleetDir = ".fleet" - ticketDir = "ticket" - imagesDir = "images" - ticketFile = "ticket.md" - promptFile = "prompt.txt" - metaFile = "meta.json" - noTicketPin = ".no-ticket" + fleetDir = ".fleet" + ticketDir = "ticket" + imagesDir = "images" + ticketFile = "ticket.md" + promptFile = "prompt.txt" + metaFile = "meta.json" ) // Result describes what a Materialize call put on disk. @@ -72,46 +70,6 @@ func TicketDir(worktreePath, id string) string { return filepath.Join(worktreePath, fleetDir, ticketDir, strings.ToUpper(id)) } -// ExistingPrompt returns a previously materialized prompt for this worktree. -// -// This is the fast path and the steady state: every session after the first in -// a ticket worktree hits it, at the cost of one ReadDir and one ReadFile, with -// no network. The filesystem is the ledger — it survives -// restarts, survives losing state.db, and a user who deletes the directory gets -// a re-fetch, which is the natural "refresh this ticket" gesture. -func ExistingPrompt(worktreePath string) (string, bool) { - base := filepath.Join(worktreePath, fleetDir, ticketDir) - entries, err := os.ReadDir(base) - if err != nil { - return "", false - } - for _, e := range entries { - if !e.IsDir() { - continue - } - data, err := os.ReadFile(filepath.Join(base, e.Name(), promptFile)) - if err == nil && len(data) > 0 { - return string(data), true - } - } - return "", false -} - -// NegativelyPinned reports whether this worktree's branch was already resolved -// to "no such issue", so inference does not re-ask on every session start. -func NegativelyPinned(worktreePath, id string) bool { - data, err := os.ReadFile(filepath.Join(worktreePath, fleetDir, ticketDir, noTicketPin)) - return err == nil && strings.EqualFold(strings.TrimSpace(string(data)), id) -} - -func pinNoTicket(worktreePath, id string) { - dir := filepath.Join(worktreePath, fleetDir, ticketDir) - if err := os.MkdirAll(dir, 0755); err != nil { - return - } - _ = os.WriteFile(filepath.Join(dir, noTicketPin), []byte(id), 0644) -} - // Materialize fetches a ticket and writes it, with its screenshots, into the // worktree. // @@ -138,12 +96,6 @@ func Materialize(ctx context.Context, o Opts) (Result, error) { // team's workflow states so the optional state write needs no second query. issue, err := fetchFull(ctx, id) if err != nil { - // errors.Is, not ==: a wrapped sentinel would skip the negative pin and - // make inference re-ask Linear on every session start — the exact cost - // NegativelyPinned exists to avoid. - if errors.Is(err, ErrNotFound) { - pinNoTicket(o.WorktreePath, id) - } return res, err } diff --git a/internal/ui/app.go b/internal/ui/app.go index 76e362b3..da14ff6c 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -1751,18 +1751,6 @@ func (h *Home) Update(msg tea.Msg) (tea.Model, tea.Cmd) { dialog, cmd := h.connectLinear.Update(msg) h.connectLinear = dialog return h, cmd - case ticketReadyMsg: - // Inference finished. The session starts either way — a Linear failure - // costs the seeded prompt, never the pane. - if line := ticketStatusLine(msg.res, msg.err); line != "" { - h.setInfo(line) - } - create := msg.create - if msg.res != nil { - create.prompt = msg.res.Prompt - } - return h, h.startSessionCmd(create) - case deleteCleanupDoneMsg: for i, pd := range h.finalizingDeletes { if pd.Session.ID == msg.sessionID { @@ -3376,16 +3364,11 @@ func (h *Home) handleSessionCreate(msg sessionCreateMsg) (tea.Model, tea.Cmd) { h.setInfo(conflict.Message(msg.account)) } } - // A branch that names a Linear issue gets the ticket read for it. The fast - // path (a worktree already materialized) is one stat and returns inline; - // only a first-time fetch defers the launch, and even then a failure starts - // the session anyway. - if prompt, cmd := h.ticketPromptFor(msg); cmd != nil { - h.setInfo("Fetching the Linear ticket for this branch…") - return h, cmd - } else if prompt != "" { - msg.prompt = prompt - } + // msg.prompt is whatever the caller set and nothing more. Deliberately no + // inference here: a seeded first message is the worktree-creation gesture + // ("start on this ticket"), and a session added by hand to a checkout that + // already holds a materialized ticket is not that gesture — it re-asked the + // original task, forever, on every session after the first. return h, h.startSessionCmd(msg) } diff --git a/internal/ui/ticket.go b/internal/ui/ticket.go index ee0bf2e4..3d54a11d 100644 --- a/internal/ui/ticket.go +++ b/internal/ui/ticket.go @@ -8,27 +8,17 @@ import ( "strings" "time" - tea "charm.land/bubbletea/v2" "github.com/brizzai/fleet/internal/debuglog" "github.com/brizzai/fleet/internal/linear" - "github.com/brizzai/fleet/internal/session" ) -// ticketMaterializeBudget bounds the whole fetch-and-write step when it runs on -// the session-creation path, where a human is waiting for a pane to appear. +// ticketMaterializeBudget bounds the whole fetch-and-write step, which runs on +// the worktree-creation path, where a human is waiting for a pane to appear. // Generous enough for a ticket with a dozen screenshots on a slow link, short // enough that a wedged request doesn't feel like a hang: the session starts either // way, and past this the prompt simply isn't seeded. const ticketMaterializeBudget = 25 * time.Second -// ticketReadyMsg carries the outcome of an inferred materialization back to the -// Update loop, with the session-creation request it was blocking. -type ticketReadyMsg struct { - create sessionCreateMsg - res *linear.Result - err error -} - // materializeTicket writes a Linear ticket and its screenshots into a freshly // created worktree. // @@ -56,83 +46,6 @@ func materializeTicket(worktreePath string, t *linear.Ticket, moveState bool) (* return &res, nil } -// ticketPromptFor resolves the first message for a session about to start in -// path, when that path's branch names a Linear issue. -// -// Runs on the Update goroutine, so it does no I/O beyond local reads on a known -// path: the branch comes from the git cache the worker already maintains, the -// identifier is a regex, and the reuse check is one ReadDir plus one ReadFile. -// That last check is the steady state — every session after the first in a -// ticket worktree hits it, with no network at all. -// -// It used to call session.GetRepoRoot, twice, and that comment was false: on a -// cache miss GetRepoRoot shells out to `git rev-parse` with an 8-second ceiling, -// on the goroutine that paints every frame. LookupRepoRoot never shells out, and -// a miss falls back to the path itself — which is the right answer for a -// worktree and for a main repo, since `rev-parse --show-toplevel` returns the -// checkout it is run in. Only a session created in a SUBDIRECTORY of a repo -// resolves differently, and there the cost of the miss is that the repo's team -// config is not found and ticket inference stays quiet — the same outcome as a -// repo that names no team, which is the designed opt-out. -// -// Returns (prompt, nil) for the fast path, ("", cmd) when a fetch is needed, and -// ("", nil) when there is nothing to do. -func (h *Home) ticketPromptFor(msg sessionCreateMsg) (string, tea.Cmd) { - if msg.prompt != "" || msg.path == "" || !linear.Available() { - return "", nil - } - if prompt, ok := linear.ExistingPrompt(msg.path); ok { - return prompt, nil - } - - repoRoot, _ := session.LookupRepoRoot(msg.path) - if repoRoot == "" { - repoRoot = msg.path - } - // The per-repo team gate is what keeps false positives free: a branch named - // fix-123 in a repo that tracks no Linear team never costs a round trip. - teamKeys := linear.TeamKeys(msg.path) - if len(teamKeys) == 0 { - if teamKeys = linear.TeamKeys(repoRoot); len(teamKeys) == 0 { - return "", nil - } - } - - branch := "" - if info, ok := h.gitInfo()[repoRoot]; ok && info != nil { - branch = info.Branch - } - id := linear.IdentifierFromBranch(branch, teamKeys) - if id == "" { - // A worktree fleet made is named <repo>-<branch>, so the directory - // still carries the identifier when the git cache is cold. - id = linear.IdentifierFromBranch(pathTailAfterRepo(msg.path, repoRoot), teamKeys) - } - if id == "" || linear.NegativelyPinned(msg.path, id) { - return "", nil - } - - create := msg - path := msg.path - return "", func() tea.Msg { - ctx, cancel := context.WithTimeout(context.Background(), ticketMaterializeBudget) - defer cancel() - // Note MoveState is false on this path, always. Creating a worktree - // from a ticket is an unambiguous "I'm starting this"; opening another - // session in a worktree that already exists is not, and by then a human - // may have moved the issue on. - res, err := linear.Materialize(ctx, linear.Opts{ - WorktreePath: path, - Identifier: id, - MoveState: false, - }) - if err != nil { - return ticketReadyMsg{create: create, err: err} - } - return ticketReadyMsg{create: create, res: &res} - } -} - // pathTailAfterRepo returns the part of a fleet-made worktree directory name // that follows the repo name, e.g. /code/brizzai-brz-3182-fix → "brz-3182-fix". // pathTailAfterRepo takes the resolved repoRoot rather than resolving it again: diff --git a/internal/ui/worktree_ticket_routing_test.go b/internal/ui/worktree_ticket_routing_test.go index a14bba1f..488c6b19 100644 --- a/internal/ui/worktree_ticket_routing_test.go +++ b/internal/ui/worktree_ticket_routing_test.go @@ -99,10 +99,10 @@ func TestWorktreeDialogAsyncMessagesAreRouted(t *testing.T) { // blocking git call. // // session.GetRepoRoot runs `git rev-parse` with an 8-second ceiling on a cache -// miss. ticketPromptFor and sessionsByTicket both run on the Bubble Tea Update -// goroutine — the one that paints every frame — and both called it, one of them -// twice, while ticketPromptFor's own comment claimed it did "no I/O beyond a -// stat". A brand-new worktree is exactly the cache miss. +// miss. sessionsByTicket runs on the Bubble Tea Update goroutine — the one that +// paints every frame — and called it once per session, while the ticket paths' +// own comments claimed they did "no I/O beyond a stat". A brand-new worktree is +// exactly the cache miss. // // Scoped to these two files on purpose. GetRepoRoot is used widely elsewhere in // this package and auditing all of it is a separate job; this pins the paths the @@ -126,3 +126,49 @@ func TestTicketInferenceNeverShellsOutFromUpdate(t *testing.T) { } } } + +// TestManualSessionCreationSeedsNoPrompt pins the one-shot rule: a seeded first +// message is the gesture of starting a worktree from a ticket, not a property of +// the directory that worktree happens to be. +// +// handleSessionCreate used to run ticketPromptFor on EVERY creation — inferring +// an identifier from the branch, and, before that, reusing the prompt.txt +// already sitting in .fleet/ticket/<ID>/. The reuse branch never looked at the +// branch at all, so a checkout that once held a ticket re-asked the original +// task on every session added by hand afterwards, forever, including long after +// the checkout had moved on to master. +// +// The invariant now: sessionCreateMsg.prompt is whatever the caller set, and the +// only caller that sets it is the worktree-creation path (plus `fleet worktree +// --ticket`/`-p`, which build the Session directly). Nothing between the message +// and the launch may write to that field. +func TestManualSessionCreationSeedsNoPrompt(t *testing.T) { + if mentions(t, "handleSessionCreate", "linear") { + t.Error("handleSessionCreate reaches into internal/linear — the only ticket " + + "fetch left is at worktree creation, and putting one back here re-seeds " + + "the prompt into every manually added session") + } + + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "app.go", nil, 0) + if err != nil { + t.Fatalf("parse app.go: %v", err) + } + ast.Inspect(f, func(n ast.Node) bool { + assign, ok := n.(*ast.AssignStmt) + if !ok { + return true + } + for _, lhs := range assign.Lhs { + sel, ok := lhs.(*ast.SelectorExpr) + if !ok || sel.Sel.Name != "prompt" { + continue + } + t.Errorf("app.go:%d assigns to a .prompt field. A first message may only be "+ + "set where the sessionCreateMsg is built (the worktree-creation path); "+ + "writing it on the way to the launch is how every manually created "+ + "session inherited a ticket's prompt.", fset.Position(assign.Pos()).Line) + } + return true + }) +} From 21ca14eb5880b5f94578fe20d10d9f5b65aba70e Mon Sep 17 00:00:00 2001 From: Yuval Hayke <yuval@brizz.ai> Date: Thu, 20 Aug 2026 14:03:42 +0300 Subject: [PATCH 2/2] fix(linear): surface ErrNotFound, and scope the prompt guard to one func Two review findings on the inference removal. ticketStatusLine swallowed ErrNotFound alongside ErrNotConnected, which was right while inference guessed identifiers out of branch names -- "no such issue" was the ordinary answer there. Removing inference left one caller, worktree creation, where the user picked the ticket in the `w` dialog and Materialize re-fetched it: "not found" now means the issue was deleted, or their access to it changed, in the seconds since. Swallowing it hands back a worktree and a session with no prompt and no message at all. It gets its own line, worded rather than %v-formatted because the sentinel reads "linear: issue not found" and would render "Linear: linear: issue not found". ErrNotConnected stays swallowed -- it can barely reach this caller, since the dialog's own fetch had to succeed for there to be a ticket to materialize. The CLI path was never affected; it prints the error to stderr itself. TestManualSessionCreationSeedsNoPrompt walked all of app.go for any AssignStmt assigning to a selector named `prompt`. sessionCreateMsg already declares that field, so the name is live in the package and any future `x.prompt = ...` -- a dialog, a form -- would fail with an error message about Linear tickets inheriting into manual sessions. Scoped to the handleSessionCreate FuncDecl, which is what the doc comment already described. Nothing is lost: the other way to set a prompt is a composite literal (sessionCreateMsg{prompt: ...}), which is how the worktree path legitimately does it and is not an AssignStmt in any file. Verified both directions -- an assignment inside the function fails, one elsewhere in app.go passes. CLAUDE.md carried the inference-era rationale for the swallow, in a line this branch wrote. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WBQUYTfR7kzUjgozd58mr6 --- CLAUDE.md | 2 +- internal/ui/ticket.go | 21 ++++++++++--- internal/ui/worktree_ticket_routing_test.go | 35 ++++++++++++++++----- 3 files changed, 46 insertions(+), 12 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e4e7bc5d..53dfee1b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -151,7 +151,7 @@ Styles in `styles.go` are declared **bare** and constructed **only** in `ApplyPa - **`Available()` and `TeamKeys()` are called from the Update goroutine** and therefore touch no network and no keychain — two atomics and two small file reads. The keychain read happens once, in `warmLinear()` from `Init`. `Resolved()` is separate from `Available()` on purpose: before the warm finishes, "no credential" is ignorance, not a fact, and anything acting on the *absence* of one (the discovery tip) must wait. - Team keys come from `.fleet.json`/`.fleet.local.json` `{"linear":{"team":"BRZ"}}` (a local `team` **replaces** the committed one; `teams` lists append and dedupe — see the `.fleet.json` bullet below), falling back to `team_id` in a committed `.linear.toml`. That file belongs to the CLI fleet no longer uses, but reading one key out of a file someone already has costs nothing and makes this zero-touch for them; **`api_key` in the same file is never read** (`TestTeamKeysReadOnlyTeamID`). Gating is on the **set**, not one key — a workspace routinely has several teams and one repo may see branches from both. - One GraphQL round trip does everything (`issueFullQuery`): description, comments with author and timestamp, labels, assignee, priority, parent/children, attachments, **and the team's workflow states**, so the optional state write needs no second query. Measured at **87 complexity points** against a 10,000-per-query cap; rate limits are 2,500 req/hr and 3M complexity/hr against roughly 2 calls per worktree, so **no throttling machinery exists**. `issue(id: "BRZ-3182")` takes the shorthand identifier. Search is `searchIssues(term:)` — confirmed against the live schema, where `issueSearch(query:)` also exists and neither is deprecated — and is deliberately **unscoped by team**: the repo gate already decides *whether* to search, and someone typing prose wants matches, not a filter they didn't ask for. -- **Error classification cannot key on HTTP status** (`TestGraphQLErrorClassification`). Captured from the live API: an unknown issue returns **HTTP 200** with an `errors[]` entry whose own `extensions` carry `statusCode 400` and the message `Entity not found: Issue`; a bad token returns 401 with code `AUTHENTICATION_ERROR`. Reading the status alone would file "no such issue" as a generic failure: `ticketStatusLine` swallows `ErrNotFound` (a branch naming no real issue is a resting state, not worth a line on the session you just started) and `fleet worktree --ticket` leans on it to reject a bad identifier *before* anything is created. +- **Error classification cannot key on HTTP status** (`TestGraphQLErrorClassification`). Captured from the live API: an unknown issue returns **HTTP 200** with an `errors[]` entry whose own `extensions` carry `statusCode 400` and the message `Entity not found: Issue`; a bad token returns 401 with code `AUTHENTICATION_ERROR`. Reading the status alone would file "no such issue" as a generic failure, and two callers separate it out: `fleet worktree --ticket` leans on it to reject a bad identifier *before* anything is created, and `ticketStatusLine` gives it its own line. That line is new — `ErrNotFound` used to be swallowed as a resting state, which was right while inference guessed identifiers out of branch names and wrong the moment that went away: the only caller left is worktree creation, where the user picked the ticket in the `w` dialog and `Materialize` re-fetched it, so "not found" means deleted or access-changed in the seconds since, and swallowing it leaves a worktree that opens with no prompt and no explanation. It is worded, not `%v`-formatted — the sentinel reads `linear: issue not found`, which would render as `Linear: linear: issue not found`. - **Extensions are recovered, not trusted.** Linear's default alt text is literally `image.png` and its upload URLs carry no filename, so a real PNG would land unnamed and unextensioned — and an agent's file-read tool dispatches on extension, making a perfectly downloaded screenshot unreadable. `detectExt` sniffs magic bytes (`http.DetectContentType`), which also rejects a 401 HTML body that would otherwise sit beside real screenshots. Recovering the extension and rewriting the markdown links are a **matched pair**: fix one and the agent still sees nothing. `findImages` takes **only** `http(s)` targets (`TestFindImagesTakesOnlyRemoteLinks`) — a relative path or a `file:` link in a description is not something fleet has any business reading off disk and copying into a worktree. - Files land at `<worktree>/.fleet/ticket/<ID>/` — inside the worktree so the agent reads them with a relative path and **no permission prompt**, since a prompt on the session's first act renders as `-` waiting, the friction this feature removes. Git exclusion uses `git rev-parse --git-path info/exclude`, **never `--git-dir` + `info/exclude`**: `info` is on git's shared-path list, so a linked worktree's `--git-dir` gives a path git never reads — the entry would look installed and exclude nothing (`TestAddFleetExcludeFromLinkedWorktree` proves it with `check-ignore`). The entry is therefore repo-wide and written once, idempotently, and it is `.gitignore`'s opposite on purpose: `.gitignore` is tracked, so writing it would dirty a fresh worktree and risk committing customer screenshots. The exclude is written **before the first byte**, since a window where the files exist and the exclude does not is a window where `git add -A` sweeps a customer screenshot into a commit. - Branch names are `brz-3182-<slug-of-title>` (`BranchNameFor`), **not** Linear's own `branchName`, which carries an owner prefix (`alice/brz-3182-…`). Linear links a PR by finding the identifier anywhere in the name, so both link identically; this form matches the convention already on disk. diff --git a/internal/ui/ticket.go b/internal/ui/ticket.go index 3d54a11d..88eb5f4b 100644 --- a/internal/ui/ticket.go +++ b/internal/ui/ticket.go @@ -64,10 +64,23 @@ func pathTailAfterRepo(path, repoRoot string) string { func ticketStatusLine(res *linear.Result, err error) string { switch { case err != nil: - // Both are resting states, not failures: a branch that names no real - // issue, and a fleet that was never connected to Linear. Neither is - // worth a line on the session the user just started. - if errors.Is(err, linear.ErrNotFound) || errors.Is(err, linear.ErrNotConnected) { + // ErrNotFound used to be swallowed beside ErrNotConnected, because + // inference guessed an identifier out of a branch name and "no such + // issue" was the ordinary answer for a branch that named none. + // Inference is gone: the only caller left is worktree creation, where + // the user picked this ticket in the `w` dialog and Materialize + // re-fetched it. "Not found" there means it was deleted, or their + // access to it changed, in the seconds since — a real event, and + // swallowing it leaves a worktree that opens with no prompt and no + // explanation. Worded rather than %v-formatted: the sentinel reads + // "linear: issue not found", which renders as "Linear: linear: …". + if errors.Is(err, linear.ErrNotFound) { + return "Linear: issue not found — worktree created without the ticket" + } + // Still a resting state, and barely reachable from this caller anyway: + // the dialog's own fetch had to succeed for there to be a ticket to + // materialize at all. + if errors.Is(err, linear.ErrNotConnected) { return "" } return fmt.Sprintf("Linear: %v — starting without the ticket", err) diff --git a/internal/ui/worktree_ticket_routing_test.go b/internal/ui/worktree_ticket_routing_test.go index 488c6b19..2dbcc0a9 100644 --- a/internal/ui/worktree_ticket_routing_test.go +++ b/internal/ui/worktree_ticket_routing_test.go @@ -140,8 +140,18 @@ func TestTicketInferenceNeverShellsOutFromUpdate(t *testing.T) { // // The invariant now: sessionCreateMsg.prompt is whatever the caller set, and the // only caller that sets it is the worktree-creation path (plus `fleet worktree -// --ticket`/`-p`, which build the Session directly). Nothing between the message -// and the launch may write to that field. +// --ticket`/`-p`, which build the Session directly). handleSessionCreate, which +// sits between the message and the launch, may not write to that field. +// +// The walk is scoped to that one function rather than to all of app.go. +// sessionCreateMsg already declares a `prompt` field, so the name is live in +// this package: a file-wide walk fails any future `x.prompt = …` — a dialog, a +// form — with an error message about Linear tickets, which is a misleading +// failure aimed at an innocent change. Scoping loses nothing, because the other +// way to set a prompt is a composite literal (`sessionCreateMsg{prompt: …}`, +// how the worktree path legitimately does it at app.go), which is not an +// AssignStmt and was never caught in any file. A rename is loud, not silent: +// the lookup fails the test rather than walking nothing. func TestManualSessionCreationSeedsNoPrompt(t *testing.T) { if mentions(t, "handleSessionCreate", "linear") { t.Error("handleSessionCreate reaches into internal/linear — the only ticket " + @@ -154,7 +164,17 @@ func TestManualSessionCreationSeedsNoPrompt(t *testing.T) { if err != nil { t.Fatalf("parse app.go: %v", err) } - ast.Inspect(f, func(n ast.Node) bool { + var fn *ast.FuncDecl + for _, decl := range f.Decls { + if d, ok := decl.(*ast.FuncDecl); ok && d.Name.Name == "handleSessionCreate" { + fn = d + break + } + } + if fn == nil { + t.Fatal("handleSessionCreate not found in app.go — renamed? this guard is now vacuous") + } + ast.Inspect(fn, func(n ast.Node) bool { assign, ok := n.(*ast.AssignStmt) if !ok { return true @@ -164,10 +184,11 @@ func TestManualSessionCreationSeedsNoPrompt(t *testing.T) { if !ok || sel.Sel.Name != "prompt" { continue } - t.Errorf("app.go:%d assigns to a .prompt field. A first message may only be "+ - "set where the sessionCreateMsg is built (the worktree-creation path); "+ - "writing it on the way to the launch is how every manually created "+ - "session inherited a ticket's prompt.", fset.Position(assign.Pos()).Line) + t.Errorf("app.go:%d assigns to a .prompt field inside handleSessionCreate. "+ + "A first message may only be set where the sessionCreateMsg is built "+ + "(the worktree-creation path); writing it on the way to the launch is "+ + "how every manually created session inherited a ticket's prompt.", + fset.Position(assign.Pos()).Line) } return true })