chore: synchronize fork main with upstream - #59
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
💡 Codex ReviewLine 388 in 8d24bc2  Pin the newly added wheel build dependency
Pin or upper-bound AGENTS.md reference: AGENTS.md:L292-L298 Mark the native PID probe as windows_only
Replace this bare AGENTS.md reference: AGENTS.md:L360-L365 hermes-agent/tools/file_operations.py Line 657 in 8d24bc2 Route the native-read switch through config
Do not introduce AGENTS.md reference: AGENTS.md:L79-L82 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Co-authored-by: crazyief <8566250+crazyief@users.noreply.github.com>
The memory_tool schema advertises new_text as an alias for content, and
memory_tool resolves it when content is None. But the table-driven inline
executor's arg_specs (agent/inline_tool_executors.py) did not list new_text,
so _call_tool's allowlist silently dropped it: a replace call using the
documented alias reached memory_tool with both fields None and failed with
"content is required for 'replace' action." — even though the caller
supplied the value. Forward new_text alongside content/old_text so the
documented alias fires and content still wins when both are set, matching
what the batch path (op.get("content") or op.get("new_text")) already
accepts.
… guidance
The corrupt-cause recovery guidance hardcoded `~/.hermes/backups/` while
every other path in the same message follows the active HERMES_HOME
(`{db_path}` is already interpolated). A custom-home or named-profile
deployment was told to restore from a directory that may not exist at all,
mid data-loss incident. Both sites (turn-completion explainer and gateway
startup broadcast) now interpolate `<hermes_root>/backups` via
get_default_hermes_root(), matching hermes_cli/backup.py's real backup
location.
Fixes NousResearch#104250
Apply the narrow registry fallback proposed in PR NousResearch#68458 without unrelated case normalization or dead legacy flags. Preserve dedicated named profiles before using CustomProfile. This corrects existing reasoning loss only; per-model dialect configuration remains a product decision. Co-authored-by: saotu <160758706+saotu@users.noreply.github.com>
Salvage NousResearch#104301 error presentation and translations. Keep drill-in outcome local to its mounted scope, ignore stale responses, retain existing rows, and show retry even when cached lanes exist. Co-authored-by: elvindu <dumanxiang@qq.com>
Adapt the callable-source diagnosis from snipecoder (NousResearch#102244) and fallback slice from BGwill-OUTLOOK (NousResearch#102721), without unrelated reasoning or override changes. Co-authored-by: CloudWishOS <99405975+snipecoder@users.noreply.github.com>\nCo-authored-by: BGwill-OUTLOOK <bgwillwork@outlook.com>
Independent review found auto-created mock attributes could replace static credentials. Read the SDK instance's stored provider without triggering attribute synthesis. Co-authored-by: CloudWishOS <99405975+snipecoder@users.noreply.github.com> Co-authored-by: BGwill-OUTLOOK <bgwillwork@outlook.com>
Deletes the temporary --user-data-dir used by the update UI shim when the browser process is shut down, preventing ~100MB leaks per update. Fixes issue NousResearch#104350.
Track the path actually launched, preserving the no-UI case and unrelated profiles. Adapted the ownership approach from NousResearch#104362. Co-authored-by: liuhao1024 <sunsky.lau@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0db7ea654
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # The fork's existing Code Scanning baseline cannot correlate the upgraded | ||
| # reusable-workflow identity on CI PR runs. The standalone weekly scan | ||
| # keeps the default upload-sarif=true for durable main-branch findings. | ||
| upload-sarif: false |
There was a problem hiding this comment.
The ci.yaml workflow runs for both pull requests and pushes to main, but this reusable-workflow call unconditionally passes upload-sarif: false. Consequently, the post-merge OSV scan also suppresses Code Scanning publication, even though the adjacent comment says the workaround is for PR runs; on main there is no PR review consumer, and the remaining short-lived artifact is not a durable alert. Pass false only when github.event_name == 'pull_request' so main-push scans retain SARIF.
Useful? React with 👍 / 👎.
| function redialPoolBackendAfterResume(poolKey: string) { | ||
| const { connectionId, profile } = parseBackendScopeKey(poolKey) | ||
|
|
||
| return runClaimedBackendDial(connectionId, profile, () => |
There was a problem hiding this comment.
When the global connection mode is remote, resolveRegistryLocalRoute deliberately stores the genuinely local backend under conn:local::<profile> so it cannot collide with the remote backend's bare profile key. This path parses that key into connectionId == "local" and then runClaimedBackendDial recomputes it through backendScopeKey, which collapses local back to the bare profile. If resume revalidation overlaps a renderer reconnect for that profile, the unrelated local and remote dials therefore join one claim and one caller can receive the other backend's descriptor; retain the original poolKey as the claim key for this path.
AGENTS.md reference: apps/desktop/AGENTS.md:L192-L196
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce7e81dc70
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # The upstream reusable workflow uploads this exact file under its | ||
| # fixed artifact name, which the wrapper downloads below. | ||
| results-file-name: osv-results.sarif | ||
| upload-sarif: ${{ inputs.upload-sarif }} |
There was a problem hiding this comment.
When this workflow runs from its own weekly schedule or workflow_dispatch trigger, it is not invoked through workflow_call, so the workflow_call.inputs.upload-sarif default is not populated; ${{ inputs.upload-sarif }} therefore resolves to an empty/false value and the child scanner still suppresses the SARIF upload. Fresh evidence after the earlier comment is that the repair only supplies true from the ci.yaml caller, leaving both direct triggers without a value; explicitly enable uploads for non-workflow_call events.
Useful? React with 👍 / 👎.
| "browser_get_images", "vision_analyze"), | ||
| "edit": ("write_file", "patch", "skill_manage"), | ||
| "search": ("search_files",), | ||
| "execute": ("terminal", "process", "execute_code", "browser_click", "browser_type", "browser_scroll", |
There was a problem hiding this comment.
When a current ACP session invokes the renamed process_manage schema, this table and the related title/formatter tables recognize only the legacy process name. As a result, get_tool_kind() classifies the call as other, and, more importantly, _tool_result_failed() treats common tool_error() responses such as {"error":"session_id is required"} as completed because process_manage is absent from _POLISHED_TOOLS; Zed/VS Code therefore show failed process operations as successful. Add the canonical name alongside the replay alias, including the polished/title/completion mappings.
Useful? React with 👍 / 👎.
|
@codex review |
💡 Codex ReviewLines 71 to 72 in dfdd0ce Preserve the synchronized upstream commit history
Fresh evidence after the earlier sync comment is that the message now names AGENTS.md reference: AGENTS.md:L71-L72 hermes-agent/tests/tools/test_file_ops_single_roundtrip.py Lines 308 to 312 in dfdd0ce Exercise native-read configuration through the real loader
When this option is loaded from a real profile, this test cannot catch an incorrect AGENTS.md reference: AGENTS.md:L65-L67 hermes-agent/tests/hermes_cli/test_stale_pid_guard.py Lines 190 to 193 in dfdd0ce Run the dashboard kill-path tests on Windows
On non-Windows CI this makes only the imported module believe it is on Windows while the interpreter and host APIs remain POSIX, so the security-sensitive stale-process cleanup can appear covered without exercising its real Windows behavior. The same file also patches AGENTS.md reference: AGENTS.md:L349-L358 hermes-agent/agent/fast_mode.py Lines 31 to 35 in dfdd0ce Reject non-finite fast-mode windows
When hermes-agent/tests/agent/test_fast_mode_auto.py Lines 110 to 111 in dfdd0ce Assert fast-mode default propagation instead of literals
These assertions freeze two configuration defaults rather than checking a behavior contract, so an intentional change to the default mode or window will fail this test even if every loader and runtime consumer remains correct. Exercise the relationship instead—for example, verify that agent initialization receives the values from AGENTS.md reference: AGENTS.md:L377-L386 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
@codex review |
💡 Codex ReviewLines 71 to 72 in 4da9713 Preserve upstream ancestry instead of squashing the sync
Fresh evidence after the target-revision correction is that AGENTS.md reference: AGENTS.md:L71-L72 hermes-agent/tests/tools/test_file_ops_single_roundtrip.py Lines 308 to 312 in 4da9713 Exercise native-read opt-out through the real config loader
When a profile's AGENTS.md reference: AGENTS.md:L65-L67 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Addressed the behavioral findings in e6c068c:\n\n- fast-mode now rejects non-finite configured windows and falls back to the bounded default; regression coverage covers NaN and both infinities.\n- the native-read config test now writes terminal.native_file_read to a temporary HERMES_HOME/config.yaml and uses the real loader.\n- default assertions now verify configuration/runtime relationship instead of freezing literal defaults.\n\nThe upstream-history finding does not match the exact checkout: 03f3b09 has parent 610c869, f93b350..03f3b09 contains 7,654 commits, and 03f3b09 is an ancestor of this PR head. The synchronized upstream history is therefore present; rewriting it would discard valid contributor ancestry. |
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
💡 Codex Reviewhermes-agent/.github/workflows/tests.yml Lines 21 to 23 in 3480079 Scale pytest workers down with the runner
When this workflow runs on the standard AGENTS.md reference: AGENTS.md:L313-L317 hermes-agent/tools/file_operations.py Lines 716 to 718 in 3480079 Keep native file reads bounded and interruptible
With the new default native path enabled on local POSIX sessions, reading a large text file enters this loop and scans through EOF solely to count lines, without consulting the tool interrupt flag or any deadline. A multi-gigabyte or continuously appended log can therefore hold the tool thread beyond the configured terminal timeout and ignore hermes-agent/.github/workflows/tests.yml Lines 21 to 23 in 3480079 Match test workers to the standard runner
When this workflow runs on the standard AGENTS.md reference: AGENTS.md:L313-L317 hermes-agent/tools/file_operations.py Lines 716 to 718 in 3480079 Keep native file reads bounded and interruptible
With the new default native path enabled on local POSIX sessions, reading a large text file enters this loop and scans through EOF solely to count lines, without consulting the tool interrupt flag or any deadline. A multi-gigabyte or continuously appended log can therefore hold the tool thread past the configured terminal timeout and ignore ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
💡 Codex Review Exercise resume routing instead of scanning main.ts
Fresh evidence after the earlier wiring-test repair is that this remaining resume test still reads AGENTS.md reference: AGENTS.md:L388-L395 Remove the deleted Perplexity provider from capability docs
Fresh evidence after the Perplexity cleanup is that this changed line still states that Perplexity is an implemented search/extract provider, even though the same change deletes AGENTS.md reference: AGENTS.md:L96-L101 Restore the shared environment timeout after this test
Because AGENTS.md reference: AGENTS.md:L326-L329 Give the asynchronous UI waits at least two seconds
After moving the JS checks onto shared standard runners, this synchronization still fails if the resumed render takes more than one second, recreating a load-dependent CI flake instead of bounding a genuinely hung test. The newly changed AGENTS.md reference: AGENTS.md:L326-L329 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
19 participants
Synchronize the fork with the current upstream main revision 03f3b09.
This PR retains the upstream tree and adds the review-repair commit series on top: pinned build tooling, canonical todo_list handling, config-gated native file reads, Windows marker compliance, and removal of the bundled Perplexity vendor connector.
Local validation: scripts/run_tests.sh on the affected ACP, agent, file-operation, run-agent, and stale-PID suites: 25 files, 480 selected tests, 0 failures.