ENG-755 - [build] A work item's remote_* fields are its ticket: rename them

[druks-operator]

Linear ticket: ENG-755

A work item is born from a ticket and carries it for life, so the fields that hold it are named for what they are.

WorkItem.remote_key/remote_url become ticket_key/ticket_url, along with get_for_remote_key() → get_for_ticket_key(), set_remote_status() → set_ticket_status(), and the work_items_remote_unique index → work_items_ticket_unique. A migration renames the columns and recreates the index; the column contract is unchanged (ticket_key required, ticket_url nullable).

The rename carries the whole contract in one move. Build's stored input drops remote_title/remote_url for ticket_title/ticket_url. WorkItemSummary emits ticketKey/ticketUrl, and DashboardItem emits ticketKey where it used to say ticketRef — the same value had two names across the board's two responses. The frontend mirror follows, and now declares ticketKey required, matching the column and dropping fallbacks that could never fire.

ticket_ref survives on Build's input and in the build prompts: that one is the reference rendered into an agent's instructions, carried on the input so the workflow needn't fetch the item. Ticket.status_name, Ticket.status_kind, and Links.ticket already read as ticket-domain names and are untouched.

Deploying

In-flight build runs must drain first. There is no dual-name path, so replaying a run whose stored input still holds remote_title/remote_url will fail signature validation.

[druks-reviewer]

Verdict: approve

All five acceptance criteria are satisfied by the diff at fe31ca8. The rename lands atomically across the migration, models, schemas, subscribers, workflow input, tests, and frontend, with no compatibility aliases and no scope creep.

Per-criterion

• AC1 — pass. backend/migrations/versions/f2c8b81a9d4e_rename_work_item_ticket_fields.py chains off d3a5c71f8e40, drops work_items_remote_unique, renames both columns via alter_column(..., new_column_name=...) with existing_nullable preserved (ticket_key NOT NULL, ticket_url nullable), and creates work_items_ticket_unique on (source, ticket_key). The downgrade is a faithful mirror. backend/tests/test_work_item_ticket_migration.py seeds a row at d3a5c71f8e40, upgrades, asserts columns/data/index, downgrades, and asserts restoration; it is registered in _OWN_DATABASE_MODULES. CI shows it ran and passed rather than skipping.
• AC2 — pass. WorkItem exposes only ticket_key, ticket_url, get_for_ticket_key(), and set_ticket_status(). get_label(), ship(), close_external(), Ticket.ref(...), the log line, the query predicate, create(), and update() all moved. Both subscribers and both PR-webhook paths call set_ticket_status(); test_ticketing.py covers the push, the non-tracker no-op, and the failure-close path.
• AC3 — pass. Build.run_multistep now takes ticket_title/ticket_url alongside the retained ticket_ref, and Build.dispatch passes all three. test_dispatch_pulls_cancelled_item_back_onto_the_board captures the full start() kwarg dict, so the stored-input shape is pinned by an assertion rather than by reading.
• AC4 — pass. WorkItemSummary declares ticket_key/ticket_url; the detail test asserts ticketKey, ticketUrl, links.ticket, and — usefully — the exact key set of the summary object, which is what actually proves remoteKey/remoteUrl are no longer emitted. The frontend interface and all five consumers read the new names, including the renamed workItemTicketKey parameter in slug.ts. DashboardItem.ticket_ref and Links.ticket are untouched, as specified.
• AC5 — pass. git grep for the legacy identifiers over backend/druks, backend/tests/ship, and frontend/src returns nothing. The only remaining hits are the baseline revision, d3a5c71f8e40, the new reversible migration, and the migration compatibility test — exactly the set the plan's migration-history reconciliation allows.

Verification

This sandbox has neither a Postgres server nor npm, so the profile commands could not run here beyond uv run ruff check backend (passed). Both GitHub checks on fe31ca8 are green and cover the same ground: On Pull Request Backend ran ruff and uv run pytest backend/ (986 passed, including the new migration test), and On Pull Request Frontend ran lint, test, and build. The drain requirement is recorded in the PR body under Risk.

Follow-up recommendations

Neither of these blocks the merge; file them as separate tickets if they're worth the churn.

1. Test-DB configuration convention. backend/tests/test_work_item_ticket_migration.py introduces its own DRUKS_TEST_PG environment variable, while the rest of the suite reads DRUKS_TEST_DATABASE_URL through druks.testing.TEST_DATABASE_URL. Combined with the skipif(not _postgres_is_reachable()) guard, a future environment that points the standard variable somewhere non-default would silently skip this test rather than fail. The constraint is that the migration test should follow the suite's single source of truth for the test database; deriving the maintenance URL from TEST_DATABASE_URL and keeping the isolated-database creation is one way, and dropping the reachability skip so an unreachable server errors loudly is another — either is reasonable, and you know the CI matrix better than I do. Whichever you pick, test_work_item_ticket_fields_upgrade_and_downgrade_preserve_schema_and_data is the test to keep green.
2. upgrade(..., "head") in the migration test. Targeting "head" means the test's assertions travel with whatever revision lands next, so an unrelated future migration could change what this test is actually exercising. Pinning to f2c8b81a9d4e would keep it a focused test of this revision. Same test as above.

Push back on either of these if you disagree — the reachability skip in particular may be a deliberate choice for local runs without a database, in which case say so and I'll drop it.

[druks-operator]

Code review: Clean, fully-consistent rename of WorkItem's remote_key/remote_url to ticket_key/ticket_url across models, schemas, workflows, migration, and both backend/frontend call sites, with a solid reversible migration test following the repo's existing scratch-database pattern.

[czpython]

1. backend/tests/test_work_item_ticket_migration.py — delete it. 129 lines proving ALTER TABLE … RENAME COLUMN works. Worse, it invents its own test infrastructure to do it: a new DRUKS_TEST_PG env var with hardcoded creds, _recreate_database()/_drop_database() via raw psycopg, and a skipif(not _postgres_is_reachable()) that silently passes when PG is absent. The house pattern is right there in test_proof_extension_migration.py:5 — TEST_DATABASE_URL from druks.testing, 54 lines, and it tests migration machinery, not one migration. None of your 8 existing migrations has a per-migration test. This one adds a permanent _OWN_DATABASE_MODULES entry and a novel precedent to cover a rename Alembic can't get wrong.

2. assert set(summary) == {...} in test_api_work_items.py. Pins the full wire key set inside a test whose job is "composes summary status and timeline". Pydantic already guarantees the shape; this just breaks an unrelated test the next time a field lands. Same shape of over-pinning as the full-kwargs assert started == {...} in the dispatch test.

3. The ticket now has two canonical names. Post-rename, WorkItemSummary.ticketKey and DashboardItem.ticketRef carry the identical string — schemas.py:157 literally does ticket_ref=item.ticket_key. Before the rename that was a translation between domains; now it's two names for one thing across two wire contracts of the same board. The PR explicitly rules out fixing it. That's the half of "a work item's fields are its ticket" that didn't land. ticket_ref on the workflow input is defensible (it's the prompt-rendered reference); DashboardItem.ticket_ref isn't.

[druks-reviewer]

@czpython — acknowledged. Three changes will be made:

1. Delete backend/tests/test_work_item_ticket_migration.py and remove its _OWN_DATABASE_MODULES entry in conftest.py. The novel test infrastructure (DRUKS_TEST_PG, raw psycopg lifecycle, reachability skip) was flagged as a follow-up recommendation in the review; you're right that the right call is deletion, not cleanup. No existing migration has a per-migration test, and Alembic column renames don't need one.

2. Remove the over-pinning assertions — drop assert set(summary) == {...} from test_api_work_items.py (the individual ticketKey/ticketUrl field assertions stay) and replace assert started == {...} in test_build_dispatch.py with targeted assertions on the three ticket kwargs (ticket_ref, ticket_title, ticket_url) only.

3. Rename DashboardItem.ticket_ref → ticket_key (wire: ticketRef → ticketKey) — this revises AC4, which had explicitly ruled it out. The argument lands: schemas.py:157 already does ticket_ref=item.ticket_key, so post-rename it's two names for one thing across two contracts of the same board. Changes:

• backend/druks/contrib/ship/schemas.py: field ticket_ref: str → ticket_key: str and from_work_item keyword arg to match
• frontend/src/extensions/ship/api.ts: DashboardItem.ticketRef → ticketKey
• frontend/src/extensions/ship/slug.ts: dashboardItemPath reads item.ticketKey instead of item.ticketRef
• frontend/src/extensions/ship/HistoryPage.tsx: both item.ticketRef reads → item.ticketKey

ticket_ref on the workflow input (Build.run_multistep) is untouched per your note that it remains defensible there.

Superseded by commit 91bb6cb implementing the requested revision.

[druks-reviewer]

Round 2 — pass

All five acceptance criteria hold at 91bb6cb. The revision-2 delta (93904ff..91bb6cb) applies exactly the three changes czpython requested, and nothing else.

Per-criterion

• AC1 — pass. backend/migrations/versions/f2c8b81a9d4e_rename_work_item_ticket_fields.py is untouched by this revision: down_revision = "d3a5c71f8e40", drops work_items_remote_unique, renames both columns with existing_nullable=False (key) / True (url), creates work_items_ticket_unique on (source, ticket_key), with a mirrored downgrade. backend/tests/test_work_item_ticket_migration.py is deleted (129 lines gone), and "test_work_item_ticket_migration" is absent from _OWN_DATABASE_MODULES in backend/tests/conftest.py.
• AC2 — pass. models.py exposes only ticket_key, ticket_url, get_for_ticket_key(), set_ticket_status(); both subscribers.py call sites use set_ticket_status(). Unchanged from round 1 and still clean.
• AC3 — pass. Build.run_multistep keeps ticket_ref and takes ticket_title/ticket_url; Build.dispatch passes all three. test_build_dispatch.py:39-41 now asserts the three keys individually — no assert started == {...} remains.
• AC4 — pass. DashboardItem declares ticket_key: str and from_work_item passes ticket_key=item.ticket_key (schemas.py:140,157). The frontend follows: api.ts interface field, slug.ts:85 dashboardItemPath, and both HistoryPage.tsx reads (search filter line 48, rendered cell line 128). WorkItemSummary and its consumers are unchanged. TicketCell.tsx and WorkItemsPage.tsx's <TicketCell ticketRef={wi.ticketKey} …> are untouched, as scoped. The over-pinned assert set(summary) == {...} block is gone; the individual summary["ticketKey"] / summary["ticketUrl"] / links.ticket assertions survive.
• AC5 — pass. grep for remote_key|remote_url|remote_title|set_remote_status|remoteKey|remoteUrl across backend/druks, backend/tests, and frontend/src returns hits only in 1ba6e314b314_baseline_schema.py, d3a5c71f8e40_…, and the new reversible migration — exactly the permitted set. The surviving ticket_ref/ticketRef occurrences are all the carved-out lineage: workflows.py (workflow input), prompt_context.py, the two build templates, test_build_prompts.py, test_build_dispatch.py's kwarg assertion, slug.ts's generic workItemPath parameter, and TicketCell's prop.

Verification

This sandbox has no Postgres server (connection to 127.0.0.1:5432 refused) and no npm/node, so most profile commands could not run locally. uv run ruff check backend passed locally ("All checks passed!"). Both GitHub checks on 91bb6cb are green and cover the same ground: On Pull Request Backend ran ruff plus uv run pytest backend/ (985 passed — one fewer than round 1's 986, consistent with the deleted migration test), and On Pull Request Frontend ran lint, test (all vitest files green), and build (tsc -b && vite build, built in 346ms). The drain note is recorded in the PR body under Risk.

Note on the ticket-vs-review conflict

The Linear ticket text says "DashboardItem.ticket_ref already reads off remote_key and needs no change." The reviewer's point 3 knowingly overrides that, and the revised plan documents the override with its reasoning. Flagging it here only so the record shows the divergence was deliberate, not accidental — no action needed.

Follow-up recommendations

1. No test pins DashboardItem's new ticketKey wire key. The history endpoint tests (test_api_work_items.py:159,181,194) exercise the serializer but only assert titles and counts, so a future regression on this field name would ship silently. Two reasonable options: extend test_history_returns_only_done_work_items with a single items[0]["ticketKey"] == "…" assertion, or add one to test_pr_closed_without_merge_is_cancelled_in_history. Either stays well clear of the whole-shape pinning you asked to remove — a single field assertion is the same shape as the summary["ticketKey"] line that survived in the detail test. Not blocking; the frontend tsc build plus the green suite give reasonable cover today.

If you disagree with the follow-up above — including "the type-level coverage is enough, skip it" — say so and I'll drop it.

[druks-operator]

Code review: The DashboardItem.ticket_ref → ticket_key rename goes beyond the ticket's stated scope (ENG-755 explicitly says that field needs no change) and this revision also deleted the only migration round-trip test plus loosened two unrelated test assertions, so I've filed a follow-up ticket for those.