Skip to content

fix(hooks): emit hookEventName on lite-mode SessionStart; make venv bootstrap self-sufficient - #2019

Closed
andywu42 wants to merge 1 commit into
OmniNode-ai:devfrom
andywu42:awu42/sessionstart-hookeventname-envelope
Closed

andywu42 wants to merge 1 commit into
OmniNode-ai:devfrom
andywu42:awu42/sessionstart-hookeventname-envelope

Conversation

@andywu42

Copy link
Copy Markdown
Contributor

Three small hook/bootstrap defects on dev, plus a gap in the OMN-9072 guard that let the first one through.

1. Lite-mode SessionStart silently drops its additionalContext

plugins/onex/hooks/scripts/session-start.sh:145 on dev:

echo '{"hookSpecificOutput":{"additionalContext":"omniclaude lite mode active — generic development tooling only"}}'

No hookEventName. The three full-mode SessionStart emissions in the same file (1533, 1542, 1546) all set it.

Impact, quoting this repo's own guard suite (tests/hooks/test_hook_event_names.py, OMN-9072):

The Claude Code client rejects payloads that lack hookEventName and silently drops any additionalContext, suppressOutput, or decision directives they carried.

This path is live — session-start.sh is wired via hooks-delegation.v1.json — so every lite-mode session loses that context. Line 145 is the only non-conforming hookSpecificOutput emission left under plugins/onex/hooks/scripts/.

2. ensure-plugin-venv.sh hard-fails when OMNI_HOME isn't exported

plugins/onex/hooks/scripts/ensure-plugin-venv.sh:19:

PROJECT_ROOT="${OMNI_HOME:?OMNI_HOME must be set}/omniclaude"

This script is invoked by hand, not as a hook: scripts/repair-plugin-venv.sh delegates to it, and the onboarding skill tells users to run it directly (plugins/onex/skills/onboarding/SKILL.md:98). A new user following onboarding is precisely who won't have OMNI_HOME exported yet — so today it aborts on their first run.

As a pre-venv bootstrap it never sources common.sh, so it can't pick up ~/.omnibase/.env the usual way. The fix mirrors common.sh's own set -a / source / set +a idiom (common.sh:391).

Incidental confirmation: this PR's own commit was initially blocked by the no-cloud-bus pre-commit hook, which failed with OMNI_HOME must be set — the same root cause, on a different script.

3. Stale header comment on ensure-plugin-venv.sh

The file's header claimed "Runs as a SessionStart hook." It isn't registered in hooks.json at all — that file carries only PreToolUse, PostToolUse, and SubagentStop. Corrected to describe the two real invocation paths. This comment is what made me misdiagnose #2 on first read.

4. Not fixed here — the OMN-9072 guard can't catch defect #1

scripts/check_hook_event_names.sh:

if ! grep -q 'hookEventName' "$file"; then

That's a whole-file test, not a per-emission one. session-start.sh contains hookEventName at 1533/1542/1546, so the file passes and line 145 stays invisible. Any file with at least one conforming emitter can hide unlimited non-conforming ones — exactly how this survived. (The hook passes on this branch, as expected.)

Arguably the more valuable fix; left out to keep this PR reviewable. Happy to take it in a follow-up.

Verification

  • Full pre-commit suite passes at commit time, including check hook scripts include hookEventName in hookSpecificOutput, Arch: No hardcoded internal IPs, Block hardcoded internal IPs, Reject committed .env files, Env Var Registry (OMN-6263), and the .secrets.baseline guard.
  • Both scripts pass bash -n; the edited line 145 is valid JSON.
  • Rebased on current dev.

Note on the Receipt Gate

This PR carries no OMN ticket — I don't have Linear access, and the gate's only exemption is the dependency-bot allowlist, so it will fail.

To green it: add an OMN- token to the PR title (the gate reads the title, not the branch — no rename needed), plus an Evidence-Source: line and the matching OCC contract. Only you can produce those. Equally happy for you to close this and re-land it yourself, as with #795 → #797.

@coderabbitai ignore

…ootstrap self-sufficient

session-start.sh: the lite-mode SessionStart envelope omitted hookEventName, so
Claude Code silently discarded its additionalContext. Every other hookSpecificOutput
emission in the repo sets it -- including the three full-mode SessionStart paths in
this same file (1533, 1542, 1546) -- making the lite-mode branch the sole outlier.
This path is live: session-start.sh is wired via hooks-delegation.v1.json.

ensure-plugin-venv.sh: the script hard-fails at ${OMNI_HOME:?} when OMNI_HOME is not
exported by the caller. It is invoked by hand -- scripts/repair-plugin-venv.sh
delegates to it, and the onboarding skill documents running it directly -- and a new
user is precisely who will not have OMNI_HOME exported yet. As a pre-venv bootstrap it
never sources common.sh, so it cannot pick up ~/.omnibase/.env the usual way. Load it
directly, using common.sh's own set -a / source / set +a idiom (common.sh:391).

ensure-plugin-venv.sh header: corrected the stale "Runs as a SessionStart hook" claim.
The script is not registered in hooks.json, which carries only PreToolUse, PostToolUse,
and SubagentStop. The comment misdescribed the script's own invocation path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jonahgabriel

Copy link
Copy Markdown
Contributor

Thanks for the thorough diagnosis and the pointer to the #795→#797 precedent -- taking that path. Re-landed under OMN-16416 as #2021 (diff unchanged, Co-authored-by trailer preserves your authorship). Closing this in favor of #2021.

jonahgabriel added a commit that referenced this pull request Aug 23, 2026
…nv bootstrap self-sufficient (#2021)

## OMN-16416

Re-land of #2019 (external contribution by
@andywu42), closed there because the contributor has no Linear access,
no OMN ticket, and no Evidence-Source -- per repo policy and the PR's
own note, re-landing under an OmniNode-owned ticket/PR with the diff
unchanged. Full credit to andywu42 for the diagnosis and fix; commit
carries `Co-authored-by`.

### What changed (diff identical to #2019)

1. **`plugins/onex/hooks/scripts/session-start.sh:145`** -- the
lite-mode `hookSpecificOutput` emission omitted `hookEventName`. Per
`tests/hooks/test_hook_event_names.py` (OMN-9072), Claude Code silently
drops `additionalContext`/`suppressOutput`/`decision` on any payload
missing it. This path is live (wired via `hooks-delegation.v1.json`), so
every lite-mode session was losing its context injection.
2. **`plugins/onex/hooks/scripts/ensure-plugin-venv.sh:19`** hard-failed
(`OMNI_HOME must be set`) when invoked directly, exactly as the
onboarding skill instructs
(`plugins/onex/skills/onboarding/SKILL.md:98`). It runs
pre-venv-bootstrap so never sources `common.sh`'s `.env` loading. Fix
mirrors `common.sh`'s own `set -a` / source `~/.omnibase/.env` / `set
+a` idiom.
3. Corrected a stale header comment on `ensure-plugin-venv.sh` claiming
it "Runs as a SessionStart hook" -- it is not registered in `hooks.json`
(only `PreToolUse`/`PostToolUse`/`SubagentStop` are).

### Explicitly not in scope (deferred, per the original PR)

`scripts/check_hook_event_names.sh` does a whole-**file** (not
per-**emission**) grep for `hookEventName`, which is how defect #1
escaped detection -- a file with one conforming emitter hides unlimited
non-conforming ones. Worth a follow-up ticket, not blocking this fix.

### dod_evidence

- Original PR verification (carried forward, diff unchanged): full
pre-commit suite passed at commit time, including `check hook scripts
include hookEventName in hookSpecificOutput`, `Arch: No hardcoded
internal IPs`, `Block hardcoded internal IPs`, `Reject committed .env
files`, `Env Var Registry (OMN-6263)`, `.secrets.baseline` guard. Both
scripts pass `bash -n`; edited JSON line is valid.
- Re-verified locally on this branch (cherry-pick onto current dev, same
pre-commit suite green).

Evidence-Ticket: OMN-16416
Evidence-Source: OCC#6912


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

- **Bug Fixes**
- Improved plugin environment setup by loading available platform
settings automatically, while safely continuing if the environment file
is unavailable.
- Enhanced lightweight session-start responses with clearer event
metadata for improved compatibility with integrations.

- **Chores**
- Refreshed security baseline metadata without changing detected secrets
or configuration.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: andywu42 <112039811+andywu42@users.noreply.github.com>
Co-authored-by: jonahgabriel <jonahgabriel@users.noreply.github.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.

2 participants