Skip to content

fix(cli): stop creating .install_method in Git checkout (#66189) - #66322

Open
lEWFkRAD wants to merge 1 commit into
NousResearch:mainfrom
lEWFkRAD:fix/66189-install-method-dirty-checkout
Open

fix(cli): stop creating .install_method in Git checkout (#66189)#66322
lEWFkRAD wants to merge 1 commit into
NousResearch:mainfrom
lEWFkRAD:fix/66189-install-method-dirty-checkout

Conversation

@lEWFkRAD

Copy link
Copy Markdown
Contributor

Fixes #66189

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades needs-decision Awaiting maintainer decision before any implementation labels Jul 17, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #54855 already supplies the focused .install_method ignore. This PR includes that correction alongside substantial unrelated fork/WIP changes; please split or rebase the targeted fix before review.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Clean, well-scoped change with good test coverage. No security concerns.


Reviewed by Hermes Agent

@teknium1 teknium1 left a comment

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.

Thanks for identifying a real updater annoyance. The premise still holds on current main: hermes_cli/config.py:503 writes the checkout-local marker, while hermes_cli/main.py:6534-6565 checks porcelain status and autostashes untracked files. The root-scoped rule in commit a1ad6a09081eb80877ed41eb9093ea8c72b0c861 addresses that path.

Problems

  • The PR diff contains 51 files and 10,516 added lines unrelated to #66189, including AMDP, platform, telemetry, pet, and backup-artifact changes. The focused final commit changes only .gitignore and tests/test_install_sh_install_method_stamp.py.
  • The unrelated AMDP work adds the user-facing reviewer_max_tokens setting at agent/amdp/config.py:172 (also documented at agent/amdp/ACTIVATION.md:48), which conflicts with the standing no-user-facing-max_tokens policy.
  • tests/test_install_sh_install_method_stamp.py:42 checks for a literal .gitignore string rather than Git ignore behavior or the updater contract.

Suggested changes

  • Rescope to the focused .install_method ignore change and add behavioral Git/update coverage.

Automated hermes-sweeper review.

Comment thread tests/test_install_sh_install_method_stamp.py
@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 18, 2026
@lEWFkRAD
lEWFkRAD force-pushed the fix/66189-install-method-dirty-checkout branch from a1ad6a0 to 63fb39f Compare July 19, 2026 13:32
@teknium1 teknium1 added the area/install-update Installer, updater, packaging, wheels, doctor label Jul 19, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Five PRs address #66189 by ignoring the installer-created root .install_method marker; #66322 and #67364 also add regression coverage, with #67364 exercising the updater's exact untracked-autostash command. #67364 is the merged reference implementation and the recorded best fix for this issue.

Related pull requests

Duplicates

#54855, #66216, #66322, #66723, and #67364 share the same core .install_method ignore correction; #66322 adds git check-ignore coverage, while #67364 adds the stronger updater-autostash regression. #66216 and #66723 were already closed as duplicates of #54855, and #67364 ultimately supersedes the remaining variants.

Suggested consolidation

Close #54855 and #66322 as already implemented on main by merged #67364: its root-scoped rule is identified in the maintainer-bot evidence at commit 211edd5e08d5, and the merged diff includes tests/hermes_cli/test_update_autostash.py::test_install_method_marker_not_autostashed_by_update. This preserves the recorded best-fix verdict for #67364; #66216 and #66723 should remain closed in the duplicate chain #66216/#66723#54855#67364.

Complex graph

flowchart TD
    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
    I66189(["issue #66189 (closed)"])
    subgraph Dup54855 ["PRs duplicating each other"]
        P54855["PR #54855 (open)"]
        P66216["PR #66216 (closed)"]
        P66322["PR #66322 (open)"]
        P66723["PR #66723 (closed)"]
        P67364["PR #67364 (merged)"]
    end
    P66322 -->|fixes| I66189
    class I66189 closed
    class P54855 open
    class P66216 closed
    class P66322 open
    class P66723 closed
    class P67364 merged
    class P67364 best
    class P66322 target
    click I66189 "https://github.com/NousResearch/hermes-agent/issues/66189"
    click P54855 "https://github.com/NousResearch/hermes-agent/pull/54855"
    click P66216 "https://github.com/NousResearch/hermes-agent/pull/66216"
    click P66322 "https://github.com/NousResearch/hermes-agent/pull/66322"
    click P66723 "https://github.com/NousResearch/hermes-agent/pull/66723"
    click P67364 "https://github.com/NousResearch/hermes-agent/pull/67364"
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 5 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 7 kB of PR diffs, 7 kB of issue/PR text, 5 kB of discussion (15 comments), 6 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 needs-decision Awaiting maintainer decision before any implementation P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Installer-created .install_method dirties Git checkouts

5 participants