feat: add workspace labels - #231
Conversation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
webmux | fa33363 | Commit Preview URL Branch Preview URL |
May 12 2026, 01:44 PM |
|
Claude finished @centdix's task in 4m 12s —— View job Code review for PR #231 — workspace labelsSolid feature: the API contract, types, mapping, snapshot, frontend rendering, CLI parity, and tests are all wired together coherently and the surface-by-surface coverage is good. A few concerns to address before merging — one is non-trivial. Main concern
That contradicts the PR description ("Labels are metadata only: branch names, paths, tmux windows, and command identifiers remain unchanged") and isn't covered by a test. See inline comment on Other findings
Nits
What's good
Todos
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| async setWorktreeLabel(branch: string, label: string | null): Promise<{ label: string | null }> { | ||
| try { | ||
| const normalizedLabel = normalizeWorktreeLabel(label); | ||
| const resolved = await this.resolveExistingWorktree(branch); | ||
| const initialized = resolved.meta | ||
| ? await this.refreshManagedArtifacts(resolved) | ||
| : await this.initializeUnmanagedWorktree(resolved); | ||
| const nextMeta = this.withUpdatedLabel(initialized.meta, normalizedLabel); | ||
| await writeWorktreeMeta(resolved.gitDir, nextMeta); | ||
| await this.deps.reconciliation.reconcile(this.deps.projectRoot, { force: true }); | ||
| return { label: normalizedLabel }; | ||
| } catch (error) { | ||
| throw this.wrapOperationError(error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Unwanted side effects for a "metadata only" operation.
The PR description says labels are metadata only ("branch names, paths, tmux windows, and command identifiers remain unchanged"), but this method calls either refreshManagedArtifacts (rewrites runtime.env and control.env) or, more concerningly, initializeUnmanagedWorktree — which allocates ports, picks a default profile/agent, and writes env files. So labeling a previously‑unmanaged worktree silently promotes it to managed.
Compare with setWorktreeArchived (line 346), which just resolves the worktree and toggles archive state — no env churn.
For label updates, the operation should be limited to:
readWorktreeMeta(gitDir)(or fail/no-op if absent)writeWorktreeMeta(gitDir, { ...meta, label })reconcile(..., { force: true })
Suggested:
async setWorktreeLabel(branch: string, label: string | null): Promise<{ label: string | null }> {
try {
const normalizedLabel = normalizeWorktreeLabel(label);
const resolved = await this.resolveExistingWorktree(branch);
if (!resolved.meta) {
throw new LifecycleError(`Worktree ${branch} has no managed metadata to label`, 409);
}
const nextMeta = this.withUpdatedLabel(resolved.meta, normalizedLabel);
await writeWorktreeMeta(resolved.gitDir, nextMeta);
await this.deps.reconciliation.reconcile(this.deps.projectRoot, { force: true });
return { label: normalizedLabel };
} catch (error) {
throw this.wrapOperationError(error);
}
}If labeling unmanaged worktrees is intentionally supported, that decision deserves to be explicit (and probably documented) rather than a side effect of reusing initializeUnmanagedWorktree. There's no test covering the unmanaged case today, so the silent promotion is unguarded.
| let label = $state(""); | ||
| let inputEl = $state<HTMLInputElement | null>(null); | ||
|
|
||
| $effect(() => { | ||
| label = initialLabel ?? ""; | ||
| }); |
There was a problem hiding this comment.
Per frontend/CLAUDE.md ("Prefer $derived over $effect for computed values — effects are for side-effects only"), this prop→state sync is the wrong tool.
It's also a subtle bug: if a websocket snapshot updates labelWorktree.label while the user is mid-typing in the dialog, initialLabel changes and the effect overwrites the user's input. Since the dialog already remounts on each open (gated by {#if labelBranch && labelWorktree} in App.svelte:1252), one-time initialization is enough:
| let label = $state(""); | |
| let inputEl = $state<HTMLInputElement | null>(null); | |
| $effect(() => { | |
| label = initialLabel ?? ""; | |
| }); | |
| let label = $state(initialLabel ?? ""); | |
| let inputEl = $state<HTMLInputElement | null>(null); | |
| $effect(() => { | |
| const currentInput = inputEl; | |
| if (!currentInput) return; | |
| queueMicrotask(() => currentInput.focus()); | |
| }); |
| if (arg === "--label" || arg.startsWith("--label=")) { | ||
| const { value, nextIndex } = readOptionValue(args, index, "--label"); | ||
| labelParts.push(value); | ||
| index = nextIndex; | ||
| continue; | ||
| } | ||
|
|
||
| if (arg.startsWith("-")) { | ||
| throw new CommandUsageError(`Unknown option: ${arg}`); | ||
| } | ||
|
|
||
| if (!branch) { | ||
| branch = arg; | ||
| continue; | ||
| } | ||
|
|
||
| labelParts.push(arg); |
There was a problem hiding this comment.
Unexpected concatenation of --label and trailing positionals.
Because --label pushes into the same labelParts array that trailing positionals also push into, webmux label feat/foo --label "Search" extra silently produces label "Search extra". Most CLIs treat --label <text> as exclusive of positional label text.
Consider either (a) erroring if both --label and positional label parts are supplied, or (b) ignoring positional words when --label was used. The current behavior is also undocumented — the help text describes --label <text> and <label> as if they were two ways to spell the same thing, not as mixable.
This is a minor UX rough edge — not a blocker — but worth tightening since the rest of the CLI parsing in this file is otherwise quite strict.
| function normalizeWorktreeMeta(meta: WorktreeMeta): WorktreeMeta { | ||
| const conversation = normalizeConversationMeta(meta.conversation); | ||
| return conversation === meta.conversation | ||
| ? meta | ||
| : { | ||
| ...meta, | ||
| conversation, | ||
| }; | ||
| const { label, conversation: rawConversation, ...rest } = meta; | ||
| const conversation = normalizeConversationMeta(rawConversation); | ||
| const normalizedLabel = normalizeOptionalString(label); | ||
| return { | ||
| ...rest, | ||
| ...(normalizedLabel ? { label: normalizedLabel } : {}), | ||
| ...(conversation !== undefined ? { conversation } : {}), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Minor: this no longer short-circuits when the meta is already normalized. Before, when conversation === meta.conversation we returned the original meta reference; now we always build a new object (even for reads of well-formed meta files). Not a correctness issue, just a small allocation regression. If you want to preserve the optimization:
function normalizeWorktreeMeta(meta: WorktreeMeta): WorktreeMeta {
const conversation = normalizeConversationMeta(meta.conversation);
const normalizedLabel = normalizeOptionalString(meta.label);
if (conversation === meta.conversation && normalizedLabel === meta.label) {
return meta;
}
const { label: _label, conversation: _conv, ...rest } = meta;
return {
...rest,
...(normalizedLabel ? { label: normalizedLabel } : {}),
...(conversation !== undefined ? { conversation } : {}),
};
}
| <div class="flex justify-between gap-2"> | ||
| <Btn type="button" onclick={onclear} disabled={loading || !initialLabel}>Clear</Btn> | ||
| <div class="flex justify-end gap-2"> | ||
| <Btn type="button" onclick={oncancel} disabled={loading}>Cancel</Btn> | ||
| <Btn type="submit" variant="cta" class="flex items-center gap-1.5" disabled={loading} | ||
| >{#if loading}<span class="spinner"></span>{/if} Save</Btn | ||
| > | ||
| </div> |
There was a problem hiding this comment.
Two small UX gaps worth considering:
- Save is enabled when the input is empty and there was no initial label. That submits
"", which the backend normalizes tonulland silently no-ops — a wasted round-trip with a confusing "label cleared" feel. Disabling Save when the trimmed input is empty and!initialLabelwould avoid this. Alternatively, disable Save when the trimmed input equals the currentinitialLabel ?? ""so unchanged submits don't fire either. - No keyboard cancel.
BaseDialogmay already handle Escape — if it doesn't, this dialog should explicitly so the modal feels consistent.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @centdix's task in 3m 45s —— View job Re-review for PR #231 — workspace labels (post-fix pass)The fix commit ( Previously raised — all resolved
One small leftover (non-blocking)
Minor doc polish
LGTM otherwise
Todos
Looks good to merge once the |
1 participant
Summary
Add editable workspace labels so users can give worktrees a human-readable name when auto-generated branch names are unclear. Labels are metadata only: branch names, paths, tmux windows, and command identifiers remain unchanged.
Changes
PUT /api/worktrees/:name/labelAPI and persist labels in existing worktree metadata.webmux label <branch> <label>andwebmux label <branch> --clear, plus label-awarewebmux listoutput/search and completions.Test plan
bun run --cwd backend checkbun run --cwd frontend checkbash scripts/run-with-isolated-tmux.sh bun run --cwd backend testbun run --cwd frontend testbun test packages/api-contract/srcbun test bin/srcgit diff --checkGenerated with Claude Code