Skip to content

fix(update): preserve and rebase committed local patches, and report real rebase conflicts - #70953

Open
mhuttes14 wants to merge 2 commits into
NousResearch:mainfrom
mhuttes14:mhuttes14/update-committed-patch-rebase
Open

mhuttes14 wants to merge 2 commits into
NousResearch:mainfrom
mhuttes14:mhuttes14/update-committed-patch-rebase

Conversation

@mhuttes14

Copy link
Copy Markdown

Problem

hermes update assumes all local work is uncommitted (and therefore stashed). An install whose main is deliberately ahead of origin — carrying committed local patches — hits the ff-only-pull failure path, which unconditionally runs a hard reset to origin/<branch> and silently destroys that work.

What this does

Commit 1 — preserve committed local patches. Adds updates.committed_local_changes (refuse | rebase, default refuse, so behavior is unchanged unless opted in):

  • refuse — if local main is ahead, stop with reconcile instructions instead of discarding commits.
  • rebase — create a durable backup ref (refs/hermes/update-backups/<branch>/<ts>, pruned to the newest 5 by reflog time), rebase the patch stack onto the remote, and continue the normal pipeline. On any failure it aborts, verifies HEAD is restored to the pre-pull SHA, and keeps the backup ref.

The historical reset is retained verbatim for the no_local_commits case, which is the only situation it was ever correct for.

Commit 2 — report what actually conflicted. When the rebase fails, the operator was shown one line: Rebasing (1/6).

git splits rebase-conflict output across streams: stdout carries CONFLICT (content): Merge conflict in <path>; stderr carries Rebasing (n/m), error: could not apply <sha>... and hint: lines. The failure path read (stderr or stdout) — stderr is non-empty, so stdout was discarded wholesale — and then took .splitlines()[0], which is the progress banner. Every actionable detail was dropped twice.

_rebase_conflict_summary scans both streams for the failing commit and reads unmerged paths from the index before git rebase --abort destroys that state, falling back to scraping stdout CONFLICT lines for rename/delete conflicts that leave no unmerged entries.

Before:

✗ Could not rebase committed local patches automatically.
  Rebasing (1/6)

After:

✗ Could not rebase committed local patches automatically.
  error: could not apply f231e8d... local patch
  Conflicting file(s): conflict.txt
  ✓ Rebase aborted; the original committed state is restored.
    Resolve by hand with:
      git -C <root> rebase --empty=drop --no-keep-empty origin/main

Testing

103 tests pass on this branch against current main (test_update_committed_changes.py, test_update_autostash.py, test_cmd_update.py), including new coverage for the conflict-reporting path and the rename/delete fallback.

Context

Both commits have been running in production on a self-hosted install that carries local patches; the rebase path reconciled a 332-commit upstream jump successfully, and the conflict reporting was what made a real two-patch conflict diagnosable.

🤖 Generated with Claude Code

Arlo and others added 2 commits July 24, 2026 15:01
When `updates.committed_local_changes: rebase` could not replay a local
patch stack, the operator was shown a single line: `Rebasing (1/6)`.

git splits rebase-conflict output across streams. stdout carries the
useful part (`CONFLICT (content): Merge conflict in <path>`); stderr
carries `Rebasing (n/m)`, `error: could not apply <sha>...` and `hint:`
lines. The failure path read `(stderr or stdout)` — stderr is non-empty,
so stdout was discarded wholesale — and then took `.splitlines()[0]`,
which is the progress banner. Every actionable detail was dropped twice.

Add `_rebase_conflict_summary`, which scans BOTH streams for the failing
commit and reads the unmerged paths out of the index while the conflicted
rebase is still live (`git rebase --abort` destroys that state moments
later), falling back to scraping stdout `CONFLICT` lines for rename/delete
conflicts that leave no unmerged entries. Also print a copy-pasteable
command to finish the rebase by hand.

Before: `Rebasing (1/6)`
After:  `error: could not apply f231e8d... local patch`
        `Conflicting file(s): conflict.txt`
        `Resolve by hand with: git -C <root> rebase --empty=drop ...`

Co-Authored-By: Claude <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) area/config Config system, migrations, profiles area/install-update Installer, updater, packaging, wheels, doctor sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 24, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for addressing a real destructive updater path. Current main still hard-resets after a failed fast-forward merge at hermes_cli/update_cmd.py:3431-3440, so the underlying fix remains needed.

Problems

  • This branch predates updater extraction commit 927463efcc: the live implementation is now hermes_cli/update_cmd.py:_cmd_update_impl (:3052), while hermes_cli/main.py only re-exports updater symbols (:4991). The reconciliation code must be ported rather than merged in its original location.
  • Default config data moved in 1fe06115d1; add the new setting beside updates.non_interactive_local_changes in hermes_cli/config_defaults.py:2674, not the old default block in config.py.
  • The cross-referenced fix(update): reconcile local branches during update #22037 local-branch case remains separate: current main switches any non-target branch at update_cmd.py:3246, and this diff reconciles only the target branch after that switch.

Suggested changes

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Summary

Three PRs address unsafe handling of committed local patches across two checkout shapes: #4142 and #70953 protect commits on the update branch from the failed-fast-forward hard-reset path, while #22037 reconciles a separate local-only branch that would otherwise be left inactive. #70953 is the stronger successor to #4142, whereas #22037 covers a complementary cause.

Related pull requests

Duplicates

#4142 is substantially duplicated and superseded by #70953 for update-branch divergence; #22037 is complementary because it handles a separate local-only branch.

Suggested consolidation

Author action on #70953: rebase onto current main and port the preserved-commit helpers, failed-merge handling, configuration default, and retained integration tests to the live updater/config locations identified by the maintainer-bot keep_open verdict. Keep #22037 open with a salvage path for separate-branch reconciliation, but require fail-closed git cherry handling, resolved-target support, and comparison-failure and rebase-conflict tests; #4142 can remain closed as a duplicate of #70953.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup4142 ["PRs duplicating each other"]
        P4142["PR #4142 (closed)"]
        P70953["PR #70953 (open)"]
    end
    class P4142 closed
    class P70953 open
    class P70953 target
    click P4142 "https://github.com/NousResearch/hermes-agent/pull/4142"
    click P70953 "https://github.com/NousResearch/hermes-agent/pull/70953"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 61 kB of PR diffs, 6 kB of issue/PR text, 3 kB of discussion (3 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles area/install-update Installer, updater, packaging, wheels, doctor comp/cli CLI entry point, hermes_cli/, setup wizard comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants