feat: keys rotate, status Codex/Gemini checks, remote developer health (#20 #21 #22) - #26
Conversation
Sorry, something went wrong.
AgentDiff ReportSummary
Review Context
Files To Review First
|
Sorry, something went wrong.
Greptile SummaryThis PR replaces the old .key.bak rotation strategy with a proper timestamped archive under ~/.agentdiff/keys/archive/, allowing agentdiff verify to fall back to locally-archived keys when the git registry has no entry for a rotated key ID. It also adds an optional --resign-last N flag to re-sign recent local trace entries with the new key after rotation, and extends agentdiff status with a remote developer health table and a --since filter.
Confidence Score: 4/5Safe to merge after addressing the non-atomic write in the re-sign path. The key archiving and verify fallback logic are sound. The one concrete defect is in resign_last_local_traces: it uses std::fs::write directly on the live trace file, which truncates before writing — a disk-full or I/O error mid-write silently destroys all trace lines, including those not being re-signed. An atomic write (write to a .tmp sibling, then rename) would eliminate this risk. src/commands/keys.rs — the resign_last_local_traces write path. Important Files Changed
Reviews (2): Last reviewed commit: "fix: address Greptile review bugs in arc..." | Re-trigger Greptile |
Sorry, something went wrong.
- keys.rs: remove redundant pub_path.exists() guard after private key rename — public key existence is already ensured before the rename, so re-checking creates a silent partial-archive path that leaves old traces unverifiable after rotation - status.rs: fix misleading ok() prefix in Gemini (true,true) arm when tools.enableHooks is not set; consolidate into a single warn() line Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| let raw = std::fs::read_to_string(&path) | ||
| .with_context(|| format!("reading {}", path.display()))?; | ||
| let mut lines: Vec<String> = raw.lines().map(String::from).collect(); | ||
| while lines.last().map(|l| l.trim().is_empty()).unwrap_or(false) { | ||
| lines.pop(); | ||
| } | ||
| anyhow::ensure!(!lines.is_empty(), "local trace buffer is empty"); | ||
|
|
||
| let take = n.min(lines.len()); | ||
| let start = lines.len() - take; | ||
| for i in start..lines.len() { | ||
| let mut val: serde_json::Value = serde_json::from_str(&lines[i]) | ||
| .with_context(|| format!("parsing trace line {}", i + 1))?; | ||
| if let Some(obj) = val.as_object_mut() { | ||
| obj.remove("sig"); | ||
| } | ||
| let sig = keys::sign_record(&val)?; | ||
| val.as_object_mut() | ||
| .context("trace entry must be a JSON object")? | ||
| .insert("sig".to_string(), serde_json::to_value(&sig)?); | ||
| lines[i] = serde_json::to_string(&val)?; | ||
| } | ||
|
|
||
| std::fs::write(&path, lines.join("\n") + "\n") | ||
| .with_context(|| format!("writing {}", path.display()))?; |
There was a problem hiding this comment.
Non-atomic write can corrupt the trace buffer on failure
std::fs::write opens the file with O_TRUNC, zeroing its contents before writing the new data. If the write is interrupted (e.g. disk-full mid-write, process killed, I/O error), the file is left truncated and partially written — all original trace lines, including the ones not being re-signed, are lost. Because this is an audit-trail store, a silently half-written file is worse than a failed write.
The fix is to write to a .tmp sibling and atomically replace the original via std::fs::rename.
Sorry, something went wrong.
What does this PR do?
Closes #20,
Closes #21,
Closes #22.
Issue #20 — agentdiff keys rotate
Issue #21 — agentdiff status Codex/Gemini enablement flags
Issue #22 — Per-developer health in agentdiff status --remote
Type of change
Greptile bugs addressed
Testing
Checklist