[qingyun-wu]

What

The Mentra slice of the owner's wearables order (Bee → Mentra → Even), implementing the #2707 design now that registration is complete (packageName sutando, server URL https://chat.ag2.space/mentra, webview /mentra/webview, MENTRAOS_API_KEY vaulted).

• scripts/core.ts — pure logic, zero imports, tested by the root tsx suite without the SDK installed: the wake-phrase gate (case/punctuation tolerant; the phrase alone yields no task; ambient speech stays out by contract), sparrow-safe deterministic task ids (task-mentra-<session>-<seq>, hostile session ids sanitized, ≤64 chars), the lane task shape, and env>manifest config resolution.
• scripts/server.ts — thin wiring only: POST /webhook/session-start spawns an AppSession (dynamic import, so --check and tests never require the dependency); final transcriptions → gate → POST /v1/ingest (source: mentra); the reply leg long-polls the broker's existing GET /v1/result/<id> and renders via showTextWall + audio.speak; a no-result timeout stays quiet because the broker's INTEGRATION_FALLBACK_ROOM_MENTRA (backend#444 mechanism) owns glasses-off delivery. /webview serves the status page the console registration points at.
• Self-contained package.json (@mentraos/sdk) — repo root dependencies untouched; tsc scope (src/ only) unaffected.

Evidence

$ npx tsx --test tests/mentra-channel-core.test.ts
tests 5 / pass 5 / fail 0
$ python3 scripts/lint-skill.py skills/mentra-channel --strict
lint-skill: 1 manifest(s), 0 error(s), 0 warning(s)
$ npx tsc --noEmit   # exit 0 — skills/ outside include scope

Before this PR no mentra source exists in the repo (git grep -i mentraos on origin/main matches nothing outside today's design doc branch) — new capability, no existing path changed.

Live path (deliberately not claimed here)

The unit suite covers the decision logic; the SDK/WS/webhook wiring is thin and its proof is the wear test: EC2 deploy (git-tracked per the owner hard rule — deploy-mentra workflow + Caddy /mentra route are the next PR), lane registry entry (mentra-lane + ingest bearer), npm install in the skill dir, then the owner wearing the glasses: wake phrase → task on the lane → reply on the HUD; glasses-off → reply in the Mentra room.

Stacking note: independent of #2707 (docs, base main) in git terms, but reviews best read together.

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

✅ No Python changes — nothing to gate (bar: 95% on changed lines).

[bassilkhilo-ag2]

Heads up - CI is red on `tsc + tests (clean install)` (typecheck:skills): two errors in `skills/mentra-channel/scripts/server.ts` -

1. line 28: importing with a `.ts` extension needs `allowImportingTsExtensions` enabled in the relevant tsconfig, or drop the extension.
2. line 81: `Cannot find module '@mentraos/sdk'` - either the package isn't declared as a dependency yet (package.json/lockfile), or the typecheck:skills tsconfig needs to skip/stub this skill's SDK-dependent file the way core.ts's zero-import design avoids the issue.

Holding off a full review until CI's green - will do a proper pass once it turns, especially given #2707's design doc (already reviewed, clean) is what this implements.

[qingyun-wu]

Changes requested at exact head 1baee01d8051679dab754a7476c601fce455af42. Formal REQUEST_CHANGES is unavailable on the author's own PR.

Three blocking findings are attached inline. The pure gate/id/config suite passes 5/5, git diff --check and the hardcoded-path gate pass, but that suite never imports the server or SDK. Hosted tsc + tests (clean install) fails at server.ts:28 (TS5097) and server.ts:81 (TS2307). Current official Mentra docs use @mentra/sdk and an AppServer subclass for authenticated webhook/session lifecycle: https://docs.mentra.glass/audio.

Worst case: the service cannot build or start; if those compile errors are bypassed, live replies are duplicated into Matrix and sanitized/restarted session IDs can silently collide under broker idempotency. Not merge-ready until the SDK/bootstrap contract, single-owner fallback delivery, and collision-resistant task IDs are fixed and exercised through the server wiring.

[qingyun-wu]

[P1] This SDK import/API does not match the current supported contract. The clean-install job fails here with TS2307, and package.json installs @mentraos/sdk; current official examples import @mentra/sdk and extend AppServer, which owns the webhook and session lifecycle. The same job also fails line 28 because .ts import extensions are disallowed by tsconfig.skills.json. Switch to the supported package/bootstrap shape, make the skill compile under the repository config, and add a test that imports/starts the server seam rather than only testing core.ts.

[qingyun-wu]

[P1] The fallback is not conditional on this timeout or session state. Backend #444 redirects every source=mentra result to INTEGRATION_FALLBACK_ROOM_MENTRA immediately because this client-side adapter is not registered as a broker deliverer. Polling and rendering here therefore duplicates live answers into Matrix. Make one session-aware component own the glasses-vs-room choice (or add an explicit claim/availability contract), and test one delivery on both live and disconnected paths.

[qingyun-wu]

[P1] Sanitization plus an in-memory sequence is not collision-resistant under broker idempotency. Distinct opaque IDs such as sess:a/b and sess:a:b both become task-mentra-sess_a_b-1; a process restart also resets an existing session to sequence 1. Those tasks can be treated as already queued and silently dropped. Hash the original opaque session ID (as the Bee adapter does) and include a restart-safe event/utterance discriminator; add collision and restart regression cases.

[qingyun-wu]

Blocking findings on head 1baee01d8051679dab754a7476c601fce455af42:

• skills/mentra-channel/scripts/server.ts:28 and skills/mentra-channel/scripts/server.ts:81: the required clean-install check is failing in npm run typecheck:skills. The root tsconfig.skills.json includes skills/**/*.ts, but the new server imports ./core.ts without allowImportingTsExtensions, and @mentraos/sdk is only declared in the skill subpackage, not in the root install that runs the typecheck. CI reports TS5097 on the .ts import and TS2307 for @mentraos/sdk. Please either make this file compile under the root skills typecheck, or exclude/gate the runtime SDK server from that root typecheck with a separate tested package contract.

• skills/mentra-channel/manifest.json:11 / skills/mentra-channel/scripts/server.ts:19: the scaffold expects MENTRA_API_KEY, but the deployment/design wording tells operators to provision MENTRAOS_API_KEY. Following the documented vault name leaves requiredConfigMissing() reporting MENTRA_API_KEY missing and the app exits before serving. Please align the name or accept both as an alias so the documented deployment path works.

Local focused checks: npx tsx --test tests/mentra-channel-core.test.ts passes, and git diff --check ae8eb605bea47d0c2752fa7a041fb2232aa3b69f...HEAD passes. The hosted clean-install check is still red, so I cannot approve this revision.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Review update for head 331316beedbd9674cbd5d34bc652a6714ed01d3c:

The new commit folds the previously separate design doc into this scaffold PR; I do not see a new blocker in that docs delta itself. The code that I blocked on is otherwise unchanged, and the current required clean-install check is still red with the same two compile errors:

• skills/mentra-channel/scripts/server.ts:28: TS5097 because the root tsconfig.skills.json typecheck does not allow .ts import extensions.
• skills/mentra-channel/scripts/server.ts:81: TS2307 because the root clean install that runs npm run typecheck:skills cannot resolve @mentraos/sdk from the skill subpackage-only dependency.

The config mismatch also remains: the folded design doc and deployment prerequisites name MENTRAOS_API_KEY, while manifest.json / requiredConfigMissing() still require MENTRA_API_KEY, so following the documented vault name still leaves the app unconfigured.

Focused local checks on this head:

• npx tsx --test tests/mentra-channel-core.test.ts
• git diff --check ae8eb605bea47d0c2752fa7a041fb2232aa3b69f...HEAD

Both passed, but the PR is still not merge-ready until the runtime server compiles under the repository contract and the documented secret name is aligned or aliased.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

All three P1s fixed at 6b00646 — each one was real, and the SDK finding especially so (the package I'd wired doesn't exist).

1. SDK/bootstrap contract: switched to the real @mentra/sdk (registry-verified 2.1.x) with the AppServer subclass shape — the SDK owns /webhook and session lifecycle; onSession wires transcription→gate→ingest. Installed as a root dependency (the repo's actual convention for skill deps — the self-contained package.json is gone), so tsc -p tsconfig.skills.json now compiles the server: exit 0. Imports use .js style per Node16; one non-obvious extra: the SDK ships CJS, so named ESM imports type-check but fail at runtime — default-import interop, caught by the new server-seam tests, which import the module (side-effect free now; start() only when executed directly) and construct the real subclass.

2. Delivery single-owner: you're right that #444's fallback fires unconditionally once the room env is set. MENTRA_DELIVERY now picks ONE owner per deployment: room (default — broker room owns all replies, glasses show an ack; safe with the env set) or glasses (explicit opt-in: app server polls /v1/result + renders; documented to REQUIRE the broker room env unset). Both paths pinned at the seam; DESIGN.md and the manifest carry the mutual-exclusion contract, with the future claim/availability upgrade path named.

3. Id collision/restart: raw session id is now sha256-hashed (your sess:a/b / sess:a:b pair is a regression test asserting distinct ids) plus a per-process boot nonce so a restart can never re-mint a pre-crash id (also a test). Broker idempotency can no longer silently drop distinct utterances.

$ npx tsc -p tsconfig.skills.json   # exit 0
$ npx tsx --test tests/mentra-channel-core.test.ts
tests 11 / pass 11 / fail 0        # incl. 2 server-seam tests on the real SDK

CI re-running on the new head.

[qingyun-wu]

Blocking finding on head 6b00646f6c7d96a1f79ea0ff0117a13c06caf6a3:

• skills/mentra-channel/DESIGN.md:38, skills/mentra-channel/DESIGN.md:63, and skills/mentra-channel/DESIGN.md:71 still direct the operator toward the stale @mentraos/sdk / MENTRAOS_API_KEY contract, while the code now installs @mentra/sdk and requires MENTRA_API_KEY (skills/mentra-channel/manifest.json:11, skills/mentra-channel/scripts/server.ts:47). Following the design/deployment text will provision the wrong vault key, so requiredConfigMissing() still exits the app as unconfigured even though the runtime code is otherwise fixed. Please align the doc with the shipped key/name or accept MENTRAOS_API_KEY as a compatibility alias.

The previous compile/runtime SDK blockers are fixed on this head. Local checks I ran:

• npm ci --ignore-scripts
• npm exec -- tsc -p tsconfig.skills.json
• npm exec -- tsx --test tests/mentra-channel-core.test.ts
• git diff --check ae8eb605bea47d0c2752fa7a041fb2232aa3b69f...HEAD

GitHub checks are green now, but I cannot approve while the documented onboarding path still starts the service with the wrong required secret.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Fixed at 1f85272 — DESIGN.md now names @mentra/sdk and MENTRA_API_KEY throughout (the two remaining @mentraos/MENTRAOS mentions are deliberate: one explains the stale docs-page name, one is the migration note). Also dropped the stale explicit wss://cloud.mentraos.com URL since the AppServer bootstrap sets no cloud URL (SDK defaults cloudApiUrl to api.mentra.glass). Operationally: I re-vaulted the owner-supplied key under MENTRA_API_KEY so the documented onboarding and requiredConfigMissing() now agree. No code change; the onboarding path provisions the right secret.

[bassilkhilo-ag2]

Independently verified — cloned fresh at head 1f852725, ran npm install (not just the pure-logic subset — with real deps so the server-seam tests actually exercise @mentra/sdk), and traced the code for each of your three self-identified P1s rather than trusting the commit message:

1. SDK contract — server.ts default-imports the @mentra/sdk CJS namespace for values (const { AppServer } = mentraSdk) and imports AppSession as a type separately, with the "why" documented inline (named ESM imports type-check but fail at runtime against this package). @mentra/sdk@^2.1.29 is declared in package.json. Ran the test suite locally with real deps installed: 11/11 pass, including server seam — real SDK contract which constructs an actual AppServer subclass — that's a runtime proof, not just a type-check. npx tsc --noEmit shows no mentra-related errors.

2. Single-owner delivery — deliveryMode() in core.ts is a hard room/glasses switch with the Matrix-duplication hazard explained inline (INTEGRATION_FALLBACK_ROOM_MENTRA set broker-side + app-server polling = double delivery), and both paths are unit-tested (delivery single-owner (review P1) — 2/2 pass).

3. Collision-resistant task ids — taskId() now hashes the original opaque session id (not a lossy slug) and mixes in a per-process boot nonce, with the original collision bug (sess:a/b and sess:a:b' both slugging to sess_a_b, silently dropped as broker-idempotent duplicates) documented in the comment. Test covers this explicitly.

Also checked against REVIEW.md's worst-case-disruption lesson: manifest.json is config-only with secrets left empty (vault-preferred, matches the MANIFEST.md convention), and requiredConfigMissing() fails closed — server.ts throws and process.exit(2)s before ever starting if MENTRA_PACKAGE_NAME/MENTRA_API_KEY/MENTRA_BROKER_URL aren't set. So an un-configured install is inert, not a live half-wired app server.

CI noise right now (refuse new inline CLAUDE_CONFIG_DIR fallback failing at exactly 2m31s, "operation was canceled") is the same GitHub Actions infra pattern I'm seeing across #2706 and #2710 this session — unrelated to this diff.

All three of your own self-flagged blockers check out as genuinely fixed. Approving.

[github-actions]

@cla-assistant check

[qingyun-wu]

Review update for head fe2a3b982c467c662c122578d36b7c96342d140c:

The stale onboarding/documentation blocker is fixed in the current diff. DESIGN.md now names @mentra/sdk and MENTRA_API_KEY for the shipped runtime path, and the remaining @mentraos / MENTRAOS_API_KEY mentions are explicitly framed as stale-docs or migration context. That now matches manifest.json and requiredConfigMissing(), so following the documented provisioning path no longer starts the app with the wrong required secret.

Focused local checks passed:

• npm exec -- tsc -p tsconfig.skills.json
• npm exec -- tsx --test tests/mentra-channel-core.test.ts
• git diff --check ae8eb605bea47d0c2752fa7a041fb2232aa3b69f...HEAD

I am not posting a final approval signal yet because hosted CI is still unsettled on this head: tsc + tests and ruff were still running at my last refresh, and two lint jobs were canceled. I do not see a remaining code or onboarding-contract blocker in the Mentra scaffold.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Changes requested at exact head fe2a3b982c467c662c122578d36b7c96342d140c; formal REQUEST_CHANGES is unavailable on my own PR.

[P1] The documented/registered HTTP contract still does not match the shipped server. I installed the exact lockfile (npm ci) and instantiated the real @mentra/sdk@2.1.29 AppServer. Its registered routes include POST /webhook; there is no /webhook/session-start. SutandoMentraServer does not pass a webhookPath override, so configuring the design/PR-body URL returns 404 and no session reaches onSession(). The PR body also claims /webview is served, but this diff defines no /webview route or publicDir.

The real-SDK seam suite passes 11/11, TypeScript/static checks pass, and hosted CI/coverage/CLA are green; those tests instantiate the server but never assert the externally registered paths. Align the design/PR body and console registration to the actual SDK route (including the ingress prefix), or configure/implement the claimed routes and pin them in a route-level test.

Worst case is a cleanly deployed app that receives zero session webhooks and exposes a dead console webview URL. Not merge-ready until the public route contract is consistent end-to-end.

[qingyun-wu]

[P1] This path is not served by the shipped AppServer. With the installed @mentra/sdk@2.1.29, the default webhookPath registers POST /webhook; SutandoMentraServer supplies no override, and route enumeration shows no /webhook/session-start. Following this registration guidance therefore 404s every session start. Document /webhook (plus the ingress prefix) or explicitly configure the server to match this path, then add a route contract test.

[john-the-dev]

Design-level review (comment, not approval — I reviewed DESIGN.md + manifest architecturally, not line-by-line across the +4416 scaffold; a full code pass on the TS is worth a second reviewer).

The design is strong and the hard parts are already right:

• SDK diligence — verifying @mentra/sdk 2.x against the registry vs the stale @mentraos/sdk docs name is exactly the kind of check that saves a broken deploy.
• Outbound double-delivery guard — the MENTRA_DELIVERY = room | glasses split with mutually-exclusive env requirements (broker fallback-room env MUST stay unset in glasses mode) cleanly resolves the backend#444 'room auto-delivers the moment the env is set' hazard. Good that the P1 is already baked into the design rather than discovered in prod.
• Client-adapter-not-broker-module placement is correctly justified (MentraOS only speaks to a server we host over its own webhook+WS), and the EKS ingress + git-tracked-only manifests follow the standing deploy rules.

Trigger discipline — this is the part worth calling out as the model: 'raw transcription is a firehose; v1 forwards ONLY wake-phrase-gated utterances; a later ambient mode reuses the events-promotion (taskify) pipeline with access_tier: ambient; NOT in v1.' That is exactly the trust boundary I just flagged on #2706 (Bee) — a wake-phrase-gated utterance is a deliberate address (owner-legit, voice-equivalent), whereas ambient capture belongs in the sandboxed ambient tier. Mentra's doc gets it right; I'd suggest #2706 adopt the same explicit discipline (gate the trigger in code, defer ambient to the ambient tier) so the two wearable channels are consistent.

Genuinely-open, owner-gated prerequisites (pre-deploy, not code blockers):

1. Wake phrase is 'TBD with owner' — needs pinning before the trigger gate is testable.
2. Mentra account identity (owner's account vs a dedicated agent account) — affects packageName/MENTRA_API_KEY provenance; worth deciding before console registration.
3. MENTRA_DELIVERY=glasses long-polls GET /v1/result/<id> — fine for v1, but note the HUD-reply latency/timeout story (a glasses user is waiting on the lens) for the follow-up that makes delivery dynamic per session.

Since the owner owns the hardware, the E2E live test she flagged is the real merge-gate here — the design explicitly leans on that. Nice work.

[qingyun-wu]

Closing in favor of ag2-space/ag2space-backend#450 (owner decision 2026-08-07: the Mentra AppServer is push-shaped hosted infra bound to our registered public URL, so it lands in the private backend as a deployable service — and the OSS side stays simple). All content moved: core.ts/server.ts/DESIGN.md/tests re-landed at experimental/services/mentra/ with Dockerfile + deploy filter + workers.tf wiring, 11/11 tests green, plus two portability fixes found in the move (SDK dual-package interop via createRequire; manifest co-location). Thanks bassil + john-the-dev for the reviews here — they carried directly into the re-landed code.