LP2: establish TypeScript server boundary and dependency proof - #6
Conversation
There was a problem hiding this comment.
💡 Codex Review
https://github.com/Sythos/GuloGulo/blob/1d55ecec8369a2ee59837d45d230b354c76212b1/src/admin/rbac.test.mjs#L1
Update release evidence references when removing MJS files
Removing the M6/runtime/integration .mjs artifacts leaves release/v1-release-evidence.template.json pointing at files that no longer exist, including src/admin/rbac.test.mjs, src/integrations/ldap-client.mjs, and src/runtime/runtime.test.mjs. The M10 audit only validates reference syntax, so it continues reporting these entries as verified evidence even though the cited artifacts are absent; update all migrated references to their .ts or compiled equivalents.
ℹ️ 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".
| export { REDACTED }; | ||
| // Compatibility bridge for legacy JavaScript consumers. The implementation is | ||
| // authored in TypeScript and emitted to dist/server by build:server. | ||
| export * from '../../dist/server/src/runtime/logger.js'; |
There was a problem hiding this comment.
Build server output before running the M7 suite
On a clean checkout, the documented npm run test:m7 command reaches src/observability/structured-event.mjs, which imports this bridge, and immediately fails with ERR_MODULE_NOT_FOUND because dist/server/src/runtime/logger.js is ignored and test:m7 does not invoke build:server. Add the build prerequisite to that focused script or keep the bridge independently loadable.
Useful? React with 👍 / 👎.
| compose(['--profile', 'lp2', 'up', '--detach', '--remove-orphans']); | ||
| started = true; |
There was a problem hiding this comment.
Mark the LP2 project started before Compose up
If docker compose up creates some LP2 containers or volumes and then returns nonzero—for example because a dependency cannot become healthy—the assignment to started is never reached, so the finally block skips down --volumes and leaves the supposedly disposable project behind. Set the cleanup flag before invoking up, since cleanup is safe even when creation was only partial.
Useful? React with 👍 / 👎.
| --command="DO \$\$ BEGIN IF NOT EXISTS (SELECT FROM pg_roles WHERE rolname = '${user_name}') THEN CREATE ROLE \"${user_name}\" LOGIN PASSWORD '${password}'; ELSE ALTER ROLE \"${user_name}\" LOGIN PASSWORD '${password}'; END IF; END \$\$;" >/dev/null | ||
|
|
There was a problem hiding this comment.
Pass fixture values without interpolating SQL
When an advertised LP2 override contains SQL quoting characters, such as LP2_POSTGRES_PASSWORD=it's-synthetic or a quoted user name, this command injects the raw value into SQL and the fixture fails to initialize; crafted values can also change the executed statement. Use psql variables with proper literal/identifier quoting or another parameterized initialization mechanism for the environment-derived role and password.
Useful? React with 👍 / 👎.
Summary\n\n- establish the strict TypeScript server build and typecheck boundary\n- migrate runtime, integrations, authentication, RBAC, and admin tooling to TypeScript with compatibility bridges only where required\n- add synthetic multi-architecture LDAP and PostgreSQL proof on Ubuntu 26.04 LTS\n\n## Verification\n\n- local npm test, typecheck, server typecheck, and LP2 suite green\n- GitHub Actions commit validation green: https://github.com/Sythos/GuloGulo/actions/runs/32711744260\n- Docker multi-architecture and LDAP/PostgreSQL integration proof green\n\n## Scope\n\nThis is an internal milestone PR. The next milestone remains gated until LP2 is merged and documented.