feat: launch Kata task workspaces#598
Conversation
roborev: Combined Review (
|
roborev: Combined Review (
|
mariusvniekerk
force-pushed
the
codex/kata-workspace-diff-targets
branch
from
704346c to
a5248f8
Compare
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
WILL NOT DO. THESE TEST ARE A CANCER AND PROMOTE BAD DESIGN PATTERNS. |
|
lol |
roborev: Combined Review (
|
roborev: Combined Review (
|
roborev: Combined Review (
|
Kata tasks need a workspace action that behaves like provider issues without making Kata part of the provider registry or requiring manual mapping in the common local-clone setup. This design pins the server-side resolver, .kata.toml discovery boundary, manual settings fallback, action visibility rules, and test expectations before implementation. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Kata issues are not provider issues, so the design cannot require a synced provider issue number or reuse the provider issue workspace endpoint. This updates the spec to require a string workspace owner key for Kata task UIDs, a Kata-backed workspace creation endpoint, and repository mapping only for choosing the worktree base. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Kata-backed workspaces should keep the workspace sidebar aligned with the live Kata task browser instead of reusing provider issue UI just because the worktree has a repository. This records that kata_task workspaces mount the existing KataIssueDetail component through a loader/adapter, fetch live daemon contents, and fall back only to stored Kata metadata when live data is unavailable. Validation: git diff --check. Generated with Codex Co-authored-by: Codex <codex@openai.com>
The workspace sidebar should reuse the live Kata task detail UI, but the existing component may need its browser-specific assumptions pulled out before it can be embedded cleanly. This keeps the requirement focused on one shared task-detail implementation while allowing the implementation plan to refactor the existing boundary instead of cloning the UI for workspaces. Validation: git diff --check. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Kata tasks need the same workspace affordance as provider issues without pretending their UID is a provider issue number. Store a string owner key and Kata metadata on workspace rows so create/reuse can key by Kata issue UID while provider workspaces preserve numeric numbers. Repo resolution is intentionally conservative: manual settings mappings or a unique watched clone .kata.toml match choose the target repo, and ambiguous tasks expose no button. Workspace sidebars render the live Kata detail component so Kata task workspaces do not fall back to provider issue UI. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Kata tasks are daemon-owned strings, so raw issue UIDs cannot safely key workspaces once multiple Kata daemons or projects are in play. Requiring the daemon ID and storing a scoped owner key prevents cross-project reuse while keeping provider issue workspaces on their numeric keys. Live Kata project payloads can expose opaque project UIDs while local clones identify the project through .kata.toml identity or name. The resolver now treats UID/identity as the stronger signal and only falls back to project name when exactly one watched clone claims it, so unclear mappings still hide the workspace action. Validation: go test ./internal/db ./internal/workspace ./internal/server -shuffle=on; cd frontend && ../node_modules/.bin/vp test run src/lib/features/kata/KataWorkspace.test.ts; bun run check; git diff --check; scripts/context-sync --check. Verified in the embedded browser by creating a Kata-backed workspace and opening the live Kata task sidebar pane. The first two commit-hook runs hit a local tmux timeout in TestFleetSnapshotEmptyTmuxServerE2E under full package concurrency; rerunning that test alone passed. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Kata and issue workspaces do not have a merge target until a PR is associated with the workspace. Rendering Target unconditionally invited guaranteed 404s, and relative worktree roots could create the worktree under the Git base repo while the API later read a different path. Normalize workspace roots to absolute paths, route non-PR merge-target lookup through associated_pr_number, and document the diff-scope contract so future workspace UI work keeps Target tied to real PR availability. Validation: go test ./internal/workspace -run 'TestCreateKataTask|TestCreateIssue' -shuffle=on; go test ./internal/server -run 'TestWorkspaceDiffEndpointReportsMergeTarget' -shuffle=on; cd frontend && ../node_modules/.bin/vp test run --project unit ../packages/ui/src/components/workspace/WorkspaceRightSidebar.test.ts; ./node_modules/.bin/vp run ui-package-check; git diff --check; scripts/context-sync --check. Verified in the embedded browser at http://127.0.0.1:64298/terminal/7c29c65820bbc431 that the Kata workspace diff shows HEAD and Branch only, with no file-list error. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Migration 000036 introduced real schema artifacts but left the SQL file as a no-op and reconciled columns later in Go. That made an impossible version/schema mismatch look supported and encouraged future migrations to hide bad fixtures behind conditional column repair. Put the owner-key schema changes back into the numbered migration and make rewind fixtures remove later artifacts before claiming an older schema version. The migration docs now state that version and physical schema must match, so duplicate-column states fail instead of being papered over. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Migration 000036 should only define the schema and backfill existing rows. A trigger that fills item_key for future inserts hides call sites that failed to adopt the new required column. Remove the fill trigger and make current-schema workspace fixtures pass item_key explicitly, matching the production InsertWorkspace path. The migration guidance now calls out that new required columns should be written by inserts rather than papered over by triggers or repair hooks. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Unsupported downgrade fixtures were modeling impossible version/schema combinations and had started driving production migration design in the wrong direction. Keep historical fixtures limited to forward migration tests, remove the ordinary downgrade/legacy-row tests, and document that invariant for future schema work. Kata workspace rows also need scoped local branch/worktree identities and live embedded task controls to match their scoped owner keys. This keeps same-short-ID tasks from colliding, hides merge-target-dependent diff controls when no PR exists, wires recurrence actions in the embedded task pane, and tolerates settings payloads that omit kata_projects. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Aliased testify/assert imports made test helpers depend on a package alias that should not exist and contradicted the test guidance. Enforce the unaliased assert import through importas, normalize existing Assert usages to assert.New/assert calls, and document the convention so future test edits have both lint and context coverage.
Validation: go test ./tools/testifyhelpercheck -shuffle=on; go test ./... -run '^$' -shuffle=on; GOCACHE="${GOCACHE:-/tmp/middleman-gocache}" mise exec -- golangci-lint run --fix; go test ./internal/server -run TestMsgvaultConfigureRejectsInvalidUpdateWithoutReplacingPriorConfig -shuffle=on; git diff --check; scripts/context-sync --check.
Generated with Codex
Co-authored-by: Codex <codex@openai.com>
Rebasing onto current main surfaced stale assert helper aliases in tests that are compiled through the PR merge ref. Normalize those call sites so the assert import policy and the merged test set agree.\n\nDeleting a configured repo also has to keep kata project mappings valid, because config save validates mappings against the remaining exact repo list. Cascade mappings for the deleted repo and roll back both config slices if persistence fails.\n\nValidation: go test ./internal/server -run 'TestHandleDeleteRepo|TestHandleDeleteRepoRemovesKataProjectMappings' -shuffle=on; go test ./internal/github ./internal/server -run '^$' -shuffle=on; make lint; make test-short. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Non-PR workspaces no longer render merge-target-dependent controls, including the commit range picker. The full-stack workspace diff spec was still opening that picker, so Chromium CI failed after the intended UI change.\n\nUpdate the spec to assert the two available scopes and the absence of Target and commit-range controls for a workspace without a merge target.\n\nValidation: cd frontend && ../node_modules/.bin/vp test run; cd frontend && ../node_modules/.bin/vp exec -- playwright test --config=playwright-e2e.config.ts --project=chromium --project=firefox tests/e2e-full/00-workspace-tab-persistence.spec.ts. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Deleting a configured repo can invalidate Kata project mappings because config saves validate those mappings against the exact configured repo set. Record that settings deletion must cascade the matching mappings in the same save and rollback path so future settings work does not rediscover the failure mode.\n\nValidation: git diff --check; scripts/context-sync --check. Generated with Codex Co-authored-by: Codex <codex@openai.com>
Automatic project-name fallback was gated on a global count: if any watched clone's .kata.toml carried a uid/identity, name matching was disabled for every clone. A valid name-only Kata project then stopped resolving as soon as an unrelated watched clone gained stable identity metadata, hiding its workspace action. Make the guardrail per-entry instead: name matching considers only clones that declare no uid/identity, and still requires exactly one match. A clone with stable identity is matched by uid/identity alone and is never resolved by a colliding name, which also removes the ambiguity a reviewer flagged in the design spec. Document the precedence and the both-fields matching rule so separate implementations cannot diverge. Validation: go test ./internal/server -run TestKataWorkspaceTarget -shuffle=on; go vet ./internal/server; golangci-lint run ./internal/server; scripts/context-sync --check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Kata workspaces were rendered through the provider PR/issue path in the workspace list. The item bubble opened the PR sidebar tab, its context menu offered "Open item on provider" / "Copy item URL" built from provider item number 0, and the row showed "#0" instead of the Kata task identity. Teach the list its third owner type. Kata rows now open the kata_task sidebar tab, render the stored short/qualified ID (falling back to a "Kata" label) with the task title in the tooltip and as the display name, suppress the provider item URL since a Kata task has no provider PR/issue, and match search against the task identity instead of the numeric item. The bubble gets its own state class and is width-capped so variable-length identifiers do not disturb the row layout. Validation: cd frontend && node ../node_modules/vite-plus/bin/vp test run src/lib/components/terminal/WorkspaceListSidebar.test.ts; full unit project run; frontend-check (fmt, lint, svelte-check). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The diff-scope contract said the merge-target scope exists "when the workspace has a PR identity," but the server only resolves it when a synced repo row, a synced merge request row, and a non-empty base branch all exist. A workspace whose associated PR is unsynced, removed, or has no base branch therefore still satisfies the documented condition while the API returns "merge target branch not available," reopening the 404 path the hidden controls were meant to prevent. State availability as a resolvable merge target branch, make the server the authority, and record that the sidebar's PR-identity gate is necessary but not sufficient so the divergence is a known, expected state rather than a surprise. Note the resolved-merge-target summary signal as the follow-up that would let the UI gate match the server check exactly. Validation: scripts/context-sync --check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The workspace list now treats kata_task as a third owner type, but that contract lived only in the implementation: a future change could diverge on how a Kata row labels its bubble, what it falls back to when short_id/qualified_id/ title are absent, whether item_number 0 is a value to show, and which provider actions apply. Document the list-row presentation in the design spec — bubble identity fallback (short_id, then qualified_id, then "Kata"), display-name fallback (title, then branch), the kata_task sidebar tab, the suppression of provider item-URL actions, and identity-based search — so the two label fallback chains and the "item_number 0 is incidental, not a sentinel" rule are explicit. Also assert the full Kata identity (project name, short/qualified ID, title) on the create-workspace response. That real-HTTP plus SQLite test is what proves the wire shape the list rendering depends on, so the frontend component fixture is backed by a backend contract test rather than asserting backend-shaped values on its own. Validation: go test ./internal/server -run TestCreateKataWorkspace -shuffle=on; scripts/context-sync --check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Kata workspace search only indexed the optional short_id, qualified_id, title, and project name. A Kata workspace created without a short or qualified ID renders the generic "Kata" bubble and was then unsearchable: none of its durable identifiers were in the haystack, so it could not be located by its actual task UID or owner key. Add the daemon, project, and issue UIDs and the stored item_key to the Kata search haystack so such a workspace stays findable by its key, and document the behavior alongside the rest of the Kata list-row contract. Validation: cd frontend && node ../node_modules/vite-plus/bin/vp test run src/lib/components/terminal/WorkspaceListSidebar.test.ts; svelte-check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The workspace list reloads from GET /workspaces, but coverage only asserted the create response. If list serialization or the DB summary hydration dropped the kata object, create could pass while the list fell back to a generic "Kata" or branch-only label after a refresh. Add a real-backend list test that seeds a kata_task workspace and asserts GET /workspaces returns the full Kata owner metadata (project UID/name, short and qualified IDs, title) plus item_number 0. Also reconcile the design spec: the ownership-schema and API-shape sections claimed item_number was absent/omitted for Kata workspaces, but it is a non-nullable integer always emitted as 0, so the contract now says exactly that and tells consumers to ignore it. Document the list-endpoint and list-row fallback expectations in the coverage section. Validation: go test ./internal/server -run TestListWorkspacesIncludesKataMetadata -shuffle=on; scripts/context-sync --check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
.kata.toml lives in a watched repository whose contents are not trusted. A contributor could commit it as a symlink to an endless or very large file such as /dev/zero; when a maintainer opens a Kata task, /kata/workspace-target scans each configured worktree and os.ReadFile followed that symlink with no file-type or size limit, so untrusted repo content could stall or exhaust memory in the local middleman process and take down the console. Lstat the path and accept only a regular file (rejecting symlinks, devices, FIFOs, and directories), re-check the opened descriptor to close the Lstat/open swap window, and read through an explicit 64 KiB cap before TOML decoding. The file only carries a tiny [project] table, so the cap is generous while making an unbounded read impossible. Validation: go test ./internal/server -run 'TestReadKataProjectTOML|TestKataWorkspaceTarget' -shuffle=on; go vet ./internal/server; golangci-lint run ./internal/server. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Capture in the Kata design spec that .kata.toml is untrusted repository content read defensively (regular-file only, bounded size), so a later resolver change does not reintroduce an unbounded os.ReadFile that a malicious clone could abuse to stall or exhaust the process. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
kata_projects is declared as a required non-null array in the settings schema, but the default no-mappings case left it as a nil slice (slices.Clone(nil)), which serializes to "kata_projects": null. That contradicts the schema's nullable:"false" contract and forces clients to handle a null where the type promises an array. Normalize the cloned mappings to an empty slice before returning, matching how agents and launch targets are already handled. Add a wire-level test asserting the default response encodes the field as [] rather than null. Validation: go test ./internal/server -run 'TestHandleGetSettings|TestHandleUpdateSettingsPersistsKataProjectMappings' -shuffle=on; go vet ./internal/server; golangci-lint run ./internal/server. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Rebasing this branch onto main combined main's newly added GitHub App token tests (which still import testify/assert aliased as Assert) with this branch's importas rule that forbids the alias. Git's three-way merge kept main's new Assert.* call sites while taking this branch's unaliased import, leaving undefined: Assert and breaking compilation, lint, and the Go test jobs on the PR merge ref. Normalize the three surviving Assert.* usages to the unaliased assert package so the merged tree compiles and satisfies the no-alias lint rule. Validation: golangci-lint run ./...; make testify-helper-check; go build ./...; go test ./internal/config ./internal/tokenauth ./internal/githubapp ./internal/github ./cmd/middleman -shuffle=on. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mariusvniekerk
force-pushed
the
codex/kata-workspace-diff-targets
branch
from
ec2bd39 to
faa6cf8
Compare
roborev: Combined Review (
|
2 participants
Kata tasks need the same workspace launch path as provider issues without treating Kata issue IDs as provider issue numbers. This keeps Kata owned by the daemon while still resolving a watched repository when the mapping is clear.
This adds conservative Kata project-to-repo resolution, daemon-scoped string workspace identity, live Kata task details inside workspace sidebars, and diff controls that only show merge target comparison when a real associated PR exists.
Validation: targeted Go and Svelte tests, ui-package-check, context-sync, and embedded-browser smoke.
generated by a clanker