feat: configuration and storybook#204
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughMoves Playground/Examples into Storybook and removes VITE_PLAYGROUND. Adds Storybook config, many stories and fixtures. Implements a tabbed Configuration UI with Console and Workers tabs, a schema-driven Workers editor (fields, env-lexical input, DnD, validation), API/hooks, utils, tests, and related docs/CSS updates. ChangesStorybook migration and Playground flag removal
Workers configuration editor and routing
Sequence Diagram(s)sequenceDiagram
participant UI as WorkerEditor
participant Mutation as useSetConfiguration
participant Bus as configuration::set
UI->>Mutation: save(id, value)
Mutation->>Bus: send payload
Bus-->>Mutation: Ok or SCHEMA_INVALID
Mutation-->>UI: invalidate queries or return parsed errors
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Contributor
skill-check — worker0 verified, 13 skipped (no docs/).
Note 17 stale rendered artifact(s) detected on main, unrelated to this PR. This PR is fine; the drift was already there. A maintainer should open a chore PR to re-render these.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
DOCUMENTATION_GUIDELINES.md (1)
21-22: 💤 Low valueConsider clarifying the section count.
The phrase "four body sections" is accurate if it refers only to the level-2 subsections (When to Use, Boundaries, Functions, Reactive triggers), but readers might count the level-1 overview heading and become confused.
✨ Optional rewording for clarity
-Every worker `SKILL.md` has YAML frontmatter and four body sections (the last -one only when the worker exposes a trigger type). +Every worker `SKILL.md` has YAML frontmatter, a level-1 overview heading, and +up to four level-2 subsections (the last one only when the worker exposes a +trigger type).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@DOCUMENTATION_GUIDELINES.md` around lines 21 - 22, Reword the ambiguous sentence "Every worker `SKILL.md` has YAML frontmatter and four body sections (the last one only when the worker exposes a trigger type)." to explicitly state that the count refers to level-2 subsections (e.g., "Every worker `SKILL.md` begins with YAML frontmatter followed by four level-2 body subsections — When to Use, Boundaries, Functions, and Reactive triggers — with the last subsection included only when the worker exposes a trigger type.") so readers cannot mistake the overview heading for an additional section.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@console/web/src/components/ui/Select.tsx`:
- Line 128: The span in the Select component uses a bare boolean attribute
aria-hidden (in the <span aria-hidden className="shrink-0 text-ink-faint">)
which is inconsistent with other SVGs; update that element in Select.tsx to use
an explicit value aria-hidden="true" so assistive tech behavior is clear and
consistent with other icons in the component.
In `@console/web/src/hooks/use-hash-route.ts`:
- Around line 56-72: In configRouteFromHash, wrap the decodeURIComponent(second)
call in a try/catch inside the branch where first === 'workers' so malformed
percent-encoding won’t throw; if decodeURIComponent throws (URIError) fall back
to a safe value (e.g., null or the original raw segment) for workerId and
optionally log the error, ensuring the function still returns { tab: 'workers',
workerId: ... } rather than propagating the exception.
In `@console/web/src/pages/Configuration/tabs/WorkersTab/index.tsx`:
- Around line 44-53: The auto-selection logic currently only triggers when
selectedId is falsy, leaving the pane empty if the URL contains a stale
selectedId not present in entries; update the useEffect in WorkersTab to also
check whether the current selectedId exists in entries and, if it does not and
entries.length > 0, call onSelect(entries[0].id). In other words, inside the
existing useEffect (which references selectedId, entries, onSelect) change the
early-return logic to: if (entries.length === 0) return; if (!selectedId ||
!entries.find(e => e.id === selectedId)) onSelect(entries[0].id); keeping the
same dependency array.
In `@console/web/src/pages/Configuration/tabs/WorkersTab/SaveBar.tsx`:
- Line 47: The save confirmation is never visible because the shouldShow boolean
(const shouldShow = dirty || inFlight || status.kind === 'error') omits
status.kind === 'saved'; update shouldShow in SaveBar.tsx to include saved
(e.g., const shouldShow = dirty || inFlight || status.kind === 'error' ||
status.kind === 'saved') so the bar remains visible when a save just succeeded,
or alternatively adjust WorkerEditor.tsx where status is set to 'saved' to delay
resetting to 'idle' (add a short timeout or clear-on-next-edit) so the transient
saved state persists long enough to be displayed; reference the shouldShow
variable and the status.kind checks when making the change.
In
`@console/web/src/pages/Configuration/tabs/WorkersTab/schema-form/DictionaryField.tsx`:
- Around line 100-122: commitRows currently loses values when a row's key is
renamed because it only looks up obj[r.key]; capture the prior per-row value by
recording a map from each existing row's id to its current value before calling
setRows (e.g. build prevValues: Map(row.id -> (obj[row.key] ??
schemaDefault(valueSchema))) using the existing rows array), then when you build
next inside commitRows use prevValues.get(r.id) first, falling back to
obj[r.key] and then schemaDefault(valueSchema) (references: commitRows,
DictionaryRow.id, rows, obj, schemaDefault, valueSchema, lastSyncedKeysRef).
In
`@console/web/src/pages/Configuration/tabs/WorkersTab/schema-form/EnumField.tsx`:
- Around line 35-38: The code treats only undefined as the unset state so
clearing the field (which sets null in onClear) becomes encodeEnumValue(null)
and shows "j:null" instead of the placeholder; update the currentKey logic (the
variable named currentKey used with encodeEnumValue and the onClear behavior) to
treat both null and undefined as unset (i.e., map value === undefined || value
=== null to undefined) so the trigger shows emptyLabel/placeholder when cleared
— assume null is not a valid enum member.
---
Nitpick comments:
In `@DOCUMENTATION_GUIDELINES.md`:
- Around line 21-22: Reword the ambiguous sentence "Every worker `SKILL.md` has
YAML frontmatter and four body sections (the last one only when the worker
exposes a trigger type)." to explicitly state that the count refers to level-2
subsections (e.g., "Every worker `SKILL.md` begins with YAML frontmatter
followed by four level-2 body subsections — When to Use, Boundaries, Functions,
and Reactive triggers — with the last subsection included only when the worker
exposes a trigger type.") so readers cannot mistake the overview heading for an
additional section.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a0f24938-1d2b-4a41-8e92-13628c519d95
⛔ Files ignored due to path filters (1)
console/web/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (129)
.github/workflows/_rust-binary.yml.github/workflows/ci.yml.gitignoreDOCUMENTATION_GUIDELINES.mdconsole/build.rsconsole/web/.gitignoreconsole/web/.storybook/main.tsconsole/web/.storybook/preview-head.htmlconsole/web/.storybook/preview.tsxconsole/web/DESIGN.mdconsole/web/PLAYGROUND.mdconsole/web/README.mdconsole/web/biome.jsonconsole/web/docs/custom-function-components.mdconsole/web/package.jsonconsole/web/src/App.tsxconsole/web/src/components/chat/ChatDock.tsxconsole/web/src/components/chat/Composer.stories.tsxconsole/web/src/components/chat/FunctionCallGroup.stories.tsxconsole/web/src/components/chat/FunctionCallMessage.stories.tsxconsole/web/src/components/chat/Message.stories.tsxconsole/web/src/components/chat/ThoughtMessage.stories.tsxconsole/web/src/components/chat/directory/SkillsViews.tsxconsole/web/src/components/chat/engine/FunctionsListView.tsxconsole/web/src/components/chat/engine/RegisteredTriggersListView.tsxconsole/web/src/components/chat/engine/TriggersListView.tsxconsole/web/src/components/chat/engine/WorkersListView.tsxconsole/web/src/components/chat/sandbox/__tests__/parsers.test.tsconsole/web/src/components/chat/sandbox/index.tsxconsole/web/src/components/chat/sandbox/parsers.tsconsole/web/src/components/permissions/DefaultPermissionModePicker.tsxconsole/web/src/components/permissions/FunctionAllowlistTree.tsxconsole/web/src/components/providers/ProviderSettingsDialog.tsxconsole/web/src/components/ui/Button.stories.tsxconsole/web/src/components/ui/Caret.stories.tsxconsole/web/src/components/ui/ModeToggle.stories.tsxconsole/web/src/components/ui/ModeToggle.tsxconsole/web/src/components/ui/Prompt.stories.tsxconsole/web/src/components/ui/Select.stories.tsxconsole/web/src/components/ui/Select.tsxconsole/web/src/components/ui/StatusDot.stories.tsxconsole/web/src/components/ui/Wordmark.stories.tsxconsole/web/src/hooks/use-approval-settings.tsconsole/web/src/hooks/use-hash-route.tsconsole/web/src/index.cssconsole/web/src/lib/backend/index.tsconsole/web/src/lib/backend/pending-approvals-store.test.tsconsole/web/src/pages/Configuration/index.tsxconsole/web/src/pages/Configuration/tabs/ConsoleSettingsTab.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/EmptyState.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/SaveBar.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/WorkerEditor.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/WorkersList.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/WorkersTab.stories.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/api.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/dirty.test.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/dirty.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/errors.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/hooks.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/index.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/ArrayField.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/BooleanField.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/DictionaryField.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/EnumField.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/FieldDispatch.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/FieldShell.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/NullableField.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/NumberField.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/ObjectSection.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/OneOfField.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/RowChrome.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/SchemaForm.stories.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/SchemaForm.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/StringField.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/env-lexical/EnvLexicalInput.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/env-lexical/EnvPlaceholderNode.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/env-lexical/EnvPlaceholderTransformPlugin.tsxconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/env-lexical/env-template.test.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/env-lexical/env-template.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/path.test.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/path.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/ref-resolver.test.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/ref-resolver.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/reorder.test.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/reorder.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/schema-form/useDragReorder.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/typography.tsconsole/web/src/pages/Configuration/tabs/WorkersTab/useUnsavedGuard.tsconsole/web/src/pages/Examples/Section.tsxconsole/web/src/pages/Examples/index.tsxconsole/web/src/pages/Examples/sections/composer-variants.tsxconsole/web/src/pages/Examples/sections/custom-function-views.tsxconsole/web/src/pages/Examples/sections/loading-states.tsxconsole/web/src/pages/Examples/sections/message-variants.tsxconsole/web/src/pages/Examples/sections/primitives.tsxconsole/web/src/pages/Examples/sections/worker-fixtures.tsconsole/web/src/pages/Playground/ScenarioPicker.tsxconsole/web/src/pages/Traces/components/WaterfallChart.tsxconsole/web/src/stories/decorators.tsxconsole/web/src/stories/design/Colors.stories.tsxconsole/web/src/stories/design/Loading.stories.tsxconsole/web/src/stories/design/Typography.stories.tsxconsole/web/src/stories/fixtures/directory-fixtures.tsconsole/web/src/stories/fixtures/engine-fixtures.tsconsole/web/src/stories/fixtures/sandbox-fixtures.tsconsole/web/src/stories/fixtures/web-fixtures.tsconsole/web/src/stories/fixtures/worker-fixtures.tsconsole/web/src/stories/playground/Agent.stories.tsxconsole/web/src/stories/playground/EventLog.tsxconsole/web/src/stories/playground/FailureModes.stories.tsxconsole/web/src/stories/playground/HappyPaths.stories.tsxconsole/web/src/stories/playground/Markdown.stories.tsxconsole/web/src/stories/playground/Timing.stories.tsxconsole/web/src/stories/playground/harness.tsxconsole/web/src/stories/playground/scenario-story.tsxconsole/web/src/stories/playground/scenarios/abort-mid-thought.tsconsole/web/src/stories/playground/scenarios/error-on-fcall.tsconsole/web/src/stories/playground/scenarios/fast-tokens.tsconsole/web/src/stories/playground/scenarios/happy-agent.tsconsole/web/src/stories/playground/scenarios/happy-ask.tsconsole/web/src/stories/playground/scenarios/happy-plan.tsconsole/web/src/stories/playground/scenarios/helpers.tsconsole/web/src/stories/playground/scenarios/index.tsconsole/web/src/stories/playground/scenarios/long-markdown.tsconsole/web/src/stories/playground/scenarios/markdown-stress.tsconsole/web/src/stories/playground/scenarios/multi-function-agent.tsconsole/web/src/stories/playground/scenarios/pending-approval.tsconsole/web/src/stories/playground/scenarios/slow-tokens.tsconsole/web/src/vite-env.d.ts
💤 Files with no reviewable changes (13)
- console/web/src/components/chat/ChatDock.tsx
- console/web/src/pages/Examples/Section.tsx
- console/web/src/pages/Examples/index.tsx
- console/web/src/vite-env.d.ts
- console/web/src/pages/Examples/sections/composer-variants.tsx
- console/web/src/pages/Examples/sections/message-variants.tsx
- console/web/src/pages/Examples/sections/worker-fixtures.ts
- console/web/src/pages/Examples/sections/loading-states.tsx
- console/web/src/pages/Playground/ScenarioPicker.tsx
- console/web/src/pages/Examples/sections/custom-function-views.tsx
- console/web/src/pages/Examples/sections/primitives.tsx
- .github/workflows/ci.yml
- .gitignore
| </SelectPrimitive.Value> | ||
| <SelectPrimitive.Icon asChild> | ||
| <span aria-hidden className="text-ink-faint"> | ||
| <span aria-hidden className="shrink-0 text-ink-faint"> |
There was a problem hiding this comment.
Add explicit value to aria-hidden.
The <span> wrapper uses aria-hidden without an explicit value, while the nested SVG and other SVGs in the component correctly use aria-hidden="true". For consistency and clarity, add the explicit value.
♿ Proposed fix
- <SelectPrimitive.Icon asChild>
- <span aria-hidden className="shrink-0 text-ink-faint">
+ <SelectPrimitive.Icon asChild>
+ <span aria-hidden="true" className="shrink-0 text-ink-faint">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <span aria-hidden className="shrink-0 text-ink-faint"> | |
| <span aria-hidden="true" className="shrink-0 text-ink-faint"> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/components/ui/Select.tsx` at line 128, The span in the Select
component uses a bare boolean attribute aria-hidden (in the <span aria-hidden
className="shrink-0 text-ink-faint">) which is inconsistent with other SVGs;
update that element in Select.tsx to use an explicit value aria-hidden="true" so
assistive tech behavior is clear and consistent with other icons in the
component.
Comment on lines
+56
to
+72
| function configRouteFromHash(hash: string): ConfigurationRoute { | ||
| // Treat bare `#/configuration` and any non-configuration hash as the | ||
| // default `console` tab. Legacy `#/providers` is funneled here too. | ||
| if (!hash.startsWith('#/configuration/')) { | ||
| return { tab: 'console', workerId: null } | ||
| } | ||
| const rest = hash.slice('#/configuration/'.length) | ||
| const segments = rest.split('/').filter(Boolean) | ||
| const [first, second] = segments | ||
| if (first === 'workers') { | ||
| return { | ||
| tab: 'workers', | ||
| workerId: second ? decodeURIComponent(second) : null, | ||
| } | ||
| } | ||
| return { tab: 'console', workerId: null } | ||
| } |
There was a problem hiding this comment.
Wrap decodeURIComponent in error handling.
Line 68 calls decodeURIComponent(second) without catching potential URIError from malformed percent-encoding. If an operator manually edits the hash to something like #/configuration/workers/%E0%A4%A, route parsing will throw and break navigation.
🛡️ Proposed fix to add try-catch
function configRouteFromHash(hash: string): ConfigurationRoute {
// Treat bare `#/configuration` and any non-configuration hash as the
// default `console` tab. Legacy `#/providers` is funneled here too.
if (!hash.startsWith('`#/configuration/`')) {
return { tab: 'console', workerId: null }
}
const rest = hash.slice('`#/configuration/`'.length)
const segments = rest.split('/').filter(Boolean)
const [first, second] = segments
if (first === 'workers') {
+ let workerId: string | null = null
+ if (second) {
+ try {
+ workerId = decodeURIComponent(second)
+ } catch {
+ // Malformed encoding — treat as no selection
+ workerId = null
+ }
+ }
return {
tab: 'workers',
- workerId: second ? decodeURIComponent(second) : null,
+ workerId,
}
}
return { tab: 'console', workerId: null }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function configRouteFromHash(hash: string): ConfigurationRoute { | |
| // Treat bare `#/configuration` and any non-configuration hash as the | |
| // default `console` tab. Legacy `#/providers` is funneled here too. | |
| if (!hash.startsWith('#/configuration/')) { | |
| return { tab: 'console', workerId: null } | |
| } | |
| const rest = hash.slice('#/configuration/'.length) | |
| const segments = rest.split('/').filter(Boolean) | |
| const [first, second] = segments | |
| if (first === 'workers') { | |
| return { | |
| tab: 'workers', | |
| workerId: second ? decodeURIComponent(second) : null, | |
| } | |
| } | |
| return { tab: 'console', workerId: null } | |
| } | |
| function configRouteFromHash(hash: string): ConfigurationRoute { | |
| // Treat bare `#/configuration` and any non-configuration hash as the | |
| // default `console` tab. Legacy `#/providers` is funneled here too. | |
| if (!hash.startsWith('`#/configuration/`')) { | |
| return { tab: 'console', workerId: null } | |
| } | |
| const rest = hash.slice('`#/configuration/`'.length) | |
| const segments = rest.split('/').filter(Boolean) | |
| const [first, second] = segments | |
| if (first === 'workers') { | |
| let workerId: string | null = null | |
| if (second) { | |
| try { | |
| workerId = decodeURIComponent(second) | |
| } catch { | |
| // Malformed encoding — treat as no selection | |
| workerId = null | |
| } | |
| } | |
| return { | |
| tab: 'workers', | |
| workerId, | |
| } | |
| } | |
| return { tab: 'console', workerId: null } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/hooks/use-hash-route.ts` around lines 56 - 72, In
configRouteFromHash, wrap the decodeURIComponent(second) call in a try/catch
inside the branch where first === 'workers' so malformed percent-encoding won’t
throw; if decodeURIComponent throws (URIError) fall back to a safe value (e.g.,
null or the original raw segment) for workerId and optionally log the error,
ensuring the function still returns { tab: 'workers', workerId: ... } rather
than propagating the exception.
Comment on lines
+44
to
+53
| useEffect(() => { | ||
| if (selectedId) return | ||
| if (entries.length === 0) return | ||
| onSelect(entries[0].id) | ||
| }, [selectedId, entries, onSelect]) | ||
|
|
||
| const selectedEntry = | ||
| selectedId != null | ||
| ? (entries.find((e) => e.id === selectedId) ?? null) | ||
| : null |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f 'WorkerEditor.tsx' --exec sed -n '1,200p' {}Repository: iii-hq/workers
Length of output: 7109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== WorkersTab/index.tsx (around lines 30-80) ==="
FILE="console/web/src/pages/Configuration/tabs/WorkersTab/index.tsx"
if [ -f "$FILE" ]; then
nl -ba "$FILE" | sed -n '1,140p' | sed -n '30,120p'
else
echo "Missing $FILE"
fi
echo
echo "=== WorkerEditorEmptySelection (rest of component) ==="
# locate WorkerEditorEmptySelection definition lines
# Use rg for the component name and then print around it.
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' . | head -n 1)"
echo "Using: $WORKER_FILE"
rg -n "function WorkerEditorEmptySelection|export function WorkerEditorEmptySelection" "$WORKER_FILE" || true
# Print from match line -20 to +120
LINE="$(rg -n "export function WorkerEditorEmptySelection" "$WORKER_FILE" | head -n1 | cut -d: -f1 || true)"
if [ -n "$LINE" ]; then
start=$((LINE-20))
end=$((LINE+200))
nl -ba "$WORKER_FILE" | sed -n "${start},${end}p"
else
echo "Could not find WorkerEditorEmptySelection export function."
fiRepository: iii-hq/workers
Length of output: 169
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== WorkersTab/index.tsx (around lines 30-120) ==="
FILE="console/web/src/pages/Configuration/tabs/WorkersTab/index.tsx"
if [ -f "$FILE" ]; then
nl -ba "$FILE" | sed -n '1,220p' | sed -n '30,160p'
else
echo "Missing $FILE"
fi
echo
echo "=== WorkerEditorEmptySelection (rest of component) ==="
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' console/web/src/pages/Configuration/tabs/WorkersTab . 2>/dev/null | head -n 1 || true)"
if [ -z "$WORKER_FILE" ]; then
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' console/web/src/pages/Configuration/tabs . 2>/dev/null | head -n 1 || true)"
fi
if [ -z "$WORKER_FILE" ]; then
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' . | head -n 1)"
fi
echo "Using: $WORKER_FILE"
rg -n "WorkerEditorEmptySelection" "$WORKER_FILE" || true
LINE="$(rg -n "export function WorkerEditorEmptySelection" "$WORKER_FILE" | head -n1 | cut -d: -f1 || true)"
if [ -n "$LINE" ]; then
start=$((LINE-30))
end=$((LINE+260))
nl -ba "$WORKER_FILE" | sed -n "${start},${end}p"
else
echo "Could not find WorkerEditorEmptySelection export function."
fiRepository: iii-hq/workers
Length of output: 170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== WorkersTab/index.tsx (around lines 30-160) ==="
FILE="console/web/src/pages/Configuration/tabs/WorkersTab/index.tsx"
if [ -f "$FILE" ]; then
nl -ba "$FILE" | sed -n '30,160p'
else
echo "Missing $FILE"
fi
echo
echo "=== WorkerEditorEmptySelection (around its definition) ==="
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' console/web/src/pages/Configuration/tabs/WorkersTab . 2>/dev/null | head -n 1 || true)"
if [ -z "$WORKER_FILE" ]; then
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' console/web/src/pages/Configuration/tabs . 2>/dev/null | head -n 1 || true)"
fi
if [ -z "$WORKER_FILE" ]; then
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' . | head -n 1)"
fi
echo "Using: $WORKER_FILE"
LINE="$(rg -n "export function WorkerEditorEmptySelection" "$WORKER_FILE" | head -n 1 | cut -d: -f1 || true)"
if [ -n "$LINE" ]; then
start=$((LINE-40))
end=$((LINE+260))
nl -ba "$WORKER_FILE" | sed -n "${start},${end}p"
else
echo "Could not find WorkerEditorEmptySelection export function."
fiRepository: iii-hq/workers
Length of output: 170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== WorkersTab/index.tsx (around lines 30-160) ==="
FILE="console/web/src/pages/Configuration/tabs/WorkersTab/index.tsx"
if [ -f "$FILE" ]; then
nl -ba "$FILE" | sed -n '30,160p'
else
echo "Missing $FILE"
fi
echo
echo "=== WorkerEditorEmptySelection (around its definition) ==="
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' console/web/src/pages/Configuration/tabs/WorkersTab . 2>/dev/null | head -n 1 || true)"
if [ -z "$WORKER_FILE" ]; then
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' console/web/src/pages/Configuration/tabs . 2>/dev/null | head -n 1 || true)"
fi
if [ -z "$WORKER_FILE" ]; then
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' . | head -n 1)"
fi
echo "Using: $WORKER_FILE"
LINE="$(rg -n "export function WorkerEditorEmptySelection" "$WORKER_FILE" | head -n 1 | cut -d: -f1 || true)"
if [ -n "$LINE" ]; then
start=$((LINE-40))
end=$((LINE+260))
nl -ba "$WORKER_FILE" | sed -n "${start},${end}p"
else
echo "Could not find WorkerEditorEmptySelection export function."
fiRepository: iii-hq/workers
Length of output: 170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== WorkersTab/index.tsx (around lines 30-160) ==="
FILE="console/web/src/pages/Configuration/tabs/WorkersTab/index.tsx"
if [ -f "$FILE" ]; then
nl -ba "$FILE" | sed -n '30,160p'
else
echo "Missing $FILE"
fi
echo
echo "=== WorkerEditorEmptySelection (around its definition) ==="
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' console/web/src/pages/Configuration/tabs/WorkersTab . 2>/dev/null | head -n 1 || true)"
if [ -z "$WORKER_FILE" ]; then
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' console/web/src/pages/Configuration/tabs . 2>/dev/null | head -n 1 || true)"
fi
if [ -z "$WORKER_FILE" ]; then
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' . | head -n 1)"
fi
echo "Using: $WORKER_FILE"
LINE="$(rg -n "export function WorkerEditorEmptySelection" "$WORKER_FILE" | head -n 1 | cut -d: -f1 || true)"
if [ -n "$LINE" ]; then
start=$((LINE-40))
end=$((LINE+260))
nl -ba "$WORKER_FILE" | sed -n "${start},${end}p"
else
echo "Could not find WorkerEditorEmptySelection export function."
fiRepository: iii-hq/workers
Length of output: 170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="console/web/src/pages/Configuration/tabs/WorkersTab/index.tsx"
echo "=== WorkersTab/index.tsx (lines 1-200) ==="
if [ -f "$FILE" ]; then
awk 'NR>=1 && NR<=200 {printf "%5d\t%s\n", NR, $0}' "$FILE"
else
echo "Missing $FILE"
fi
echo
echo "=== WorkerEditorEmptySelection definition (find file + show component) ==="
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' console/web/src/pages/Configuration/tabs/WorkersTab . 2>/dev/null | head -n 1 || true)"
if [ -z "$WORKER_FILE" ]; then
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' console/web/src/pages/Configuration/tabs . 2>/dev/null | head -n 1 || true)"
fi
if [ -z "$WORKER_FILE" ]; then
WORKER_FILE="$(fd -t f 'WorkerEditor.tsx' . 2>/dev/null | head -n 1 || true)"
fi
echo "Using: ${WORKER_FILE:-<not found>}"
if [ -z "$WORKER_FILE" ]; then
exit 0
fi
LINE="$(rg -n "export function WorkerEditorEmptySelection" "$WORKER_FILE" | head -n 1 | cut -d: -f1 || true)"
if [ -z "$LINE" ]; then
exit 0
fi
start=$((LINE-40))
end=$((LINE+260))
echo "=== $WORKER_FILE (around definition: $start-$end) ==="
awk -v s="$start" -v e="$end" 'NR>=s && NR<=e {printf "%5d\t%s\n", NR, $0}' "$WORKER_FILE"Repository: iii-hq/workers
Length of output: 5566
Stale selectedId (not present in entries) leaves the Workers right pane empty with no recovery
WorkersTab only auto-selects when selectedId is falsy; if the URL carries an id that isn’t in entries, selectedEntry becomes null and the UI renders WorkerEditorEmptySelection (which only shows the empty state—no redirect/reselect). Add a guard to auto-select the first entry when selectedId doesn’t match any loaded item (while keeping the existing entries.length === 0 load guard):
🐛 Proposed guard
useEffect(() => {
- if (selectedId) return
if (entries.length === 0) return
+ if (selectedId && entries.some((e) => e.id === selectedId)) return
onSelect(entries[0].id)
- }, [selectedId, entries, onSelect])
+ }, [selectedId, entries, onSelect])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/pages/Configuration/tabs/WorkersTab/index.tsx` around lines
44 - 53, The auto-selection logic currently only triggers when selectedId is
falsy, leaving the pane empty if the URL contains a stale selectedId not present
in entries; update the useEffect in WorkersTab to also check whether the current
selectedId exists in entries and, if it does not and entries.length > 0, call
onSelect(entries[0].id). In other words, inside the existing useEffect (which
references selectedId, entries, onSelect) change the early-return logic to: if
(entries.length === 0) return; if (!selectedId || !entries.find(e => e.id ===
selectedId)) onSelect(entries[0].id); keeping the same dependency array.
| saveDisabled, | ||
| }: SaveBarProps) { | ||
| const inFlight = status.kind === 'saving' | ||
| const shouldShow = dirty || inFlight || status.kind === 'error' |
There was a problem hiding this comment.
The "saved" status display is unreachable.
The visibility condition on Line 47 does not include status.kind === 'saved', so the bar hides when status is 'saved' and dirty is false. Meanwhile, the "saved" message (Lines 96-98) only renders when status.kind === 'saved' && !dirty, which means:
- When the bar is visible (
dirty || inFlight || error), the saved state never displays because dirty is true. - When
status.kind === 'saved' && !dirty, the bar is hidden entirely.
Looking at WorkerEditor.tsx, the 'saved' status is transient—it's set on Line 99 after mutation success, but then immediately reset to 'idle' when the refetch completes and triggers the effect on Lines 50-54. Users never see the "saved" confirmation.
The comment on Lines 26-28 suggests the bar should show "just succeeded", but the current logic prevents this.
💡 Proposed fix to show "saved" feedback
Option 1: Include 'saved' in the visibility condition:
- const shouldShow = dirty || inFlight || status.kind === 'error'
+ const shouldShow = dirty || inFlight || status.kind === 'error' || status.kind === 'saved'Option 2: In WorkerEditor.tsx, delay the status reset to 'idle' so 'saved' persists briefly:
useEffect(() => {
if (valueQuery.data === undefined) return
setDraft(valueQuery.data)
- setStatus((cur) => (cur.kind === 'saving' ? cur : { kind: 'idle' }))
+ setStatus((cur) => (cur.kind === 'saving' || cur.kind === 'saved' ? cur : { kind: 'idle' }))
}, [valueQuery.data])Then add a timeout to clear 'saved' after a few seconds, or let the next edit clear it (which already happens on Line 80).
Also applies to: 96-98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@console/web/src/pages/Configuration/tabs/WorkersTab/SaveBar.tsx` at line 47,
The save confirmation is never visible because the shouldShow boolean (const
shouldShow = dirty || inFlight || status.kind === 'error') omits status.kind ===
'saved'; update shouldShow in SaveBar.tsx to include saved (e.g., const
shouldShow = dirty || inFlight || status.kind === 'error' || status.kind ===
'saved') so the bar remains visible when a save just succeeded, or alternatively
adjust WorkerEditor.tsx where status is set to 'saved' to delay resetting to
'idle' (add a short timeout or clear-on-next-edit) so the transient saved state
persists long enough to be displayed; reference the shouldShow variable and the
status.kind checks when making the change.
Comment on lines
+100
to
+122
| function commitRows(nextRows: DictionaryRow[]) { | ||
| setRows(nextRows) | ||
| const counts = new Map<string, number>() | ||
| for (const r of nextRows) { | ||
| if (r.key.length === 0) continue | ||
| counts.set(r.key, (counts.get(r.key) ?? 0) + 1) | ||
| } | ||
| const allUnique = [...counts.values()].every((c) => c === 1) | ||
| const allNonEmpty = nextRows.every((r) => r.key.length > 0) | ||
| if (allUnique && allNonEmpty) { | ||
| const next: { [key: string]: JsonValue } = {} | ||
| for (const r of nextRows) { | ||
| next[r.key] = obj[r.key] ?? schemaDefault(valueSchema) | ||
| } | ||
| onChange(next) | ||
| // Order-independent: reordering keeps the same key set, so external | ||
| // sync won't fight a drag. | ||
| lastSyncedKeysRef.current = nextRows | ||
| .map((r) => r.key) | ||
| .sort() | ||
| .join('\u0000') | ||
| } | ||
| } |
There was a problem hiding this comment.
Renaming a key silently drops its value.
commitRows rebuilds the object keyed by each row's current key and reads obj[r.key]. When a key is renamed, the new key doesn't exist in obj, so the value falls through to schemaDefault(valueSchema) and the previously-entered value is lost on the first keystroke of the rename. For an env-style editor where the value (e.g. ${VAR}) is the important part, this is meaningful data loss.
Carry the prior value across a rename by mapping the previous key through the stable row id:
🐛 Proposed fix to preserve value on rename
function commitRows(nextRows: DictionaryRow[]) {
setRows(nextRows)
const counts = new Map<string, number>()
for (const r of nextRows) {
if (r.key.length === 0) continue
counts.set(r.key, (counts.get(r.key) ?? 0) + 1)
}
const allUnique = [...counts.values()].every((c) => c === 1)
const allNonEmpty = nextRows.every((r) => r.key.length > 0)
if (allUnique && allNonEmpty) {
+ const prevKeyById = new Map(rows.map((r) => [r.id, r.key]))
const next: { [key: string]: JsonValue } = {}
for (const r of nextRows) {
- next[r.key] = obj[r.key] ?? schemaDefault(valueSchema)
+ const prevKey = prevKeyById.get(r.id)
+ const carried = prevKey !== undefined ? obj[prevKey] : undefined
+ next[r.key] = carried ?? obj[r.key] ?? schemaDefault(valueSchema)
}
onChange(next)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function commitRows(nextRows: DictionaryRow[]) { | |
| setRows(nextRows) | |
| const counts = new Map<string, number>() | |
| for (const r of nextRows) { | |
| if (r.key.length === 0) continue | |
| counts.set(r.key, (counts.get(r.key) ?? 0) + 1) | |
| } | |
| const allUnique = [...counts.values()].every((c) => c === 1) | |
| const allNonEmpty = nextRows.every((r) => r.key.length > 0) | |
| if (allUnique && allNonEmpty) { | |
| const next: { [key: string]: JsonValue } = {} | |
| for (const r of nextRows) { | |
| next[r.key] = obj[r.key] ?? schemaDefault(valueSchema) | |
| } | |
| onChange(next) | |
| // Order-independent: reordering keeps the same key set, so external | |
| // sync won't fight a drag. | |
| lastSyncedKeysRef.current = nextRows | |
| .map((r) => r.key) | |
| .sort() | |
| .join('\u0000') | |
| } | |
| } | |
| function commitRows(nextRows: DictionaryRow[]) { | |
| setRows(nextRows) | |
| const counts = new Map<string, number>() | |
| for (const r of nextRows) { | |
| if (r.key.length === 0) continue | |
| counts.set(r.key, (counts.get(r.key) ?? 0) + 1) | |
| } | |
| const allUnique = [...counts.values()].every((c) => c === 1) | |
| const allNonEmpty = nextRows.every((r) => r.key.length > 0) | |
| if (allUnique && allNonEmpty) { | |
| const prevKeyById = new Map(rows.map((r) => [r.id, r.key])) | |
| const next: { [key: string]: JsonValue } = {} | |
| for (const r of nextRows) { | |
| const prevKey = prevKeyById.get(r.id) | |
| const carried = prevKey !== undefined ? obj[prevKey] : undefined | |
| next[r.key] = carried ?? obj[r.key] ?? schemaDefault(valueSchema) | |
| } | |
| onChange(next) | |
| // Order-independent: reordering keeps the same key set, so external | |
| // sync won't fight a drag. | |
| lastSyncedKeysRef.current = nextRows | |
| .map((r) => r.key) | |
| .sort() | |
| .join('\u0000') | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@console/web/src/pages/Configuration/tabs/WorkersTab/schema-form/DictionaryField.tsx`
around lines 100 - 122, commitRows currently loses values when a row's key is
renamed because it only looks up obj[r.key]; capture the prior per-row value by
recording a map from each existing row's id to its current value before calling
setRows (e.g. build prevValues: Map(row.id -> (obj[row.key] ??
schemaDefault(valueSchema))) using the existing rows array), then when you build
next inside commitRows use prevValues.get(r.id) first, falling back to
obj[r.key] and then schemaDefault(valueSchema) (references: commitRows,
DictionaryRow.id, rows, obj, schemaDefault, valueSchema, lastSyncedKeysRef).
Comment on lines
+35
to
+38
| // Leave the key `undefined` when the value is unset so the trigger shows | ||
| // the placeholder instead of stringifying `undefined` into `j:undefined`. | ||
| const currentKey = | ||
| value === undefined ? undefined : encodeEnumValue(value as JsonValue) |
There was a problem hiding this comment.
null is not treated as the unset state.
onClear commits null as the canonical empty value (Line 60), but currentKey only maps undefined to the placeholder. A cleared field therefore produces encodeEnumValue(null) → j:null, which matches no option, so the trigger renders a mismatched value instead of the emptyLabel/placeholder. Treat null as unset as well (assuming null isn't itself an enum member):
🐛 Proposed fix
const currentKey =
- value === undefined ? undefined : encodeEnumValue(value as JsonValue)
+ value === undefined || value === null
+ ? undefined
+ : encodeEnumValue(value as JsonValue)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Leave the key `undefined` when the value is unset so the trigger shows | |
| // the placeholder instead of stringifying `undefined` into `j:undefined`. | |
| const currentKey = | |
| value === undefined ? undefined : encodeEnumValue(value as JsonValue) | |
| // Leave the key `undefined` when the value is unset so the trigger shows | |
| // the placeholder instead of stringifying `undefined` into `j:undefined`. | |
| const currentKey = | |
| value === undefined || value === null | |
| ? undefined | |
| : encodeEnumValue(value as JsonValue) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@console/web/src/pages/Configuration/tabs/WorkersTab/schema-form/EnumField.tsx`
around lines 35 - 38, The code treats only undefined as the unset state so
clearing the field (which sets null in onClear) becomes encodeEnumValue(null)
and shows "j:null" instead of the placeholder; update the currentKey logic (the
variable named currentKey used with encodeEnumValue and the onClear behavior) to
treat both null and undefined as unset (i.e., map value === undefined || value
=== null to undefined) so the trigger shows emptyLabel/placeholder when cleared
— assume null is not a valid enum member.
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.
Summary by CodeRabbit
New Features
Documentation
Chores