Repository navigation
🤖 fix: reject leading-dash workspace names and let a heartbeat go back to the default interval - #5726
Merged
Merged
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
ThomasK33
force-pushed
the
fix/workspace-name-leading-dash
branch
from
October 6, 2026 13:01
668197d to
6557b38
Compare
Member
Author
|
Readiness record for
Generated with |
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
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.
Summary
Fixes two Settings bugs from the agent bug bash in #5670.
-now fails validation in the form and in the backend. Before the fix,-dash-passed the check, andgit worktree add -b -dash-then failed with git's option-parsing error.Fixes #5686
Fixes #5692
#5686: leading hyphen
-ingit worktree add -b <name>as an option. A-after a/is safe:git worktree add -b feat/-xand-b a-/bboth work (checked against git directly). So only the first character is rejected.validateWorkspaceNameandvalidateWorkspaceBranchNamereject a leading-with "… names cannot start with a hyphen". The creation form (useWorkspaceName) and the backend create/rename/fork paths use these validators, so both reject the name.x-,a-/b,feat/-xand_all work with git, so I changed nothing else.#5692: back to the global default interval
intervalMsis now optional inWorkspaceHeartbeatSettingsSchema. An absent value means "use the global default". This PR removes a requirement on an existing field and adds no new persisted field.heartbeat.settakesintervalMs: nullto clear the override, and an absent key keeps the saved value (the same pattern astrigger/whenBusy).Downgrade: an older build reads a record without
intervalMsas the global default.normalizeWorkspaceMetadataHeartbeatinsrc/node/config/index.ts(unchanged) falls back toheartbeatDefaultIntervalMs. The new test reads workspace metadata after a global change and gets the new default. Older builds load and save such a record normally, because they do not parse workspace entries through the zod schema. One exception: an older build'smux_config_writeagent tool validates the whole file against the old schema and refuses to write while such a record exists. Tracked in #5742 (workaround: set an interval in the dialog).Every reader of a workspace heartbeat
intervalMs, and how it handles an absent value:src/node/config/index.tsnormalizeWorkspaceMetadataHeartbeat(workspace metadata)heartbeatService.getSanitizedTrackingIntervalMs(scheduling: activity events, metadata events, config resync on every tick)raw ?? heartbeatDefaultIntervalMs ?? built-in. A global change applies on the next tick.workspaceServiceheartbeat executor (sanitizeHeartbeatIntervalMs)workspaceService.getHeartbeatSettingsheartbeat.getIPC passesresolveDefaultInterval: falseand gets the saved shape.workspaceService.setHeartbeatSettingsnullclears the override, and an absent key preserves it. ThescheduleUpdatedAtstamp compares effective intervals. Returns the effective interval.tools/heartbeat.ts) andsummarizeHeartbeatSettingsHeartbeatToolCallcarduseWorkspaceHeartbeatandWorkspaceHeartbeatModalnullfor the default./heartbeat off(chatCommands.ts)intervalMs. Before, it wrote the built-in default, which silently pinned a value.src/cli)Behavior change: a heartbeat created without an interval (dialog default,
/heartbeat off, or the agent tool withoutintervalMs) now follows the global default. Before, it saved a copy of the default at creation time. A dialog save with the checkbox unchecked pins the shown value as an override, as before.Known limit: a global default change does not stamp
scheduleUpdatedAt. A fixed-interval (trigger: "interval") workspace that follows the default re-anchors on the next tick, but the restart-anchoring rule uses the older stamp. I did not add a persisted field for this. Tracked in #5741.No new operation: the checkbox is a form control (Tab and Space), and Save stays the operation. So I added no keyboard shortcut.
Validation
Pre-fix failures on
origin/main(tests written first):After the fix,
make static-checkpasses, along with 691 tests in 16 related files (on the rebased head) (validation, heartbeat settings/service/tool, modal, hook, Settings section, chat commands, config, workspace/task service).One existing test changed on purpose:
chatCommands.test.tsexpected/heartbeat offto write the built-in default interval. It now expects no interval.Screenshots (Storybook
Components/WorkspaceHeartbeatModal/LongMessageDesktop, a workspace with a 7-minute override):Before, 1200px: no way back to the default.

After, 1200px, with "Use the global default" checked:

After, 390px:

Final check
A clean-context reviewer checked head
668197d12eagainst the criteria and recommended "ready with tracked follow-ups". Follow-ups: #5741 (re-anchoring after a global default change) and #5742 (downgrade note formux_config_write). Its third note is cosmetic: the disabled interval field keeps showing an old default if the global default changes while the dialog is open. I did not file it.Rebase: main gained #5717, which conflicted in the heartbeat hook and modal. The rebased head
6557b3828bkeeps #5717's rule: when the config cannot load,globalDefaultIntervalMsstays undefined, and the checkbox label leaves out the number. The normal and security reviews on6557b3828bwere clean. A second clean-context final check on6557b3828brecommended "ready with tracked follow-ups" and named one more: #5745 (the disabled field shows the built-in value when the default is unknown).Review assessments used: 6 of 6 (normal and security reviews on
668197d12eand on6557b3828b, and the two final checks).Risks
Medium-low. The persisted shape changes from "always has intervalMs" to "intervalMs optional". All readers already had a fallback (see the table), and the downgrade read path is tested. The modal unit tests save the shown interval without typing, because happy-dom does not fire
onChangefor this number input. Typing is pre-existing behavior.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high