Closed
Conversation
Signed-off-by: Connor Braa <connor@dagger.io>
Contributor
Author
|
switching back to draft. there's something fishy going on here. been testing with this one liner |
cwlbraa
added a commit
that referenced
this pull request
Jun 26, 2025
this PR implements "real" container resume where we store each Run WithExec in the container LLB definition and shores up the base of that container by saving off InitialSourceDir in state. InitialSourceDir is a made consistent and re-hydratable by using dag.Host().Directory(forkRepoPath).AsGit().Ref(initialWorktreeHead).Tree(). because we can rely on that SHA always pointing at the exact right base content and the forkRepo always having that SHA, we can source the base of sourcedir without keeping a copy of the first commit on disk. 6/25: this PR does have a big piece of "magic" in it. we template in the .git file to be gitdir: repos/name/worktrees/id so that, on export, we don't constantly override reset the git history. Other stuff buried in here: env.apply no longer takes arguments that give the false impression it's gonna make a commit. it just takes newState (the container) env.run calls apply run_background still doesn't. to make those restore we gotta do some additional thinking around how we data model service persistence. additionally on 6/25 cc @aluzzardi: better error handling in the event of failed commands. we now no longer fail the withexec for failed commands, but still propagate error information to the git notes and the tool responses. this is necessary for better prompt engineering around endpoints. the agent was getting confused and using localhost to try to talk to things it had run_backgrounded. closes #115. * working Signed-off-by: Connor Braa <connor@dagger.io> * cleanup Signed-off-by: Connor Braa <connor@dagger.io> * clean up apply Signed-off-by: Connor Braa <connor@dagger.io> * shore up error handling in run command Signed-off-by: Connor Braa <connor@dagger.io> * assorted fixes: - stop saving initialSourceDir - its saved in the llb. - have environment_update rebuild the base from workdir - improve prompt engineering around internal/external ports - improve run_command error handling so that failed commands can be saved into the container state Signed-off-by: Connor Braa <connor@dagger.io> * resolve build failure post-merge Signed-off-by: Connor Braa <connor@dagger.io> * make tests pass Signed-off-by: Connor Braa <connor@dagger.io> * shoutout guillaume's tests, i totally forgot about this part of the magic Signed-off-by: Connor Braa <connor@dagger.io> * remove unnecessary git notes initialization Signed-off-by: Connor Braa <connor@dagger.io> --------- Signed-off-by: Connor Braa <connor@dagger.io>
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.
we incidentally got a bunch of the "real" container resume logic working when @aluzzardi reworked state storage into git.
this PR shores up a hole in that resume: we weren't saving run commands into the ongoing container state.
the other hole is run_background... maybe that should just call AddService? does that make sense to you @aluzzardi?
this also includes minor fixes to env.container concurrency safety.