agentHost: recheck Codex hook trust before the first turn - #336358
Draft
Dmitriy Vasyura (dmitrivMS) wants to merge 2 commits into
Draft
Dmitriy Vasyura (dmitrivMS) wants to merge 2 commits into
Dmitriy Vasyura (dmitrivMS) wants to merge 2 commits into
Conversation
Refresh the thread-scoped hook grant after native project trust becomes available. Reuse the existing pre-turn replacement path only when trust changes, and preserve existing grants when discovery fails. Cover late discovery, unchanged trust, lookup failures, and revocation. Fixes microsoft/vscode-engineering#3840 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot started reviewing on behalf of
Dmitriy Vasyura (dmitrivMS)
September 16, 2026 00:57
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Replacement materialization can discard a successful recheck when its redundant hook lookup fails.
Get a fresh assessment by requesting another Copilot review.
Review tier: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds a first-turn Codex hook-trust recheck to address cold-start hook discovery.
Changes:
- Tracks applied hook trust and restarts when discovered hashes change.
- Adds four regression cases covering discovery, failure, and revocation.
- Documents fresh-workspace integration-test requirements.
| File | Description |
|---|---|
codexAgent.ts |
Implements hook-trust tracking and rechecking. |
codexCreateChat.test.ts |
Adds first-turn trust regression tests. |
providerIntegration/README.md |
Documents Codex hook test isolation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Carry successful hook discovery into the replacement instead of repeating a lookup that can fail. Revalidate workspace and hook-source trust immediately before startup, and rediscover if the working directory changed. Cover the redundant lookup failure and trust/cwd changes in regression tests. Addresses review feedback on #336358. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.

Fixes microsoft/vscode-engineering#3840
Summary
Root cause and evidence
In a fresh workspace,
hooks/listcan return no project hooks beforethread/startestablishes native project trust. The thread then starts withouthooks.state, and a model turn can finish successfully without running the synchronousSessionStartmarker hook. Incidental pre-turn restarts can mask this ordering problem.The original failure in build 473136 took 30.450 seconds. Checkout logs confirm the failed attempt and existing passing retry both used
d550ca3ed31712f730403f337ce7bcae62aadf1b; the retry is not evidence of a resolving change.Reproduced with the locked Codex 0.153.0 SDK on main
aa513800f750ab6a2edf90d45fa28bfe7e411749. Native controls returned zero hooks before startup and one afterward; restarting with the discovered hash produced the marker in 3/3 controls. The original integration assertions remain unchanged and pass with this fix.Original provider-internal CI logs were not published, so the historical restart trigger is inferred; the failure mechanism and fix were reproduced locally. This does not claim a common cause for the other timeouts aggregated in the tracker.
Overlapping work
Searches of open VS Code hook, Codex, Workspace Trust, and exact-marker reports found no additional open issue that this patch can confidently close.
Validation
Windows, Electron 42.10.0, locked Codex 0.153.0:
scripts\test.bat --run src\vs\platform\agentHost\test\node\codex\codexCreateChat.test.ts: 98 passing.scripts\test-integration.bat --run src\vs\platform\agentHost\test\node\providerIntegration\codexCustomizations.integrationTest.ts: 6 passing (five provider cases plus the harness check).git diff --check: passed.No full build, CI rerun, or Linux/macOS validation was performed.