core-lead/molecule-core
1
0
Fork 0
forked from molecule-ai/molecule-core
Code Pull Requests Activity
36 Commits 73 Branches 39 Tags 58 MiB
2f0c708d81
Commit Graph

3 Commits

Author SHA1 Message Date
Hongming Wang
c84b9998b6 chore: apply code-review round-6 suggestions 
All 5 suggestions from the latest review pass.

## tests/e2e/_extract_token.py (new)
Extracted the 14-line python-in-bash heredoc from _lib.sh into a real
Python file. Easier to edit, fewer escaping traps, same behavior.
Shell helper now just shells out to it.

## tests/e2e/_lib.sh
- Replaced inline python with: python3 "$(dirname "${BASH_SOURCE[0]}")/_extract_token.py"
- Removed redundant sys.exit(0) as part of the extraction

## Shellcheck-clean scripts (new CI job enforces)
- Removed dead captures: BEFORE_COUNT (test_activity_e2e.sh), ORIG_SKILLS,
  REIMPORT_SKILLS (test_api.sh), QA_TOKEN (test_comprehensive_e2e.sh)
- Renamed unused loop vars `i`, `j` -> `_` in 4 sites
- Added `# shellcheck disable=SC2046` on the two intentional word-splits
  in test_claude_code_e2e.sh (docker stop/rm of multiple container IDs)
- Removed a useless re-register of QA mid-script (was done in Section 2)

## CI (.github/workflows/ci.yml)
- Replaced `sudo apt-get install postgresql-client` + psql with a direct
  `docker exec` into the existing postgres:16 service container. Saves
  ~10-20s per CI run.
- Added new `shellcheck` job that lints tests/e2e/*.sh on every PR.
  Local: shellcheck --severity=warning returns 0 across all 5 scripts.

## Verification
- go test -race ./internal/handlers/... : pass
- mcp-server: 96/96 jest
- canvas: 357/357 vitest + clean build
- tests/e2e/test_api.sh: 62/62
- tests/e2e/test_comprehensive_e2e.sh: 67/67
- shellcheck tests/e2e/*.sh : clean
- CI YAML: valid

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-13 17:08:45 -07:00
Hongming Wang
3130fe0144 chore: address follow-up review — dead helpers, lib polish, CI hardening 
Last sweep of code-review items before merging PR #5.

## _lib.sh cleanup

- Removed unused e2e_register and e2e_heartbeat helpers (dead code —
  no caller ever invoked them)
- Standardized on $BASE variable set via : "${BASE:=...}" so every
  script uses one name (was mixed $BASE / $e2e_base)
- e2e_extract_token now writes stderr warnings on JSON parse failure
  or missing auth_token, instead of silently returning empty. Previous
  behavior made downstream "missing workspace auth token" 401s much
  harder to diagnose

## Script cleanup

- test_api.sh, test_comprehensive_e2e.sh, test_activity_e2e.sh all
  drop the redundant `e2e_base + BASE="$e2e_base"` aliasing; sourcing
  _lib.sh sets BASE via : "${BASE:=...}" default

## CI hardening (.github/workflows/ci.yml)

- Postgres credentials now match .env.example (dev:dev — was
  molecule:molecule, caused confusion for local repros)
- Added Go module cache via actions/setup-go cache:true +
  cache-dependency-path: platform/go.sum. ~30s cold-run improvement
- New pre-E2E step asserts migrations actually ran by checking for
  the 'workspaces' table. Catches future migration-author mistakes
  before they surface as obscure E2E failures

## Follow-up issue

Filed Molecule-AI/molecule-monorepo#6 for the deterministic token-
mint admin endpoint. PR #5 uses an empirical "beat the container"
race (5/5 wins in benchmarks); issue #6 tracks the real fix for
any future CI load that invalidates the assumption.

## Verification

- bash tests/e2e/test_api.sh              -> 62/62
- bash tests/e2e/test_comprehensive_e2e.sh -> 67/67
- python3 -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))" -> ok

## Operational note

Hourly PR-triage + issue-pickup cron scheduled this session (job id
0328bc8f, fires at :17 past each hour). Runtime reports it as
session-only despite durable:true — re-invoke via /loop or
CronCreate in a fresh session if needed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-13 17:08:45 -07:00
Hongming Wang
f9803ec55e fix(e2e): comprehensive + activity_e2e + shared lib + CI smoke job 
Follow-up to the test_api.sh fix. Same Phase 30.1 + 30.6 staleness
existed in the other E2E scripts; same pattern applied.

## New tests/e2e/_lib.sh
Shared bash helpers so future scripts don't reimplement:
- e2e_extract_token — parse auth_token from register response
- e2e_register       — register + echo token
- e2e_heartbeat      — heartbeat with bearer auth
- e2e_cleanup_all_workspaces — pre-test state reset

## test_comprehensive_e2e.sh (14 fail -> 0 fail)
Root cause was deeper than test_api.sh: the script creates workspaces
at Section 2 but doesn't register them until Section 3. In between,
the platform provisioner spawns the Docker container, whose main.py
calls /registry/register first and claims the single-issue token.
The script's later register gets no auth_token back.

Fix: register each workspace immediately after POST /workspaces,
beating the container to the token. Empirically 5/5 wins in a tight
loop. PM/Dev/QA tokens captured at creation time; bearer auth threaded
through all heartbeat/update-card/discover/peers calls.

Removed the duplicate register calls in Section 3/4 that followed
(tokens already captured).

Result: 53/68 -> 67/67 (one duplicate check dropped).

## test_activity_e2e.sh
Same pattern applied on faith. Script still SKIPs cleanly when no
online agent is present; when an agent IS online, it now re-registers
it to mint a fresh bearer token and threads Authorization: Bearer on
the 3 heartbeat calls.

## test_api.sh refactor
Now sources _lib.sh and uses the shared helpers. No behavior change,
still 62/62.

## .github/workflows/ci.yml — new e2e-api job
Spins up Postgres 16 + Redis 7 as GitHub Actions services, builds the
platform binary, runs it in background with DATABASE_URL/REDIS_URL,
polls /health for 30s, then runs tests/e2e/test_api.sh. On failure
dumps platform.log for triage. 10-min job timeout.

This is the watchdog that would have caught Phase 30.1 auth drift
the day it landed. Picks test_api.sh not test_comprehensive_e2e.sh
because the latter depends on Docker-in-Docker for container
provisioning which is heavier than a PR gate should carry.

## Verification
- bash tests/e2e/test_api.sh                -> 62/62
- bash tests/e2e/test_comprehensive_e2e.sh  -> 67/67
- bash tests/e2e/test_activity_e2e.sh       -> cleanly SKIPs (no agent)
- go build ./...                            -> clean
- .github/workflows/ci.yml                  -> valid YAML, new job added

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
 2026-04-13 17:08:45 -07:00