Repository navigation
docs: unify Hermes OpenMax onboarding prompt - #8
Conversation
zylos-luna-coco
left a comment
There was a problem hiding this comment.
Comprehensive onboarding prompt overhaul — replaces the old manual-step template with a prescriptive, security-hardened flow. Well done.
Security posture:
- Explicit prohibition on printing/logging API keys, tokens, JWTs, and signed URLs — correct for a prompt that carries a real invitation token at render time.
chmod 600on env file, existing-key reuse check before re-registration, D8 envelope handling to prevent double-unwrapping mistakes.- Docstring correctly states callers must deliver the rendered prompt directly and never persist it.
Channel-liveness boundary (from PR #6):
Baked into both the prompt and tests — "CWS WebSocket 是传输连接,不是 IM channel,不得调用 /channel-liveness". Consistent with the reporter removal.
Evidence boundary:
Good 4-level distinction: Gateway running → cws connected → online-report ok → online_status=online. Prevents false positives in verification. The message E2E path (inbound → response ready → send succeeded → history check) is the right completeness bar.
Prompt generator (build_prompt):
Clean placeholder→value substitution. New invitation metadata params (invitation_id, invitation_token, organization_name, display_name, owner_name, owner_member_id, expires_at) cover the full Buy Agent rendering surface. CLI argparse generated dynamically from the param list — no duplication.
Tests:
3 tests covering full-value rendering (18+ assertions), placeholder-only mode, and transport/IM boundary. test_prompt_does_not_treat_openmax_transport_as_im_channel is a good regression guard.
102 passed per the PR description. LGTM.
zylos-luna-coco
left a comment
There was a problem hiding this comment.
Re-approved after 2 new commits (head e658dda). Both additions are clean.
ce2f61f — generate prompt from current checkout:
sys.path.insert(0, ...) in the CLI script ensures python scripts/generate_openmax_prompt.py always uses this checkout's hermes_openmax/prompt.py, not a stale installed wheel. The test (test_cli_script_loads_current_checkout_and_renders_complete_prompt) validates this by running from cwd="/" — good coverage.
e658dda — English onboarding prompt variant:
build_prompt(language="en"|"zh")withValueErroron unknown language — correct.- English prompt is a faithful translation: all security rules, D8 envelope handling, channel-liveness boundary, evidence hierarchy, and final report format preserved.
- Chinese prompt slightly condensed (steps merged into shorter blocks) but content-equivalent.
- Value key mapping cleaned up (
CWS_BFF_URL→BFF, etc.) — cleaner substitution dict. - CLI gains
--languagewithchoices=("en", "zh"), defaults to"en". - Tests: 3 → 7 (added CLI subprocess, zh variant, en transport/IM, invalid language).
- README documents the generator commands.
LGTM.
Summary
Security
.envVerification
uv run python -m pytest -q tests/test_prompt.py tests/test_reporters.py— 10 passeduv run python -m pytest -q -m "not live"— 102 passed, 1 skipped, 1 deselecteduv run python -m pytest -q— 102 passed, 2 skippedpython3 -m compileall -q cws_agent_sdk hermes_openmax testsgit diff --checkNote
An independent reviewer was requested, but its model call returned HTTP 503 after retries; no review result was fabricated.