fix(update): refuse the version-floor auto-update on a checkout with local commits

[leonlaiyc]

Problem / Motivation

GatewayOrchestrator._auto_apply_update (src/kiro_crew/slack/gateway.py) ends in git reset --hard origin/<branch>. Reached through the mandatory policy version-floor trigger, it does that to a checkout carrying committed local work — and the work is gone.

The two triggers do not carry the same guarantee:

• The ordinary auto_update trigger is safe by construction. It requires the update check's can_fast_forward verdict (behind > 0 and ahead == 0, dashboard/handlers/updates.py:691), so a checkout with local commits is never offered to it.
• The version-floor trigger (update_required(_running_version) branch) deliberately bypasses available and gates only on can_apply. can_apply mirrors CommandProvider.can_apply() — an apply command exists and can run — so it reports the install shape (a git checkout can apply; a wheel or .deb cannot). It says nothing about how the tree relates to the remote. Every git checkout below the floor reaches the reset.

Neither pre-existing check in _auto_apply_update stops it:

Check	Why a tree with local commits passes
git diff HEAD origin/<branch> --quiet	such a tree has a content diff like any other — that is what "there are new commits" looks like
git status --porcelain	only warns, and only about uncommitted edits, then proceeds

So a developer checkout running below a policy min_version loses committed local work unattended, with only a tracked-file log line as evidence.

Why it matters

Unattended, unrecoverable loss of committed work on the most privileged path in the product: no auth, no click, git reset --hard + pip install + os.execv on boot. The user never sees a prompt, and reset --hard leaves nothing to recover from the working tree — only the reflog, which the affected user has no reason to know to check.

The trigger is a policy floor, so the hosts most likely to hit it are exactly the ones with local commits: a developer machine below the org's min_version.

What changed (motivation → approach → change)

Motivation — the destructive step must not run on a tree where it destroys committed work.

Approach — mirror the verdict the safe trigger already relies on, at the site that actually performs the reset, rather than at the callers. can_fast_forward lives in the update check; the version-floor path never consults it. Putting the equivalent test inside _auto_apply_update means every future caller inherits it, and no caller has to remember to.

Change — after the fetch (so the counts are against the revision this would reset to) and before the reset:

• count git rev-list --left-right --count HEAD...origin/<branch>;
• when ahead > 0, refuse: log the counts and report them through the existing update-status surface (push_update_progress("failed", …), the same surface handlers/updates.py uses for every other refusal), then return without resetting;
• otherwise proceed exactly as before.

The condition is ahead > 0, not ahead > 0 and behind > 0. The issue proposed the diverged case, and the diverged case is real — but can_fast_forward is behind > 0 **AND ahead == 0**, so mirroring it faithfully means refusing whenever the tree is ahead. An ahead-only checkout (local commits, remote unmoved) reaches this reset for exactly the same reason a diverged one does — can_apply gates on install shape — and loses its commits just as thoroughly. On a developer box that has simply not pushed yet, ahead-only is the likelier shape. Narrowing to the diverged case would have left the more common half of the same defect open, so it is covered and separately tested.

The floor mandate stands without the reset being its mechanism. "This host must not stay below the floor" and "discard this developer's committed work to get there" are different statements, and only the first is policy — a violation that can only be cleared by discarding committed work needs a human, so the host stays below the floor and says why.

The gate fails CLOSED. If rev-list fails, or returns output that does not parse as two counts, the update is refused rather than proceeding. On a path this privileged, "we could not establish that the reset is safe" is not a reason to reset; it is the same posture as the existing source-pin check above it, which refuses a blocked host rather than assuming.

Not changed, deliberately. #4503 already covers this site's handling of uncommitted tracked edits; that path is left exactly as it was so this diff shows one behaviour change.

One production file, 84 lines.

Tests

test/test_slack_gateway.py::TestAutoApplyUpdateDivergenceGuard — six cases, all driving the real _auto_apply_update and asserting on the argv actually handed to create_subprocess_exec, so "no reset happened" is observed at the process boundary rather than inferred from a return value. The fake dispatches on the git subcommand rather than a call index, so reordering an unrelated git call cannot make it assert against the wrong process.

Test	Behavior locked in
test_diverged_checkout_is_not_reset	3 ahead / 5 behind: no reset --hard runs, and the refusal reaches the update-status surface carrying both counts
test_ahead_only_checkout_is_not_reset	2 ahead / 0 behind: unpushed commits are equally protected — the half the narrow reading would have missed
test_fast_forward_checkout_still_updates	preservation: 0 ahead / 5 behind — the ordinary path is not narrowed
test_identical_checkout_still_updates	preservation: 0/0 with a content diff still proceeds
test_unreadable_counts_fail_closed	rev-list exiting non-zero refuses instead of falling through
test_unparseable_counts_fail_closed	rev-list succeeding with junk is not read as "no local commits"

Fail-before / pass-after, with the production file reverted to origin/main and the tests left in place:

FAILED ...::test_diverged_checkout_is_not_reset
  - AssertionError: git reset --hard ran on a checkout with 3 committed local commits:
    the version-floor trigger discarded work no human agreed to lose
FAILED ...::test_ahead_only_checkout_is_not_reset
  - AssertionError: git reset --hard ran on a checkout holding 2 unpushed commits
FAILED ...::test_unreadable_counts_fail_closed
FAILED ...::test_unparseable_counts_fail_closed

4 failed, 2 passed

The two preservation tests pass on both trees by design — they are the guard against over-correcting into refusing every update, not evidence of a defect, and are reported as such rather than folded into the fail-before count.

pytest test/test_slack_gateway.py -q                     238 passed, 8 skipped
pytest test/test_spawn_audit.py \
       test/test_update_check_install_aware.py -q                     81 passed

Four existing _fake_exec helpers were taught the new call (test_venv_update_full_path, test_reset_then_frontend_then_pip, test_no_restart_after_any_unclean_sync_even_when_the_repair_works, test_a_failed_install_with_a_failed_repair_does_not_restart). They dispatch on a call counter, so a new git call in the sequence would have shifted every position after it. Rather than renumber them, each answers rev-list by args and returns before the counter is touched, so every existing position keeps the meaning it already had — and all four still pass against pristine origin/main, which the control run above confirms.

Gates: flake8 · isort --check-only — both clean on the two changed files. black --diff reports no hunk overlapping any changed region in either file; both are pre-existing baseline offenders on origin/main, so they are left unformatted rather than graduated off .github/black-baseline.txt.

Manual verification

N/A — unit coverage sufficient: the assertion is on the exact argv the method hands to create_subprocess_exec, so the property under test ("no git reset --hard is issued") is checked at the boundary where the damage would occur. Reproducing by hand means building a checkout with local commits on a host below a policy min_version and waiting for the floor trigger to fire on boot, which is what these tests drive directly.

Related Issues

Refs #5163.

Surfaced by the First Principles review on #5158 (the CLI divergence guard for #5143), which guards the other reset-to-origin apply site, cli_server.py::_update. That PR is still open; this change touches a different file and does not depend on it.

#4503 covers the same site's handling of uncommitted tracked edits — deliberately not addressed here.

Checklist

• Single commit with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) — no user-facing doc describes the auto-update reset preconditions
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

[NicholasRBowers]

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]: This PR has been inactive for 7+ days with failing CI. I've assessed the blockers and they appear resolvable — I'll push fixes directly to this branch as a co-author.

Assessment: Two blockers, both mechanical: (1) merge conflict with current main; (2) two deterministic failures in test/test_driver_session_directives.py (TestChannelApplierBoundary, shard 2 on 3.10/3.12/Windows) — a file this 2-file diff does not touch, consistent with base drift rather than the PR's own change. Plan: rebase onto fresh origin/main resolving conflicts per the PR's clear intent (refuse the version-floor auto-update on a checkout with local commits), verify the shard-2 failures against the rebased head, fix if the diff genuinely reaches them, and drive to green.

If you'd prefer I don't touch this PR, add the pr-no-autofix label.

[NicholasRBowers]

🤖 Kiro Crew [operator: NicholasRBowers#a942f9ca]

Drive-to-green halted before any push: this PR has been superseded by work that merged to main after it was opened.

While preparing the rebase onto current main, I found that #5387 (fix(update): gate auto-update on an allowlist, and harden the path it re-enables, merged 2026-08-25) independently landed the same guard this PR proposes — in a strictly stronger form:

• The refusal lives in the shared body of _auto_apply_update, so it covers both triggers, including the mandatory version-floor path this PR targets.
• It counts local commits against the pinned reset target OID via the commits_ahead helper, avoiding the ref re-resolution race inherent in git rev-list HEAD...origin/<branch> (a concurrent fetch can advance origin/<branch> between the count and the reset).
• It refuses whenever ahead != 0 — ahead-only, diverged, and unknown/None — i.e. it fails closed exactly as this PR intended.
• Main also carries test coverage for the same cases: test_local_commits_refuse_before_the_reset (ahead > 0 refuses) and test_unknown_ahead_count_also_refuses (unreadable count refuses), plus positive-path tests with commits_ahead == 0.

The tracked issue also resolved independently: #5163 was closed 2026-08-25 as a duplicate of #4503, which #5387 fixed.

Consequence: resolving this PR's rebase conflicts by re-applying its guard logic onto main's current structure produces an empty diff — the guard is already there. There is nothing left for this PR to add, so there is no "green" to drive it to, and per the drive-to-green rules I will not repurpose or re-scope a contributor's PR.

Recommendation: close this PR as superseded by #5387, with credit to @leonlaiyc for independently identifying and correctly diagnosing the version-floor hard-reset data-loss path (the PR predates #5387 and reached the same fail-closed design). @leonlaiyc — if you see a residual delta main still lacks, please comment and this can be revisited.

No commits were pushed to this branch; it is untouched at 5682633fcf72bab63254bfaed94962cdac62f308.

[leonlaiyc]

Closing as superseded by #5387. Current main now contains the same fail-closed protection in a stronger shared implementation, including pinned-target ahead-count handling and regression coverage; rebasing this branch leaves no residual change to contribute. Thanks to the maintainer bot for verifying the overlap without modifying this branch.