Address second review pass: fix start-time race + minor cleanups
Build App / compute-version (pull_request) Successful in 9s
Build Container / build-container (pull_request) Successful in 1m48s
Build App / build-macos (pull_request) Successful in 2m16s
Build App / build-windows (pull_request) Successful in 3m11s
Build App / build-linux (pull_request) Successful in 4m51s
Build App / create-tag (pull_request) Has been skipped
Build App / sync-to-github (pull_request) Has been skipped
Build App / compute-version (pull_request) Successful in 9s
Build Container / build-container (pull_request) Successful in 1m48s
Build App / build-macos (pull_request) Successful in 2m16s
Build App / build-windows (pull_request) Successful in 3m11s
Build App / build-linux (pull_request) Successful in 4m51s
Build App / create-tag (pull_request) Has been skipped
Build App / sync-to-github (pull_request) Has been skipped
- M1 (race): don't mount the host AWS dir for static-credential Bedrock. sync_bedrock_credentials() is the sole writer of ~/.aws/credentials in that mode, and mounting /tmp/.host-aws let the entrypoint's `rm -rf ~/.aws; cp -a` race that write at startup (only when a global aws_config_path was also set). Static keys + AWS_REGION env are self-sufficient and don't need the host config, so skipping the mount removes the dual-writer entirely. - L-a: exit codes are now read via wait_for_exec_exit(), which polls inspect_exec until the exec reports finished, so a non-zero tar/cred exit isn't missed by reading exit_code too early. The backup only fails on a definitively non-zero code (falls back to the empty-output check if undeterminable). - L-b: fixed two comments that referenced the old write_bedrock_static_credentials name (now sync_bedrock_credentials). - L-c: entrypoint only rewrites ~/.claude.json when awsAuthRefresh is actually present, avoiding a needless jq reformat on every non-SSO start. - L-d: backup script traps EXIT to remove its mktemp staging dir even when tar fails, so failed backups don't accumulate temp dirs (with the sanitized config copy) in the container. L-e (drop routing) is a non-issue: the layout is tabbed, so only one terminal pane is ever visible; the active-guard routing is correct. Verified the race fix, trap cleanup, grep guard, and exit-code polling. cargo check / tsc / vitest all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -278,7 +278,7 @@ fn compute_bedrock_fingerprint(project: &Project, global_aws: &GlobalAwsSettings
|
||||
// NOTE: the static credential fields (access key / secret / session
|
||||
// token) are intentionally NOT part of the fingerprint. They are
|
||||
// written to ~/.aws/credentials on every start by
|
||||
// write_bedrock_static_credentials(), so a key rotation should refresh
|
||||
// sync_bedrock_credentials(), so a key rotation should refresh
|
||||
// in place rather than force a full container recreation. Region,
|
||||
// profile, and bearer token remain env-based and so stay here.
|
||||
let parts = vec![
|
||||
@@ -674,7 +674,7 @@ pub async fn create_container(
|
||||
BedrockAuthMethod::StaticCredentials => {
|
||||
// Static/session credentials are NOT injected as env vars.
|
||||
// They are written to ~/.aws/credentials by
|
||||
// write_bedrock_static_credentials() on every container
|
||||
// sync_bedrock_credentials() on every container
|
||||
// start, so rotated/updated keys are picked up without a
|
||||
// full container recreation (and never get baked into the
|
||||
// snapshot image). The empty values set by the
|
||||
@@ -934,7 +934,19 @@ pub async fn create_container(
|
||||
false
|
||||
};
|
||||
|
||||
if should_mount_aws || aws_config_path.is_some() {
|
||||
// For static-credential Bedrock, sync_bedrock_credentials() is the sole
|
||||
// owner of ~/.aws/credentials (it rewrites it on every start). Mounting the
|
||||
// host AWS dir would make the entrypoint's `rm -rf ~/.aws; cp -a` race that
|
||||
// write at startup, so we never mount it in that case — the static keys
|
||||
// (+ AWS_REGION env) are self-sufficient and don't need the host config.
|
||||
let is_bedrock_static = project.backend == Backend::Bedrock
|
||||
&& project
|
||||
.bedrock_config
|
||||
.as_ref()
|
||||
.map(|b| b.auth_method == BedrockAuthMethod::StaticCredentials)
|
||||
.unwrap_or(false);
|
||||
|
||||
if (should_mount_aws || aws_config_path.is_some()) && !is_bedrock_static {
|
||||
let aws_dir = aws_config_path
|
||||
.map(|p| std::path::PathBuf::from(p))
|
||||
.or_else(|| dirs::home_dir().map(|h| h.join(".aws")));
|
||||
|
||||
Reference in New Issue
Block a user