[codex] TASK-000001: Rebuild theme system with Linear-style light mode and sidebar toggle

[zhangweijian97]

Linear issue: TASK-000001

This PR was created by the devos.ing ADHD (Agentic Development Hub & Daemon) workflow.

Includes:

• plan + implement session output
• separate review/testing session

[zhangweijian97]

Review status: FAIL (TASK-000001)

The theme preference primitives were added, but two blocking issues remain before merge:

1. Theme utility contract regression in web UI

• Components now reference classes like bg-theme-canvas, text-theme-primary, border-theme-default, and hover-bg-theme-subtle, but these selectors are no longer defined in packages/web/src/app/globals.css.
• Impact: referenced classes resolve to no-op styles, so themed surfaces regress across sidebar/board/project UI.
• Fix expectation: restore the removed @layer utilities theme class mappings (or fully migrate all consumers to equivalent existing styles/CSS variables) so every referenced theme-* class has concrete CSS behavior.
• Verify with: bun run --filter web build + manual light/dark/system toggle check via sidebar control.

2. Root quality gate is red (bun test)

• Current run reports 4 failures:
  • 3x packages/server/tests/express-server-listen.test.ts (EADDRINUSE on server.listen(0))
  • 1x packages/server/tests/task-lifecycle-e2e.test.ts (Unable to bind task lifecycle test server)
• Fix expectation: harden test server bind behavior (allocation/retry/teardown isolation) so bind failures do not occur in this environment.
• Verify with: rerun bun test and confirm all suites pass.

Please address both blockers and re-run the full gates before re-review.

[zhangweijian97]

devos.ing implementation feedback for TASK-000001

Review/testing summary:
The PR introduces the requested theme primitives (system/light/dark preference model, persisted override, pre-hydration init script, and sidebar toggle wiring), but it regresses the web UI theme contract by removing the custom theme-* utility class definitions that the updated components now depend on, so large parts of the UI no longer receive intended themed styles; additionally, the required bun test run completed but failed with 4 failing tests (packages/server/tests/express-server-listen.test.ts and packages/server/tests/task-lifecycle-e2e.test.ts) due local server bind failures, so the workspace is not currently green.

Fix instructions for the implementation agent:
Address every item below, use each body as the repair checklist, and rerun the listed verification checks.

Bugs to fix:

1. Theme utility classes were removed but are still referenced across the UI
   Failing command/repro: RIPGREP_CONFIG_PATH=/dev/null rg -n "bg-theme-canvas|text-theme-primary|border-theme-default|hover-bg-theme-subtle" packages/web/src/components | head and RIPGREP_CONFIG_PATH=/dev/null rg -n "^\.(bg-theme-canvas|text-theme-primary|border-theme-default|hover-bg-theme-subtle)" packages/web/src/app/globals.css.
   Observed: Many components now use theme-* classes, but packages/web/src/app/globals.css no longer defines those utility selectors, so those classes have no CSS effect and themed styling regresses.
   Expected: All referenced theme-* utility classes should be defined so light/dark/system theming applies consistently across the UI.
   Likely files/code path: packages/web/src/app/globals.css (missing @layer utilities theme class definitions), plus consumers under packages/web/src/components/**.
   Fix expectation: Restore the removed @layer utilities theme class mappings (or migrate all affected components to equivalent existing classes/CSS variables) so every referenced theme-* class resolves to real styles.
   Verification: bun run --filter web build, then manual UI check that issue board/projects/sidebar surfaces render with expected themed colors in light, dark, and system modes while toggling via the sidebar button.

2. Root quality gate is red: bun test fails on local server bind tests
   Failing command/repro: bun test.
   Observed: Test run ends with 4 failures: three in packages/server/tests/express-server-listen.test.ts (EADDRINUSE on server.listen(0)) and one in packages/server/tests/task-lifecycle-e2e.test.ts (Unable to bind task lifecycle test server).
   Expected: bun test should pass for a workable workspace gate.
   Likely files/code path: packages/server/tests/express-server-listen.test.ts, packages/server/tests/task-lifecycle-e2e.test.ts (test server bind/retry strategy).
   Fix expectation: Stabilize server test binding behavior (port allocation/retry/teardown isolation) so these tests pass reliably in this environment.
   Verification: Re-run bun test and confirm all suites pass with no bind-related failures.

[zhangweijian97]

Validation failed due to a UI regression in the theme rollout.

bun test passes, but several updated web components reference undefined CSS custom properties, causing dropped styles and broken themed surfaces (transparent panels, missing borders/highlights).

Required fix before merge

1. Undefined theme tokens break themed panels
   • Tokens currently unresolved in packages/web/src/app/globals.css: --bg-card, --bg-control, --border-strong, --bg-interactive, --bg-control-subtle.
   • Impacted UI areas: Create Task, Agent Monitor, Operator Section panel, and issue cards.
   • Likely files:
     • packages/web/src/app/globals.css
     • packages/web/src/components/task-create/task-create-panel.tsx
     • packages/web/src/components/agent-monitor/agent-monitor-panel.tsx
     • packages/web/src/components/web-shell/operator-section-panel.tsx
     • packages/web/src/components/issues-board/issue-card.tsx
   • Fix expectation: define missing tokens in the shared theme set (light + dark), or replace usages with already-defined tokens.
   • Verify by rerunning bun test and visually checking affected panels in both themes via bun run --filter web dev.

[zhangweijian97]

devos.ing implementation feedback for TASK-000001

Review/testing summary:
bun test passed, but the theme rollout has a concrete UI regression: several changed components now reference CSS custom properties that are not defined in packages/web/src/app/globals.css, so the browser drops those styles and the themed panels/buttons lose their intended borders/backgrounds. That means the acceptance scope is not fully met yet.

Fix instructions for the implementation agent:
Address every item below, use each body as the repair checklist, and rerun the listed verification checks.

Bugs to fix:

1. undefined theme tokens break themed panels
   Failing command/repro: bun run --filter web dev, then open the operator UI and inspect the Create Task, Agent Monitor, and issue cards.
   Observed: the browser drops declarations that use var(--bg-card), var(--bg-control), var(--border-strong), var(--bg-interactive), and var(--bg-control-subtle) because those custom properties are not defined in packages/web/src/app/globals.css, so several surfaces render transparent or lose borders/highlights.
   Expected: every theme token referenced by the new UI should resolve in both light and dark modes, so the themed panels/buttons render consistently.
   Likely files/code path: packages/web/src/app/globals.css, packages/web/src/components/task-create/task-create-panel.tsx, packages/web/src/components/agent-monitor/agent-monitor-panel.tsx, packages/web/src/components/web-shell/operator-section-panel.tsx, packages/web/src/components/issues-board/issue-card.tsx.
   Fix expectation: define the missing custom properties in the theme token set or swap the references to existing tokens already defined in globals.css.
   Verification: rerun bun test, then open the web UI and confirm the affected panels keep their intended borders/backgrounds in both themes.

[zhangweijian97]

Review result: FAIL

The PR is close to scope and bun test is currently healthy in this workspace (655 passed, 0 failed), but there is one blocking regression risk:

1. Theme provider crashes when localStorage access throws
   • readStoredThemePreference in packages/web/src/lib/theme/theme.ts is not exception-safe.
   • In environments where storage.getItem(...) throws, theme initialization can crash via ThemeProvider (packages/web/src/components/providers/theme-provider.tsx → getInitialThemeState).
   • Fix expected: wrap storage reads in try/catch and fall back to a safe default (for example system) while keeping existing preference validation behavior unchanged.
   • Verification expected:
     1. Re-run the provided repro and confirm it exits 0 with fallback behavior.
     2. Add/extend a web theme unit test for throwing storage access, then run bun test.

[zhangweijian97]

devos.ing implementation feedback for TASK-000001

Review/testing summary:
The implementation meets most of the scoped goal (theme helpers, init script, and sidebar toggle wiring are present) and bun test is runnable/workable in this workspace (655 passed, 0 failed), but there is a concrete regression risk: persisted-preference reads in the runtime provider are not exception-safe, so environments where localStorage.getItem throws can crash the UI during theme initialization instead of safely falling back.

Fix instructions for the implementation agent:
Address every item below, use each body as the repair checklist, and rerun the listed verification checks.

Bugs to fix:

1. Theme provider crashes when localStorage access throws
   Failing command/repro: bun -e 'import { readStoredThemePreference } from "./packages/web/src/lib/theme/theme.ts"; const storage={getItem(){ throw new Error("blocked") }} as Pick<Storage,"getItem">; try { readStoredThemePreference(storage); console.log("ok"); } catch (error) { console.error("threw", (error as Error).message); process.exit(1); }'.
   Observed: The command exits non-zero and logs threw blocked; in app runtime this means ThemeProvider initialization can throw when storage access is denied.
   Expected: Theme preference read should gracefully fall back (e.g., to system) when storage APIs throw, matching the defensive behavior already used in the init script.
   Likely files/code path: packages/web/src/lib/theme/theme.ts (readStoredThemePreference and/or readThemePreference) called by packages/web/src/components/providers/theme-provider.tsx via getInitialThemeState.
   Fix expectation: Wrap storage reads in try/catch and return fallback on exception; keep preference validation logic unchanged.
   Verification: 1) Re-run the repro command above and confirm exit code 0 with fallback behavior. 2) Add/extend a web theme unit test for throwing storage access and run bun test.

[zhangweijian97]

Review Result: FAIL

bun test passed (656 passing, 0 failing), but the PR does not fully meet the success goal due to a persistence-path correctness bug.

Blocking bug

1. Theme toggle crashes when storage writes fail

• Repro:
  bun -e "import { writeStoredThemePreference } from './packages/web/src/lib/theme/theme'; const storage={setItem(){throw new Error('blocked')}}; try { writeStoredThemePreference(storage as any, 'light'); console.log('NO_THROW'); } catch { console.log('THREW'); }"
  outputs THREW.
• Observed: writeStoredThemePreference throws when localStorage.setItem fails, so manual theme toggling can error in blocked/quota/privacy contexts.
• Expected: Manual override should still apply/cycle theme even if persistence fails; storage failure should be non-fatal.
• Likely path: packages/web/src/lib/theme/theme.ts (writeStoredThemePreference), called by setPreference in packages/web/src/components/providers/theme-provider.tsx.
• Fix expectation: Guard storage writes with try/catch (or return success/failure without throwing) and continue applying in-memory/document theme updates regardless.
• Verification: Re-run repro and confirm no throw; add/extend unit test for write-failure handling; run bun test.

[zhangweijian97]

devos.ing implementation feedback for TASK-000001

Review/testing summary:
Reviewed the PR against the stated success goal (light/dark/system support, system detection, manual override, persisted preference, no initial theme flash, and sidebar toggle placement) and ran bun test successfully (656 passing, 0 failing), but found a correctness defect in the persistence path: when localStorage.setItem throws (quota/privacy/security cases), the manual theme toggle flow throws instead of degrading gracefully, which can break the user’s ability to switch theme and therefore does not fully satisfy the manual override/persistence reliability requirement.

Fix instructions for the implementation agent:
Address every item below, use each body as the repair checklist, and rerun the listed verification checks.

Bugs to fix:

1. Theme toggle crashes when storage writes fail
   Failing command/repro: bun -e "import { writeStoredThemePreference } from './packages/web/src/lib/theme/theme'; const storage={setItem(){throw new Error('blocked')}}; try { writeStoredThemePreference(storage as any, 'light'); console.log('NO_THROW'); } catch { console.log('THREW'); }" prints THREW (or reproduce in browser contexts where localStorage writes are blocked/quota-exceeded, then click the sidebar theme toggle).
   Observed: Theme preference write throws, so the manual toggle path can error instead of reliably applying/cycling theme.
   Expected: Manual override should still work even if persistence fails; storage failures should be swallowed/fallback-only.
   Likely files/code path: packages/web/src/lib/theme/theme.ts (writeStoredThemePreference), called by setPreference in packages/web/src/components/providers/theme-provider.tsx.
   Fix expectation: Wrap storage.setItem in try/catch (or make write helper return success/failure without throwing) and keep applying in-memory/document theme update regardless of persistence success.
   Verification: Re-run the repro command and confirm no throw; add/extend unit test to assert write-failure handling; run bun test.

[zhangweijian97]

Review/testing failed after 3 automated fix passes. Parked for human review and PR updates.

[zhangweijian97]

Review status: FAIL

Architecture is clean — theme system is well-structured with proper separation of concerns (theme.ts for logic, theme-provider.tsx for React state, CSS variables for styling). The light/dark/system cycle with localStorage persistence and FOUC-prevention init script is solid.

Blocking issues:

1. README.md polluted with test comments

   • Two lines added to README: // Codex direct test and // Codex daemon test
   • These are agent debugging artifacts, not documentation. Remove them.

2. Light mode color system not fully implemented

   • :root[data-theme="light"] only defines status colors (status-done-bg, etc.) in the diff
   • But all other CSS variables (bg-app, text-primary, etc.) are NOT redefined for light mode
   • All components already reference variables like var(--bg-sidebar), so without light overrides the UI will render identically in both themes
   • The dark :root block defines the full variable set, but :root[data-theme="light"] only has status colors
   • This means light mode toggle will have zero visible effect on the main UI

Expected: Every CSS variable defined in :root { } must have a corresponding override in :root[data-theme="light"] { }. The status colors are a good start but the bulk of the palette (backgrounds, text, borders, etc.) is missing.

Non-blocking observations:

• Test refactors (express-server-listen, task-lifecycle-e2e) look correct — removing port-binding dependencies improves test reliability
• Theme toggle placement in sidebar footer is correct per task description
• getThemeInitScript() FOUC prevention is a nice touch