test: combined 797 + 798 + 799 for CI validation - #800
Closed
skevetter wants to merge 7 commits into
Closed
Conversation
✅ Deploy Preview for devsydev canceled.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for images-devsy-sh canceled.
|
skevetter
changed the title
workspace up currently runs synchronously end-to-end, streaming a single JSON blob to the caller only once the container is ready. That blocks the CLI/desktop for the whole provisioning window and gives up all in-progress state on a crash or disconnect. Adds a submit/poll durable execution model alongside the existing synchronous flow: - pkg/task: a JSON-file task store (atomic writes, cross-process flock) recording status/PID/result for background-launched work. - workspace up --detach re-execs itself as a background process and returns a task ID immediately. - workspace task list/get/logs/cancel/rm: standard verb commands (kubectl/docker/gh-style) to manage submitted tasks. - pkg/devcontainer/status: a structured Phase/Event/Reporter model threaded through Runner.Up and the devcontainer build/run/setup pipeline, replacing ad hoc log lines with typed progress events. - A StatusUpdate RPC on the agent tunnel streams those events back to the CLI host live instead of waiting for the final result. - Desktop app updated to submit+poll via the new task model instead of parsing a single terminal JSON blob. See docs/rfcs/async-workspace-up.md for the full design.
Fixes the unused status import left over from the intermediate up.go split, plus cyclop, funcorder, errcheck, forbidigo, goconst, golines, modernize, and revive findings across the new task/status/detach code.
fmt.Printf is disallowed by the forbidigo rule; fmt.Fprintf(os.Stdout, ...) is the same output without needing a suppression comment.
workspace_up now submits `up --detach` and polls `workspace task logs
<id> --follow` instead of running `up` to completion. The e2e mock CLI
still implemented the old synchronous `up` only: `--detach` was
silently swallowed by parseArgs's generic flag skip, so mock-devsy
wrote plain progress text instead of a {kind:"task", id} envelope.
cli.run's JSON.parse then threw, the up handler took its catch path,
and the wizard's success check never ran -- so the "Open Workspace"
button never appeared, timing out the three affected e2e specs.
Adds a `task` verb to the mock (list/get/logs/cancel/rm) backed by a
`tasks` map in the persisted mock state, and makes `up --detach` write
the {kind:"task", id} envelope and defer the actual workspace/progress
simulation to `task logs --follow`, mirroring pkg/task and
pkg/devcontainer/config/envelope.go.
…passthrough git clone already runs with --progress (added in #590), but its \r-delimited "Receiving objects: NN% (x/y)" frames were logged verbatim, producing a wall of near-duplicate lines for large clones. progressWriter splits on \r as well as \n (git overwrites its progress line with \r rather than emitting a new one), logs percentage updates thinned to one per 10 points, and passes any other line through unchanged.
…passthrough
log.Writer() is used as a subprocess's Stdout/Stderr at ~65 call sites
across the codebase (docker build, buildkit, SSH, IDE installers, git
clone, etc). Its levelWriter.Write wrote raw bytes directly to the
stderr sink, bypassing zap's encoder entirely -- contradicting its own
doc comment ("writes each line as a log entry") and producing
unencoded lines in what's supposed to be a uniform JSON/text/logfmt
stream in --log-output json mode.
levelWriter now line-buffers and calls Info/Debug/Error per complete
line, so subprocess output goes through the same encoder as every
other log call. Close flushes any trailing line left without a
newline (previously silently dropped).
Reorder levelWriter's exported Close before the unexported logLine helper, and hoist the repeated "json" format literal in writer_test.go into a constant.
skevetter
force-pushed
the
test/combined-log-and-git-progress
branch
from
July 28, 2026 23:30
3fcdd07 to
1398645
Compare
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
1 participant
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.
Not for merge — combines all three PRs (#797 log fix, #798 async workspace up, #799 git clone progress) into one branch purely to exercise the full CI/e2e integration suite with all three changes applied together in GitHub runners.
#797, #798, and #799 remain open as the actual review/merge units. Close this once CI has run.