🤖 feat: iterate goals UI with completed-goals history and editable objective#3301
Conversation
|
@codex review Please review this iteration of the workspace Goal UI:
Reads + writes happen under the per-workspace file lock; history is best-effort (failures logged, lifecycle mutation never aborted). Curious for feedback on UX wording ("Archive this goal" vs "Clear goal") and the editInPlace flag design vs a dedicated endpoint. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed6b31e0b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Pushed a fix: added |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f4fbff035
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Addressed your two pieces of feedback:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4bfc593db
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Pushed two more fixes addressing your latest feedback:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b771205e0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Addressed your tie-breaker feedback: |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
ammar-agent
force-pushed
the
goals-ui-qfdv
branch
2 times, most recently
from
e6614a7 to
921cff4
Compare
|
@codex review Three new iterations on the Goal UX:
Plus 12 new tests (5 create-form, 5 label-accent, 2 layout-visibility integration). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1eb4b3d602
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Iteration 3 just landed (commit Key things to look at:
Risks called out in the PR body. |
|
@codex review Addressed P2 in commit Iteration 3 (goal defaults consolidation) is also up for review on top of the previous always-visible/accent/create-form work — please cover the new code as well:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b19d5b4ece
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Addressed your P2 in
Please re-review iteration 3 (goal defaults consolidation). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18e48a9395
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Addressed your latest P2 in
Tests still pass:
|
There was a problem hiding this comment.
💡 Codex Review
mux/src/browser/features/RightSidebar/RightSidebar.tsx
Lines 739 to 743 in 3634340
Fetch provider config before rejecting budget edits
When the user edits an existing goal's budget while useProvidersConfig() is still loading, providersConfig can be null; for custom/provider-mapped models this makes modelHasPricingData(sendMessageOptions.model, providersConfig) return false even though the backend config would resolve pricing. The create path just below fetches api.providers.getConfig() at submit time for this exact case, but inline budget edits still throw UNPRICED_CURRENT_MODEL_GOAL_MESSAGE and block a valid budget until the hook catches up.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1285584a12
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review P1 (mid-stream promote) addressed by rejecting promote when P2 (queued-goal defaults) addressed by applying New test Please take another look. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e7ce9b869
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review Both P2s addressed:
New test: Please take another look. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97ca0ba8bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review P2 (gate promoted budgeted goals on pricing data) addressed by adding
Please take another look. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f5c70b11e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review P2 (double-archive on skipped auto-promote) fixed by hoisting the pricing gate check above the history append in Please take another look. |
`promoteUpcomingGoal` and `promoteNextUpcomingUnlocked` set `status: "active"` and wrote the record without running `applyBudgetDrivenStatus`. A previously-active goal that was demoted back into upcoming may already have exhausted limits (cost >= budget or turnsUsed >= turnCap), so re-promoting it would let the user fire one extra send before the next chunk's accounting flipped it back to `budget_limited`. Fix: run `applyBudgetDrivenStatus` on the parsed record before persisting. Matches `createGoal` (which has the same conditional for new active goals). Both promote paths benefit because `maybeAutoPromoteOnComplete` and `runDeferredAutoPromoteAfterStreamEnd` delegate to `promoteNextUpcomingUnlocked`. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `max` • Cost: `$55.32`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=max costs=55.32 -->
…mpleted goals (Codex P3) The 'Archive this goal' link for completed goals called `clearGoal()`, which records the goal as 'completed' in `goal-history.jsonl`. After the goal-board work, the renderer surfaces those entries under the Completed section — not Archived. Label and behavior were contradicting. Fix: route the click to `workspace.archiveGoal` when the goal is lifecycle-complete; everything else still uses `clearGoal()`. The backend's `archiveGoal` handles the active-goal source path (append history under 'cleared' end-reason + write to archived), and the board's existing dedup keeps it in the Archived section only. Test updated to assert the new `Archive goal` aria-label. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `max` • Cost: `$55.32`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=max costs=55.32 -->
Adding `useAPI()` directly in GoalTab (for the archive-on-complete handler) broke storybook smoke tests — the stories render the GoalTab in isolation without an APIProvider, and `useAPI` throws when the context is null. Fix: mirror the same opt-in pattern already used by `useGoalDefaults` and `useGoalBoard` — read `APIContext` directly with `useContext`, fall back to a null `api`, and have the archive handler short-circuit when `api` isn't available. Same defensive posture, same code path. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `max` • Cost: `$55.32`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=max costs=55.32 -->
When an archived goal that was originally completed gets revived and later promoted, the promote paths spread the goal's existing fields into the new active record — including `completionSummary`. The agent's `get_goal` reads `goal.json` directly and would see the stale 'done' message, potentially confusing the next turn. Fix: explicitly set `completionSummary: undefined` in both promote paths (`promoteUpcomingGoal` and `promoteNextUpcomingUnlocked`) so the active record always lands with a clean summary. Matches the existing `completionSummaryPatch` invariant in `applyMutableFields` which clears the summary for any non-complete status transition. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `max` • Cost: `$55.32`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=max costs=55.32 -->
Child task workspaces (those with `parentWorkspaceId`) can't run any goal action — `WorkspaceGoalService.assertParentWorkspace` rejects every write (setGoal / clearGoal / setGoalDefaults / all board ops). Before this change the GOALS experiment unconditionally injected the Goal tab on every workspace, including child tasks, so users on a child workspace would see the create / queue form whose submits silently failed (`child_workspace` typed error). Fix: read `parentWorkspaceId` from `useWorkspaceMetadata` in RightSidebar and gate Goal tab insertion on it being null. Existing 'remove tab' branch still runs for non-experiment workspaces; child workspaces with the experiment now also fall through removeTab if the tab was previously inserted (handles the case where the user turned the experiment on at a parent, then jumped to a child). --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `max` • Cost: `$55.32`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=max costs=55.32 -->
…x P2) A goal that was completed → archived → revived → promoted → completed-again has BOTH a stale 'completed' history entry AND is the currently-active goal. The previous archiveGoal source order (archived → upcoming → history → active) hit the history branch first, snapshotted the stale row, and returned — leaving the live active goal in place. Result: archive appeared to silently fail for users who had revived a previously-completed goal. Fix: move the active-slot branch to the top of the source-priority ladder (after the already-archived idempotent check). The current ACTIVE record is used for the archived snapshot, so we get the latest accounting rather than a stale history row. All 111 service tests pass; static check passes. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `max` • Cost: `$55.32`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=max costs=55.32 -->
A goal completed → archived → revived → promoted → completed-again has TWO 'completed' rows in goal-history.jsonl with the same id. The board's Completed section was rendering both, surfacing the goal as two cards. Fix: in `getGoalBoard`, track seen goalIds in a `Set<string>` while iterating the (already newest-first sorted) history list, and skip duplicates. We keep only the most recent 'completed' row per goalId. The other dedup passes (active / archived / upcoming) are unchanged. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `max` • Cost: `$55.32`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=max costs=55.32 -->
ammar-agent
force-pushed
the
goals-ui-qfdv
branch
from
1ba1275 to
640495e
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…inline edit Three UX improvements to the multi-goal queue: 1. **Promote mid-stream**: User can now promote a queued goal even while the current goal is streaming. The active stream is interrupted first (via `WorkspaceService.interruptStream` wired into the goal service as `streamInterrupter`), then promotion proceeds under the file lock. This preserves cost attribution — the in-flight turn's tokens belong to the goal that ran them; the next user message starts fresh under the promoted goal. 2. **Surfaced backend errors**: `WorkspaceGoalTransitionError` / `WorkspaceGoalChildWorkspaceError` are translated to `ORPCError` `BAD_REQUEST` at the router boundary so the message reaches the renderer instead of being collapsed to a generic "Internal server error". The GoalBoardSections error banner now shows the real reason a click had no effect. 3. **Inline edit + DnD reorder**: Upcoming rows expose a Pencil button that opens an inline objective+budget editor (new `updateUpcomingGoal` endpoint), and reordering uses `@dnd-kit` (same pattern as Settings → Providers route-priority list) instead of up/down arrow buttons. The "Archive" action is relabeled "Remove" to better match the user's mental model. Backend additions: - `WorkspaceGoalService.setStreamInterrupter` + `waitForStreamSettled` - `WorkspaceGoalService.updateUpcomingGoal` - `GoalBoardUpdateUpcomingInputSchema` + `workspace.updateUpcomingGoal` - Router-side `withGoalErrorTranslation` wrapper for all 7 board handlers - `coreServices.ts` wires the interrupter via `workspaceService.interruptStream` 5 new service tests (116 pass total). --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `xhigh` • Cost: `$179.68`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=xhigh costs=179.68 -->
|
@codex review Follow-up to address three reviewer points:
5 new service tests (116 total pass). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 261e3734d5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… snapshot (Codex P2) When an inline objective edit arrives while the workspace is streaming and `editInPlace: true`, the projected snapshot published to the UI was built via `createGoal(...)`, which minted a fresh goalId + zeroed cost/ turns + cleared budget/turnCap until stream end drained the queued mutation. The persisted mutation renames in place, so the panel briefly showed inconsistent accounting that disappeared on drain. Fix: overlay the rename onto the current record (preserving goalId, costCents, turnsUsed, attributedChildren, budgetCents, turnCap) for the editInPlace branch only. Non-editInPlace branches (replace / new goal) keep `createGoal` semantics. Test: new "mid-stream editInPlace rename returns an optimistic snapshot that preserves goalId + accounting" guards the projection. 117 service tests pass. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `xhigh` • Cost: `$179.68`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=xhigh costs=179.68 -->
|
@codex review Addressed the P2 on mid-stream editInPlace optimistic snapshot:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b3de54041
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…+ project drain semantics on mid-stream rename (Codex P2)
Two goal-lifecycle hygiene fixes:
1. **Hide "Goal cleared" synthetic messages from the chat transcript.**
`appendClearSummary` had been persisting an assistant-role synthetic
message with `uiVisible: true` whenever a goal was cleared, replaced,
or auto-promoted on complete. The renderer stripped the `Goal cleared:`
prefix but still showed the rest as a full assistant chat bubble
("X" — spent $0.00 over N turns (status: paused)) — pure noise, since
the right-sidebar Goal Board already surfaces the same information in
its Completed / Archived sections. Drop `uiVisible: true` so the
message stays in the AI request payload (the model still knows the
previous goal was cleared) but no longer renders as a chat bubble.
Updated `workspaceGoalService.test.ts` assertion accordingly. The
`StreamingMessageAggregator` legacy-prefix strip stays — it still
handles older chat.jsonl entries that have `uiVisible: true`.
2. **Mid-stream editInPlace optimistic snapshot mirrors drain semantics.**
Codex P2 follow-up: when a rename arrives mid-stream and also
tightens `budgetCents` below the already-accrued cost, the previous
projection applied raw mutable fields, so the UI would show an
`active` snapshot until stream end while the eventual persisted record
immediately becomes `budget_limited`. Project through
`applyMutableFields` (which itself ends in `applyBudgetDrivenStatus`)
so the optimistic state matches the drain state. Added a guarding
test that asserts a rename + tightening budget projects directly to
`budget_limited`.
118 service tests pass.
---
_Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `xhigh` • Cost: `$179.68`_
<!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=xhigh costs=179.68 -->
|
@codex review Addressed both items in one push:
118 service tests pass; static-check green. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 354c21f175
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…eExplicitBudget (Codex P2) When `alwaysRequireExplicitBudget` is OFF in goal defaults, blank budget input is intentionally resolved to `null` (no budget) by `resolveGoalSetIntent`, not to `defaultBudgetCents`. But both the empty- state create form (`GoalTab.tsx`) and the queued-goal Adder (`GoalBoardSections.tsx`) were advertising "$X (default)" as the placeholder unconditionally, and the helper text said blank uses the shown defaults — so users with that setting off could submit a blank budget expecting the default to apply when in fact an unbudgeted goal would be created. Fix: gate the placeholder + helper text on `defaults.alwaysRequireExplicitBudget`. - Placeholder when ON: `$X (default)` - Placeholder when OFF: `no budget (default)` - Helper text adjusts to match: when ON, the existing "Leave Budget / Turn cap blank to use the defaults shown above" wording is correct; when OFF, the text now clarifies that blank Budget creates an unbudgeted goal while blank Turn cap still uses the default. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `xhigh` • Cost: `$179.68`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=xhigh costs=179.68 -->
|
@codex review Addressed P2 on budget placeholder/helper text. Both
This matches |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a78e245e91
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…dex P2) The Upcoming Adder only rendered objective + budget fields, so `resolveGoalSetIntent` silently inherited the workspace/global `defaultTurnCap` for every queued goal — users couldn't see or override the turn cap before queuing, and an auto-promoted goal could hit an unexpected turn limit. Added an explicit turn-cap field to both: - `UpcomingAdder` (the "Queue another goal" form): three-column row of budget / turn-cap / Queue button. Placeholder uses the same default-aware pattern as the budget field — "no cap (default)" when the workspace has no default turn cap, "N turns (default)" otherwise. Explicit blank still falls through `resolveGoalSetIntent` (which then applies the default); explicit non-positive integers surface a form error. - `UpcomingRowEditor` (the inline edit form on existing queued rows): new turn-cap input alongside budget, with the same blank-means-no-cap semantics the budget field already uses on this surface (an existing goal's defaults were resolved when it was queued, so blank-on-edit is "clear the cap", not "re-apply the default"). Plumbing: `update` callback in `UpcomingSection` and `onSubmit` shape on `UpcomingRowEditorProps` now carry `turnCap`. The `workspace.updateUpcomingGoal` endpoint already accepted `turnCap`. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `xhigh` • Cost: `$179.68`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=xhigh costs=179.68 -->
|
@codex review Addressed P2 on hidden default turn cap. Added an explicit turn-cap field to both the queue Adder ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 626414bbbd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ace-default saves (Codex P2) Two follow-ups: 1. **Reject partial turn-cap strings.** The new turn-cap inputs in the Upcoming Adder and the inline UpcomingRowEditor used `Number.parseInt`, which silently truncates inputs like `1.5` → 1 and `12abc` → 12. The user would type one value and the queued goal would persist a different cap. Extracted `parseGoalTurnCapInput` (full-string `Number` + `Number.isSafeInteger` validation) to `src/common/utils/goals/ budgetParser.ts` so every entry point — slash command, GoalTab inline editor, Adder, inline editor — validates the same way. `GoalTab.tsx`'s local `parseTurnCapInput` is now an alias for the shared canonical parser (eliminates the duplicated definition that the Codex note flagged as drifting risk). 2. **Serialize workspace-default saves.** Codex P2: rapid edits to two workspace defaults fired multiple concurrent full-record `set` calls to `workspace.goalDefaults.set`. The backend reads → mutates → writes the full config record per request, so a slower first request could complete after a later edit and overwrite it on disk while the UI showed the optimistic latest state — silent reverts on next refetch or restart. Fix in `GoalDefaultsSection.tsx`: chain saves through a `useRef<Promise<void>>` so each request awaits the previous before issuing its own write. The optimistic local `setOverride` still fires immediately so the UI feels snappy, but on-disk state is now guaranteed to match the user's final edit. --- _Generated with `mux` • Model: `anthropic:claude-opus-4-7` • Thinking: `xhigh` • Cost: `$179.68`_ <!-- mux-attribution: model=anthropic:claude-opus-4-7 thinking=xhigh costs=179.68 -->
|
@codex review Addressed both P2s:
|
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Summary
Iterates the right-sidebar Goal tab into the long-lived home for goal
configuration. Builds on the original "completed-goals history + in-place
objective editing" change with three follow-on iterations: always-visible
tab, active-goal accent, in-tab create form, and a new defaults panel
that owns both the per-workspace override and the global defaults that
used to live in Settings → Experiments.
Background
After the first round, the Goal tab still:
workspace had to learn
/goalto start one.turn-cap defaults, and offered no way to keep different defaults per
workspace (e.g., higher budget for the workspace handling a tough
refactor, low budget for everything else).
The user asked us to consolidate goal configuration into the Goal tab
itself — the natural home for goal lifecycle.
Implementation
Original change set (kept)
goal-history.jsonl— Append-only per-workspace history file.workspace.getGoalHistoryreturns newest-first, capped at 200, withcorrupt-line tolerance.
WorkspaceGoalServicehooks —clearGoaland the archive+replacebranch of
setGoalImmediatelywrite history under the file lock beforethe canonical
goal.jsonis rewritten.editInPlaceflag onGoalSetInputSchemaso the objective can berenamed in place (preserves
goalId+ accounting). Inline objectiveeditor on the active-goal card.
Clear is a small underlined link, relabeled to "Archive this goal" for
completed goals.
GoalHistorySectionunder the present goal (or under the emptystate), expandable read-only rows with end-reason + accounting +
completion summary.
Iteration 1: Always-visible Goal tab + active-goal accent + in-tab create form
goalsexperiment is enabled — nolonger gated on
goal != null || goalHistory.length > 0. Surfaces anempty-state with a create form so users don't need
/goalto begin.GoalTabLabelsubscribes to the workspace sidebar state and appliesthe canonical
text-successaccent when a live (status === "active",non-pending) goal exists — mirrors the
useActiveGoalCountpredicate.GoalCreateFormin the empty state with slash-command parity(objective + optional budget + optional turn cap). Cmd/Ctrl+Enter
submits. Blank budget/turn-cap defers to defaults so
/goal, palette,and tab paths produce identical goals.
Iteration 2: Per-workspace overrides + Goal-tab defaults panel
WorkspaceGoalDefaultsOverrideSchemaonboth
WorkspaceConfigSchemaandWorkspaceMetadataSchema. Each field(
defaultBudgetCents,defaultTurnCap,alwaysRequireExplicitBudget)is independently nullable;
nullmeans "inherit from global".workspace.goalDefaults.{get,set}oRPC modeled onworkspace.heartbeat.{get,set}. The service drops the whole recordwhen every field is null, keeping
~/.mux/config.jsonminimal.mergeGoalDefaults(global, override)layers the override on topof the global.
loadGoalDefaults(api, workspaceId?)now accepts anoptional workspace; the slash command (
chatCommands.ts), commandpalette (
sources.ts), and in-tab create form (RightSidebar.tsx)all pass
workspaceIdso workspace rules win over the global default.GoalDefaultsSectionat the bottom of the Goal tab (empty + activestates). Collapsible
<details>with summary showing the effectivebudget + turn cap and a "Workspace" badge when any override is active.
Two stacked sub-panels:
inheriting, copy reads "Inherits $X from All workspaces". Toggle on
seeds the input with the inherited value; toggle off persists null.
GoalDefaultsControlsfrom Settings(now with an
onPersistcallback so the workspace panel's inheritlabels re-pull on every global edit).
GoalDefaultsControls; itemits a small pointer toward the Goal tab as the new home.
Validation
make static-checkclean (lint + format + typecheck + docs link check).Targeted tests (all pass):
src/browser/utils/goals/resolveGoalSetIntent.test.ts— 15 pass,including 7 new
mergeGoalDefaultsprecedence tests.src/node/services/workspaceService.goalDefaults.test.ts— 8 pass(round-trip, partial, all-null clears, no-op short-circuit,
validation, not-found).
src/node/services/workspaceGoalService.test.ts— 91 pass.src/browser/features/RightSidebar/GoalTab.test.tsx— 17 pass,including 5 create-form tests.
src/browser/features/RightSidebar/Tabs/TabLabels.goal.test.tsx—5 pass (label accent matrix).
src/browser/features/Settings/Sections/ExperimentsSection.test.tsx—9 pass (updated to assert goal defaults moved out of the panel).
tests/ui/layout/rightSidebar.test.ts— 2 new layout-visibilityguards (tab hidden when GOALS off; always present when GOALS on).
Risks
setGoalreplace +clearGoalis best-effort: ahistory write failure logs but never aborts the lifecycle mutation.
editInPlaceis opt-in; existing/goal, palette, and kickoff pathscontinue to take archive+recreate unchanged.
with no override is indistinguishable from one whose override was
never set, so reverting an override never strands stale data on disk.
loadGoalDefaults(workspaceId?)signature change is backwardscompatible — callers that don't yet know the workspace (Settings
global panel) still get global-only defaults.
Generated with
mux• Model:anthropic:claude-opus-4-7• Thinking:high• Cost:$16.93