-
Notifications
You must be signed in to change notification settings - Fork 46.7k
fix(oneshot): honor resume state #74397
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
SE87H
wants to merge
8
commits into
NousResearch:main
Choose a base branch
from
SE87H:agent/fix-oneshot-resume
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
f78a7b2
fix(oneshot): honor resume state
6d0d3e3
fix(oneshot): scope continue to workspace
SE87H 8239b10
fix(oneshot): fail when resumed workspace is unavailable
SE87H a0b7ab3
test(oneshot): cover unavailable resumed workspace
SE87H e784efc
fix(oneshot): synchronize terminal workspace on resume
SE87H f6e2819
test(oneshot): cover terminal workspace synchronization
SE87H 123dc83
test(oneshot): restore process cwd after workspace tests
SE87H 5931f54
test(oneshot): preserve resume invariants after fresh-main rebase
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| import pytest | ||
|
|
||
|
|
||
| def test_oneshot_resume_fails_before_reopen_when_recorded_cwd_is_missing(monkeypatch): | ||
| import hermes_cli.oneshot as oneshot_mod | ||
|
|
||
| reopened = [] | ||
|
|
||
| class FakeSessionDB: | ||
| def get_session(self, session_id): | ||
| return {"id": session_id, "cwd": "/recorded/workspace"} | ||
|
|
||
| def resolve_session_by_title(self, _title): | ||
| return None | ||
|
|
||
| def resolve_resume_session_id(self, session_id): | ||
| return session_id | ||
|
|
||
| def get_resume_conversations(self, _session_id): | ||
| return ([{"role": "user", "content": "prior context"}], []) | ||
|
|
||
| def reopen_session(self, session_id): | ||
| reopened.append(session_id) | ||
|
|
||
| monkeypatch.setattr(oneshot_mod.os.path, "isdir", lambda _path: False) | ||
|
|
||
| with pytest.raises( | ||
| FileNotFoundError, | ||
| match="Recorded session working directory is unavailable", | ||
| ): | ||
| oneshot_mod._load_oneshot_resume( | ||
| FakeSessionDB(), | ||
| resume_session_id="session-1", | ||
| continue_last=False, | ||
| restore_resume_cwd=True, | ||
| ) | ||
|
|
||
| assert reopened == [] | ||
|
|
||
|
|
||
| def test_oneshot_resume_allows_explicit_cwd_restore_opt_out(monkeypatch): | ||
| import hermes_cli.oneshot as oneshot_mod | ||
|
|
||
| reopened = [] | ||
|
|
||
| class FakeSessionDB: | ||
| def get_session(self, session_id): | ||
| return {"id": session_id, "cwd": "/recorded/workspace"} | ||
|
|
||
| def resolve_session_by_title(self, _title): | ||
| return None | ||
|
|
||
| def resolve_resume_session_id(self, session_id): | ||
| return session_id | ||
|
|
||
| def get_resume_conversations(self, _session_id): | ||
| return ([{"role": "user", "content": "prior context"}], []) | ||
|
|
||
| def reopen_session(self, session_id): | ||
| reopened.append(session_id) | ||
|
|
||
| monkeypatch.setattr(oneshot_mod.os.path, "isdir", lambda _path: False) | ||
|
|
||
| session_id, history = oneshot_mod._load_oneshot_resume( | ||
| FakeSessionDB(), | ||
| resume_session_id="session-1", | ||
| continue_last=False, | ||
| restore_resume_cwd=False, | ||
| ) | ||
|
|
||
| assert session_id == "session-1" | ||
| assert history == [{"role": "user", "content": "prior context"}] | ||
| assert reopened == ["session-1"] |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also set
os.environ["TERMINAL_CWD"] = saved_cwdafter this succeeds. Interactive resume does this incli.py:7178-7179; prompt, terminal, and file resolution prefer that variable, soos.chdir()alone can leave a resumed one-shot operating on the prior configured workspace.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in
e9db5835a: afteros.chdir(saved_cwd)succeeds, one-shot resume now setsos.environ["TERMINAL_CWD"] = saved_cwdbefore the durable session is reopened and beforeAIAgentconstruction. Added focused coverage inb5123936cfor stale-value replacement, missing/unenterable workspaces leaving the prior environment and session untouched,--no-restore-cwdpreservation, and the value observed during agent construction. The implementation commit diff is limited to this synchronization. I reproduced the transition logic in an isolated environment (4/4); the full repository pytest run still requires a runnable checkout.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Full validation is now complete. The exact PR code head
b5123936cb13005ba446835ee46b5da5d24564f7was tested against the samemainbase in a disposable internal PR; its only additional changes were contributor-attribution mappings required by the fork's governance check. CI run #19 passed all 8 Python slices (including the focused one-shot tests), Python E2E, desktop Playwright E2E, ruff/ty and Windows checks, OSV/supply-chain scans, and lockfile/history gates. The validation PR was closed without merge. Leaving this thread for reviewer confirmation rather than self-resolving it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Final verification on head
c55ce708ed2889ca3e9ec4529f6da9b1fd48b609: the focused workspace tests now also restore the process CWD after each test, preventing their intentional productionchdir()calls from leaking into later tests. Disposable validation PRSE87H/hermes-agent#4completed full CI run #24 successfully; one unrelated asynchronous TTS timing flake passed when only its job was rerun, with no code change. Leaving the thread unresolved for reviewer confirmation.