[RAPTOR-19729] fix(workload): recovery never ends at the file, and carries its own exit by cdevent · Pull Request #863 · datarobot-oss/cli

cdevent · 2026-08-28T13:06:52Z

RATIONALE

RAPTOR-19729. Follow-up to #846, which is already merged and closed two of this ticket's messages. The ticket stayed open because its other half — "up must never leave 'delete the file/state' as the way out" — was undone, and because the fix landed on the wrong end of the sequence.

The message that gets there first was missed. deployable() speaks for the run once the workload is already stuck. awaitRunning is what prints at the moment it becomes stuck, and it still ended at "finished as errored; check the logs" — precisely the dead end the ticket describes, and the reason the folk remedy was hand-deleting workloadId, after which the create hit 409 and the conflict advised setting the exact line just deleted.

Four more refusals ended at deleting the state directory, because re-pointing a project at another artifact had no command: dr artifact code init refused a linked directory and said to delete the directory too. Nothing could be reworded until something existed to reword onto.

CHANGES

awaitRunning carries the same exit as its sibling — the delete that clears the binding, hedged with dr workload logs for errored and certain for terminated, matching what nameTaken and deployable already do.

dr workload up --recreate performs that exit inside the deploy: delete the bound workload, clear the binding, create it again under the same name.

Acts only on errored/terminated. Any other state is refused — a workload a deploy can act on is deployed onto, not replaced.
Asks for the workload name to be typed back (the gesture confirmLock already uses for production) unless --yes.
Deletes nothing under --dry-run. This is the one ordering hazard: the delete must precede the plan for the plan to describe the create it now is.
A failed binding-clear does not fail the run — the irreversible half is already done, and the create overwrites the key anyway.

dr artifact code init --force re-points an already-linked directory in place, via a new wapi.Relink. The catalog comes from the artifact named on the command line, exactly as a fresh init takes it; the BASE manifest resets, because it describes the tree synced into the artifact being left. The ignore file and history.log survive — which is the point, since deleting the directory discards them along with the code catalog.

Five messages now name a command instead of a path: reusedArtifactConflict (up), the locked-artifact refusal in code sync, init's own already-linked refusal, and the artifact note workload delete prints.

NOTES

Two sites are deliberately left, per scope agreed before implementation: manifest/write.go's "Delete the file instead" and workload config's "Delete it to run setup again". Both need dr workload config --force before they can name a command. The first is the most literal instance of this ticket's title and sits on the supported dr workload delete --dir recovery path, so it is the natural next follow-up.

One correction worth flagging: linkProject preserves catalogId/lastSyncedVersionId only because it re-points within the same lineage. An arbitrary relink cannot, so the new messages do not claim it does — what --force genuinely buys over a deletion is that it is a command, and that the ignore file and history survive.

This PR also unbreaks internal/workload/up's tests, which have not compiled since 6bea35a changed waitWorkloadFn's signature and left one test fake behind. That is unrelated to this ticket but blocked running anything here.

TESTING

task lint — clean across all three GOOS legs.
task test — green under -race. (internal/plugin's TestDiscoverWithContextSuite flakes under full-suite load; it passes in isolation and on a clean tree, and that package is untouched here.)
New: up/recreate_test.go (deletes for both dead states, refuses a live one, no-ops unbound, dry-run deletes nothing, no-terminal-no---yes refuses, failed clear does not stop the run, failed delete does); wapi/relink_test.go; sync/phase1_locked_test.go; plus cases in up/run_test.go, init/cmd_test.go, init/display_test.go, up/cmd_test.go.
Regression guards assert NotContains on the deletion advice, so these messages cannot quietly revert.
Verified --help and the live already-linked refusal against a real binary; confirmed the refusal leaves config.json untouched.
Live scenario RAPTOR-19729-E-recovery.sh added and registered (task smoke-test-workload). Not yet run against a tenant — it needs a real API token.

Note

Medium Risk
--recreate performs irreversible workload deletes on a core deploy path; guards limit it to dead states and require confirmation, but misuse or flag bugs could remove production-adjacent resources.

Overview
Workload recovery adds dr workload up --recreate, which deletes a bound workload that is errored or terminated, clears workloadId in .datarobot.yaml, and continues the deploy as a fresh create under the same name. It refuses other states, requires typing the workload name (or --yes), and under --dry-run only prints what would happen—no delete before the plan.

Artifact relinking adds dr artifact code init --force and wapi.Relink so an already-linked project can point at another artifact without removing .datarobot/workload/. Catalog and sync baseline come from the new artifact; ignore file and history are kept, and the previous artifact id is shown for undo.

Error and help text no longer tell users to delete .datarobot/workload/ or hand-edit bindings. Stuck deploy paths (awaitRunning, deployable, locked sync, artifact conflicts, workload delete notes) now point at dr workload delete, --recreate, or init --force.

Docs, changelog, unit tests, and live smoke scenario E (RAPTOR-19729-E-recovery.sh) cover the new behavior.

^{Reviewed by Cursor Bugbot for commit 853a872. Configure here.}

datarobot-pr-review-router · 2026-08-28T13:07:07Z

🎫 Jira: RAPTOR-19729 — CLI recovery must never end at "delete the file"

cursor

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 853a872. Configure here.}

wojtekwdr · 2026-09-01T08:41:08Z

+		return opts.Confirm(fmt.Sprintf(
+			"Workload %s is %s. It will be deleted, which cannot be undone, and created again "+
+				"under the same name.\nType the workload name to recreate it, anything else to stop: ",
+			tui.WarnStyle.Render("`"+workloadName+"`"), live.Status), workloadName)


[medium] Nothing wires a Confirm that returns true, so this want argument is never checked. I changed it to a literal "yes" and the whole up suite stayed green, which means the type-the-name gesture the flag's safety story rests on isn't asserted anywhere. confirmRoll has that test at roll_test.go:298.

wojtekwdr · 2026-09-01T08:41:09Z

+	// The JSON envelope is the same shape a fresh link produces: a caller
+	// parsing it is asking which artifact this directory pushes to, and the
+	// answer does not depend on whether it had a different one a moment ago.
+	if outputFormat == outputformat.OutputFormatJSON {


[medium] This returns before printRelinked and initResult has no field for previous, so --force --output-format json never emits the id it replaced. The comment a few lines up says that id is the only thing left that remembers where the link came from, and docs/commands/artifact.md says the same. A nullable previousArtifactId on initResult would keep both paths honest.

wojtekwdr · 2026-09-01T08:41:09Z

+
+	now := time.Now().UTC()
+
+	cfg := Config{


[low] question: Config also carries LastBuiltVersionID and this literal drops it, so imageStale sees nil and the next deploy rebuilds the image. I think that's the right answer after re-pointing at a different artifact, but the comment above settles every other field by name and is silent on this one.

…s undo Addresses review on #863. --recreate keyed its consent on NonInteractive, which `up` also sets for a run whose stdin is a pipe. `dr workload up --recreate < /dev/null` therefore deleted the workload with no prompt and no --yes, and the refusal below it was unreachable from the command — while the flag help, the docs and the changelog all promise it asks. Options grows a Yes that means only the flag and its env var, and confirmRecreate reads that. Rolling a locked version can still be waved through by a reviewed pipeline; deleting cannot, because there is nothing on the other side of it. Nothing asserted the name typed back, so the gesture the flag's safety rests on could have been a literal. Both are covered now: the piped case fails on the unwired delete seam if it regresses, and the prompt test reads what the deploy hands Confirm as the expected answer. `init --force --output-format json` returned before printRelinked and had no field for it, so JSON was the one way to run the command that lost the id it replaced — the id that is the undo. initResult carries previousArtifactId, null for a fresh link. Scenario E asserted less than it looked like. `.action` is null on every run because the envelope is `{"up": {...}}`, so E.4 failed unconditionally; the forbidden-pattern regex matched none of the strings it guards against, since a path sits between the verb and the dot; and two skips scored as passes, one of them before the workload was registered for cleanup. Both skips are now failures, registration moved ahead of them, and the pattern anchors on `.datarobot/` so it stays off `.datarobot.yaml`. Relink's comment settled four of Config's five fields by name; CLIVersion is named now, with a note that the list is the whole struct.

chasdr · 2026-09-01T16:00:44Z

+
+	// Read before write, so a corrupted config is reported as itself rather
+	// than silently replaced by this one. It also carries CreatedAt across.
+	previous, err := LoadConfig(projectDir)


does init --force need a guard for the same artifact id? relinking to the artifact already linked resets the BASE manifest and lastSyncedVersionId, so the next sync diffs everything against an empty base and any unsynced local edit becomes a remote-wins conflict with a *.LOCAL copy.

no-op when previous.ArtifactID matches, or is same-id re-init intended?

that was a good point added guard in next commit

…rries its own exit PR #846 gave the deployable() refusals and the name conflict their exit. It missed the message that gets there first, and left the other half of the ticket — that `up` must never leave "delete the file/state" as the way out — undone. The first message. A deploy whose workload finishes errored ended at "check the logs", which is precisely where recovery ran out of documented steps: the folk remedy was hand-deleting the workloadId, after which the create hit 409 and the conflict advised setting the exact line just deleted. deployable() speaks for the run once the workload is already stuck; awaitRunning is what prints at the moment it becomes stuck, and it now names the same delete, hedged for errored and certain for terminated, exactly as its sibling is. The exit, performed. `dr workload up --recreate` deletes the bound workload and creates it again under the same name — the recovery the refusals name, folded into the deploy that was already asked for. It acts only on errored and terminated, because a workload a deploy can act on is deployed onto rather than replaced; it asks for the name to be typed back, the gesture confirmLock already uses for production; and it deletes nothing under --dry-run, which is the one ordering hazard, since the delete has to precede the plan for the plan to describe the create it now is. The state directory. Four messages ended at deleting it, because re-pointing a project at another artifact had no command: `dr artifact code init` refused a linked directory and said to delete the directory too. `--force` re-points in place, taking the catalog from the artifact named on the command line as a fresh init does, and keeping the ignore file and history a deletion discards. The refusal in `up`, the one in `code sync`, `init`'s own, and the note `workload delete` prints now all name it. Two sites are deliberately left: the manifest's "Delete the file instead" and `workload config`'s "Delete it to run setup again". Both need `dr workload config --force` before they can name a command. The first is the most literal instance of this ticket and sits on the supported recovery path, so it is the next follow-up. Also unbreaks `internal/workload/up`'s tests, which have not compiled since 6bea35a changed waitWorkloadFn's signature and left one fake behind. RAPTOR-19729 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The dry run skipped the delete, correctly, but then handed the plan the dead workload it had just announced it would delete. So the preview described a deploy that cannot happen: the plan rendered a roll onto a workload the very next check refuses ("errored, so there is nothing healthy to deploy onto"), and the JSON envelope reported action "rolled" where the real run reports "created" — directly under a hint line saying the workload would be deleted and recreated. A preview that names a different action than the run is not a preview, and --dry-run is documented as the answer to "what would this deploy" in CI. The delete is still skipped; what changes is that the state handed back is the one the real run reaches. Nothing can act on it: Run stops at its own DryRun branch before anything is applied. The test pins the two against each other rather than against a literal, so a preview and a run can only drift apart by failing. The live scenario asserts the same on the JSON envelope. RAPTOR-19729 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…s undo Addresses review on #863. --recreate keyed its consent on NonInteractive, which `up` also sets for a run whose stdin is a pipe. `dr workload up --recreate < /dev/null` therefore deleted the workload with no prompt and no --yes, and the refusal below it was unreachable from the command — while the flag help, the docs and the changelog all promise it asks. Options grows a Yes that means only the flag and its env var, and confirmRecreate reads that. Rolling a locked version can still be waved through by a reviewed pipeline; deleting cannot, because there is nothing on the other side of it. Nothing asserted the name typed back, so the gesture the flag's safety rests on could have been a literal. Both are covered now: the piped case fails on the unwired delete seam if it regresses, and the prompt test reads what the deploy hands Confirm as the expected answer. `init --force --output-format json` returned before printRelinked and had no field for it, so JSON was the one way to run the command that lost the id it replaced — the id that is the undo. initResult carries previousArtifactId, null for a fresh link. Scenario E asserted less than it looked like. `.action` is null on every run because the envelope is `{"up": {...}}`, so E.4 failed unconditionally; the forbidden-pattern regex matched none of the strings it guards against, since a path sits between the verb and the dot; and two skips scored as passes, one of them before the workload was registered for cleanup. Both skips are now failures, registration moved ahead of them, and the pattern anchors on `.datarobot/` so it stays off `.datarobot.yaml`. Relink's comment settled four of Config's five fields by name; CLIVersion is named now, with a note that the list is the whole struct.

Relink rebuilds the sync baseline from the artifact it is given: BASE goes back to empty and lastSyncedVersionId is cleared. That is correct when the artifact changes — the old baseline describes a code store the new artifact never had — and pure loss when it does not. With BASE empty, Classify takes the absent-base branch, so a file present on both sides with different bytes is ADD_CONFLICT rather than LOCAL_MODIFIED. Phase 5 renames the local copy to <path>.LOCAL.<timestamp> and downloads the remote over it. The same edit uploads cleanly against the baseline the relink discarded, so `init --force <same-id>` silently traded an unsynced edit for a sidecar file. --force says do not refuse, and the end state it asks for is the one the directory is already in, so the same id is answered by doing nothing and saying what was left alone — not by an error. The text path stops printing "Re-pointed <dir> from artifact A to A"; the JSON path reports the current link with previousArtifactId null, because nothing was replaced. The check is the caller's: only it knows whether being handed the current id is a mistake or a deliberate baseline reset. Relink's contract now says so, and says what performing one costs.

The field is real — it arrived with RAPTOR-19519, after the base this branch was written on — and Relink's literal drops it, which the comment did not say. Worse, the comment claimed to have settled the whole of Config, so it asserted completeness while one field went unmentioned. Dropping it is the right answer and now says so: the id records a code version in the catalog of the artifact being left, so it cannot vouch for what the new one serves. nil is what imageStale already treats as stale, so the next deploy buys one rebuild instead of inheriting an image from another lineage. The completeness line is corrected to six fields and now says why it has to stay complete: the literal is the file after the write, so a field left out of it is a field cleared. TestRelink_ClearsTheLastBuiltVersion pins the behaviour so it is a decision rather than an omission.

github-actions Bot added the go Pull requests that update go code label Aug 28, 2026

cdevent marked this pull request as ready for review August 28, 2026 13:09

cdevent requested review from a team as code owners August 28, 2026 13:09

cursor Bot reviewed Aug 28, 2026

View reviewed changes

Comment thread internal/workload/up/recreate.go Outdated

cdevent force-pushed the RAPTOR-19729-recovery-never-ends-at-the-file branch from e1356e8 to da617cc Compare August 31, 2026 12:00

wojtekwdr requested changes Sep 1, 2026

View reviewed changes

chasdr reviewed Sep 1, 2026

View reviewed changes

cdevent and others added 5 commits September 2, 2026 16:27

cdevent force-pushed the RAPTOR-19729-recovery-never-ends-at-the-file branch from 674727e to bf8be84 Compare September 2, 2026 13:30

cdevent requested a review from wojtekwdr September 3, 2026 12:48

Navigation Menu

Uh oh!

[RAPTOR-19729] fix(workload): recovery never ends at the file, and carries its own exit - #863

[RAPTOR-19729] fix(workload): recovery never ends at the file, and carries its own exit#863
cdevent wants to merge 5 commits into
mainfrom
RAPTOR-19729-recovery-never-ends-at-the-file

cdevent commented Aug 28, 2026 •

edited by cursor Bot

Loading

Uh oh!

datarobot-pr-review-router Bot commented Aug 28, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

Uh oh!

wojtekwdr Sep 1, 2026

Uh oh!

Uh oh!

Uh oh!

wojtekwdr Sep 1, 2026

Uh oh!

wojtekwdr Sep 1, 2026

Uh oh!

Uh oh!

chasdr Sep 1, 2026

Uh oh!

cdevent Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

cdevent commented Aug 28, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

RATIONALE

CHANGES

NOTES

TESTING

Uh oh!

datarobot-pr-review-router Bot commented Aug 28, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wojtekwdr Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wojtekwdr Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

wojtekwdr Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chasdr Sep 1, 2026

Choose a reason for hiding this comment

Uh oh!

cdevent Sep 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cdevent commented Aug 28, 2026 •

edited by cursor Bot

Loading