Commit graph

3 commits

Author SHA1 Message Date
Abhay Singh
17ff13ccbe
fix(install): migrate deployments off the retired chopratejas image repo (#2427)
## Description

Fixes #2426.

Persistent Docker deployments store their image in the deployment
manifest. The image org moved from the personal
`ghcr.io/chopratejas/headroom` repo to the project org
`ghcr.io/headroomlabs-ai/headroom`, and the personal repo is frozen at
0.27.0. Because the manifest image is only ever read back verbatim
(`build_runtime_command`, `docker run`, status output), a deployment
created before the move keeps pulling 0.27.0 forever, several minor
versions behind the CLI, with no drift signal to the user.

Two related gaps:

- `headroom/install/state.py` reads the recorded image straight back
with no migration, so an old manifest is stuck on the dead repo.
- `headroom/cli/install.py` `deploy --image` still defaulted to
`ghcr.io/chopratejas/headroom:latest`, so brand new deploys through that
command also pinned the retired repo (the `install-apply` default was
already correct).

## Fix

- Rewrite the retired repo to the org repo when a manifest is loaded, in
both `load_manifest` and `list_manifests`, preserving whatever tag was
recorded. The rewrite is surgical: it only matches the exact retired
`ghcr.io/chopratejas/headroom` repo and leaves already-current images
and any third-party image untouched. The migrated value persists on the
next apply/save.
- Change the `deploy --image` default to
`ghcr.io/headroomlabs-ai/headroom:latest` so it matches `install-apply`.

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- `headroom/install/state.py`: add `_migrate_deprecated_image` and apply
it in `load_manifest` and `list_manifests` before constructing the
manifest.
- `headroom/cli/install.py`: `deploy --image` default now points at the
org repo.
- `tests/test_install/test_state.py`: new tests covering load and list
migrating the retired repo (tag preserved) and leaving
current/third-party images untouched.

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [ ] Manual testing performed

### Test Output

```text
$ uvx ruff@0.15.17 check headroom/install/state.py headroom/cli/install.py tests/test_install/test_state.py
All checks passed!
$ uvx mypy@1.20.2 --ignore-missing-imports headroom/install/state.py
Success: no issues found in 1 source file
```

## Real Behavior Proof

- Environment: Windows 11, Python 3.12, project venv (`uv sync --extra
proxy`), `uvx ruff@0.15.17` / `uvx mypy@1.20.2`.
- Exact command / steps: wrote a manifest.json pinning
`ghcr.io/chopratejas/headroom:latest` (and `:0.27.0`) under a temp home,
then called the real `load_manifest` and `list_manifests`.
- Observed result: both returned a manifest with `image ==
ghcr.io/headroomlabs-ai/headroom:latest` (tag preserved on the `0.27.0`
case too); an already-current image and a third-party image passed
through unchanged. Ran against the actual module.
- Not tested: a live `docker run` against the migrated image.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] I have updated the CHANGELOG.md if applicable
2026-07-19 22:15:01 -07:00
Leoy
42bdf23d24
fix(install): write deployment manifest atomically and tolerate corrupt manifests (#1303)
## Description

Make deployment-manifest persistence in `headroom/install/state.py`
crash-safe by writing the manifest **atomically**. `save_manifest` used
a plain `path.write_text(...)` (truncate-then-write), so an interrupted
save (Ctrl-C, system restart, container OOM/SIGKILL) could leave a
truncated `manifest.json` on disk.

> **Note (rebased onto current `main`):** since this PR was opened,
#1491 hardened `load_manifest` to raise a typed `ManifestError` on a
corrupt manifest. I've rebased and **dropped my original `load_manifest
→ return None` change in favour of that deliberate typed-error design**,
so this PR now scopes down to the still-missing piece: the **atomic
write** (upstream `save_manifest` is still a plain `write_text`), plus a
regression test for the `ManifestError` path that `main` added without
test coverage.

Closes #

## Type of Change

- [x] Bug fix (non-breaking change that fixes an issue)
- [ ] New feature (non-breaking change that adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)
- [ ] Documentation update
- [ ] Performance improvement
- [ ] Code refactoring (no functional changes)

## Changes Made

- Add `_atomic_write_text(path, data)`: write to a same-directory temp
file → `flush()` + `os.fsync()` → `os.replace()` (atomic rename on POSIX
and Windows); the temp file is cleaned up if anything fails.
- `save_manifest` now persists via `_atomic_write_text` instead of
`path.write_text(...)`, so a crash between truncate and full write
leaves either the previous file or the complete new one — never a
truncated manifest.
- `load_manifest` is left exactly as `main` has it (raises
`ManifestError` on a corrupt payload) — no behavioural change from me
there.
- Tests: add `test_save_manifest_writes_atomically` (no leftover temp
file; manifest round-trips) and
`test_load_manifest_raises_manifest_error_on_corrupt_payload` (covers
the `ManifestError` path #1491 introduced but did not test).

## Testing

- [x] Unit tests pass (`pytest`)
- [x] Linting passes (`ruff check .`)
- [x] Type checking passes (`mypy headroom`)
- [x] New tests added for new functionality
- [x] Manual testing performed

### Test Output

```text
$ pytest tests/test_install/test_state.py -q
.....                                                                    [100%]
5 passed in 0.11s

$ ruff check headroom/install/state.py tests/test_install/test_state.py
All checks passed!

$ ruff format --check headroom/install/state.py tests/test_install/test_state.py
2 files already formatted

$ mypy headroom/install/state.py
Success: no issues found in 1 source file
```

## Real Behavior Proof

- **Environment:** macOS (Darwin), Python 3.13, rebased onto current
`main`.
- **Exact command / steps:** `save_manifest(manifest)` then inspect the
profile dir and reload.
- **Observed result:** after a save the profile directory contains only
`manifest.json` (no leftover `.manifest.json.*.tmp`), and
`load_manifest("default")` round-trips the persisted manifest. A
deliberately-corrupt `manifest.json` (`"{not json"`) makes
`load_manifest` raise `ManifestError` (typed), not a raw
`JSONDecodeError`.
- **Not tested:** the physical-crash-mid-write window is reasoned about
via `os.replace()` atomicity, not fault-injected.

## Review Readiness

- [x] I have performed a self-review
- [x] This PR is ready for human review

## Checklist

- [x] My code follows the project's style guidelines
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [ ] I have updated the CHANGELOG.md if applicable

## Additional Notes

Documentation / CHANGELOG boxes are unchecked as N/A — this is an
internal persistence-hardening fix with no user-facing surface. The diff
is now small (atomic write + two tests); the corrupt-manifest handling
itself lives in `main` via #1491.
2026-07-09 14:07:05 -05:00
JerrettDavis
bd242fc62d test: expand persistent install coverage
Add focused regression coverage for install, runtime, provider, state, health, supervisor, and persistent wrap flows so the new persistent deployment surfaces are exercised more thoroughly in CI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-04-11 18:24:15 -05:00