Qualify remote refs for worktrees and collapse duplicate Plain intents at boot - #338
johnnylinsf wants to merge 4 commits into
Conversation
…s at boot A local branch literally named origin/main (what a `git fetch origin main:origin/main` typo creates) appeared in the shared tella-fusion checkout at 17:02 UTC on 2026-09-09. Git resolves a bare `origin/main` through refs/heads before refs/remotes, so `rev-parse origin/main` silently answered with the stray branch and `git worktree add -b … origin/main` refused with "ambiguous object name". Every worktree shares those refs, so until the branch was deleted at 22:10 every Plain ticket triage run, both docs-sync runs and a new tella-fusion session died at workspace setup. - worktree.ts, worktree-reaper.ts, warm-template.ts: name remote-tracking refs in full (refs/remotes/origin/<branch>, git-remote-ref.ts) wherever a worktree is created, reset or compared, so a stray local branch can no longer shadow them. Upstream tracking is unchanged: Git still recognises the full ref as a remote-tracking branch. - automations.ts: the boot replay of durable intents keeps one intent per Plain thread and drops intents for threads that already have a live session. The outage left 20 pending intents for 9 tickets (7 for one ticket, from repeated support-card clicks); replaying each would have opened one triage session per attempt. - Tests: a stray refs/heads/origin/main no longer breaks createWorktree, and the intent dedupe keeps the earliest intent, an interrupted run's own intent, and one intent per automation. Co-authored-by: Johnny Lin <[email protected]>
🕙 Outdated review — superseded by a newer review below🤖 OS review · request changes · quality 3/5 · risk highSafe once the P1 below is fixed. Boot recovery can discard an explicitly requested Plain retrigger. 🔴 Risk high · not fully recoverable · irreversible delete 1 inline comment below. Reviewed 🔁 Not merge-ready and no live session owns this branch — add the |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![open-session-os-tella-dev[bot]](https://avatars.githubusercontent.com/u/65127121?s=60&v=4){kind=small}
| isAutomationRunning(automation.id)) | ||
| ) | ||
| continue; | ||
| const supersededReason = superseded.get(intent.sessionId); |
There was a problem hiding this comment.
🔴 P1 — Do not coalesce explicit Plain retriggers
retriggerAutomationSession deliberately starts a fresh event run using the prior session's automationEvent. If worktree setup fails before the new session is persisted, its durable intent remains, while the original non-archived Plain session is returned by livePlainThreadSessions. On restart, this branch treats the new intent as superseded and deletes it, so the explicitly requested retrigger never runs despite durable acceptance. Persist an intent flag indicating eligibility for Plain launch coalescing, set it only for automatic/thread-card launches, and exclude retriggers from supersededPlainThreadIntents.
There was a problem hiding this comment.
Fixed in 7a1b58e: intents now carry an opt-in coalescePlainThread flag, set only by the automatic launches (the plain:thread_created webhook in agents/plain/handlers.ts and the support-card route in routes/plain.ts) and carried through resume. supersededPlainThreadIntents ignores intents without it, so an explicit retriggerAutomationSession run is never coalesced against the session it was retriggered from; covered by a new unit test. The unit test file passes locally; the host is under heavy IO and memory pressure, so the full bun run check is left to CI.
johnnylinsf
added
the
os-auto-fix
|
🛠️ OS auto-fix — Fixed: comment 3974990785 (pre-flag Plain intents excluded from boot dedupe) in fbdf8cb |
Boot dedupe of durable Plain intents must not swallow an explicit retrigger: retriggerAutomationSession replays the prior session's automationEvent, so if its launch failed before the new session persisted, the original live session made the retrigger's intent look superseded and boot deleted it. Persist a coalescePlainThread flag on the intent, set only by the automatic launches (the plain:thread_created webhook and the support-card triage route), carry it through resume, and have supersededPlainThreadIntents ignore intents without it. Retriggers never set it, so they always replay. Co-authored-by: Johnny Lin <[email protected]>
🕙 Outdated review — superseded by a newer review below🤖 OS review · request changes · quality 3/5 · risk mediumSafe once the P1 below is fixed. The retrigger fix protects new intents but excludes the pre-upgrade outage intents this PR is meant to recover. 🟠 Risk medium · recovery in hours · irreversible delete, wide blast radius 1 inline comment below. Reviewed |
|
|
||
| function plainThreadIdOf(intent: ReplayableIntent): string | null { | ||
| if ( | ||
| !intent.coalescePlainThread || |
There was a problem hiding this comment.
🔴 P1 — Do not exclude the existing outage intents from recovery
coalescePlainThread was not persisted before this PR, so every intent already left by the reported outage has this field absent. This guard therefore ignores all 20 existing intents: for the ticket with seven attempts, supersededPlainThreadIntents returns no superseded entries and boot launches seven sessions. Preserve the retrigger fix while handling legacy Plain intents, for example by identifying pre-upgrade intents belonging to the Plain automation and allowing pending-vs-pending deduplication when no live session exists, while requiring the explicit flag before suppressing against a live session.
There was a problem hiding this comment.
Fixed in fbdf8cb: plainThreadIdOf no longer requires the flag, so every Plain event intent takes part in pending-vs-pending dedupe (earliest accepted per automation and thread replays), including intents written before coalescePlainThread existed. Only flagged intents are dropped against a thread that already has a live session, and the live session's own intent stays the replay for its key, so an explicit retrigger still replays. Three new unit tests cover legacy intents with no live session, legacy intents alongside a live session, and an interrupted run's own unflagged intent. Full bun run check timed out under host load again (load avg ~37); the unit file passes and prettier is clean on the touched files, the rest is left to CI.
supersededPlainThreadIntents ignored every intent without coalescePlainThread, which no intent written before that flag existed carries, so the outage's own 20 intents would all have replayed. Pending intents now dedupe per thread whether or not they are flagged; only flagged ones are dropped for a thread with a live session, so explicit retriggers still replay. Co-authored-by: Johnny Lin <[email protected]>
🕙 Outdated review — superseded by a newer review below🤖 OS review · request changes · quality 2/5 · risk highSafe once the blockers below are fixed. The latest commit resolves the legacy-outage gap but is not yet merge-ready. 🔴 Risk high · not fully recoverable · irreversible delete 2 inline comments below. Reviewed |
| } | ||
| if (coveredByLive(intent, live)) continue; | ||
| if (!kept.has(key)) kept.set(key, intent.sessionId); | ||
| } |
There was a problem hiding this comment.
🔴 P1 — Preserve every explicitly requested retrigger
The latest fix still coalesces unflagged intents against each other. retriggerAutomationSession permits concurrent event runs and leaves coalescePlainThread absent, so if two explicit retriggers for th_a persist and both fail during workspace setup, this map keeps the earlier intent and boot permanently deletes the later user-requested run. Persist coalescePlainThread: false (or a dedicated origin) for new retriggers, reserve absence for legacy automatic intents, and exempt explicit false intents from pending-to-pending coalescing.
| let disposableSandbox: | ||
| | { provider: ReturnType<typeof getSandboxProvider>; id: string } | ||
| | undefined; | ||
| { provider: ReturnType<typeof getSandboxProvider>; id: string } | undefined; |
There was a problem hiding this comment.
🟠 P2 — Restore the formatter-compliant union layout
Required CI currently fails in bun run format:check because oxfmt rejects this changed declaration, so the PR cannot merge as written. Restore the formatter-produced layout before pushing.
| { provider: ReturnType<typeof getSandboxProvider>; id: string } | undefined; | |
| | { provider: ReturnType<typeof getSandboxProvider>; id: string } | |
| | undefined; |
open-session-os-tella-dev
Bot
removed
the
os-auto-fix
fbdf8cb left the type annotation on one line; oxfmt wants each member on its own line, and format:check was the first CI step to fail on the PR. Co-authored-by: Johnny Lin <[email protected]>
🤖 OS review · request changes · quality 3/5 · risk highSafe once the P1 below is fixed. The formatting failure and two earlier recovery issues are addressed, but pending explicit retriggers can still be discarded. 🔴 Risk high · not fully recoverable · irreversible delete 1 inline comment below. Reviewed 🔁 Not merge-ready and no live session owns this branch — add the |
| continue; | ||
| } | ||
| if (coveredByLive(intent, live)) continue; | ||
| if (!kept.has(key)) kept.set(key, intent.sessionId); |
There was a problem hiding this comment.
🔴 P1 — Preserve every explicitly requested retrigger
The latest fix still coalesces unflagged intents against each other. Two calls to /api/automations/retrigger for the same Plain session may overlap; both create event intents without coalescePlainThread, and a workspace setup failure leaves both durable. At restart this map keeps the earlier intent and resumePendingAutomationRuns deletes the later one without executing it, despite retrigger explicitly promising a fresh concurrent run. Persist coalescePlainThread: false from retriggerAutomationSession and exclude explicitly false intents from pending-to-pending coalescing, while continuing to collapse legacy undefined outage intents.
1 participant
Why
At 17:02 UTC on 2026-09-09 a local branch literally named
origin/main(what agit fetch origin main:origin/maintypo creates) appeared in the shared tella-fusion checkout. Git resolves a bareorigin/mainthroughrefs/headsbeforerefs/remotes, sorev-parse origin/mainsilently answered with the stray branch andgit worktree add -b … origin/mainrefused withfatal: ambiguous object name: 'origin/main'. Every worktree shares those refs, so until the branch was deleted at 22:10 every Plain ticket triage run (20 attempts), both docs-sync runs and a new tella-fusion session died at workspace setup with no session persisted.What
worktree.ts,worktree-reaper.ts,warm-template.ts: name remote-tracking refs in full (refs/remotes/origin/<branch>, newgit-remote-ref.ts) wherever a worktree is created, reset or compared. A stray local branch can no longer shadow them. Upstream tracking is unchanged: Git still treats the full ref as a remote-tracking branch (verified in a scratch repo,@{u}=remotes/origin/main).automations.ts: boot replay of durable intents keeps one intent per Plain thread and drops intents for threads that already have a live session. The outage left 20 pending intents for 9 tickets (7 for one ticket, from repeated support-card clicks); replaying each at the next gateway boot would have opened one triage session per attempt. The interrupted run's own intent is never dropped.worktree-start-point.test.tsreproduces the strayrefs/heads/origin/mainand asserts creation from both the default start point and a named base;automation-intent-recovery.test.tscovers the dedupe rules.Other bare
origin/<branch>uses (git-status, pr-info, diff, sandbox bootstrap) are display or sandbox paths and are left for a follow-up.Verification
bun run checkclean in the shared checkout.refs/heads/origin/mainpresent,worktree add -b,worktree add --detach,reset --hard,rev-list --notandmerge-base --is-ancestorall work againstrefs/remotes/origin/mainand fail or mis-resolve against bareorigin/main.Started by Johnny Lin in this OS session