Tracking two non-blocking items surfaced during review of #24 (approved & merged separately).
1. composeTemplateEnvValue regex is brittle to inline comments
internal/localmode/compose_test.go — the value-extraction regex ends in "?\s*$, which breaks if an inline comment is ever added to one of the three timing lines:
- Quoted + comment (
REDIS_TIMEOUT: "60s" # note) → FindStringSubmatch returns nil → require.Len(match, 2) fails with a misleading "missing env var".
- Unquoted + comment (
USAGE_SYNC: 30s # note) → the comment is swallowed into the capture group → time.ParseDuration errors.
The runtime YAML/compose parser strips inline comments fine (verified with yaml.v3), so this is test-only — but the template already uses inline comments heavily, so it's an easy trap. Suggested fix:
re := regexp.MustCompile(`(?m)^\s*` + regexp.QuoteMeta(key) + `:\s*"?([^"#\n]+?)"?\s*(?:#.*)?$`)
Ref: #24 (comment)
2. Confirm info_test.go DB-URL fixture matches the server contract (cross-repo)
internal/localmode/info_test.go now asserts a new database_url shape:
postgres://volcano_client_<uuid>:vpg_local_secret@localhost:8002/app?sslmode=disable&application_name=volcano_full_access
This is a pure passthrough unit test (no CLI bug — info.go forwards the URL verbatim to psql), but the format isn't what the server currently emits. In volcano-hosting, local info builds the URL via localmode.DatabaseConnectionString, which still returns the old shape (volcano:volcano@... & application_name=volcano_full_access:<db>).
The new format drops the :<db> suffix from application_name and introduces a scoped role (volcano_client_<uuid> + real password). The server's pgproxy currently identifies full_access/admin connections by parsing that suffix — strings.CutPrefix(appName, "volcano_full_access:") (pooling_proxy.go:847). Confirm the paired server change (a) emits exactly this URL from local info and (b) moves routing/auth onto the scoped role, otherwise admin connections lose their target-DB routing.
Ref: #24 (comment)
Tracking two non-blocking items surfaced during review of #24 (approved & merged separately).
1.
composeTemplateEnvValueregex is brittle to inline commentsinternal/localmode/compose_test.go— the value-extraction regex ends in"?\s*$, which breaks if an inline comment is ever added to one of the three timing lines:REDIS_TIMEOUT: "60s" # note) →FindStringSubmatchreturns nil →require.Len(match, 2)fails with a misleading "missing env var".USAGE_SYNC: 30s # note) → the comment is swallowed into the capture group →time.ParseDurationerrors.The runtime YAML/compose parser strips inline comments fine (verified with
yaml.v3), so this is test-only — but the template already uses inline comments heavily, so it's an easy trap. Suggested fix:Ref: #24 (comment)
2. Confirm
info_test.goDB-URL fixture matches the server contract (cross-repo)internal/localmode/info_test.gonow asserts a newdatabase_urlshape:This is a pure passthrough unit test (no CLI bug —
info.goforwards the URL verbatim topsql), but the format isn't what the server currently emits. Involcano-hosting,local infobuilds the URL vialocalmode.DatabaseConnectionString, which still returns the old shape (volcano:volcano@... & application_name=volcano_full_access:<db>).The new format drops the
:<db>suffix fromapplication_nameand introduces a scoped role (volcano_client_<uuid>+ real password). The server's pgproxy currently identifies full_access/admin connections by parsing that suffix —strings.CutPrefix(appName, "volcano_full_access:")(pooling_proxy.go:847). Confirm the paired server change (a) emits exactly this URL fromlocal infoand (b) moves routing/auth onto the scoped role, otherwise admin connections lose their target-DB routing.Ref: #24 (comment)