TKT-021 + TKT-022: vet chat and studio project switching - #12
Merged
Merged
Conversation
In a studio, `bobby ticket move/view/assign` run from a worktree reach the studio board only because findProjectRoot walks UP from the worktree cwd to the studio root — which holds only while the worktree is a filesystem descendant of the studio root. A worktree_root configured outside the studio would silently write stage moves to the wrong board (or fail to find one). resolveWorktreeRoot now takes an optional studioRoot and, when config.studio is set and studioRoot is known, throws a clear config error naming worktree_root, the studio root, and the fix if the resolved path lands outside the studio. computeWorktreePlacement forwards studioRoot so the guard fires at worktree creation, before any agent is spawned; the orchestrator passes this.repoRoot (the studio/launch root). Path containment is symlink-safe: the longest existing prefix is realpath'd (macOS /var → /private/var) and the not-yet-created tail re-appended, since worktree_root usually does not exist yet. Non-studio (v1) projects are unaffected — no `studio` key, no validation — and both new params are optional, so existing callers keep their behaviour. The two functions have no other call sites. 4 tests: refuse-outside, accept-under, accept-default-descendant, non-studio-unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add conversational planning: a ChatManager holding sessions, executor --resume passthrough for continuity across turns, a plan permission mode so the agent cannot write while the user is still discussing, and .bobby/chats.json for persistence. - executor.js: --resume flag; claudeSessionIdFromEvent to capture the Claude session id off the stream. - state.js: chatMode / chatId / chatHistory workspace fields. - orchestrator.js: runChatTurn (plan + commit modes) with its own exit handler so a read-only discussion turn is never mislabeled a no-op; _launch gains permissionMode/resume/onEvent/onExit overrides. - chat.js (NEW): ChatManager — startChat/sendMessage/commitPlan/listChats, persisted to .bobby/chats.json. - server.js: POST/GET /api/chats, /api/chats/:id, /message, /commit (501 when no manager is wired). - app.js/dashboard.js/remote.js: wire the ChatManager. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add ProjectContext, a thin mutable holder of the active studio project, and wire it through the orchestrator and server so the app can switch projects without a restart. - lib/dashboard/project-context.js (NEW): resolves the active project's board paths, switchTo() re-points them in place and persists via .bobby/active- project, isStudio()/listProjects() gate the feature. Inert off-studio. - Orchestrator: ticketsDir/sessionsDir are now getters that read live off the project context (fallback to constructor args); switchProject() delegates and broadcasts project_switched. No lifecycle event — running agents untouched. - server.js: GET /api/projects, POST /api/projects/select (400 off-studio), /api/config now carries isStudio + activeProject, ticket/brief/feature routes read the active board live, /api/workspaces re-scopes to the active project by board membership (repo runs always shown). - commands/dashboard.js + remote.js: construct and pass a ProjectContext. Tests: project-context.test.js (TC-1..4,12) and project-api.test.js (TC-5..11 + workspace scoping + persistence). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Switching projects mid-run moved the board out from under the running agent's bookkeeping. `this.ticketsDir` is a live getter over the project context, and the exit path read it: with the UI on beta, an alpha run's exit asked beta's board how the run went. Absent id -> newStage null -> stageAdvanced false, so a successful run that had moved its ticket was recorded as a no-op and never reached awaiting_approval. Where both boards held the same id it was worse and silent — an unrelated ticket's stage decided nextStatus, and auto-approve could launch the next agent against the wrong project. Session logs split: header in A, tail in B. A workspace now pins the board it was created on, exactly as TKT-069 pinned its target repo, and every per-workspace read goes through _ticketsDirFor(ws)/_sessionsDirFor(ws) — the exit stage re-read, prompt building, the existence check, feature children, session init and exec-event logging, readLatestSessionFile, mergedAt. The getters stay live for the UI's board and for picking a NEW ticket off it, so switchProject re-scopes the UI exactly as before. Records with no pin fall back to the live board, which is the only board they have. scopeToProject answers ownership from the pinned dir instead of looking the ticket up on the active board — exact, collision-proof, and one listTickets Set per request rather than a findTicket readdir per workspace (the review's perf note). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Re-review artifact. AC3 verified fixed adversarially (pin reverted → 5/6 of the new orchestrator-project-pin suite red; restored → 6/6 green). Rejected on a different defect: ProjectContext is wired into commands/dashboard.js, which bin/bobby.js never registers, and not into commands/app.js — so the shipped `bobby app` still reports isStudio:false and 400s both project routes on a real two-project studio. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The feature was complete and unreachable. ProjectContext was threaded
into commands/dashboard.js — a module bin/bobby.js does not register,
since commands/app.js carries .alias('dashboard') — so the shipped
`bobby app` built an Orchestrator without it and answered isStudio:false
and 400 on both project routes. Every unit test was green throughout,
because each one hand-wires the orchestrator the command does not.
commands/app.js now constructs the context and passes it. The orphan is
deleted (TKT-070) rather than left as a second, more correct-looking copy
of the same wiring for the next feature to land in.
The new e2e suite spawns the real `node bin/bobby.js app` against a real
two-project studio and asserts the switch re-scopes GET /api/tickets, and
against a real single-project repo to hold the off-studio case inert. Remove
the wiring and its 4 studio tests go red while the 2 off-studio stay green.
A unit test that builds its own server cannot make either claim.
Also from review:
- /api/sessions read the BOOT config's dir and never followed a switch.
- createRepoRun pinned sessionsDir but not ticketsDir, so a repo run
created before a switch would prompt against one board and log to another.
- Decision hygiene: orchestrator-reads-tickets-from-the-shared-board still
literally named this.ticketsDir for four reads that are now per-workspace.
Invalidated and replaced by orchestrator-reads-tickets-from-the-workspaces-
own-board, which restates TKT-051's substance and absorbs yesterday's pin
decision, so one active entry covers which board is read.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cycle-3 review artifact. Rejected on two new defects: B1 (regression, b823e54): ProjectContext re-derives the active project from getActiveProject() alone, dropping the BOBBY_PROJECT/--project tier that readConfig already resolved into config._project. Because orchestrator .ticketsDir prefers the context, `bobby app --project beta` serves alpha's board. Proven before/after against e1e932c on one fixture. B2 (AC2): switching re-scopes paths but not the project config, so a ticket created after a switch takes the boot project's ticket_prefix onto the new project's board (AL-001 on beta). ProjectContext.config exists but nothing consumes it, and its cascade leaks the boot project's keys. Verified good: the cycle-2 wiring fix, the AC3 pin (re-broken and restored), the commands/dashboard.js deletion, and the decision swap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects, one cause. ProjectContext reimplemented config resolution
and got it wrong in both directions.
B1 (a regression from b823e54). `bobby app --project beta` served
ALPHA's board. bin/bobby.js turns --project into BOBBY_PROJECT and
resolveActiveProject resolves explicit > env > active-project file >
sole project into config._project. ProjectContext threw that away and
re-derived from the FILE alone, and since orchestrator.ticketsDir prefers
the context, the narrower answer won — while /api/config still reported
"beta" beside an alpha board. The bug was latent from the start; wiring
the context into the shipped command is what made it live.
B2 (AC2). Switching re-scoped paths but not config. ticket_prefix is a
project key read from the boot closure, so booting on alpha and switching
to beta minted AL-001 into beta's board. ProjectContext.config existed but
nothing consumed it — and its cascade was `{...bootConfig, ...projectFile}`,
which both skipped the real cascade's rules (ticket_prefix precedence, the
git_conventions/dashboard/workflows deep merges, project_repos vs
repo_group) and started from an already-cascaded config, so a key present
in alpha survived a switch to beta. Consuming it naively would have traded
a stale prefix for a leaky one.
The fix is to delete the second implementation. lib/config.js now exposes
configForProject(root, project) — the same cascade readConfig runs, over a
pristine studio base — and readConfig's studio path and the new function
share one cascadeProject so they cannot drift. ProjectContext takes
config._project and calls configForProject. server.js gains activeConfig()
alongside boardDir()/sessionsBoardDir(), feeding both createTicket sites.
Reverting either half turns 5 of the 18 new tests red.
Also the five non-blocking notes on the e2e suite: kill the child on
startup timeout; listen on 'close' not 'exit' so the EADDRINUSE retry can
actually see the message (it was dead code); say what the off-studio
describe really guards instead of overclaiming; make the 400 test read its
own before-state instead of depending on its predecessor; and set
BOBBY_NO_REGISTRY=1 and clear BOBBY_PROJECT in the spawn env so a test run
cannot write to the developer's ~/.bobby or be masked by a stale export.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cycle-4 review artifact. Rejected on B3: the project switch re-scopes the board and (as of 8d469a2) the ticket prefix, but not the orchestrator's config — so a workspace created after a switch is cut from the previous project's REPO, and /api/config + /api/workflows still answer for the boot project. B1, B2, the AC3 pin and the cycle-2 reachability fix all re-verified live from scratch. The lib/config.js refactor is proven behaviour-preserving: readConfig at efbf2cd vs HEAD deep-compared across 8 fixture shapes plus the BOBBY_PROJECT case, all identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
B3, the worst defect this ticket has produced. `project_repos` is a
PROJECT key and decides which repository a ticket's worktree is cut from.
The Orchestrator held the boot config and nothing re-pointed it, so a
workspace created after a switch resolved its target repo from the
PREVIOUS project: a beta ticket getting a worktree of alpha's repo, where
an agent then edits the wrong codebase with nothing in the UI to hint at
it. Two more from the same cause: /api/config reported the old project's
name and stack beside the new activeProject, and /api/workflows listed the
old project's pipelines, which createWorkspace then refused.
The seam existed — activeConfig() — and had been applied to the two sites
the failing test pointed at. That was fixing the repro, not the defect.
Now the same live/pinned split the board already uses:
- `get config()` follows the active project (the UI's question)
- `_configFor(ws)` reads the project the workspace was created in, and
every run-scoped read goes through it: permission posture, executor,
model, workflow resolution, and the whole prompt context
- the workspace record pins `project`, the NAME the dirs and the config
are both derived from, so three parallel pins cannot disagree
C2 was scored non-blocking but is worse than that: the agent's own
`bobby ticket move` resolves its board through resolveActiveProject, which
falls back to `.bobby/active-project` — the file the UI rewrites on every
switch. An alpha run whose user moved to beta moved its ticket on BETA's
board. That is the AC3 bug one process further out, straight through the
pin. The child is now launched with BOBBY_PROJECT set to the run's own
project, the top of that precedence chain.
C1: `_resolveTo` now reads the target config BEFORE assigning any state.
It goes to disk and can throw, and a half-applied switch — projectName on
beta, board paths on alpha — is worse than a refused one.
Reverting B3, the config pin, or C2 fails exactly its own test and no
others.
Decision hygiene: `concurrency-cap-refuses-per-server-process` said the cap
was "per project per server process", which this made false — one
orchestrator now spans projects, so the budget is shared across them.
Invalidated and re-recorded as concurrency-cap-refuses-per-orchestrator
with that consequence stated; the code comment matches.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cycle-5 review artifact. Rejected on D1 (auto_approve_stages read live in _onExit) and D2 (_pipelineFor short-circuits to the boot-captured this.pipeline for the default workflow) — both proven, both invisible to the green suite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two reads in the exit path still resolved against the boot/live config after a mid-run switch, both invisible to the green suite because the fixtures gave every project identical settings. D1 (AC3) — `auto_approve_stages` (a per-project `dashboard.*` key) was read live in `_onExit`, the one method where every other decision is pinned. Alpha `[]` / beta `['planning']`: an alpha run exiting with the UI on beta got swept forward and `approve()` launched an agent alpha never sanctioned. The mirror stranded a run alpha meant to advance. Now `_configFor(ws)`. D2 (AC2) — `_pipelineFor` short-circuited to the constructor-captured `this.pipeline` whenever `ws.pipeline === this.pipelineName`, i.e. for every `default` workspace. `workflows` is per-project and a workspace records only the NAME, so a beta run advanced through alpha's `default`, silently skipping any stage beta added — a security stage among them. Now always resolved against the run's own project config, degrading a deleted named workflow to that project's default rather than the boot one. This was not a `this.config` read, so the getter-conversion grep could not surface it: converting a field to a getter is only half the audit, the other half is every constructor field derived from the config. Test rework (the fixtures are what hid both): - makeStudio takes per-project config and deep-merges `dashboard`, so two projects can hold DIFFERENT settings — the single-root fixture could not express the live-vs-pinned distinction, which is why D1 hid. - seed() now goes through the REAL createWorkspace (studio root is a git repo), so it can no longer pin project/dirs differently than production. Three fixture divergences in this ticket's history were hand-built records; this closes that off. - two behavioural tests: auto-approve follows the run's project both directions; a run advances through its own workflow and does not skip beta's security stage. Reverting either fix fails exactly its test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… seam Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… pkg) RelayTransport plumbing verified end to end against a real loopback relay and a headless phone (AC1 pass). AC2/AC3 fail on a deterministic, view-observable bug: the app boots loadConfig() before the relay socket is online, the request fails and is swallowed, and only refresh() — not loadConfig() — is retried on presence-online, so the phone shows no project name and degraded board lane order for the whole session. Violates one-frontend-two-transports. The buggy code (app/app/app.js, RelayTransport) ships in @bobbycode/pro-dashboard, not this repo, so the fix lands there. Recording the finding, evidence, and diagnosis on the ticket; rejected to building. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cycle-6 review: D1/D2 verified fixed (adversarial reverts each fail exactly their own test), constructor-captured config derivatives re-audited, B3/C1/C2 re-verified from source. Approved with notes. Records decision run-scoped-reads-pin-to-the-workspaces-project. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
N1 — _configFor's error fallback (run's project deleted/corrupted mid-run) returns the BOOT config instead of the live one: immutable, always valid, and exactly right in the common case where the run's project is the boot project. A plain switch never reaches this catch, so AC3 is unaffected; this only hardens the degenerate path. N2 — the pin-suite header still described the old hand-built, plain-dir fixture; updated to reflect the real createWorkspace / git-backed studio it now uses, and to note headSha/commitCheckpoint run for real but are not load-bearing (every run advances a stage, short-circuiting the no-op check). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TKT-021 and TKT-022 are PR'd as #12 and stay in shipping: CI is red on the PR for a failure that predates it (3 tests in test/lib/project.test.js, already failing on main since a3fe211), so neither ticket is moved to done. Filed TKT-072 for that CI failure with what is ruled out so far. Also picks up the bobby-lighthouse skill scaffold and detected feature areas, both regenerated by running the CLI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The chat and project API suites, and the one HTTP case in the FSM suite, awaited server.close() without first dropping the client connection that fetch leaves open. On Node 18 close() waits on that idle keep-alive socket, so its callback never fires and every HTTP round-trip test in those files hit the 5s jest timeout — 15 failures on the Node 18 leg of the matrix, including all of TKT-021's chat routes and all of TKT-022's project routes. Node 20 and 22 close idle connections themselves, which is why this was invisible locally and on two thirds of the matrix. server-api.test.js already does exactly this; these three just missed it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The three createProject failures on the Linux runners assert `committed` first, which prints a bare `false`, and `makes the initial commit` shells out to `git log` and dies on "branch 'master' has no commits yet". Neither says why the commit was refused, so CI has never reported the actual git error behind a failure that has been red on main since a3fe211. Assert `commitError` first in all three — it holds git's own message. Diagnostic value only; no production code changes and the suite is unchanged on a machine where the commit succeeds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
commitError kept only the first line of the exec error, which is the command line itself — so a refused scaffold commit reported "Command failed: git commit -m ..." and nothing about the refusal. That is the whole point of the field: `bobby new` shows it to the user, and the app renders it on a page. execSync puts the child's stderr on `e.stderr` (stdio is already 'pipe'), so append it. Verified against a forced failure: commitError now reads "Command failed: git commit -m ...: <git's own words>". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Root cause of the three createProject failures, red on main since a3fe211: git refuses the scaffold commit with "empty ident name (for <runner@...>)". A bare runner has no git identity and its `runner` user has an empty gecos, so git cannot auto-detect a name either. The test tried to arrange one by setting GIT_AUTHOR_NAME and friends on process.env, which does nothing here — a Jest ESM module gets a COPY of process.env that spawned children never see, a fact this same file already records in the comment above its commit-outcome test. macOS git auto-detects user@host and commits regardless, which is why it only ever failed on Linux. Configure the identity on the runner instead, and drop the env dance that never worked, replacing it with what the file actually depends on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ccevans
added a commit
that referenced
this pull request
Aug 18, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ships the two tickets sitting in
shippingunder the TKT-020 epic ("the app beyond a single project"), plus the follow-up work that landed alongside them.Tickets
TKT-021 — Vet chat: conversational planning with executor
--resumePlanning was one-shot: an agent ran, wrote
plan.md, exited, with no way to push back before it committed. Adds a ChatManager holding sessions, executor--resumefor continuity across turns, plan permission mode so the agent cannot write while you are still arguing,.bobby/chats.jsonfor persistence, and/api/chatsroutes.All 4 ACs verified against the live running server (curl + a PATH-shimmed fake executor capturing real spawn args) — evidence in the ticket's
test-evidence/results.md.TKT-022 — Studio mode: switch projects from inside the app
bobby appserved only the project it was launched in. Adds a single mutableProjectContextand/api/projects/selectso you can move between registered projects without restarting the server; tickets, workspaces and the brief re-scope on switch, and a run in project A is unaffected by switching to project B.Took six review cycles. The recurring failure was config reads left live that should be pinned to the run's own project — the last two (D1
auto_approve_stagesin_onExit, D2_pipelineForshort-circuiting to a constructor-capturedthis.pipeline) were caught only after the test fixture was reworked to give two projects genuinely different config. Both are now proven by adversarial revert: reverting either fix fails exactly its own test and no other.Also included
worktree_rootthat resolves outside the studiocommands/dashboard.js, superseded bycommands/app.jsbuilding, its fix is in the Pro package)Known follow-ups (non-blocking, filed)
server.jshands plugins the boot config/board atbuildServer, so a plugin reading its passed-in config rather thanorchestrator.configsees the boot project after a switch. Pre-existing, but TKT-022's switching is what makes it stale.plan.md— worth a v1 follow-up.Verification
npm test— 1275 passed, 46 skipped, 70 of 71 suites (1 skipped), exit 0npm run lint— 0 errors (37 pre-existing unused-var warnings)origin/main(post-Integrate app + studio (incl. TKT-062 critical permissions fix) #11); replay was conflict-free and left the tree byte-identical.