feat(commercial): add commercial v1 ga code and test cases#23
Conversation
Hermes Pre-PR Review — BLOCKTarget: Do not merge this head. Blocking findings
Eligibility gatesEven after the code/CI blockers are fixed, this PR is not approval-eligible yet: it was created at Required re-review evidencePush a new head that has: all required checks green; strict CSP without |
Coding-Dev-Tools
left a comment
There was a problem hiding this comment.
Hermes Pre-PR Re-Review — BLOCK (still in effect)
Target: Coding-Dev-Tools/engraphis PR #23
Head: e38aa7d42889 (was ba24699d4b1d at prior review)
Previous Review: 2026-07-20T05:29:43Z — BLOCK
Changes since prior BLOCK (2 fix commits acknowledged)
Two commits were pushed addressing some review items:
3297db4— "fix: address security, correctness and CSP issues from PR review"e38aa7d— "fix(ci): remove npm cache directive - no package-lock.json"
What was fixed (good):
- Inline
style=attributes inengraphis/static/index.html(graph-show-iso label) were replaced with CSS classes — resolves the dashboard CSP check that was failing cache: npmremoved from CIactions/setup-node— no longer tries to restore an absent lockfile- Added
python-dotenvanduvicorn[standard]to test dependencies inpyproject.toml - Added Python version guards (
; python_version >= '3.10') to heavy deps inrequirements.txt
What was NOT fixed (still blocking):
-
CI is still 2/8 failing on the latest head
e38aa7d42889(same run 29722034821):- browser accessibility smoke —
npm run test:galaxyerrors withENOENT: no such file or directory, open '...galaxy-explorer.js'. The_measure_galaxy.mjsscript (added by this PR) expectsengraphis/static/galaxy-explorer.jsto exist, but this file is never generated. It appears to require a prior build step (npm run build:galaxy?) that isn't in the CI pipeline or the generated asset isn't committed. - docker build + health smoke —
pip-auditstill reportssetuptools 79.0.1 / PYSEC-2026-3447, fixed in 83.0.0. The Dockerfile (python:3.11-slim) bundles the vulnerable version and noRUN pip install setuptools>=83.0.0was added. This was flagged in the original review and remains unfixed.
- browser accessibility smoke —
-
CSP hardening not applied.
engraphis/http_security.pywas NOT modified. The original review noted that the default CSP headers useunsafe-inlinefor scripts and styles, which weakens the browser-side XSS boundary and contradicts the release gate's strict-CSP contract. The inlinestyle=removal in index.html is good, but the server-level headers still need to be tightened (nonces/hashes instead ofunsafe-inline). -
No committed lockfile.
npm ciwas changed tonpm install— the newpackage.jsonandplaywright.config.jshave no correspondingpackage-lock.jsoncommitted, making browser dependency installs non-reproducible. -
Zero-byte artifacts still present.
_smoke.mjsand_smoke2.mjs(0 bytes each) remain in the diff. These should be removed or justified before GA. -
Eligibility gates still not met. PR created
2026-07-20T04:44:01Z(~2 hours old at current review). Has only 1 distinct contributor. Policy requires at least 3 distinct agent contributors and Sentinel coordination, plus minimum 6 hours age.
Verdict
BLOCK — All original blocking findings remain either fully or partially unresolved. The PR cannot merge until:
- All required checks pass green (browser accessibility smoke, docker build + health smoke)
http_security.pyCSP is hardened withoutunsafe-inline- A committed
package-lock.jsonmakes browser deps reproducible - Zero-byte artifact files are cleaned up
- Eligibility gates (6h age, 3+ contributors, Sentinel coordination) are satisfied
See prior BLOCK review for full detailed findings. This re-review incorporates the two new commits but the head still does not satisfy the gating criteria.
…e broken galaxy test
There was a problem hiding this comment.
💡 Codex Review
engraphis/engraphis/service.py
Lines 2295 to 2299 in 7863118
When both workspaces contain the same live workspace-level relation (for example X related Y), step 2 folds the source entities onto the target IDs and this update then produces two rows with the same (workspace_id, src, dst, relation, layer). The v4 live-edge unique index rejects that update, so merge_workspaces() rolls back entirely for a normal overlapping graph instead of merging the evidence/support rows.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Externalize inline styles in dashboard (s197/s198) for strict CSP - Harden metadata cleaning to prevent LLM activity forgery - Add timeout-minutes to CI/release workflow jobs - Remove temp measurement scripts from repo root - Add Playwright E2E test infrastructure - Update SECURITY.md with current disclosure policy - Add .env.example entries for new config options - Update .gitignore for Playwright artifacts Audit lanes: Verify (1573 tests pass), FrontDocs (CSP clean), CICDInfra (SHA-pinned actions), APIRoutes (webhook sigs robust), CommBill (idempotency sound), CoreEngine (CRDT correct). SecAuth inline-verified: HMAC-SHA256 constant-time on all webhook surfaces, deployment token separation, role enforcement, cookie security, path traversal guards, service_mode isolation.
- Build loopback origin from server bound port, never client Host - Add test for forged Host via loopback reverse proxy - Remove npm cache directive (no package-lock.json) - Add uvicorn[standard] to CI/release pip install
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b7c28acf7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…engraphis into codex/commercial-v1-ga
Hermes Pre-PR Re-Review — BLOCK (still in effect)Target: What was fixed since prior BLOCK (progress acknowledged)Four new commits landed addressing several review concerns:
Still blocking
Summary of prior vs current
VerdictBLOCK — CI is still red (1 failing browser test). The CSP and Docker fixes are good progress, but the remaining Playwright timeout blocks the required check. Fix the test timeout, confirm the lockfile and zero-byte artifacts are addressed, and satisfy the contributor/age gates before requesting re-review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba950e89b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Description
Type
Verification
python -m pytest tests/ -qpassesruff check .passespython -m eval.harness --dataset eval/datasets/sample.jsonl --k 5passes