Fix repo update silently dropping agent_cli; single-source the domain→row column mapping - #398
Merged
Merged
Conversation
…→row column mapping
`_update_repo` hand-copied ten columns onto the loaded row and omitted `agent_cli`, so
`PATCH /repos/{id}` with a new CLI reported success (the service returns its in-memory
merge, never a re-read) while the row kept its old value — an existing repo could never
be switched to codex, and the dashboard's edit reverted on refresh. Insert was fine:
`_RepoRow.from_domain` carried the field. Only the update path lost it, since #384.
Each row class now states its domain→column mapping once, in `column_values`, which both
the insert (`from_domain`) and the update (`_apply_columns`) go through, so a newly added
column can't be carried on create and silently dropped on update.
`_update_task` had the same hand-copied shape and was dropping `memo`, `initial_prompt`,
`starting_model` and `agent_cli` from `save_task` — also fixed by the shared mapping.
History stays append-only: it's a relationship, not a column, so `column_values` never
touches it.
The store contract tests had a reflective guard proving a *created* repo/task round-trips
with every field intact, but nothing equivalent for the update path. Added it for both,
with `_assert_every_field_changed` forcing the mutation fixture to move every new field so
the round-trip genuinely exercises it. Also fixed a false-positive REST test that asserted
only the PATCH *response* body — it passed against a row that still said `codex`.
No migration: the columns were always correct.
tildesrc
marked this pull request as ready for review
August 24, 2026 19:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PATCH /repos/{id}with a newagent_clireported success but never persisted it — an existing repo could never be switched to codex, and the dashboard's repo-edit reverted on the next refresh.Root cause
_update_repohand-copied ten columns onto the loaded row and omittedagent_cli. Insert was fine (_RepoRow.from_domaincarried the field), so only the update path lost it — introduced by #384, unnoticed until the dashboard grew the CLI picker.Store.update_repodocuments itself as "a full-row write … the store just overwrites the row", so this was a contract violation.Two things hid it:
TaskService.update_reporeturns its in-memory merge rather than a re-read, so the API response cheerfully reported the value that was never stored.test_repo_agent_cli_defaults_and_round_trips_over_restlooked like coverage but asserted only the PATCH response body, never re-reading. It passed against a row that still saidcodex.The fix
Each row class now states its domain→column mapping once, in
column_values, which both the insert (from_domain) and the update (_apply_columns) go through — so a newly added column can't be carried on create and silently dropped on update. One line would have fixedagent_cli; this fixes the class of bug._update_taskhad the identical hand-copied shape and was droppingmemo,initial_prompt,starting_modelandagent_clifromsave_task— the same shared mapping fixes it. History stays append-only: it's a relationship, not a column, socolumn_valuesnever touches it.Tests
The store contract tests already had a reflective guard proving a created repo/task round-trips with every field intact — but nothing equivalent for the update path, which is exactly the hole this bug fell through. Added it for both, with
_assert_every_field_changedforcing the mutation fixture to move every new field so the round-trip genuinely exercises it (a new field is guarded by default). Also tightened the false-positive REST test to assert the re-read.All five new/tightened assertions were confirmed to fail before the fix, on both store backends.
No migration — the columns were always correct.
Plan: the task's
plan.mdartifact.