Skip to content

Tighten frontend TypeScript contracts#404

Merged
mariusvniekerk merged 18 commits into
mainfrom
codex/typescript-contract-cleanup
May 30, 2026
Merged

Tighten frontend TypeScript contracts#404
mariusvniekerk merged 18 commits into
mainfrom
codex/typescript-contract-cleanup

Conversation

@mariusvniekerk
Copy link
Copy Markdown
Collaborator

  • Tightens settings API types so generated OpenAPI clients carry the real contract.
  • Removes frontend compatibility shims and broad runtime normalization around typed settings data.
  • Centralizes async command handling and removes promise-shape probing.
  • Adds oxfmt as the explicit frontend formatter without enabling a baseline format check yet.
  • Documents frontend TypeScript cleanup rules to avoid defensive casts and fabricated tests.

@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (140b8f0)

Medium issues found: settings defaults may leak invalid zero/null values, and the formatter dependency is missing from the lockfile.

Medium

  • internal/server/settings_handlers.go:56
    buildLocalSettingsResponse now returns s.cfg.Terminal directly after the response schema/frontend types were tightened to require full non-null terminal settings. Configs that omit optional terminal fields can still expose font_size: 0, line_height: 0, or cursor_blink: null, which can break the settings UI or terminal defaults.
    Fix: Normalize the response here, or move defaulting into config load/validation so config.Terminal is always fully populated before API responses.

  • package.json:6
    The root package adds oxfmt scripts and devDependencies.oxfmt, but the diff does not update bun.lock. Frozen or CI installs using the lockfile may not resolve the declared formatter dependency consistently.
    Fix: Run bun install from the repo root and commit the resulting bun.lock update.


Synthesized from 2 reviews (agents: codex | types: default, security)

mariusvniekerk and others added 17 commits May 29, 2026 17:10
The kata CLI needs the workspace binding registered before issue tasks can be created for this repo. Initializing the existing middleman binding adds the per-machine override file to gitignore so local daemon configuration stays out of commits.\n\nValidation: not run (gitignore-only change).

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The Pierre diff wrapper had duplicated structural casts around getLineIndex even though the dependency now exposes that API in its public type surface. Keeping one typed accessor makes the adapter boundary explicit and avoids repeating an unchecked shape in the render helpers.\n\nValidation: npx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/components/diff/PierreFileDiff.svelte --svelte-version 5; bun run typecheck (packages/ui).

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The settings API normalizers were relying on broad response assertions, which hid nullable generated fields and broader string values from the app-facing types. Build the returned settings and preview objects field-by-field so defaults and supported enum fallbacks are explicit at the client boundary.\n\nValidation: bun --cwd frontend test src/lib/api/settings.test.ts; bun run typecheck from frontend

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The markdown renderer still needs adapter boundaries for Marked extension tokens, but broad assertions inside renderer bodies make the rendering logic harder to audit. Using Marked's renderer types for built-in overrides and a named item-ref token assertion keeps the unchecked boundary explicit without changing output.

Validation: bun run --cwd frontend test ../packages/ui/src/utils/markdown.test.ts; bun run --cwd packages/ui typecheck

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
Malformed SSE JSON should be treated as a bad frame, but a consumer callback throwing is application behavior that callers and tests need to observe. Splitting decode from invocation keeps bad payloads quiet without hiding downstream failures.

Validation: bun run --cwd packages/ui vitest run src/stores/events.svelte.test.ts; bun run --cwd packages/ui typecheck; npx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/stores/events.svelte.ts --svelte-version 5

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The draft store tests repeated provider refs and API response shapes in every scenario, which made stale-load and publish cases harder to adjust without changing unrelated setup details. Local builders keep the scenario-specific expectations visible while centralizing the default test double data.

Validation: bun run --cwd frontend test ../packages/ui/src/stores/diff-review-draft.svelte.test.ts
Validation: bun run --cwd packages/ui typecheck

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
Keyboard command handlers can return either void or Promise<void>, but dispatch and the command palette each open-coded promise handling through assertions just to reach `.then`. Centralizing the typed result handling keeps rejection surfacing consistent without weakening the command handler contract.

Validation: npx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/keyboard/Palette.svelte --svelte-version 5; bun run test src/lib/stores/keyboard/dispatch.svelte.test.ts src/lib/components/keyboard/Palette.svelte.test.ts; bun run typecheck

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
Some production Svelte handlers read DOM-specific properties from EventTarget via broad assertions. Narrowing currentTarget and target before reading input values, tagName, and contentEditable keeps the same behavior while making the event boundary explicit.\n\nValidation: bun run typecheck in frontend and packages/ui.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The settings client had accumulated response normalizers because the OpenAPI contract described typed settings fields as nullable or wider than the server actually returns. That made the TypeScript layer compensate for a schema problem instead of relying on generated types.

Tightening the source Huma/config type metadata lets the generated Go and TypeScript clients represent the real response shape. The frontend settings API can now return generated responses directly, while update requests keep using the same generated request types.

Validation: make api-generate; bun run typecheck in packages/ui and frontend; bun run test src/lib/api/settings.test.ts in frontend; go test ./internal/server ./internal/server/e2etest ./internal/config ./internal/apiclient/... -run 'Test.*Settings|Test.*RepositoryGlob|Test.*RepoImport|Test.*BulkAdd|Test.*Preview' -shuffle=on; go test ./... -shuffle=on.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The settings API contract now returns and accepts a complete typed terminal settings object, so the frontend should not preserve a parallel partial input type or fallback retry path for older response shapes.

Keeping those compatibility shims would reintroduce the same client-side normalization layer that the schema cleanup was meant to remove. The terminal form now turns blank numeric fields into the normal defaulted TerminalSettings object locally and sends that typed object directly.

Validation: npx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/settings/TerminalSettings.svelte --svelte-version 5; npx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/Provider.svelte --svelte-version 5; bun run typecheck in packages/ui and frontend; bun run test src/lib/components/settings/TerminalSettings.test.ts src/lib/components/terminal/TerminalOptionsMenu.test.ts src/lib/api/settings.test.ts in frontend.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The settings schema cleanup accidentally ran formatting over an existing API test and changed line wrapping without changing behavior. Keep this follow-up focused on restoring the prior style so the type/schema commits do not carry unrelated test churn.

Validation: bun run test src/lib/api/settings.test.ts in frontend.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The TypeScript and Svelte code had no repository-owned formatter, which made formatting changes depend on whichever tool happened to touch a file. Add oxfmt as the explicit formatter while leaving pre-commit unchanged until the existing frontend tree gets a deliberate baseline format pass.

Validation: bun install --frozen-lockfile; ./node_modules/.bin/oxfmt --version; ./node_modules/.bin/oxfmt --check --no-error-on-unmatched-pattern .oxfmtrc.json frontend/package.json. Known follow-up: bun run fmt:check currently reports 396 existing frontend/package files that would change, so it is intentionally not wired into pre-commit yet.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The EventSource regression test added a large fake browser boundary for a narrow callback behavior that is not worth carrying in this branch. The FocusListView search handler was also made more defensive even though the handler is only bound to an input and matching nearby code uses the direct cast.

Keep this follow-up focused on removing noisy cleanup so the type-safety branch does not expand into unrelated test scaffolding or cosmetic defensive code.

Validation: npx @sveltejs/mcp@0.1.22 svelte-autofixer packages/ui/src/views/FocusListView.svelte --svelte-version 5; bun run --cwd packages/ui typecheck; bun run --cwd packages/ui lint.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The async command rejection scenario was introduced to prove a helper extraction, not to cover behavior that changed in the palette itself. Keep the keyboard test suite focused on existing user interactions and avoid extra console-spy scaffolding for a type-cleanup branch.

Validation: bun run --cwd frontend test src/lib/components/keyboard/Palette.svelte.test.ts; bun run --cwd frontend typecheck.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
Recent frontend type-safety work showed that vague guidance lets cleanup drift into call-site assertions, defensive branches, normalization helpers, and fabricated tests. Capture the preferred pattern directly in the frontend and testing context docs: tighten the owning contract, keep assertions at real boundaries, and validate local type cleanup with existing behavior coverage.

Validation: git diff --check.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
Promise-like runtime checks are exactly the TypeScript pattern this branch is trying to avoid. The terminal panes were probing document.fonts.ready with typeof .then even though that browser API is already promise-typed; use the typed promise directly and make the frontend guidance explicitly ban thenable probing.

Validation: npx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/terminal/GhosttyTerminalPane.svelte --svelte-version 5; npx @sveltejs/mcp@0.1.22 svelte-autofixer frontend/src/lib/components/terminal/XtermTerminalPane.svelte --svelte-version 5; rg promise-shape probes in frontend/packages; bun run --cwd frontend typecheck; bun run --cwd frontend lint; git diff --check.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
The settings type cleanup removed frontend fallbacks, which exposed that the isolated e2e server was constructing config directly and serving zero-value terminal settings instead of the validated defaults used by normal config loading. Apply validation before the e2e server saves and serves its generated config so the API contract remains complete in full-stack tests.

The same CI run also caught a real DiffView regression from an over-defensive window key handler, plus two brittle e2e assertions that failed under full-suite background refresh and async diff scrolling. Keep those checks focused on user-visible behavior so local e2e failures point at product regressions instead of incidental timing.

Validation: go test ./cmd/e2e-server -shuffle=on; bun run --cwd packages/ui typecheck; bun run --cwd frontend typecheck; bun run --cwd frontend test; bun run --cwd frontend test ../packages/ui/src/components/diff/DiffView.test.ts; MIDDLEMAN_E2E_OUTPUT_FILE=/tmp/middleman-e2e-focused.log bun run --cwd frontend test:e2e -- --project=chromium tests/e2e-full/activity-collapse.spec.ts tests/e2e-full/workspace-tab-persistence.spec.ts; MIDDLEMAN_E2E_OUTPUT_FILE=/tmp/middleman-e2e-sidebar.log bun run --cwd frontend test:e2e -- --project=chromium tests/e2e-full/sidebar-collapse.spec.ts; MIDDLEMAN_E2E_OUTPUT_FILE=/tmp/middleman-e2e-terminal.log bun run --cwd frontend test:e2e -- --project=chromium --project=firefox --project=webkit tests/e2e-full/settings-terminal-font.spec.ts (Chromium passed; Firefox/WebKit blocked by broken local Playwright browser installs: missing libmozglue.dylib and missing WebKit executable). Full Chromium e2e was attempted twice and did not pass cleanly locally; failures moved between unrelated activity/sidebar assertions while the focused files passed.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
@mariusvniekerk mariusvniekerk force-pushed the codex/typescript-contract-cleanup branch from 745c98d to ff5e3d7 Compare May 29, 2026 21:11
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (ff5e3d7)

Summary verdict: Two medium issues need fixing; no high or critical findings were reported.

Medium

  • packages/ui/src/utils/markdown.ts:229
    The extracted listitem renderer calls this.parser.parse(token.tokens) without passing !!token.loose. Marked uses that flag to preserve tight vs loose list output, so tight markdown lists may start rendering with paragraph wrappers.
    Fix: Restore the loose flag: this.parser.parse(token.tokens, !!token.loose).

  • package.json:7
    The new fmt / fmt:check scripts depend on oxfmt, but the checked-in Bun lockfile was not updated. Clean or frozen installs may miss the new binary and cause formatter scripts to fail.
    Fix: Run bun install with the repo’s Bun workspace setup and commit the resulting bun.lock changes.


Synthesized from 2 reviews (agents: codex | types: default, security)

Playwright was pinned behind the current release, which meant browser installs and local e2e tooling could drift from the versions developers expect when refreshing shared browser caches. Move both the runner and CLI package together so their browser revision metadata stays aligned.

Validation: bun install --frozen-lockfile. Direct Bun Playwright invocations in this sandbox failed with CouldntReadCurrentDirectory before execution, so the lockfile check is the reliable local verification here.

Generated with Codex
Co-authored-by: Codex <codex@openai.com>
@roborev-ci
Copy link
Copy Markdown

roborev-ci Bot commented May 29, 2026

roborev: Combined Review (4fe1a05)

High-risk settings regression found; dependency lockfiles also need updating before merge.

High

  • internal/server/settings_handlers.go:56
    GET /settings now returns raw s.cfg.Terminal, while the schema/frontend expect terminal fields to be complete and non-null. Existing/default configs can still have zero or nil terminal values, so the API may emit font_size: 0, line_height: 0, or cursor_blink: null, and the frontend no longer normalizes them.
    Fix: Restore response normalization or apply terminal defaults during config load/validation before serving settings.

Medium

  • package.json:7
    The package manifests change Playwright/prettier dependencies and add oxfmt, but the Bun lockfiles are not updated. CI uses bun install --frozen-lockfile, so installs can fail or use stale dependency versions.
    Fix: Regenerate and commit the affected bun.lock files.

Synthesized from 2 reviews (agents: codex | types: default, security)

@mariusvniekerk mariusvniekerk merged commit be7d5db into main May 30, 2026
16 checks passed
@mariusvniekerk mariusvniekerk deleted the codex/typescript-contract-cleanup branch May 30, 2026 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant