feat(codex): add native hooks and project install - #47
Conversation
📝 WalkthroughWalkthroughVersion 0.3.0 adds native Codex lifecycle hook integration: new ChangesCodex Native Lifecycle Hook Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces native Codex integration with Hebb Mind, adding support for native lifecycle hooks (SessionStart, UserPromptSubmit, and Stop) and project-scoped configuration, alongside a Codex transcript parser and updated documentation. Feedback from the review highlights a few critical issues: a parsing bug in remove_project_mcp_table that could corrupt configuration files if trailing comments are present, a logical error in _clean_user_text that incorrectly filters out short useful keywords while retaining greetings, and unnecessary codex CLI presence checks during project-scoped installation and uninstallation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def remove_project_mcp_table(text: str) -> str: | ||
| """Remove Hebb's MCP TOML table while preserving unrelated config. | ||
|
|
||
| Args: | ||
| text: Existing Codex TOML source. | ||
|
|
||
| Returns: | ||
| TOML source without ``mcp_servers.hebb`` tables. | ||
| """ | ||
| lines = text.splitlines(keepends=True) | ||
| output: list[str] = [] | ||
| skipping = False | ||
| for line in lines: | ||
| stripped = line.strip() | ||
| if stripped.startswith("[") and stripped.endswith("]"): | ||
| table = stripped.strip("[]").strip() | ||
| skipping = table == "mcp_servers.hebb" or table.startswith("mcp_servers.hebb.") | ||
| if not skipping: | ||
| output.append(line) | ||
| return "".join(output) |
There was a problem hiding this comment.
If a user has a comment on the same line as a table header (e.g., [mcp_servers.other] # comment), stripped.endswith("]") will evaluate to False. This prevents the parser from recognizing the new table header, causing it to keep skipping lines and inadvertently delete the entire rest of the configuration file.
We can fix this by stripping trailing comments before checking for table headers, while still preserving the original line in the output.
def remove_project_mcp_table(text: str) -> str:
"""Remove Hebb's MCP TOML table while preserving unrelated config.
Args:
text: Existing Codex TOML source.
Returns:
TOML source without ``mcp_servers.hebb`` tables.
"""
lines = text.splitlines(keepends=True)
output: list[str] = []
skipping = False
for line in lines:
# Strip trailing comments to correctly identify table headers
content = line.split("#", 1)[0].strip()
if content.startswith("[") and content.endswith("]"):
table = content.strip("[]").strip()
skipping = table == "mcp_servers.hebb" or table.startswith("mcp_servers.hebb.")
if not skipping:
output.append(line)
return "".join(output)| def _clean_user_text(raw: str) -> str: | ||
| """Apply Hebb's storage filter and length bound to Codex user text.""" | ||
| cleaned = clean_user_input(raw) | ||
| if not cleaned: | ||
| return "" | ||
| if not is_greeting_only(cleaned) and len(cleaned) < _MIN_USER_LEN: | ||
| return "" | ||
| return _truncate(cleaned, _MAX_USER_LEN) |
There was a problem hiding this comment.
The current condition not is_greeting_only(cleaned) and len(cleaned) < _MIN_USER_LEN has a logical bug:
- If the input is a greeting (e.g., "Hi"),
is_greeting_onlyisTrue, so the condition isFalseand the greeting is kept and stored. - If the input is a short but useful keyword (e.g., "pnpm"),
is_greeting_onlyisFalse, so the condition isTrueand it is discarded.
We should discard the input if it is a greeting OR if it is shorter than the minimum length.
| def _clean_user_text(raw: str) -> str: | |
| """Apply Hebb's storage filter and length bound to Codex user text.""" | |
| cleaned = clean_user_input(raw) | |
| if not cleaned: | |
| return "" | |
| if not is_greeting_only(cleaned) and len(cleaned) < _MIN_USER_LEN: | |
| return "" | |
| return _truncate(cleaned, _MAX_USER_LEN) | |
| def _clean_user_text(raw: str) -> str: | |
| """Apply Hebb's storage filter and length bound to Codex user text.""" | |
| cleaned = clean_user_input(raw) | |
| if not cleaned: | |
| return "" | |
| if is_greeting_only(cleaned) or len(cleaned) < _MIN_USER_LEN: | |
| return "" | |
| return _truncate(cleaned, _MAX_USER_LEN) |
| def install(scope: str) -> None: | ||
| """Install Hebb Mind MCP into Codex. | ||
| """Install Hebb Mind MCP and lifecycle hooks into Codex. | ||
|
|
||
| Codex registers MCP servers globally via ``codex mcp add`` — there is no | ||
| per-project scope, so this command is global-only. | ||
| Project scope writes ``.codex/config.toml`` and ``.codex/hooks.json``. | ||
| User scope registers MCP through ``codex mcp add`` and writes the user | ||
| hooks file. | ||
| """ | ||
| _ensure_codex() | ||
|
|
||
| # Resolve absolute path to hebb-mcp — Codex launches the MCP server as a | ||
| # subprocess whose PATH may not include `pip install --user` bin dirs. | ||
| mcp_argv = hebb_mcp_command() | ||
| # Replace any prior entry so a fresh install picks up a moved binary. | ||
| subprocess.run(["codex", "mcp", "remove", "hebb"], capture_output=True, check=False) | ||
| result = subprocess.run(["codex", "mcp", "add", "hebb", "--", *mcp_argv], check=False) | ||
| if result.returncode != 0: | ||
| raise click.ClickException("codex mcp add failed") | ||
| from hebb.integrations.codex.install import handle | ||
|
|
||
| click.secho("Installed hebb MCP server for Codex.", fg="green") | ||
| click.echo(f" MCP: {shell_quote(mcp_argv)}") | ||
| click.echo("Verify with: codex mcp list") | ||
| handle(scope) |
There was a problem hiding this comment.
When installing with scope="project" (the default), we only write local configuration files and do not run any codex CLI commands. Therefore, we shouldn't block the installation if the codex CLI is not present on the user's PATH. We should only require and verify the codex CLI when installing with scope="user".
| def install(scope: str) -> None: | |
| """Install Hebb Mind MCP into Codex. | |
| """Install Hebb Mind MCP and lifecycle hooks into Codex. | |
| Codex registers MCP servers globally via ``codex mcp add`` — there is no | |
| per-project scope, so this command is global-only. | |
| Project scope writes ``.codex/config.toml`` and ``.codex/hooks.json``. | |
| User scope registers MCP through ``codex mcp add`` and writes the user | |
| hooks file. | |
| """ | |
| _ensure_codex() | |
| # Resolve absolute path to hebb-mcp — Codex launches the MCP server as a | |
| # subprocess whose PATH may not include `pip install --user` bin dirs. | |
| mcp_argv = hebb_mcp_command() | |
| # Replace any prior entry so a fresh install picks up a moved binary. | |
| subprocess.run(["codex", "mcp", "remove", "hebb"], capture_output=True, check=False) | |
| result = subprocess.run(["codex", "mcp", "add", "hebb", "--", *mcp_argv], check=False) | |
| if result.returncode != 0: | |
| raise click.ClickException("codex mcp add failed") | |
| from hebb.integrations.codex.install import handle | |
| click.secho("Installed hebb MCP server for Codex.", fg="green") | |
| click.echo(f" MCP: {shell_quote(mcp_argv)}") | |
| click.echo("Verify with: codex mcp list") | |
| handle(scope) | |
| def install(scope: str) -> None: | |
| """Install Hebb Mind MCP and lifecycle hooks into Codex. | |
| Project scope writes ``.codex/config.toml`` and ``.codex/hooks.json``. | |
| User scope registers MCP through ``codex mcp add`` and writes the user | |
| hooks file. | |
| """ | |
| if scope == "user": | |
| _ensure_codex() | |
| from hebb.integrations.codex.install import handle | |
| handle(scope) |
| def uninstall(scope: str) -> None: | ||
| """Remove Hebb Mind MCP from Codex (global-only).""" | ||
| """Remove Hebb Mind MCP and lifecycle hooks from Codex.""" | ||
| _ensure_codex() | ||
|
|
||
| result = subprocess.run(["codex", "mcp", "remove", "hebb"], check=False) | ||
| if result.returncode != 0: | ||
| raise click.ClickException("codex mcp remove failed") | ||
| from hebb.integrations.codex.uninstall import handle | ||
|
|
||
| handle(scope) |
There was a problem hiding this comment.
Similarly to the install command, we should only require and verify the codex CLI when uninstalling with scope="user".
| def uninstall(scope: str) -> None: | |
| """Remove Hebb Mind MCP from Codex (global-only).""" | |
| """Remove Hebb Mind MCP and lifecycle hooks from Codex.""" | |
| _ensure_codex() | |
| result = subprocess.run(["codex", "mcp", "remove", "hebb"], check=False) | |
| if result.returncode != 0: | |
| raise click.ClickException("codex mcp remove failed") | |
| from hebb.integrations.codex.uninstall import handle | |
| handle(scope) | |
| def uninstall(scope: str) -> None: | |
| """Remove Hebb Mind MCP and lifecycle hooks from Codex.""" | |
| if scope == "user": | |
| _ensure_codex() | |
| from hebb.integrations.codex.uninstall import handle | |
| handle(scope) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@repo_pages/zh/index.md`:
- Line 40: The section title in the page metadata is still scoped to Claude
Code, but the updated details now cover both Claude Code and Codex workflows.
Update the title used alongside this card so it matches the broader scope, or
split the content into separate entries if needed; locate the label/title in the
same index page section as the details text referencing Claude Code, Codex, and
MCP memory tools.
In `@src/hebb/integrations/codex/cli.py`:
- Around line 11-12: The public CLI handlers in this module, including codex and
the other exported command functions referenced in the diff, have incomplete
docstrings and need the required structured sections. Update each public API
docstring to include Args, Returns, and Raises in the same style used elsewhere
in the repository, making sure the function names and their behavior are clearly
documented. Keep the content aligned with the existing CLI entrypoints and
lifecycle hooks, and apply the same docstring format consistently across all
affected public functions.
- Around line 33-37: The unconditional _ensure_codex() call in the Codex CLI
entrypoints is forcing a binary dependency even when scope is project, which
only operates on local .codex files. Update the codex command flow in cli.py so
_ensure_codex() is only invoked for user-scoped install/uninstall paths, and let
the project-scoped path call handle(scope) without requiring Codex; apply the
same conditional behavior to the matching uninstall entrypoint as well.
In `@src/hebb/integrations/codex/install.py`:
- Around line 36-44: The public API docstrings are incomplete in install.py and
must include the required Args, Returns, and Raises sections. Update the
docstrings for the affected public functions, including hooks_path(),
hooks_config(), handle(), and the other listed APIs, so each one documents its
parameters, return value, and any exceptions it may raise in the same style as
the existing docstring. Keep the documentation consistent across these symbols
and ensure no public function is missing any required section.
- Around line 173-176: The TOML table detection in the install path is too
strict, so headers like [mcp_servers.hebb] with trailing comments are not
recognized and get duplicated. Update the header parsing in the install logic
around the stripping/skipping checks so it identifies table names before any
inline comment text, then use that normalized table name to decide whether to
skip mcp_servers.hebb and its nested tables.
- Around line 101-110: The command matching logic in the hook installer is
relying on raw substring checks, which misses quoted absolute paths and can
cause duplicate reinstall/uninstall behavior. Update the hook detection in the
relevant helper in install.py to parse the command into argv first, then match
on the executable and subcommand values instead of looking for strings like
“hebb codex ” or “/hebb claude-code ” in the raw command text.
In `@src/hebb/integrations/codex/recall.py`:
- Around line 6-17: The public handlers handle_session_start and handle_prompt
in the Codex recall module only have summary docstrings, so update each
docstring to include the required Args, Returns, and Raises sections. Keep the
behavior unchanged, but document the no-argument signature, the None return
value, and any exceptions that may propagate from the delegated recall call
imported from hebb.integrations.claude_code.recall.
In `@src/hebb/integrations/codex/stop.py`:
- Around line 24-25: The public function handle in stop.py has a docstring but
is missing the required Args, Returns, and Raises sections. Update handle’s
docstring to follow the project Python API docstring convention by adding those
sections, even if they are empty or note that no arguments or exceptions are
expected, so the documentation is complete and consistent.
In `@src/hebb/integrations/codex/transcript.py`:
- Around line 49-57: The transcript selection logic in the user-turn parsing
path currently picks the last raw user record and then returns None if
_clean_user_text filters it out, which can hide an earlier valid user message.
Update the selection in the transcript parser so it chooses the last user record
that still has non-empty cleaned text, using the existing helpers like
_raw_user_text and _clean_user_text in the relevant transcript handling flow.
Apply the same fix in both affected parsing sections so the “last valid user
input” behavior is consistent.
- Around line 166-168: The _truncate helper currently exceeds the configured
limit because it appends the ellipsis after slicing to limit; update _truncate
so the returned string always stays within limit, likely by reserving space for
the ellipsis when truncation is needed. Keep the fix localized to _truncate in
transcript.py and preserve the existing behavior for already-short values.
- Around line 21-26: `CodexTurn` is a public dataclass whose docstring is
missing the required API sections; update the class docstring in `CodexTurn` to
include Args, Returns, and Raises sections in the prescribed style. Use the
existing `summary` and `timestamp` fields to describe the arguments, note the
dataclass construction result in Returns, and include any exceptions that may be
raised during initialization or parsing if applicable.
In `@src/hebb/integrations/codex/uninstall.py`:
- Around line 58-66: Complete the public API docstrings for
uninstall_project_mcp and handle by adding the missing sections required by the
coding guidelines. Update uninstall_project_mcp to include a Raises section
describing any exceptions it can propagate, and update handle to include a
Returns section describing its return value. Keep the docstrings consistent with
the existing Args/Returns style used in
src/hebb/integrations/codex/uninstall.py.
In `@tests/unit/integrations/test_codex_hooks.py`:
- Around line 184-190: The test fixture in hook_input hardcodes an absolute /tmp
transcript_path, which should be replaced with a workspace-safe path. Update the
Codex hook tests that build hook_input to use tmp_path / "rollout.jsonl" (or
another relative placeholder) instead of "/tmp/rollout.jsonl", keeping the
change localized to the test data used around extract_last_turn and the related
hook_input setup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 146e5fec-b857-46e3-bfc6-8ac7625674d0
📒 Files selected for processing (33)
.claude-plugin/plugin.json.codex/hooks.json.release-please-manifest.jsonCHANGELOG.mdREADME.mdREADME_ZH.mdpyproject.tomlrepo_pages/api/cli.mdrepo_pages/guide/codex.mdrepo_pages/guide/installation.mdrepo_pages/guide/mcp-integration.mdrepo_pages/index.mdrepo_pages/public/llms.txtrepo_pages/quick-start.mdrepo_pages/zh/api/cli.mdrepo_pages/zh/guide/codex.mdrepo_pages/zh/guide/installation.mdrepo_pages/zh/guide/mcp-integration.mdrepo_pages/zh/index.mdrepo_pages/zh/quick-start.mdreports/design/codex-native-integration-design.mdsrc/hebb/__init__.pysrc/hebb/cli/commands/doctor.pysrc/hebb/cli/commands/setup.pysrc/hebb/integrations/codex/cli.pysrc/hebb/integrations/codex/install.pysrc/hebb/integrations/codex/recall.pysrc/hebb/integrations/codex/stop.pysrc/hebb/integrations/codex/transcript.pysrc/hebb/integrations/codex/uninstall.pysrc/hebb/upgrade/helper.pytests/unit/integrations/test_codex_cli.pytests/unit/integrations/test_codex_hooks.py
| - icon: 🔌 | ||
| title: REST + MCP + Claude Code Hooks | ||
| details: 三行命令为 Claude Code 启用跨会话记忆；hebb codex install 一键将能力以 MCP 工具形式接入 Codex。REST 文档位于 /docs。 | ||
| details: 三行命令为 Claude Code 或 Codex 启用自动跨会话召回、回合写入与 MCP 记忆工具。REST 文档位于 /docs。 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the feature label aligned with the updated copy.
The details now describe both Claude Code and Codex flows, but the section title still says “Claude Code Hooks,” so this card reads as Claude-only. Rename the title (or split the card) to match the broadened scope.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@repo_pages/zh/index.md` at line 40, The section title in the page metadata is
still scoped to Claude Code, but the updated details now cover both Claude Code
and Codex workflows. Update the title used alongside this card so it matches the
broader scope, or split the content into separate entries if needed; locate the
label/title in the same index page section as the details text referencing
Claude Code, Codex, and MCP memory tools.
| def codex() -> None: | ||
| """Codex integration — configure Hebb Mind as an MCP server.""" | ||
| """Codex integration — native MCP and lifecycle hooks.""" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add the required Args / Returns / Raises sections to these public CLI APIs.
These new public handlers have docstrings, but not the structured sections required by the repository rules. As per coding guidelines, "**/*.py: Include docstring with Args, Returns, and Raises sections for all public APIs" and "src/**/*.py: All public APIs in Python MUST have docstrings with Args, Returns, and Raises sections."
Also applies to: 26-32, 51-52, 61-62, 69-70, 77-78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/codex/cli.py` around lines 11 - 12, The public CLI
handlers in this module, including codex and the other exported command
functions referenced in the diff, have incomplete docstrings and need the
required structured sections. Update each public API docstring to include Args,
Returns, and Raises in the same style used elsewhere in the repository, making
sure the function names and their behavior are clearly documented. Keep the
content aligned with the existing CLI entrypoints and lifecycle hooks, and apply
the same docstring format consistently across all affected public functions.
Source: Coding guidelines
| _ensure_codex() | ||
|
|
||
| # Resolve absolute path to hebb-mcp — Codex launches the MCP server as a | ||
| # subprocess whose PATH may not include `pip install --user` bin dirs. | ||
| mcp_argv = hebb_mcp_command() | ||
| # Replace any prior entry so a fresh install picks up a moved binary. | ||
| subprocess.run(["codex", "mcp", "remove", "hebb"], capture_output=True, check=False) | ||
| result = subprocess.run(["codex", "mcp", "add", "hebb", "--", *mcp_argv], check=False) | ||
| if result.returncode != 0: | ||
| raise click.ClickException("codex mcp add failed") | ||
| from hebb.integrations.codex.install import handle | ||
|
|
||
| click.secho("Installed hebb MCP server for Codex.", fg="green") | ||
| click.echo(f" MCP: {shell_quote(mcp_argv)}") | ||
| click.echo("Verify with: codex mcp list") | ||
| handle(scope) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't require codex for project-scoped install/uninstall.
src/hebb/integrations/codex/install.py:181-222 and src/hebb/integrations/codex/uninstall.py:77-109 only invoke the Codex binary for scope == "user". With project now the default, these unconditional _ensure_codex() calls make the default path fail even though those branches only touch local .codex/ files.
Suggested fix
def install(scope: str) -> None:
@@
- _ensure_codex()
+ if scope == "user":
+ _ensure_codex()
@@
def uninstall(scope: str) -> None:
@@
- _ensure_codex()
+ if scope == "user":
+ _ensure_codex()Also applies to: 53-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/codex/cli.py` around lines 33 - 37, The unconditional
_ensure_codex() call in the Codex CLI entrypoints is forcing a binary dependency
even when scope is project, which only operates on local .codex files. Update
the codex command flow in cli.py so _ensure_codex() is only invoked for
user-scoped install/uninstall paths, and let the project-scoped path call
handle(scope) without requiring Codex; apply the same conditional behavior to
the matching uninstall entrypoint as well.
| def hooks_path(scope: str) -> Path: | ||
| """Resolve the Codex hooks path for an installation scope. | ||
|
|
||
| Args: | ||
| scope: Either ``project`` or ``user``. | ||
|
|
||
| Returns: | ||
| Path to the active ``hooks.json`` layer. | ||
| """ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the required public API docstring sections.
Several public functions are missing one or more required Args, Returns, or Raises sections; for example, hooks_config() has no Args/Returns/Raises, and handle() has no Returns.
As per coding guidelines, "**/*.py: Include docstring with Args, Returns, and Raises sections for all public APIs."
Also applies to: 48-49, 92-100, 114-121, 145-151, 181-189, 225-230, 255-261
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/codex/install.py` around lines 36 - 44, The public API
docstrings are incomplete in install.py and must include the required Args,
Returns, and Raises sections. Update the docstrings for the affected public
functions, including hooks_path(), hooks_config(), handle(), and the other
listed APIs, so each one documents its parameters, return value, and any
exceptions it may raise in the same style as the existing docstring. Keep the
documentation consistent across these symbols and ensure no public function is
missing any required section.
Source: Coding guidelines
| return any( | ||
| marker in command | ||
| for marker in ( | ||
| "hebb codex ", | ||
| "/hebb codex ", | ||
| "hebb.cli.main codex ", | ||
| "hebb claude-code ", | ||
| "/hebb claude-code ", | ||
| "hebb.cli.main claude-code ", | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and inspect the relevant section
ast-grep outline src/hebb/integrations/codex/install.py --view expanded || true
wc -l src/hebb/integrations/codex/install.py
sed -n '1,220p' src/hebb/integrations/codex/install.py
# Find the hook-related helpers and any quoting logic
rg -n "is_hebb_hook|hooks_config|shlex|quote|codex|claude-code" src/hebb/integrations/codex/install.py src -g '*.py'Repository: afx-team/hebb-mind
Length of output: 22060
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '220,270p' src/hebb/integrations/codex/install.py
sed -n '1,140p' src/hebb/utils/cli_paths.py
sed -n '1,120p' src/hebb/integrations/claude_code/install.pyRepository: afx-team/hebb-mind
Length of output: 7442
Match hook commands after shell parsing, not raw substrings. shell_quote() will quote an absolute Hebb path with spaces, so commands like '/path with spaces/hebb' codex recall bypass the current "hebb codex " checks. That lets reinstall duplicate hooks and uninstall miss them; parse the command argv before matching.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/codex/install.py` around lines 101 - 110, The command
matching logic in the hook installer is relying on raw substring checks, which
misses quoted absolute paths and can cause duplicate reinstall/uninstall
behavior. Update the hook detection in the relevant helper in install.py to
parse the command into argv first, then match on the executable and subcommand
values instead of looking for strings like “hebb codex ” or “/hebb claude-code ”
in the raw command text.
| @dataclass | ||
| class CodexTurn: | ||
| """A parsed Codex turn and its source timestamp.""" | ||
|
|
||
| summary: TurnSummary | ||
| timestamp: str | None = None |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required public API docstring sections.
CodexTurn is a public dataclass but its docstring lacks the required Args/Returns/Raises sections.
Proposed docstring update
`@dataclass`
class CodexTurn:
- """A parsed Codex turn and its source timestamp."""
+ """A parsed Codex turn and its source timestamp.
+
+ Args:
+ summary: Memory-ready turn summary extracted from the rollout.
+ timestamp: Timestamp of the source user message, if present.
+
+ Returns:
+ None.
+
+ Raises:
+ None.
+ """As per coding guidelines, "src/**/*.py: All public APIs in Python MUST have docstrings with Args, Returns, and Raises sections."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @dataclass | |
| class CodexTurn: | |
| """A parsed Codex turn and its source timestamp.""" | |
| summary: TurnSummary | |
| timestamp: str | None = None | |
| `@dataclass` | |
| class CodexTurn: | |
| """A parsed Codex turn and its source timestamp. | |
| Args: | |
| summary: Memory-ready turn summary extracted from the rollout. | |
| timestamp: Timestamp of the source user message, if present. | |
| Returns: | |
| None. | |
| Raises: | |
| None. | |
| """ | |
| summary: TurnSummary | |
| timestamp: str | None = None |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/codex/transcript.py` around lines 21 - 26, `CodexTurn`
is a public dataclass whose docstring is missing the required API sections;
update the class docstring in `CodexTurn` to include Args, Returns, and Raises
sections in the prescribed style. Use the existing `summary` and `timestamp`
fields to describe the arguments, note the dataclass construction result in
Returns, and include any exceptions that may be raised during initialization or
parsing if applicable.
Source: Coding guidelines
| user_indices = [index for index, record in enumerate(records) if _raw_user_text(record)] | ||
| if not user_indices: | ||
| return None | ||
|
|
||
| user_index = user_indices[-1] | ||
| user_record = records[user_index] | ||
| user_text = _clean_user_text(_raw_user_text(user_record)) | ||
| if not user_text: | ||
| return None |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Select the last clean user turn, not just the last raw user record.
Line 49 records raw user messages, but Lines 55-57 return None if the last raw user message is filtered out. A trailing short/noisy prompt can suppress an earlier complete turn, which breaks the “last valid user input” parser contract.
Proposed fix
- user_indices = [index for index, record in enumerate(records) if _raw_user_text(record)]
- if not user_indices:
- return None
-
- user_index = user_indices[-1]
- user_record = records[user_index]
- user_text = _clean_user_text(_raw_user_text(user_record))
- if not user_text:
+ user_indices: list[int] = []
+ last_valid_user: tuple[int, str, int] | None = None
+ for index, record in enumerate(records):
+ raw_user_text = _raw_user_text(record)
+ if not raw_user_text:
+ continue
+ turn_number = len(user_indices)
+ user_indices.append(index)
+ user_text = _clean_user_text(raw_user_text)
+ if user_text:
+ last_valid_user = (index, user_text, turn_number)
+ if last_valid_user is None:
return None
+
+ user_index, user_text, turn_number = last_valid_user
+ user_record = records[user_index]
@@
- turn=len(user_indices) - 1,
+ turn=turn_number,Also applies to: 81-87
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/codex/transcript.py` around lines 49 - 57, The
transcript selection logic in the user-turn parsing path currently picks the
last raw user record and then returns None if _clean_user_text filters it out,
which can hide an earlier valid user message. Update the selection in the
transcript parser so it chooses the last user record that still has non-empty
cleaned text, using the existing helpers like _raw_user_text and
_clean_user_text in the relevant transcript handling flow. Apply the same fix in
both affected parsing sections so the “last valid user input” behavior is
consistent.
| def _truncate(value: str, limit: int) -> str: | ||
| """Truncate text with a visible ellipsis.""" | ||
| return value if len(value) <= limit else value[:limit] + "…" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep truncated output within the configured limit.
The current branch returns limit + 1 characters because the ellipsis is appended after slicing to limit.
Proposed fix
def _truncate(value: str, limit: int) -> str:
"""Truncate text with a visible ellipsis."""
- return value if len(value) <= limit else value[:limit] + "…"
+ if len(value) <= limit:
+ return value
+ if limit <= 0:
+ return ""
+ return value[: limit - 1] + "…"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _truncate(value: str, limit: int) -> str: | |
| """Truncate text with a visible ellipsis.""" | |
| return value if len(value) <= limit else value[:limit] + "…" | |
| def _truncate(value: str, limit: int) -> str: | |
| """Truncate text with a visible ellipsis.""" | |
| if len(value) <= limit: | |
| return value | |
| if limit <= 0: | |
| return "" | |
| return value[: limit - 1] + "…" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/codex/transcript.py` around lines 166 - 168, The
_truncate helper currently exceeds the configured limit because it appends the
ellipsis after slicing to limit; update _truncate so the returned string always
stays within limit, likely by reserving space for the ellipsis when truncation
is needed. Keep the fix localized to _truncate in transcript.py and preserve the
existing behavior for already-short values.
| def uninstall_project_mcp(path: Path) -> bool: | ||
| """Remove project-scoped Hebb MCP configuration. | ||
|
|
||
| Args: | ||
| path: Project ``config.toml`` path. | ||
|
|
||
| Returns: | ||
| Whether the file changed. | ||
| """ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the required public API docstring sections.
uninstall_project_mcp() is missing a Raises section, and handle() is missing a Returns section.
As per coding guidelines, "**/*.py: Include docstring with Args, Returns, and Raises sections for all public APIs."
Also applies to: 77-85
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/codex/uninstall.py` around lines 58 - 66, Complete the
public API docstrings for uninstall_project_mcp and handle by adding the missing
sections required by the coding guidelines. Update uninstall_project_mcp to
include a Raises section describing any exceptions it can propagate, and update
handle to include a Returns section describing its return value. Keep the
docstrings consistent with the existing Args/Returns style used in
src/hebb/integrations/codex/uninstall.py.
Source: Coding guidelines
| hook_input = { | ||
| "session_id": "session-1", | ||
| "turn_id": "turn-3", | ||
| "cwd": "/workspace/project", | ||
| "transcript_path": "/tmp/rollout.jsonl", | ||
| "last_assistant_message": "Recorded.", | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the hardcoded /tmp transcript path.
These tests never read that file because extract_last_turn is mocked, so baking in "/tmp/rollout.jsonl" just adds a POSIX-only absolute path and violates the repo rule against absolute paths outside the workspace. Use tmp_path / "rollout.jsonl" (or a relative placeholder string) instead. As per coding guidelines, "**/*.{py,json,yaml,yml,env,toml,txt}: MUST NOT hardcode API keys, secrets, or absolute paths outside the user's workspace".
Also applies to: 221-224
🧰 Tools
🪛 ast-grep (0.44.0)
[info] 187-187: Do not hardcode temporary file or directory names
Context: "/tmp/rollout.jsonl"
Note: [CWE-377] Insecure Temporary File.
(hardcoded-tmp-file)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/integrations/test_codex_hooks.py` around lines 184 - 190, The test
fixture in hook_input hardcodes an absolute /tmp transcript_path, which should
be replaced with a workspace-safe path. Update the Codex hook tests that build
hook_input to use tmp_path / "rollout.jsonl" (or another relative placeholder)
instead of "/tmp/rollout.jsonl", keeping the change localized to the test data
used around extract_last_turn and the related hook_input setup.
Sources: Coding guidelines, Linters/SAST tools
1 participant
Summary
Verification
PYTHONPATH=src pytest -q tests/unitPYTHONPATH=src pytest -q tests/integrationPYTHONPATH=src pytest -q tests/integration/test_facade.py tests/unit/integrations/test_codex_cli.py tests/unit/integrations/test_codex_hooks.pymypy src/hebb/ruff check src tests/unit/integrations/test_codex_cli.py tests/unit/integrations/test_codex_hooks.pynpm run docs:buildinrepo_pages/Summary by CodeRabbit
New Features
Bug Fixes
Documentation