mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-09 03:20:56 +00:00
feat(config): make the per-node checkpoint commit timeout configurable (#552)
Some checks failed
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Has been cancelled
TypeScript / Test (push) Has been cancelled
TypeScript / Build (push) Has been cancelled
Some checks failed
Rust / Format (push) Waiting to run
Rust / Clippy (push) Waiting to run
Rust / Generated Docs (push) Waiting to run
Rust / Test (Linux) (push) Waiting to run
Rust / Test (macOS) (push) Waiting to run
TypeScript / Typecheck (push) Has been cancelled
TypeScript / Test (push) Has been cancelled
TypeScript / Build (push) Has been cancelled
## Problem
The post-node run-branch checkpoint commit runs repository commit hooks
unless `skip_git_hooks` is enabled, but its sandbox command timeout was
hardcoded to 30 seconds. Consumers whose hooks run a multi-minute gate
cannot complete a checkpoint.
## Change
Adds `commit_timeout_ms` to the existing `[run.checkpoint]` table.
- Defaults to `30000`, preserving existing behavior.
- Threads the value through config raw layer -> merge -> resolve ->
resolved settings -> `RunOptions` -> `GitState` -> both checkpoint call
sites.
- Applies the configured timeout to checkpoint `git add -A` and `git
commit`.
- Keeps old serialized run manifests compatible via serde default.
## Testing
- `cargo +nightly-2026-04-14 fmt --all`
- `cargo +nightly-2026-04-14 clippy --locked --workspace --all-targets
-- -D warnings`
- `cargo nextest run --locked -p fabro-config -p fabro-types -p
fabro-workflow`
- 1795 passed, 31 skipped
- `cargo nextest run --locked -p fabro-cli
attach_json_errors_without_prompting_for_human_input`
- `cargo nextest run --locked --workspace --status-level slow --profile
ci --no-fail-fast`
- 6951 passed, 3 timed out, 187 skipped
- The 3 timeouts are preexisting on clean `upstream/main`: verified by
running `CARGO_TARGET_DIR=/data/projects/fabro/target cargo nextest run
--locked -p fabro-cli --profile ci --no-fail-fast workflow::acp::acp`
from a detached worktree at `upstream/main` (`8c7d5dc7d`), which timed
out the same three tests:
-
`workflow::acp::acp_artifacts_are_listed_when_touched_file_mtime_precedes_attempt_start`
-
`workflow::acp::acp_backend_does_not_inject_registered_provider_credentials`
- `workflow::acp::acp_backend_workflow`
## Compatibility
No behavior change without explicit opt-in. Omitted config resolves to
the existing 30 second timeout, and old serialized run manifests
deserialize unchanged.
---------
Co-authored-by: thewoolleyman <chad@thewoolleyman.com>
Co-authored-by: Bryan Helmkamp <bryan@brynary.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
8c3f035ea9
commit
790762fb8d
15 changed files with 202 additions and 74 deletions
|
|
@ -112,6 +112,7 @@ team = "platform"
|
|||
[run.checkpoint]
|
||||
exclude_globs = ["**/node_modules/**", "**/.cache/**"]
|
||||
skip_git_hooks = false
|
||||
commit_timeout = "30s"
|
||||
|
||||
[run.inputs]
|
||||
default_branch = "main"
|
||||
|
|
@ -329,8 +330,9 @@ Configure checkpoint behavior for all runs.
|
|||
|---|---|
|
||||
| `exclude_globs` | Glob patterns for files to exclude from checkpoint commits (for example, `["**/node_modules/**"]`) |
|
||||
| `skip_git_hooks` | When `true`, Fabro-managed run-branch checkpoint commits bypass local Git commit hooks. Defaults to `false`. Does not affect Fabro workflow `[[run.hooks]]` or metadata-branch snapshots. |
|
||||
| `commit_timeout` | Max duration for the per-node run-branch checkpoint commit (e.g. `"30s"`, `"10m"`). This commit runs repository commit hooks unless `skip_git_hooks` is `true`. Defaults to `"30s"`. |
|
||||
|
||||
`exclude_globs` replaces across layers — the highest-precedence layer wins wholesale. `skip_git_hooks` uses normal override semantics. See [Run Configuration — Checkpoint](/execution/run-configuration#runcheckpoint) for per-run configuration.
|
||||
`exclude_globs` replaces across layers — the highest-precedence layer wins wholesale. `skip_git_hooks` and `commit_timeout` use normal override semantics. See [Run Configuration — Checkpoint](/execution/run-configuration#runcheckpoint) for per-run configuration.
|
||||
|
||||
## Secrets and environment variables
|
||||
|
||||
|
|
|
|||
|
|
@ -356,14 +356,16 @@ Configure how git checkpoint commits behave.
|
|||
[run.checkpoint]
|
||||
exclude_globs = ["**/node_modules/**", "**/.cache/**", "**/dist/**"]
|
||||
skip_git_hooks = false
|
||||
commit_timeout = "30s"
|
||||
```
|
||||
|
||||
| Field | Description |
|
||||
|---|---|
|
||||
| `exclude_globs` | Glob patterns for files to exclude from checkpoint commits. Uses git pathspec `:(glob,exclude)` syntax. |
|
||||
| `skip_git_hooks` | When `true`, Fabro-managed run-branch checkpoint commits bypass local Git commit hooks (e.g. `pre-commit`, `commit-msg`). Defaults to `false`. Does not affect Fabro workflow `[[run.hooks]]` or metadata-branch snapshots. |
|
||||
| `commit_timeout` | Max duration for the per-node run-branch checkpoint commit (e.g. `"30s"`, `"10m"`). This commit runs repository commit hooks unless `skip_git_hooks` is `true`. Defaults to `"30s"`. |
|
||||
|
||||
`exclude_globs` replaces across layers — the higher-precedence layer wins wholesale. `skip_git_hooks` uses normal override semantics: the highest layer that sets it wins.
|
||||
`exclude_globs` replaces across layers — the higher-precedence layer wins wholesale. `skip_git_hooks` and `commit_timeout` use normal override semantics: the highest layer that sets the field wins.
|
||||
|
||||
### `[run.inputs]`
|
||||
|
||||
|
|
|
|||
|
|
@ -927,6 +927,7 @@ fn attach_json_errors_without_prompting_for_human_input() {
|
|||
"include": []
|
||||
},
|
||||
"checkpoint": {
|
||||
"commit_timeout_ms": 30000,
|
||||
"exclude_globs": [],
|
||||
"skip_git_hooks": false
|
||||
},
|
||||
|
|
|
|||
|
|
@ -148,7 +148,8 @@ fn inspect_resolves_selector_via_server_endpoint() {
|
|||
},
|
||||
"checkpoint": {
|
||||
"exclude_globs": [],
|
||||
"skip_git_hooks": false
|
||||
"skip_git_hooks": false,
|
||||
"commit_timeout_ms": 30000
|
||||
},
|
||||
"clone": {
|
||||
"enabled": true
|
||||
|
|
|
|||
|
|
@ -168,9 +168,11 @@ impl Combine for RunCheckpointLayer {
|
|||
self.exclude_globs
|
||||
};
|
||||
let skip_git_hooks = self.skip_git_hooks.or(other.skip_git_hooks);
|
||||
let commit_timeout = self.commit_timeout.or(other.commit_timeout);
|
||||
Self {
|
||||
exclude_globs,
|
||||
skip_git_hooks,
|
||||
commit_timeout,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -286,6 +286,9 @@ pub struct RunCheckpointLayer {
|
|||
pub exclude_globs: Vec<String>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub skip_git_hooks: Option<bool>,
|
||||
/// Optional timeout for the per-node run-branch checkpoint commit.
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub commit_timeout: Option<Duration>,
|
||||
}
|
||||
|
||||
/// `[run.clone]` — source workspace clone policy.
|
||||
|
|
|
|||
|
|
@ -202,9 +202,9 @@ fn resolve_prepare(
|
|||
|
||||
RunPrepareSettings {
|
||||
steps,
|
||||
timeout_ms: prepare.timeout.map_or(300_000, |timeout| {
|
||||
u64::try_from(timeout.as_std().as_millis()).unwrap_or(u64::MAX)
|
||||
}),
|
||||
timeout_ms: prepare
|
||||
.timeout
|
||||
.map_or(300_000, |timeout| timeout.as_millis()),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -223,12 +223,18 @@ fn resolve_execution(execution: Option<&RunExecutionLayer>) -> RunExecutionSetti
|
|||
|
||||
fn resolve_checkpoint(checkpoint: Option<&RunCheckpointLayer>) -> RunCheckpointSettings {
|
||||
RunCheckpointSettings {
|
||||
exclude_globs: checkpoint
|
||||
exclude_globs: checkpoint
|
||||
.map(|checkpoint| checkpoint.exclude_globs.clone())
|
||||
.unwrap_or_default(),
|
||||
skip_git_hooks: checkpoint
|
||||
skip_git_hooks: checkpoint
|
||||
.and_then(|checkpoint| checkpoint.skip_git_hooks)
|
||||
.unwrap_or(false),
|
||||
commit_timeout_ms: checkpoint
|
||||
.and_then(|checkpoint| checkpoint.commit_timeout)
|
||||
.map_or(
|
||||
RunCheckpointSettings::DEFAULT_COMMIT_TIMEOUT_MS,
|
||||
|timeout| timeout.as_millis(),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -514,9 +520,7 @@ fn resolve_hook(hook: &HookEntry, index: usize, errors: &mut Vec<ResolveError>)
|
|||
hook_type,
|
||||
matcher: hook.matcher.clone(),
|
||||
blocking: hook.blocking,
|
||||
timeout_ms: hook
|
||||
.timeout
|
||||
.map(|timeout| u64::try_from(timeout.as_std().as_millis()).unwrap_or(u64::MAX)),
|
||||
timeout_ms: hook.timeout.map(|timeout| timeout.as_millis()),
|
||||
sandbox: hook.sandbox,
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -917,8 +917,8 @@ fabro_tools = true
|
|||
}
|
||||
}
|
||||
|
||||
mod run_checkpoint_skip_git_hooks {
|
||||
//! Layer + resolver tests for `[run.checkpoint] skip_git_hooks`.
|
||||
mod run_checkpoint {
|
||||
//! Layer + resolver tests for `[run.checkpoint]`.
|
||||
|
||||
use crate::SettingsLayer;
|
||||
use crate::layers::Combine;
|
||||
|
|
@ -954,6 +954,31 @@ skip_git_hooks = true
|
|||
assert!(!settings.checkpoint.skip_git_hooks);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolves_commit_timeout_default_when_omitted() {
|
||||
let settings = super::workflow_settings_from_layer(SettingsLayer::default())
|
||||
.expect("empty settings should resolve")
|
||||
.run;
|
||||
|
||||
assert_eq!(settings.checkpoint.commit_timeout_ms, 30_000);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolves_commit_timeout_when_set() {
|
||||
let settings = super::workflow_settings_from_toml(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.checkpoint]
|
||||
commit_timeout = "10m"
|
||||
"#,
|
||||
)
|
||||
.expect("settings should resolve")
|
||||
.run;
|
||||
|
||||
assert_eq!(settings.checkpoint.commit_timeout_ms, 600_000);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn higher_layer_false_overrides_lower_layer_true() {
|
||||
let workflow = parse_settings(
|
||||
|
|
@ -981,6 +1006,33 @@ skip_git_hooks = true
|
|||
assert!(!settings.checkpoint.skip_git_hooks);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn higher_layer_commit_timeout_overrides_lower_layer() {
|
||||
let workflow = parse_settings(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.checkpoint]
|
||||
commit_timeout = "10m"
|
||||
"#,
|
||||
);
|
||||
let user = parse_settings(
|
||||
r#"
|
||||
_version = 1
|
||||
|
||||
[run.checkpoint]
|
||||
commit_timeout = "30s"
|
||||
"#,
|
||||
);
|
||||
let merged = workflow.combine(user);
|
||||
|
||||
let settings = super::workflow_settings_from_layer(merged)
|
||||
.expect("merged settings should resolve")
|
||||
.run;
|
||||
|
||||
assert_eq!(settings.checkpoint.commit_timeout_ms, 600_000);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exclude_globs_replace_behavior_preserved_when_skip_git_hooks_added() {
|
||||
// Higher layer provides skip_git_hooks but no exclude_globs;
|
||||
|
|
|
|||
|
|
@ -35,6 +35,12 @@ impl Duration {
|
|||
pub const fn from_millis(millis: u64) -> Self {
|
||||
Self(StdDuration::from_millis(millis))
|
||||
}
|
||||
|
||||
/// Total milliseconds, saturating at `u64::MAX`.
|
||||
#[must_use]
|
||||
pub fn as_millis(&self) -> u64 {
|
||||
u64::try_from(self.0.as_millis()).unwrap_or(u64::MAX)
|
||||
}
|
||||
}
|
||||
|
||||
impl From<StdDuration> for Duration {
|
||||
|
|
|
|||
|
|
@ -541,8 +541,9 @@ mod run_namespace_variable_substitution_tests {
|
|||
fn substitutes_variables_in_string_backed_settings_families() {
|
||||
let mut run = RunNamespace {
|
||||
checkpoint: RunCheckpointSettings {
|
||||
exclude_globs: vec!["tmp/{{ vars.ENV }}/**".to_string()],
|
||||
skip_git_hooks: false,
|
||||
exclude_globs: vec!["tmp/{{ vars.ENV }}/**".to_string()],
|
||||
skip_git_hooks: false,
|
||||
commit_timeout_ms: 30_000,
|
||||
},
|
||||
environment: RunEnvironmentSettings {
|
||||
image: EnvironmentImageSettings {
|
||||
|
|
@ -845,15 +846,37 @@ impl Default for RunExecutionSettings {
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
pub struct RunCheckpointSettings {
|
||||
pub exclude_globs: Vec<String>,
|
||||
pub exclude_globs: Vec<String>,
|
||||
/// When `true`, Fabro-managed run-branch checkpoint commits bypass
|
||||
/// local Git commit hooks (e.g. `pre-commit`, `commit-msg`). This does
|
||||
/// not affect Fabro workflow `[[run.hooks]]` or metadata-branch
|
||||
/// snapshots, which already bypass repository hooks.
|
||||
#[serde(default)]
|
||||
pub skip_git_hooks: bool,
|
||||
pub skip_git_hooks: bool,
|
||||
/// Timeout (ms) for the per-node run-branch checkpoint commit, which runs
|
||||
/// repository commit hooks unless `skip_git_hooks` is set. Default 30_000.
|
||||
#[serde(default = "default_checkpoint_commit_timeout_ms")]
|
||||
pub commit_timeout_ms: u64,
|
||||
}
|
||||
|
||||
impl RunCheckpointSettings {
|
||||
pub const DEFAULT_COMMIT_TIMEOUT_MS: u64 = 30_000;
|
||||
}
|
||||
|
||||
fn default_checkpoint_commit_timeout_ms() -> u64 {
|
||||
RunCheckpointSettings::DEFAULT_COMMIT_TIMEOUT_MS
|
||||
}
|
||||
|
||||
impl Default for RunCheckpointSettings {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
exclude_globs: Vec::new(),
|
||||
skip_git_hooks: false,
|
||||
commit_timeout_ms: default_checkpoint_commit_timeout_ms(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
|
||||
|
|
|
|||
|
|
@ -193,9 +193,8 @@ impl Handler for ParallelHandler {
|
|||
"parallel_base",
|
||||
0,
|
||||
None,
|
||||
&gs.checkpoint_exclude_globs,
|
||||
&gs.checkpoint,
|
||||
&gs.git_author,
|
||||
gs.checkpoint_skip_git_hooks,
|
||||
)
|
||||
.await;
|
||||
match result {
|
||||
|
|
@ -320,9 +319,10 @@ impl Handler for ParallelHandler {
|
|||
.as_ref()
|
||||
.map(|gs| gs.git_author.clone())
|
||||
.unwrap_or_default();
|
||||
let skip_git_hooks = git_state
|
||||
let checkpoint = git_state
|
||||
.as_ref()
|
||||
.is_some_and(|gs| gs.checkpoint_skip_git_hooks);
|
||||
.map(|gs| gs.checkpoint.clone())
|
||||
.unwrap_or_default();
|
||||
let group_id = parallel_group_id.clone();
|
||||
let branch_scope = StageScope::for_parallel_branch(
|
||||
setup.target_id.clone(),
|
||||
|
|
@ -407,7 +407,7 @@ impl Handler for ParallelHandler {
|
|||
let add_cmd = format!("{git_r} add -A");
|
||||
let add_result = setup
|
||||
.sandbox
|
||||
.exec_command(&add_cmd, 30_000, None, None, None)
|
||||
.exec_command(&add_cmd, checkpoint.commit_timeout_ms, None, None, None)
|
||||
.await;
|
||||
if add_result
|
||||
.as_ref()
|
||||
|
|
@ -419,11 +419,17 @@ impl Handler for ParallelHandler {
|
|||
&git_author.name,
|
||||
&git_author.email,
|
||||
&msg,
|
||||
skip_git_hooks,
|
||||
checkpoint.skip_git_hooks,
|
||||
);
|
||||
let _ = setup
|
||||
.sandbox
|
||||
.exec_command(&commit_cmd, 30_000, None, None, None)
|
||||
.exec_command(
|
||||
&commit_cmd,
|
||||
checkpoint.commit_timeout_ms,
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
}
|
||||
let sha_cmd = format!("{git_r} rev-parse HEAD");
|
||||
|
|
|
|||
|
|
@ -280,9 +280,8 @@ impl RunLifecycle<WorkflowGraph> for GitLifecycle {
|
|||
&result.outcome.status.to_string(),
|
||||
completed_count,
|
||||
shadow_sha,
|
||||
&self.run_options.checkpoint_exclude_globs(),
|
||||
self.run_options.checkpoint(),
|
||||
&git_author,
|
||||
self.run_options.checkpoint_skip_git_hooks(),
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
|
|||
|
|
@ -62,8 +62,7 @@ pub async fn execute(init: Initialized) -> Executed {
|
|||
base_sha,
|
||||
run_branch: git.run_branch.clone(),
|
||||
meta_branch: git.meta_branch.clone(),
|
||||
checkpoint_exclude_globs: run_options.checkpoint_exclude_globs(),
|
||||
checkpoint_skip_git_hooks: run_options.checkpoint_skip_git_hooks(),
|
||||
checkpoint: run_options.checkpoint().clone(),
|
||||
git_author: run_options.git_author(),
|
||||
}))
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
use std::collections::HashMap;
|
||||
use std::path::PathBuf;
|
||||
|
||||
use fabro_types::settings::run::RunMode;
|
||||
use fabro_types::settings::run::{RunCheckpointSettings, RunMode};
|
||||
use fabro_types::{ForkSourceRef, GitContext, RunId, WorkflowSettings};
|
||||
use tokio_util::sync::CancellationToken;
|
||||
|
||||
|
|
@ -50,12 +50,8 @@ impl RunOptions {
|
|||
self.settings.run.execution.mode == RunMode::DryRun
|
||||
}
|
||||
|
||||
pub fn checkpoint_exclude_globs(&self) -> Vec<String> {
|
||||
self.settings.run.checkpoint.exclude_globs.clone()
|
||||
}
|
||||
|
||||
pub fn checkpoint_skip_git_hooks(&self) -> bool {
|
||||
self.settings.run.checkpoint.skip_git_hooks
|
||||
pub fn checkpoint(&self) -> &RunCheckpointSettings {
|
||||
&self.settings.run.checkpoint
|
||||
}
|
||||
|
||||
pub fn git_author(&self) -> GitAuthor {
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ use fabro_checkpoint::trailer as trailerlink;
|
|||
use fabro_checkpoint::trailer::Trailer;
|
||||
use fabro_sandbox::shell_quote;
|
||||
use fabro_types::RunId;
|
||||
use fabro_types::settings::run::RunCheckpointSettings;
|
||||
use fabro_util::error::SharedError;
|
||||
|
||||
use crate::artifact_snapshot;
|
||||
|
|
@ -22,13 +23,12 @@ pub struct GitCommandError {
|
|||
/// Captured git state for a workflow run, shared with handlers.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct GitState {
|
||||
pub run_id: RunId,
|
||||
pub base_sha: String,
|
||||
pub run_branch: Option<String>,
|
||||
pub meta_branch: Option<String>,
|
||||
pub checkpoint_exclude_globs: Vec<String>,
|
||||
pub checkpoint_skip_git_hooks: bool,
|
||||
pub git_author: GitAuthor,
|
||||
pub run_id: RunId,
|
||||
pub base_sha: String,
|
||||
pub run_branch: Option<String>,
|
||||
pub meta_branch: Option<String>,
|
||||
pub checkpoint: RunCheckpointSettings,
|
||||
pub git_author: GitAuthor,
|
||||
}
|
||||
|
||||
pub const GIT_REMOTE: &str =
|
||||
|
|
@ -58,7 +58,7 @@ pub(crate) fn exec_err(label: &str, r: fabro_sandbox::ExecResult) -> GitCommandE
|
|||
/// Run a git checkpoint commit via the sandbox.
|
||||
#[allow(
|
||||
clippy::too_many_arguments,
|
||||
reason = "Checkpointing needs explicit run metadata, excludes, and author inputs."
|
||||
reason = "Checkpointing needs explicit run metadata, checkpoint settings, and author inputs."
|
||||
)]
|
||||
pub async fn git_checkpoint(
|
||||
sandbox: &dyn Sandbox,
|
||||
|
|
@ -67,15 +67,14 @@ pub async fn git_checkpoint(
|
|||
status: &str,
|
||||
completed_count: usize,
|
||||
shadow_sha: Option<String>,
|
||||
exclude_globs: &[String],
|
||||
checkpoint: &RunCheckpointSettings,
|
||||
author: &GitAuthor,
|
||||
skip_git_hooks: bool,
|
||||
) -> std::result::Result<String, GitCommandError> {
|
||||
let mut all_excludes: Vec<String> = artifact_snapshot::EXCLUDE_DIRS
|
||||
.iter()
|
||||
.map(|d| format!("**/{d}/**"))
|
||||
.collect();
|
||||
all_excludes.extend(exclude_globs.iter().cloned());
|
||||
all_excludes.extend(checkpoint.exclude_globs.iter().cloned());
|
||||
|
||||
let pathspecs: Vec<String> = all_excludes
|
||||
.iter()
|
||||
|
|
@ -83,7 +82,7 @@ pub async fn git_checkpoint(
|
|||
.collect();
|
||||
let add_cmd = format!("{GIT_REMOTE} add -A -- . {}", pathspecs.join(" "));
|
||||
let add_result = sandbox
|
||||
.exec_command(&add_cmd, 30_000, None, None, None)
|
||||
.exec_command(&add_cmd, checkpoint.commit_timeout_ms, None, None, None)
|
||||
.await;
|
||||
match add_result {
|
||||
Ok(r) if r.is_success() => {}
|
||||
|
|
@ -127,14 +126,18 @@ pub async fn git_checkpoint(
|
|||
}
|
||||
|
||||
let msg_path_q = shell_quote(&msg_path);
|
||||
let no_verify = if skip_git_hooks { " --no-verify" } else { "" };
|
||||
let no_verify = if checkpoint.skip_git_hooks {
|
||||
" --no-verify"
|
||||
} else {
|
||||
""
|
||||
};
|
||||
let commit_cmd = format!(
|
||||
"{GIT_REMOTE} -c user.name={name} -c user.email={email} commit --allow-empty{no_verify} -F {msg_path_q}",
|
||||
name = shell_quote(&author.name),
|
||||
email = shell_quote(&author.email),
|
||||
);
|
||||
let commit_result = sandbox
|
||||
.exec_command(&commit_cmd, 30_000, None, None, None)
|
||||
.exec_command(&commit_cmd, checkpoint.commit_timeout_ms, None, None, None)
|
||||
.await;
|
||||
let _ = sandbox.delete_file(&msg_path).await;
|
||||
match commit_result {
|
||||
|
|
@ -165,7 +168,7 @@ pub async fn git_checkpoint(
|
|||
/// Run a git checkpoint after the per-run sandbox git capability probe.
|
||||
#[allow(
|
||||
clippy::too_many_arguments,
|
||||
reason = "Checkpointing needs explicit run metadata, excludes, and author inputs."
|
||||
reason = "Checkpointing needs explicit run metadata, checkpoint settings, and author inputs."
|
||||
)]
|
||||
pub(crate) async fn checked_git_checkpoint(
|
||||
runtime: &SandboxGitRuntime,
|
||||
|
|
@ -175,9 +178,8 @@ pub(crate) async fn checked_git_checkpoint(
|
|||
status: &str,
|
||||
completed_count: usize,
|
||||
shadow_sha: Option<String>,
|
||||
exclude_globs: &[String],
|
||||
checkpoint: &RunCheckpointSettings,
|
||||
author: &GitAuthor,
|
||||
skip_git_hooks: bool,
|
||||
) -> std::result::Result<String, SharedError> {
|
||||
runtime.ensure_git_available(sandbox).await.map_err(|err| {
|
||||
SharedError::new(anyhow::Error::new(err).context("sandbox git unavailable"))
|
||||
|
|
@ -189,9 +191,8 @@ pub(crate) async fn checked_git_checkpoint(
|
|||
status,
|
||||
completed_count,
|
||||
shadow_sha,
|
||||
exclude_globs,
|
||||
checkpoint,
|
||||
author,
|
||||
skip_git_hooks,
|
||||
)
|
||||
.await
|
||||
.map_err(|err| SharedError::new(anyhow::Error::new(err)))
|
||||
|
|
@ -877,6 +878,7 @@ mod tests {
|
|||
struct ScriptedSandbox {
|
||||
exec_results: Mutex<VecDeque<ExecResult>>,
|
||||
commands: Mutex<Vec<String>>,
|
||||
timeouts: Mutex<Vec<u64>>,
|
||||
write_paths: Mutex<Vec<String>>,
|
||||
delete_paths: Mutex<Vec<String>>,
|
||||
}
|
||||
|
|
@ -886,6 +888,7 @@ mod tests {
|
|||
Self {
|
||||
exec_results: Mutex::new(exec_results.into()),
|
||||
commands: Mutex::new(Vec::new()),
|
||||
timeouts: Mutex::new(Vec::new()),
|
||||
write_paths: Mutex::new(Vec::new()),
|
||||
delete_paths: Mutex::new(Vec::new()),
|
||||
}
|
||||
|
|
@ -898,6 +901,13 @@ mod tests {
|
|||
.clone()
|
||||
}
|
||||
|
||||
fn timeouts(&self) -> Vec<u64> {
|
||||
self.timeouts
|
||||
.lock()
|
||||
.expect("timeouts lock poisoned")
|
||||
.clone()
|
||||
}
|
||||
|
||||
fn write_paths(&self) -> Vec<String> {
|
||||
self.write_paths
|
||||
.lock()
|
||||
|
|
@ -950,7 +960,7 @@ mod tests {
|
|||
async fn exec_command(
|
||||
&self,
|
||||
command: &str,
|
||||
_timeout_ms: u64,
|
||||
timeout_ms: u64,
|
||||
_working_dir: Option<&str>,
|
||||
_env_vars: Option<&std::collections::HashMap<String, String>>,
|
||||
_cancel_token: Option<CancellationToken>,
|
||||
|
|
@ -959,6 +969,10 @@ mod tests {
|
|||
.lock()
|
||||
.expect("commands lock poisoned")
|
||||
.push(command.to_string());
|
||||
self.timeouts
|
||||
.lock()
|
||||
.expect("timeouts lock poisoned")
|
||||
.push(timeout_ms);
|
||||
self.exec_results
|
||||
.lock()
|
||||
.expect("exec_results lock poisoned")
|
||||
|
|
@ -1066,9 +1080,8 @@ mod tests {
|
|||
"success",
|
||||
1,
|
||||
None,
|
||||
&[],
|
||||
&RunCheckpointSettings::default(),
|
||||
&crate::git::GitAuthor::default(),
|
||||
false,
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
|
@ -1093,9 +1106,8 @@ mod tests {
|
|||
"success",
|
||||
1,
|
||||
None,
|
||||
&[],
|
||||
&RunCheckpointSettings::default(),
|
||||
&crate::git::GitAuthor::default(),
|
||||
false,
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
|
@ -1124,9 +1136,8 @@ mod tests {
|
|||
"success",
|
||||
1,
|
||||
None,
|
||||
&[],
|
||||
&RunCheckpointSettings::default(),
|
||||
&crate::git::GitAuthor::default(),
|
||||
false,
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
|
@ -1144,9 +1155,8 @@ mod tests {
|
|||
"success",
|
||||
1,
|
||||
None,
|
||||
&[],
|
||||
&RunCheckpointSettings::default(),
|
||||
&crate::git::GitAuthor::default(),
|
||||
false,
|
||||
)
|
||||
.await
|
||||
.unwrap_err();
|
||||
|
|
@ -1173,9 +1183,8 @@ mod tests {
|
|||
"success",
|
||||
1,
|
||||
None,
|
||||
&[],
|
||||
&RunCheckpointSettings::default(),
|
||||
&author,
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
let second = git_checkpoint(
|
||||
|
|
@ -1185,9 +1194,8 @@ mod tests {
|
|||
"success",
|
||||
1,
|
||||
None,
|
||||
&[],
|
||||
&RunCheckpointSettings::default(),
|
||||
&author,
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
|
||||
|
|
@ -1225,6 +1233,29 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn git_checkpoint_uses_configured_timeout_for_add_and_commit() {
|
||||
let sandbox = ScriptedSandbox::new(vec![exec_ok(), exec_ok(), exec_ok()]);
|
||||
let checkpoint = RunCheckpointSettings {
|
||||
commit_timeout_ms: 600_000,
|
||||
..RunCheckpointSettings::default()
|
||||
};
|
||||
git_checkpoint(
|
||||
&sandbox,
|
||||
"run1",
|
||||
"work",
|
||||
"success",
|
||||
1,
|
||||
None,
|
||||
&checkpoint,
|
||||
&crate::git::GitAuthor::default(),
|
||||
)
|
||||
.await
|
||||
.expect("checkpoint should succeed");
|
||||
|
||||
assert_eq!(sandbox.timeouts(), vec![600_000, 600_000, 10_000]);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn git_diff_reports_timeout() {
|
||||
let sandbox = ScriptedSandbox::new(vec![exec_timed_out(99)]);
|
||||
|
|
@ -1253,6 +1284,10 @@ mod tests {
|
|||
async fn git_checkpoint_appends_no_verify_when_skip_hooks_enabled() {
|
||||
// add, commit, rev-parse
|
||||
let sandbox = ScriptedSandbox::new(vec![exec_ok(), exec_ok(), exec_ok()]);
|
||||
let checkpoint = RunCheckpointSettings {
|
||||
skip_git_hooks: true,
|
||||
..RunCheckpointSettings::default()
|
||||
};
|
||||
git_checkpoint(
|
||||
&sandbox,
|
||||
"run1",
|
||||
|
|
@ -1260,9 +1295,8 @@ mod tests {
|
|||
"success",
|
||||
1,
|
||||
None,
|
||||
&[],
|
||||
&checkpoint,
|
||||
&crate::git::GitAuthor::default(),
|
||||
true,
|
||||
)
|
||||
.await
|
||||
.expect("checkpoint should succeed");
|
||||
|
|
@ -1288,9 +1322,8 @@ mod tests {
|
|||
"success",
|
||||
1,
|
||||
None,
|
||||
&[],
|
||||
&RunCheckpointSettings::default(),
|
||||
&crate::git::GitAuthor::default(),
|
||||
false,
|
||||
)
|
||||
.await
|
||||
.expect("checkpoint should succeed");
|
||||
|
|
@ -1350,9 +1383,8 @@ mod tests {
|
|||
"success",
|
||||
1,
|
||||
None,
|
||||
&[],
|
||||
&RunCheckpointSettings::default(),
|
||||
&author,
|
||||
false,
|
||||
)
|
||||
.await;
|
||||
assert!(result.is_ok(), "git_checkpoint failed: {:?}", result.err());
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue