Use verified Python launchers for agent hooks - #1496
Conversation
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Deliberate self-review — exact head
|
Review fix evidence — exact head
|
|
@codex review Please review exact head |
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh adversarial review — exact head 8c2ef1e7e1f3f1428c50f753836a7453f9fcba69
Verdict: changes requested.
I inspected all existing PR discussion before submitting: 3 issue comments (self-review, fix evidence, and the review request), 0 prior submitted reviews, and 0 inline review threads. I independently reviewed the live exact-head diff and the unchanged hook policy source.
CRITICAL
None.
HIGH
- The PreToolUse launcher fails open when
pyor the Python policy process is unavailable. A shell-form command-not-found returns 1; Claude Code documents that PreToolUse exit codes other than 2 are non-blocking, so the attempted tool call proceeds without the deny floor. The manual smoke/preflight is not an active runtime guard. Map launcher/policy failures to blocking exit 2 and test the unavailable/broken-launcher path. See the inline finding and the Claude hook exit-code contract. - The native
PowerShelltool is outside both project deny layers. The hook matcher isBashonly, the Python policy ignores tool names other thanBash/Shell, and the static denies are allBash(...). Running the handler under PowerShell with aBashpayload does not test native PowerShell interception. Extend and canary the PowerShell path, or explicitly de-scope and track the security gap before this T4 configuration merges. Claude documents PowerShell as a distinct Windows tool and permission surface: tools reference, permissions.
MEDIUM
- The asserted allow-rule matrix does not authorize the copied Windows workflow. It expects
Bash(py ...)rules even though the Windows commands are documented for PowerShell, whose permissions are separate. It also omits the documentedpy -3 -B -m unittest ...renderer-test command. Align the rules and matrix test with the actual tool/platform invocations.
LOW
None.
Evidence
- Live PR head remained the reviewed SHA immediately before review creation.
- Exact-head
.claude/settings.jsonparsed successfully. - Exact-head
smoke_test.pycompiled in memory under CPython 3.14 and 3.13. - The policy-script blobs are identical at base, reviewed head, and local main; direct
py -3 -Bandpy -3.13 -Bprobes both emitted the expected structured deny forgit restore --worktree. - A missing-command PowerShell probe exited 1, which is non-blocking under the documented PreToolUse contract.
git diff --check base...headpassed.- Current local Claude Code: 2.1.219.
Residual risk / NOT verified
- No live Claude
/hooksinventory or real PowerShell-tool deny canary was run. - Hosted exact-head checks were still in progress during review.
- No product/backend/frontend suites were run; the PR does not touch product runtime/UI files.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c2ef1e7e1
ℹ️ 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".
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Review repair evidence — exact head
|
|
@codex review Please perform a fresh adversarial review of exact head
This remains a draft, human-held T4 change. Please do not infer native PowerShell interception from a PowerShell-hosted handler receiving a |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5f9773cc7
ℹ️ 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".
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Review repair evidence — exact head
|
|
@codex review Please perform a fresh adversarial review of exact head Focus on the repaired failure-ledger ordering contract: local Windows/POSIX update workflows must render before synchronization testing, while Required Docs Governance must continue testing the checked-in projection without running a renderer. Please also examine whether the new static smoke assertions cover every copied local mirror without weakening the existing launcher, fail-closed, or #1497 boundary claims. All prior exact-head review and CI evidence is invalid after this push. The PR remains draft and human-held. |
|
Codex Review: Didn't find any major issues. Bravo. 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". |
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh adversarial review — exact head f2551ebfd987bccfe7eab8f4a1e459629ad7b942
Verdict: one LOW finding; no CRITICAL, HIGH, or MEDIUM findings.
I re-read all current PR discussion before submission: 8 issue comments, 10 submitted reviews, and all 7 inline threads (all resolved). The new Codex no-major-issues comment is informational. I independently reviewed the complete 12-file 6fd39367...f2551ebf diff, the linked #1487 scope, open #1497 de-scope, and the unchanged Required Docs Governance workflow.
CRITICAL
None.
HIGH
None.
MEDIUM
None.
LOW
- The missing-launcher catch path does not emit the stable diagnostic claimed by the PR. It blocks correctly with exit 2, but writes only PowerShell's environment-/localization-dependent exception text; the fixed Taskdeck blocking message and assertion cover only nonzero policy-process failures. See the inline finding.
Prior-thread disposition
The six findings repaired at f5f9773c and the render-order finding repaired at f2551ebf are individually resolved in their threads. The full diff now preserves exit-2 blocking, the exact platform/tool permission inventories, command-not-found-safe copied workflows, Bash-payload-only claims, and the explicit #1497 native-PowerShell boundary. The new static contract covers all seven copied local render-before-sync sequences and separately requires Required Docs Governance to retain the synchronization test without invoking the renderer.
Exact-head evidence
- Guarded worktree was clean and at
f2551ebfd987bccfe7eab8f4a1e459629ad7b942. .mcp.jsonand.claude/settings.jsonparsed successfully.- Configured-handler smoke passed on native Windows CPython 3.14 and 3.13.
- Renderer synchronization suite passed 11/11 on both CPython 3.14 and 3.13.
- All six mirrored skill validations passed.
- Docs, golden-principles, and GitHub-operations governance checks passed.
git diff --check 6fd39367...HEADpassed; worktree remained clean.- Focused wrapper probes: arbitrary policy exit 17 mapped to exit 2 with the fixed diagnostic; missing launcher mapped to exit 2 but emitted only the PowerShell exception, reproducing the LOW finding.
- Immediately before submission, the live head still matched the reviewed SHA. At the last check-run read, 23 checks were successful, 11 skipped, and the Windows API Integration job remained in progress.
NOT verified / residual risk
- WSL/POSIX direct utilities were not rerun on this head.
- No fresh interactive Claude
/hooksinventory or live allow/deny canary was run. - Native PowerShell-tool interception is intentionally not claimed and remains open as Priority-I T4 issue #1497.
- Backend/full frontend/Playwright suites were not run locally because no product runtime/UI files changed.
- The PR remains draft and human-held; hosted exact-head gates and the LOW finding must be settled before any merge decision.
Signed-off-by: Chris0Jeky <jeky.tck@gmail.com>
Review repair evidence — exact head
|
|
@codex review Please perform a fresh adversarial review of exact head Focus on whether both PreToolUse failure classes now remain fail closed with deterministic Taskdeck-owned diagnostics: launcher/handler invocation failure must return exit 2 plus All prior exact-head review and CI evidence is invalid after this push. Do not broaden into the separately tracked #1497 PowerShell-tool policy gap; this PR remains draft and human-held. |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
Chris0Jeky
left a comment
There was a problem hiding this comment.
Fresh adversarial review — exact head 3d66cc0b71aeb78d1ad1a60bde97ffcf5ae0e8ba
Verdict: clean. No CRITICAL, HIGH, MEDIUM, or LOW findings.
I re-read all current PR discussion before submission: 11 issue comments, 12 submitted reviews, and all 8 inline threads. Every prior thread is resolved, including the stable-diagnostic LOW repaired by 3d66cc0b; the exact-head Codex no-major-issues comment is informational. I independently reviewed the complete 12-file 6fd39367...3d66cc0b diff, linked #1487, open #1497, and the unchanged Required Docs Governance workflow.
CRITICAL
None.
HIGH
None.
MEDIUM
None.
LOW
None.
Contract review
- Missing launcher/handler invocation returns exit 2 with exactly
Taskdeck PreToolUse handler launch failed; blocking command.. - A launched policy process returning arbitrary nonzero (probed with 17) returns exit 2 with the distinct fixed policy-process diagnostic.
- Safe Bash payloads and structured denials still exit 0; the deny result remains structured JSON with
permissionDecision: deny. - The exact configured Python-handler multiset and the five-utilities-plus-renderer-test permission inventories remain pinned per platform, with forward-slash and duplicate mutation controls.
- Copied Windows command lookup fails loudly rather than inheriting a stale
$LASTEXITCODE. - Seven local Windows/POSIX/mirror contracts remain render-before-synchronization, while Required Docs Governance retains its synchronization test and contains no renderer.
- Claims remain explicitly Bash-payload-only; native PowerShell-tool interception is not inferred and remains the open Priority-I T4 boundary in #1497.
- The failure-ledger change remains append-only and its checked-in projection is synchronized.
Exact-head evidence
- Guarded worktree was clean and at
3d66cc0b71aeb78d1ad1a60bde97ffcf5ae0e8ba. - Both JSON configuration files parsed and Windows
py/nodepreflights passed. - All six hook sources compiled in memory on CPython 3.14 and 3.13.
- Configured-handler smoke passed on CPython 3.14 and 3.13.
- Renderer synchronization suite passed 11/11 on both interpreters.
- All six mirrored skill validations passed.
- Docs, golden-principles, and GitHub-operations governance checks passed.
git diff --check 6fd39367...HEADpassed; the worktree remained clean.- Immediately before submission, the live PR head still matched the reviewed SHA.
- At the final pre-review check read, 22 jobs were successful, 11 skipped, and the Windows and Ubuntu API Integration jobs were still in progress.
NOT verified / residual risk
- I did not independently rerun the WSL/POSIX direct-utility gate in this Windows-focused review.
- No fresh interactive Claude
/hooksinventory or live allow/deny canary was run. - Native PowerShell-tool interception remains explicitly outside this PR and tracked by #1497.
- Backend/full frontend/Playwright suites were not run locally because no product runtime/UI files changed.
- The PR remains draft and human-held; exact-head hosted gates must finish green before any maintainer merge decision.
Chris0Jeky
left a comment
There was a problem hiding this comment.
Independent POSIX/docs-governance adversarial review — exact head 3d66cc0b71aeb78d1ad1a60bde97ffcf5ae0e8ba
Verdict: clean. No actionable CRITICAL, HIGH, MEDIUM, or LOW findings.
I started cold from all 11 issue comments, all 13 submitted reviews, and all 8 inline threads (all resolved), then reviewed the complete 6fd39367...3d66cc0b71aeb78d1ad1a60bde97ffcf5ae0e8ba diff across all 12 changed files.
Findings
- CRITICAL: None.
- HIGH: None.
- MEDIUM: None.
- LOW: None.
POSIX, governance, and claim evidence
- Direct WSL/POSIX checks on this exact clean worktree used Python 3.11.2: both JSON files parsed, all six hook sources parsed in memory, and the renderer synchronization suite passed 11/11.
- The stable diagnostics did not regress portability: the Taskdeck-owned launcher and policy-failure lines are ASCII/stable, the POSIX direct commands remain
python3 -B, and the PowerShell-hosted handler is accurately described as receivingBashpayloads rather than proving native PowerShell-tool interception. - All seven copied local update contracts contain render-before-synchronization order. Required Docs Governance still runs the checked-in projection synchronization test and has no renderer invocation.
- The failure ledger remains append-only in JSONL, and the Markdown projection reflects the new #1487/#1497 rows without rewriting earlier history.
- #1497 is correctly scoped as the separate Priority I/T4 native PowerShell-tool deny-policy gap; #1495 retains the PowerShell-to-WSL handoff concern.
git diff --checkpassed and the review worktree remained clean.
Not independently re-run
- I did not invoke the renderer entrypoint because this review was read-only; the 11/11 synchronization suite independently compared the checked-in projection.
- I did not re-run the native-Windows configured-handler smoke, backend/frontend/Playwright suites, or mutate canaries. The exact-head hosted checks were still completing when this review began.
Residual risk
This remains a human-held T4 hook/configuration change. Native PowerShell-tool interception is intentionally not established here and remains blocked on #1497. Any head change invalidates this review.
Exact-head verification checkpoint —
|
Summary
pythonhook launchers with the verifiedpy -3 -BformPowerShell(py -3 -B ...)on Windows andBash(python3 -B ...)on POSIX, including a narrowly scoped renderer unittest rule on eachsys.executable -B, inventory configured Python handlers independent of slash direction, reject duplicate/stale permissions, and exercise missing-launcher, missing-policy, forward-slash-regression, and command-lookup controlsThis is a T4-class hook-configuration change. On 2026-07-27, the maintainer explicitly authorized agents to merge all PR types once their exact-head gates are satisfied.
Verification
.mcp.jsonand.claude/settings.jsonparse successfully;pyandnodepreflights passpy -3): 6 hook files compile in memory, configured-handler smoke passes (including rc2, stable-diagnostic, and mutation controls), renderer suite 11/11node scripts/check-docs-governance.mjsnode scripts/check-golden-principles.mjsnode scripts/check-github-ops-governance.mjsgit diff --check origin/main...HEADDocumentation
docs/STATUS.mdupdateddocs/IMPLEMENTATION_MASTERPLAN.mdupdateddocs/TESTING_GUIDE.mdupdated with platform-specific, fail-fast commandsTracking
Review / Priority IIPending / Priority II)CI Workflow Validation
3d66cc0b(run30201593348; 16/16 jobs)3d66cc0b(run30201593328; required becausescripts/changed)3d66cc0b(dynamic run30201591953; 4/4 analyses)Risk Notes
PreToolUsematcher and policy still evaluateBashpayloads only. The native-Windows smoke proves Bash payloads passing through PowerShell-hosted handlers; it does not prove native PowerShell-tool interception. Cover native PowerShell calls in Taskdeck's Claude deny floor #1497 owns that T4 policy change.python3 -B; no interpreter install, global PATH mutation, resolver wrapper, deny regex, product runtime, or product authorization behavior is changed.