feat(dispatch): let a repository preserve the agent run already in flight #7007
There are no files selected for viewing
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
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
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -489,6 +489,20 @@ func newRunCmd() *cobra.Command { | |
| } | ||
|
|
||
| func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRepo, fullsendBinary string, envFiles []string, noPostScript bool, debug string, forgeFlag string, eventFile string, rFlags resolveFlags, sOpts statusOpts, printer *ui.Printer, keepSandbox bool, oFlags runOverrideFlags) (runErr error) { | ||
| // Captured first, before harness resolution, token minting and env | ||
| // expansion, each of which can take real time — a mint call retries over | ||
| // the network. Anything on the work item after this instant is activity | ||
| // the agent must reconcile against, and a later capture would classify | ||
| // some of it as predating the run. | ||
| // | ||
| // This is still not the honest baseline. The run was dispatched before | ||
| // this process started, so the true start is the workflow run's | ||
| // server-side created_at, which costs an API call to read; no host clock | ||
| // inside this process can reach it. Recorded here so the two halves do | ||
| // not disagree about what "run start" means — the follow-up run watcher | ||
| // reached the same conclusion and uses the run record's created_at. | ||
| runStartedAt := time.Now().UTC() | ||
|
|
||
| printer.Banner(Version()) | ||
| printer.Blank() | ||
| printer.Header("Running agent: " + agentName) | ||
|
|
@@ -1884,7 +1898,8 @@ func runAgent(ctx context.Context, agentName, fullsendDir, outputBase, targetRep | |
| printer.StepFail("Failed to bootstrap sandbox") | ||
| return err | ||
| } | ||
| if err := bootstrapEnv(sandboxName, remoteRepositoryDir, h, rt.EnvExports(), | ||
| runFacts{headSHA: runHeadSHA(forgePlatform), startedAt: runStartedAt}, fetchEnvVal); err != nil { | ||
| printer.StepFail("Failed to bootstrap sandbox") | ||
| return err | ||
| } | ||
|
|
@@ -2777,6 +2792,8 @@ var reservedSandboxKeys = map[string]bool{ | |
| "FULLSEND_SLUG": true, | ||
| "FULLSEND_TIMEOUT_MINUTES": true, | ||
| "FULLSEND_ITERATION_DEADLINE": true, | ||
| "FULLSEND_RUN_HEAD_SHA": true, | ||
| "FULLSEND_RUN_STARTED_AT": true, | ||
| // OPENAI_API_KEY is reserved through oidcDenyKeys (merged by init()). | ||
| } | ||
|
|
||
|
|
@@ -2932,8 +2949,49 @@ func runTerminalError(hasLoop, validationPassed, timedOut bool, runCount int, el | |
| return nil | ||
| } | ||
|
|
||
| // runFacts is what the work item looked like when this run started. It is | ||
| // exported into the sandbox so an agent can tell, before it writes its | ||
| // result, whether the item moved under it — which it must do once a | ||
| // repository sets FULLSEND_PRESERVE_RUNS, because the run in flight is then | ||
| // no longer cancelled when a newer event arrives. | ||
| type runFacts struct { | ||
| // headSHA is the work item's head at run start; empty for issues. | ||
| headSHA string | ||
| // startedAt is when the run started, in UTC. | ||
| startedAt time.Time | ||
| } | ||
|
|
||
| func bootstrapEnv(sandboxName, remoteRepositoryDir string, h *harness.Harness, runtimeEnvExports []string, facts runFacts, fetchEnv ...fetchServiceEnv) error { | ||
| remoteEnvFile := sandbox.SandboxWorkspace + "/.env" | ||
|
|
||
| content := strings.Join(buildEnvScriptLines(sandboxName, remoteRepositoryDir, h, runtimeEnvExports, facts, fetchEnv...), "\n") + "\n" | ||
|
|
||
| tmpFile, err := os.CreateTemp("", "fullsend-env-*.sh") | ||
| if err != nil { | ||
| return fmt.Errorf("creating temp env file: %w", err) | ||
| } | ||
| defer os.Remove(tmpFile.Name()) | ||
|
|
||
| if _, err := tmpFile.WriteString(content); err != nil { | ||
| tmpFile.Close() | ||
| return fmt.Errorf("writing temp env file: %w", err) | ||
| } | ||
| tmpFile.Close() | ||
|
|
||
| if err := sandbox.UploadFile(sandboxName, tmpFile.Name(), remoteEnvFile); err != nil { | ||
| return fmt.Errorf("copying .env file to sandbox: %w", err) | ||
| } | ||
|
|
||
| return uploadHostFiles(sandboxName, h) | ||
| } | ||
|
|
||
| // buildEnvScriptLines assembles the sandbox .env script. | ||
| // | ||
| // The ORDER of these lines is behaviour, not formatting: later exports win, | ||
| // and .env.d is sourced in the middle, so anything that must outrank a | ||
| // harness-supplied env file has to come after it. There is a test pinning | ||
| // that for the run facts. | ||
| func buildEnvScriptLines(sandboxName, remoteRepositoryDir string, h *harness.Harness, runtimeEnvExports []string, facts runFacts, fetchEnv ...fetchServiceEnv) []string { | ||
| outputDir := sandbox.SandboxWorkspace + "/output" | ||
|
|
||
| var lines []string | ||
|
|
@@ -2999,29 +3057,29 @@ func bootstrapEnv(sandboxName, remoteRepositoryDir string, h *harness.Harness, r | |
| // overriding a single var from a shared host_files .env file. | ||
| lines = append(lines, buildSandboxEnvLines(h)...) | ||
|
|
||
| // Expose this run's baseline so the agent can re-check, once, whether the | ||
| // work item moved under it before it writes its result. | ||
| // | ||
| // After .env.d, for the same reason env.sandbox is: a file sourced from | ||
| // .env.d would otherwise overwrite these. reservedSandboxKeys stops an | ||
| // env.sandbox entry shadowing them, but it says nothing about .env.d, so | ||
| // position is what actually protects them. It does not close every route — | ||
|
There was a problem hiding this comment. [low] defense-in-depth Pre-existing gap: a host_files entry targeting the runner env file can bypass reservedSandboxKeys and ordering-based protection for the new run facts. Tracked in #7010. There was a problem hiding this comment. [low] defense-in-depth Pre-existing gap acknowledged in the PR: a host_files entry whose destination is the runner's own .env file can replace it wholesale, bypassing both reservedSandboxKeys and ordering-based protection for the new run facts. This gap is repo-wide (not introduced by this PR) and is tracked in #7010. |
||
| // a host_files entry whose dest is the runner's own .env replaces the file | ||
| // wholesale — and that gap is repo-wide rather than specific to these two | ||
| // keys; see the tracking issue. | ||
| lines = append(lines, buildRunFactsEnvLines(facts)...) | ||
|
|
||
| // Runner-owned budget and deadline come after every harness-controlled | ||
| // entry so none of them can shadow the values (#7042). This stays the last | ||
| // line: unlike the static exports above it sources a file rewritten before | ||
| // every iteration, so it must be re-read after everything else. | ||
| lines = append(lines, iterationEnvSourceLine()) | ||
|
|
||
| return lines | ||
| } | ||
|
|
||
| // uploadHostFiles copies the harness's host_files into the sandbox. | ||
| func uploadHostFiles(sandboxName string, h *harness.Harness) error { | ||
| for _, hf := range h.HostFiles { | ||
| // Use safeExpandEnv instead of os.ExpandEnv to refuse OIDC | ||
| // credential vars in host_files src path expansion (#5832). | ||
|
|
@@ -4887,6 +4945,65 @@ func extractMapString(m map[string]any, keys ...string) string { | |
| return "" | ||
| } | ||
|
|
||
| // runHeadSHA returns the work item's head at run start, or "" when the run | ||
| // is not against a pull or merge request. | ||
| // | ||
| // GITHUB_SHA is deliberately not a fallback: on pull_request_target it is the | ||
| // base branch, and a base SHA presented as the head would make an agent | ||
| // report a head move on every run. | ||
| func runHeadSHA(forgePlatform string) string { | ||
| if forgePlatform == "gitlab" { | ||
| return gitlabMergeRequestHeadSHA() | ||
| } | ||
| if sha := os.Getenv("PR_HEAD_SHA"); sha != "" { | ||
| return sha | ||
| } | ||
| return prHeadSHAFromEventPath(os.Getenv("GITHUB_EVENT_PATH")) | ||
| } | ||
|
|
||
| // gitlabMergeRequestHeadSHA returns the source-branch head of the merge | ||
| // request this run was dispatched for, or "" when the run is not against one. | ||
| // | ||
| // CI_MERGE_REQUEST_SOURCE_BRANCH_SHA is preferred but is not always set. Of | ||
| // it, GitLab's predefined-variables reference says: "The variable is empty in | ||
| // merge request pipelines. The SHA is present only in merged results | ||
| // pipelines." So on an ordinary merge request pipeline it is empty, and | ||
| // exporting that would tell the agent this run has no head at all — the | ||
| // signal reserved for issue runs — leaving it unable to notice the branch | ||
| // moving underneath it. | ||
| // | ||
| // CI_COMMIT_SHA fills that gap, but only conditionally: in a merged results | ||
| // pipeline it is the merge-result commit rather than the source head, so an | ||
| // unconditional fallback would export the wrong SHA there. Guarding on the | ||
| // pipeline source is self-correcting — in a merged results pipeline the | ||
| // first variable is populated, so the fallback is never reached — and a | ||
| // pipeline that is not a merge request keeps returning "", which is correct | ||
| // because there is no merge request head to report. | ||
| func gitlabMergeRequestHeadSHA() string { | ||
| if sha := os.Getenv("CI_MERGE_REQUEST_SOURCE_BRANCH_SHA"); sha != "" { | ||
| return sha | ||
| } | ||
| if os.Getenv("CI_PIPELINE_SOURCE") == "merge_request_event" { | ||
| return os.Getenv("CI_COMMIT_SHA") | ||
| } | ||
| return "" | ||
| } | ||
|
|
||
| // buildRunFactsEnvLines exports this run's baseline into the sandbox. | ||
| // | ||
| // They are written here rather than through env.sandbox or an env/*.env file: | ||
| // .env.d files are sourced after this one and would expand ${VAR} host-side to | ||
| // an empty string, and a ${VAR} in harness env.sandbox hard-fails | ||
| // ValidateRunnerEnvWith for consumers that do not define it. | ||
| func buildRunFactsEnvLines(facts runFacts) []string { | ||
| return []string{ | ||
| fmt.Sprintf("export FULLSEND_RUN_STARTED_AT='%s'", facts.startedAt.UTC().Format(time.RFC3339)), | ||
| // The SHA is forge-supplied; single-quote it the way every other | ||
| // exported value here is, rather than trusting its shape. | ||
| fmt.Sprintf("export FULLSEND_RUN_HEAD_SHA='%s'", strings.ReplaceAll(facts.headSHA, "'", "'\\''")), | ||
| } | ||
| } | ||
|
|
||
| // prHeadSHAFromEventPath extracts pull_request.head.sha from the event | ||
| // payload embedded in a workflow_dispatch event file. For workflow_dispatch | ||
| // events, the file contains {"inputs": {"event_payload": "<json-string>"}}. | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[low] api-shape
bootstrapEnv signature correctly places the required runFacts parameter before the variadic fetchEnv. No change needed.