-
Notifications
You must be signed in to change notification settings - Fork 650
fix(codex): wrap SessionStart hook output in hookSpecificOutput envelope #705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -137,8 +137,21 @@ fi | |
| ENCODED_PROJECT=$(printf '%s' "$PROJECT" | jq -sRr @uri) | ||
| CONTEXT=$(curl -sf "${ENGRAM_URL}/context?project=${ENCODED_PROJECT}" --max-time 3 2>/dev/null | jq -r '.context // empty') | ||
|
|
||
| # Inject Memory Protocol + context — stdout is returned to Codex as additionalContext | ||
| cat <<'PROTOCOL' | ||
| # Build Memory Protocol + context, then emit it as Codex's hookSpecificOutput | ||
| # JSON envelope. Codex's SessionStart parser rejects raw stdout text as a hook | ||
| # failure (non-fatal, but reports "hook: SessionStart Failed" every run — see | ||
| # codex-review SKILL.md "Failure modes", 2026-08-08 silent-death-after-hooks). | ||
| # Claude Code's SessionStart contract stays tolerant of raw text, so this is a | ||
| # codex/-only change; plugin/claude-code/scripts/session-start.sh is untouched. | ||
| # | ||
| # The protocol text is written to a FILE, never captured via `$(cat <<'EOF' ...)` | ||
| # — macOS ships bash 3.2 as /bin/bash, which mis-parses a heredoc nested inside | ||
| # a command substitution once the heredoc body contains an apostrophe (e.g. | ||
| # "user's"), throwing "unexpected EOF while looking for matching `)'" at | ||
| # script-load time. Writing to a file sidesteps the parser bug entirely. | ||
| PROTOCOL_FILE=$(mktemp) | ||
| trap 'rm -f "$PROTOCOL_FILE"' EXIT | ||
| cat <<'PROTOCOL' > "$PROTOCOL_FILE" | ||
|
Comment on lines
+140
to
+154
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift Move protocol construction out of the plugin adapters. These scripts build Memory Protocol policy and require
As per path instructions, 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Path instructions |
||
| ## Engram Persistent Memory — ACTIVE PROTOCOL | ||
|
|
||
| You have engram memory tools. This protocol is MANDATORY and ALWAYS ACTIVE. | ||
|
|
@@ -174,9 +187,8 @@ Call `mem_save` IMMEDIATELY after ANY of these: | |
| Call `mem_session_summary` with: Goal, Discoveries, Accomplished, Next Steps, Relevant Files. | ||
| PROTOCOL | ||
|
|
||
| # Inject memory context if available | ||
| if [ -n "$CONTEXT" ]; then | ||
| printf "\n%s\n" "$CONTEXT" | ||
| fi | ||
| jq -n --rawfile protocol "$PROTOCOL_FILE" --arg ctx "$CONTEXT" \ | ||
| '{hookSpecificOutput: {hookEventName: "SessionStart", | ||
| additionalContext: ($protocol + (if $ctx != "" then "\n\n" + $ctx else "" end))}}' | ||
|
|
||
| exit 0 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Stop when temporary-directory creation fails.
If
mktemp -dfails,TMPDis empty. The next redirect writes to/head.txt. The cleanup trap does not remove those files.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents