Skip to content

fix hook context output - Fixes #533 #534

Open
danielmorozoff wants to merge 6 commits intosafishamsi:v5from
danielmorozoff:v5
Open

fix hook context output - Fixes #533 #534
danielmorozoff wants to merge 6 commits intosafishamsi:v5from
danielmorozoff:v5

Conversation

@danielmorozoff
Copy link
Copy Markdown

Summary

  • Fix PreToolUse hook output to emit top-level systemMessage instead of unsupported hookSpecificOutput.additionalContext.
  • Add a regression test covering unsupported hook fields.
  • Ignore local assistant artifacts in .gitignore.

Why

Newer CLI versions reject hook output containing additionalContext with:

PreToolUse hook returned unsupported additionalContext

The generated hook had regressed to a Claude-specific output shape.

Validation

  • .venv/bin/pip install -e '.[all]'
  • .venv/bin/pip check
  • env HOME=/tmp/graphify-pytest-home .venv/bin/python -m pytest -q
  • .venv/bin/graphify update .
  • Smoke-tested the platform install and verified the generated hook output contains systemMessage and no additionalContext.

Fixes #533
Resolves #533

Dan Morozoff added 2 commits April 24, 2026 07:26
@qodo-ai-reviewer
Copy link
Copy Markdown

Hi, test_codex_agents_install_writes_supported_hook_output only asserts substrings on str(hooks) and never validates the JSON that the hook command actually echoes at runtime, so malformed/invalid echo output can ship while tests still pass. This undermines the regression coverage for the Codex CLI schema error this PR is trying to prevent.

Severity: remediation recommended | Category: correctness

How to fix: Parse and validate echoed JSON

Agent prompt to fix - you can give this to your LLM of choice:

Issue description

The regression test verifies only stringified Python structures from hooks.json, not the actual JSON emitted by the hook command at runtime.

Issue Context

The Codex hook is a shell command that echos a JSON object; Codex rejects unsupported fields at runtime. The test should validate the command’s emitted JSON (at least syntactically, ideally also schema-wise for allowed keys).

Fix Focus Areas

  • tests/test_install.py[132-145]
  • graphify/main.py[704-717]

Suggested approach

  • In the test, locate the hook command string (e.g., hooks[0]["hooks"][0]["command"]).
  • Extract the echoed JSON literal and json.loads(...) it, then assert the resulting dict contains only supported keys (e.g., {"systemMessage"}) and explicitly does not contain additionalContext, hookSpecificOutput, permissionDecision.
  • Optionally, execute the command via subprocess.run(..., shell=True, cwd=tmp_path, check=False) with a created graphify-out/graph.json to ensure the && branch runs, and then parse stdout as JSON.

Found by Qodo code review

rosschurchill added a commit to rosschurchill/graphify-super that referenced this pull request Apr 27, 2026
…ession guard)

additionalContext was already present in _CODEX_HOOK; add a test so it
cannot be silently removed in future refactors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

codex not support additionalContext on PreToolUse

2 participants