Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,9 @@ lazy_static = "1.4"
# Semantic versioning for skill update resolution
semver = "1.0"

# Terminal detection for update check
is-terminal = "0.4"

# Temp directories for skill archive extraction
tempfile = "3.15"
urlencoding = "2.1"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Archive Manifest: background-version-check

**Date Archived:** 2026-03-22
**Archived By:** opencode (SDD ARCHIVE sub-agent)

---

## Verdict from Verify

**PASS**

All 12 acceptance criteria satisfied. 333 tests pass (290 lib + 34 binary + 9 integration), 0 failures. cargo clippy and fmt clean. Minor naming discrepancies had no behavioral impact.

---

## Summary of Implementation

Implemented a non-blocking background version check that:

1. Spawns a detached daemon thread (`"agentsync-update-check"`) before CLI parsing in `src/main.rs:119`
2. Queries `https://crates.io/api/v1/crates/agentsync` with a 3s timeout via `reqwest::blocking::Client`
3. Caches results at `~/.cache/agentsync/update-check.json` with 24h TTL
4. Respects `AGENTSYNC_NO_UPDATE_CHECK=1` and `CI=true` env var opt-outs
5. Only prints to stderr when connected to a TTY
6. Compares versions via `semver::Version`, ignoring pre-releases
7. Prints a one-time hint per new version (tracked via `notified_for_version`)
8. All errors silently dropped — zero impact on CLI execution

### Files Added
- `src/update_check.rs` — module with `Cache`, `CheckedVersion`, and `spawn()` function

### Files Modified
- `Cargo.toml` — added `is-terminal = "0.4"` dependency
- `src/lib.rs` — added `pub(crate) mod update_check;`
- `src/main.rs` — added `agentsync::update_check::spawn()` call

### Tests
- Unit tests for cache load/save, version comparison, pre-release skipping
- All 333 tests pass

---

## Delta Spec Location
`openspec/changes/archive/2026-03-22-background-version-check/specs/version-check/spec.md`

## Main Spec Location
`openspec/specs/version-check/spec.md`
217 changes: 217 additions & 0 deletions openspec/changes/archive/2026-03-22-background-version-check/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,217 @@
# Technical Design: background-version-check

## 1. Architecture

### Module Structure

```
src/
update_check.rs # New module (pub(crate))
lib.rs # Add: pub(crate) mod update_check
main.rs # Add: update_check::spawn() call
```
Comment on lines +7 to +12

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add language identifiers to fenced code blocks.

These fences are missing language tags, which is currently triggering markdownlint MD040 warnings.

Also applies to: 76-103, 109-121, 205-207

🧰 Tools
🪛 markdownlint-cli2 (0.21.0)

[warning] 7-7: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@openspec/changes/archive/2026-03-22-background-version-check/design.md`
around lines 7 - 12, Add explicit language identifiers to the fenced code blocks
in the design document: change the module tree fence (the block showing
update_check.rs, lib.rs, main.rs) to use a language tag such as ```text (or
```bash) and update any Rust code fences to use ```rust; apply the same fix to
the other fenced blocks mentioned (the additional code/design snippets later in
the file) so all triple-backtick fences include the appropriate language tag.


**Public API surface** (all `pub(crate)`):

- `update_check::spawn()` — Call site: `main()`, fires and forgets
- No other public items; all internals are `pub(super)` or private

### Spawn Point

`srC/main.rs:118` — after `tracing_subscriber::fmt::init()`, before `Cli::parse()`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix incorrect file path casing in spawn-point reference.

srC/main.rs:118 should be src/main.rs:118; the current path is invalid and can misdirect readers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@openspec/changes/archive/2026-03-22-background-version-check/design.md` at
line 21, Update the spawn-point reference that currently reads `srC/main.rs:118`
to the correct casing `src/main.rs:118` so links and references point to the
valid file; locate and replace the `srC/main.rs:118` token in the design
document (the spawn-point reference) with `src/main.rs:118`.


**Rationale**: The check must run for every command invocation regardless of which subcommand fires. Spawning before CLI parsing also means it starts as early as possible, maximizing the window for the background thread to complete before the process exits. Tracing must be initialized first so the background thread can use `tracing` for any diagnostics.

---

## 2. Code Structure

### `src/update_check.rs`

#### `CheckedVersion` struct

```rust
struct CheckedVersion {
last_checked: DateTime<Utc>,
latest_version: String,
notified_for_version: Option<String>,
}
```

Fields:
- `last_checked`: when the cache was written
- `latest_version`: the version string returned by crates.io (e.g. `"1.33.0"`)
- `notified_for_version`: the version the user was already notified about (`None` = never notified)

**Rationale**: `notified_for_version` is a separate field so the "once per version" behavior survives across cache refreshes. The hint only re-appears when a *new* version is detected.

#### `Cache` struct

```rust
struct Cache {
path: PathBuf,
}
impl Cache {
fn load(&self) -> Option<CheckedVersion>;
fn save(&self, v: &CheckedVersion) -> anyhow::Result<()>;
}
```

- `load()` returns `None` on any I/O or parse error (silent drop)
- `save()` creates parent directories via `fs::create_dir_all`; wraps errors in `anyhow`
- File path: `~/.cache/agentsync/update-check.json`

#### `spawn()` — Entry Point

```rust
pub(crate) fn spawn()
```

1. Checks opt-out conditions
2. Spawns a detached thread named `"agentsync-update-check"`
3. Returns immediately; no `JoinHandle` kept

### Thread Work Flow

```
1. Opt-out guard
└─ AGENTSYNC_NO_UPDATE_CHECK=1 → return
└─ CI=true → return
└─ !stderr.is_terminal() → return

2. Load cache
├─ Read ~/.cache/agentsync/update-check.json
├─ Parse → CheckedVersion
├─ If fresh (< 24h) AND notified_for_version == latest_version → return (already told)
└─ If stale or absent → proceed

3. HTTP fetch (reqwest blocking, 3s timeout)
├─ GET https://crates.io/api/v1/crates/agentsync
├─ Extract JSON: `crate.newest_version` (semver string)
└─ On error → return silently

4. Compare versions
├─ Parse current: `env!("CARGO_PKG_VERSION")`
├─ Parse remote: `semver::Version::parse`
├─ Skip pre-releases (`.pre.is_empty()`)
└─ If remote > current → proceed

5. Write cache + emit hint
├─ Update: notified_for_version = remote version
├─ Write ~/.cache/agentsync/update-check.json
└─ eprintln!(yellow bold hint)
```

---

## 3. Data Flow

```
main()
└─ update_check::spawn()
└─ std::thread::Builder::spawn (detached)
├─ opt-out check (env + TTY)
├─ cache load ─────────────────────────────────┐
│ └─ ~/.cache/agentsync/update-check.json │
├─ reqwest::blocking::Client (3s timeout) │
│ └─ GET crates.io/api/v1/crates/agentsync │
├─ semver::Version::parse comparison │
├─ cache save ────────────────────────────────┘
└─ eprintln!(hint) → stderr (TTY-only)
```

---

## 4. Key Decisions

| Decision | Rationale |
|---|---|
| **Daemon thread (no `join()`)** | The thread is purely advisory; it must not block or delay process exit. Process termination kills the thread automatically. |
| **`is_terminal` on `stderr`** | The hint must not pollute redirected output in scripts/CI. Using `stderr` (not `stdout`) also avoids interfering with command output. |
| **Skip pre-releases** | Avoids noisy hints for beta/alpha users when a stable release is behind a pre-release. |
| **`CheckedVersion.notified_for_version`** | Enables "once per new version" without a separate flag file. The cache itself records what was already seen. |
| **Silent error handling** | All errors in the background thread are swallowed. No logging, no user-facing errors. The feature is purely advisory. |
| **Cache dir creation via `create_dir_all`** | Avoids failure if `~/.cache` doesn't exist on first run. |
| **`chrono::DateTime<Utc>` for timestamps** | Already a dependency (used elsewhere in the crate), provides reliable UTC timestamps for cache freshness checks. |
| **`reqwest::blocking::Client`** | Simpler than async for a single short-lived HTTP call in a background thread. Avoids bringing async runtime concerns into `main`. |
| **`env!("CARGO_PKG_VERSION")`** | Compile-time constant — no need to pass the version at runtime. Works reliably in any build. |

---

## 5. Dependencies

| Dependency | Change | Purpose |
|---|---|---|
| `is-terminal = "0.4"` | **Add** | `is_terminal(Stderr)` guard |
| `reqwest` (with `blocking`) | Existing | crates.io HTTP fetch |
| `semver` | Existing | Version parse + compare |
| `chrono` (with `serde`) | Existing | UTC timestamp for cache TTL |
| `serde` + `serde_json` | Existing | Cache serialization |
| `colored` | Existing | Yellow bold hint formatting |
| `anyhow` | Existing | Error wrapping in `Cache::save` |

**No new crates needed.** `is-terminal` is the only addition.

---

## 6. Integration Points

### `src/main.rs`

```rust
fn main() -> Result<()> {
tracing_subscriber::fmt::init();
update_check::spawn(); // <— insert here
let cli = Cli::parse();
// ...
}
```

### `src/lib.rs`

```rust
pub(crate) mod update_check;
```

### `Cargo.toml`

```toml
# Add to [dependencies]
is-terminal = "0.4"
```

---

## 7. Cache File Format

`~/.cache/agentsync/update-check.json`:

```json
{
"last_checked": "2026-03-22T10:00:00Z",
"latest_version": "1.33.0",
"notified_for_version": "1.33.0"
}
```

All fields are strings. Missing or malformed cache files are treated as absent cache.

---

## 8. Hint Output

Format (written to stderr, yellow, bold):

```
⚡ A new version of agentsync is available: 1.33.0 (you have 1.32.0). Run `cargo install agentsync` to update.
```

Only emitted once per new version. Subsequent runs with the same cached `latest_version` are silent.

---

## 9. Testing Approach

- **Unit tests** in `src/update_check.rs`: mock the cache path via a `Cache` constructor that accepts a `PathBuf`; test TTL logic, pre-release skipping, once-per-version logic.
- **Integration tests** in `tests/`: use `cargo test --test all_tests` with a temporary cache dir to verify the spawn doesn't panic and doesn't block.
- **Contract test**: verify the hint output format matches the expected emoji + version pattern.
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
## Exploration: background-version-check

### Current State
The CLI entry point is `src/main.rs` → `fn main()`. It:
1. Initializes tracing subscriber
2. Parses CLI via `clap::Parser`
3. Dispatches to command handlers (`run_skill`, `run_status`, `run_doctor`, etc.)
4. Returns `Result<()>` — on `Ok`, process exits cleanly

Exit flow is clean; no threading exists (only one `thread::sleep` in `linker.rs:2825`).

### Affected Areas
- `src/main.rs` — Spawn the background thread immediately after `tracing_subscriber::fmt::init()`, before CLI parsing (so it runs regardless of command)
- `Cargo.toml` — Add `is-terminal = "0.4"` for TTY detection (or use `atty` crate)
- `src/` — New module `src/update_check.rs` + expose in `src/lib.rs`

### Dependencies Analysis
| Dependency | Available? | Notes |
|---|---|---|
| reqwest (blocking) | Yes | Already has `blocking` feature (Cargo.toml:54) |
| semver | Yes | Already available (Cargo.toml:62) |
| dirs | No | Not in deps — need to use `dirs_next` or implement manually via `home_dir()` + `.cache` |
| is-terminal | No | Not in deps — use `is-terminal = "0.4"` or `atty` |
| colored | Yes | Already available (Cargo.toml:44) |

### Approach
Create `src/update_check.rs` with:
- `CheckedVersion` struct (version, checked_at timestamp, notified_for_version)
- `spawn_version_check()` — spawns `std::thread::Builder::spawn` with `"agentsync-update-check"` name, detached (no `.join()`), uses `reqwest::blocking::Client` with 3s timeout to GET `https://crates.io/api/v1/crates/agentsync`
- Cache file: `~/.cache/agentsync/update-check.json`
- Logic:
1. Check `AGENTSYNC_NO_UPDATE_CHECK` / `CI` env vars → skip
2. Check `stderr.is_terminal()` → skip if not TTY
3. Read cache; if fresh (24h TTL) and already notified for current cached version → skip HTTP
4. Fetch crates.io API; compare with current binary version via `semver::Version::parse`
5. If newer → write cache + print hint to stderr (once per version via `notified_for_version` field)
- Import `VERSION` from crate or use `env!("CARGO_PKG_VERSION")`

Module should be `pub(crate)` in lib.rs. Call `update_check::spawn_version_check()` at top of `main()` before any CLI work.

### Risks
- Thread may outlive main process on very fast exits — acceptable since output goes to stderr only
- Cache dir may not exist — use `dirs::cache_dir()` or `home_dir()` with fallback
- crates.io rate limiting — catch errors silently in background thread
- Version string from crates.io may include pre-release (e.g., `1.33.0-beta.1`) — handle with `semver::Version::parse` or fallback gracefully

### Ready for Proposal
Yes. The implementation is straightforward: ~150-200 LOC in a new module, minimal deps, follows existing patterns (blocking HTTP, serde JSON, colored output, chrono timestamps). No breaking changes.
Loading
Loading