Skip to content

feat(stop): explain blocking change conflicts#829

Open
tlm wants to merge 7 commits into
mainfrom
wsp-960-stop-start-error-output
Open

feat(stop): explain blocking change conflicts#829
tlm wants to merge 7 commits into
mainfrom
wsp-960-stop-start-error-output

Conversation

@tlm

@tlm tlm commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Description

This improves the workshop stop UX when the stop action is blocked by another
workshop change, especially a refresh that is paused on error.

Previously, if a user ran workshop stop dev while a refresh was paused on
error, the CLI only received a generic daemon error. The output did not explain
which change was blocking the stop or what the user should do next.

Before:

error: cannot stop "dev": waiting on error

After:

error: cannot stop "dev": refresh change is waiting on error
To view details: "workshop tasks 29"

To abort and undo the refresh: "workshop refresh --abort dev"
Otherwise, resolve the error, then run "workshop refresh --continue dev"
After that, run "workshop stop dev" again.

For non-refresh change conflicts, the CLI also now has enough context to show a
more specific blocking-change message:

error: cannot stop "dev": change launch is in progress
To view details: "workshop tasks 30"

Design

This change keeps the existing project layering:

  • workshopstate.StopMany remains responsible for deciding whether stop can
    proceed.
  • conflict.CheckChangeConflict remains the source of blocking change details.
  • The daemon API layer translates typed domain errors into API error responses.
  • The client translates API error values into typed client errors via
    errors.As.
  • The CLI formats user-facing recovery guidance.

The daemon now returns structured change-conflict API errors when a workshop
action is blocked by a change conflict. The response includes the blocking
change ID, kind, status, project ID, and workshop name.

Example API error value:

{
  "kind": "change-conflict",
  "message": "workshop \"dev\" has \"refresh\" change in progress",
  "value": {
    "change-id": "29",
    "change-kind": "refresh",
    "change-status": "Wait",
    "project-id": "...",
    "workshop": "dev"
  }
}

The client exposes this through client.ChangeConflictError, allowing CLI
commands to inspect structured change details without parsing human-readable
daemon messages.

QA

Unit and package tests run:

go test ./client
go test ./cmd/workshop -check.f TestStopChangeConflict
go test ./internal/overlord/conflict
go test ./internal/overlord/workshopstate -check.f TestStopManyWaitingReturnsChangeConflict
go test ./internal/daemon -check.f TestStopWorkshopChangeConflict

End-to-end manual QA:

  1. Created a local workshop with a project SDK hook that initially succeeds.
  2. Launched the workshop.
  3. Changed the SDK hook to fail.
  4. Ran refresh with --wait-on-error.
  5. Ran workshop stop while the refresh was paused.

Observed output:

error: cannot stop "refresh-output": refresh change is waiting on error
To view details: "workshop tasks 4"

To abort and undo the refresh: "workshop refresh --abort refresh-output"
Otherwise, resolve the error, then run "workshop refresh --continue refresh-output"
After that, run "workshop stop refresh-output" again.

The paused refresh was then cleaned up with:

workshop refresh --abort refresh-output

Self-review quick check

  • Make decisions that cost a lot to reverse explicit in the PR description.
  • Avoid nested conditions.
  • Delete dead code and redundant comments.
  • Normalise symmetries by sticking to doing identical things identically.
  • Check that coupled code elements, files, and directories are adjacent. For example, test data is stored as close as possible to a test.
  • Put variable declaration and initialisation together.
  • Divide large expressions into digestable and self-explanatory ones. Use multiple variables if required.
  • Put a blank line between two logically different chunks of code.
  • Follow the style guide for new error messages.

Docs

Procedure:

  • I have checked and added or updated relevant documentation.
  • I have checked and added or updated relevant release notes.
  • I have included the technical author in the review.

Content:

  • Headings and titles accurately describe the content.
  • New and updated pages include correct metadata.
  • Documentation tests are added or updated where applicable (for tutorial/ and how-to/ sections).
  • Documentation follows the style guide.
  • If needed, docs/.coverage.yaml updated, coverage tags added (.. artefact).

Or:

  • I confirm the PR has no implications for documentation.

@tlm tlm self-assigned this Jun 5, 2026
@dmitry-lyfar

Copy link
Copy Markdown
Collaborator

@tlm thank you, I just noted, we don't use any commit message annotation. Just capital letter, preferably start with a verb.

@tlm

tlm commented Jun 7, 2026

Copy link
Copy Markdown
Contributor Author

@dmitry-lyfar ta, will update going forward.

@tlm tlm requested a review from dmitry-lyfar June 7, 2026 23:20
@tlm tlm mentioned this pull request Jun 8, 2026
18 tasks

@dmitry-lyfar dmitry-lyfar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are unit tests and linter issues

@dmitry-lyfar dmitry-lyfar marked this pull request as draft June 9, 2026 04:50
@tlm tlm mentioned this pull request Jun 9, 2026
18 tasks
tlm added 6 commits June 9, 2026 23:39
Add a change-conflict API error kind and client-side conversion to a typed ChangeConflictError so commands can inspect blocking change details without parsing daemon error messages.
Show a clearer error when stop is blocked by a refresh change, including the tasks command and the refresh abort/continue steps needed before retrying stop.
Preserve blocking change details when stop is requested for a workshop
waiting
on an errored change, so API clients can surface the change kind,
status, and
task ID.
Expose blocking change details in change-conflict API errors so clients
can guide users to the relevant tasks output and recovery commands
without parsing human-readable daemon messages.
Reorder the test imports so they match the project's gci configuration.
Update the mid-refresh removal test to expect the blocking change status
now preserved by ChangeConflictError.
@tlm tlm force-pushed the wsp-960-stop-start-error-output branch from 69ed36c to 34de700 Compare June 10, 2026 00:15
@tlm tlm marked this pull request as ready for review June 10, 2026 00:15
"Change" is internal overlord terminology that the CLI never exposes;
users inspect work through "workshop tasks". Reword the stop conflict
errors to say task instead, and drop the change kind from the generic
conflict message since it added internal jargon without helping the
user act on the error.
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.

2 participants