refactor: extract dual-bot support (Clawd + Gremlin)#8
Conversation
Extract bot configuration into bot-config.ts with per-bot prompts, wandering behavior, and identity settings. Add Gremlin as a second bot with name-addressed chat routing (only responds when mentioned). Thread BotConfig through all handlers to replace hardcoded Clawd values. Update ecosystem.config.cjs for dual-bot PM2 management. Co-Authored-By: Claude <noreply@anthropic.com>
Add GitHub Actions CI (build + lint on PR/push to main) and /ship initialization config with 80% coverage threshold target. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review - PR #8
Summary: Cleanly extracts bot identity, prompts, and wandering config into a per-bot configuration system, enabling Clawd and Gremlin to run as separate processes from a single codebase.
Changes reviewed:
- New
BotConfiginterface and config registry (src/bot-config.ts) - Clawd and Gremlin prompt modules (
src/prompts/clawd.ts,src/prompts/gremlin.ts) - Threading
BotConfigthrough all handlers insrc/index.tsandsrc/agent-loop.ts - Name-addressed chat routing via
shouldRespondToChat() - Dual-bot PM2 config (
ecosystem.config.cjs) - CI workflow (
.github/workflows/ci.yml)
Quality Assessment:
| Aspect | Status | Notes |
|---|---|---|
| Code Quality | pass | Clean extraction — config threaded consistently, good doc comments, nullable prompts handled well |
| Tests | warning | No tests exist yet (test-command: none in ship-config). Acceptable for this refactor scope |
| Security | pass | No new secrets, env vars, or external inputs introduced |
| Performance | pass | No performance implications — same runtime behavior, just parameterized |
Issues found:
-
CI lint step fails (blocking) — The CI workflow runs
npm run lintbut noeslint.config.jsexists in the repo. ESLint 9.x requires a flat config file. CI is currently red: https://github.com/enspyrco/tech_world_bot/actions/runs/23194165834. Either add aneslint.config.jsor remove the lint step until one is configured. -
shouldRespondToChathardcodes Clawd as the "responds to everything" bot (src/index.tsline 545) — This embeds routing policy in a function that should ideally be config-driven. Consider adding arespondsToAll: booleanfield toBotConfiginstead of checkingagentName === "clawd". Low priority since it works correctly today, but it would make adding a third bot cleaner. -
Shared
messageHistorybetween bots — Both bots share the same chat room, but each process has its ownmessageHistoryarray. This means Clawd's conversation context won't include Gremlin's responses and vice versa. This is probably fine (even desirable) for now, but worth noting as a conscious design choice. -
Gremlin's fallback hint message references Clawd by name (
src/index.tsline 344:"try asking Clawd!") — This works for the current two-bot setup but would need updating if the tutor bot is ever renamed. Minor.
Suggestions:
-
Deployment note needed: The CLAUDE.md still references a single
tech-world-botPM2 process. The deployment section should be updated to reflect dual-bot management (e.g.,pm2 restart clawd-bot gremlin-bot). This PR description says "Stack: 1/3" so maybe a later PR handles docs. -
Imports before interface declaration in
bot-config.ts(lines 26-27): Theimportstatements come after theBotConfiginterface. While TypeScript is fine with this, conventionally imports go at the top of the file. Nitpick. -
Agent dispatch coordination: The existing Firebase Cloud Function dispatches
agentName: "clawd". For Gremlin to also be dispatched, the Cloud Function will need a secondRoomAgentDispatch. Presumably handled in a later PR in this stack, but flagging for awareness.
Verdict: REQUEST_CHANGES
The code quality of the refactor itself is solid — config extraction is clean, nullable prompts are handled consistently, and the name-addressed routing is a nice touch. However, CI is red due to the missing ESLint config, which needs to be fixed before merge. Once that's resolved, this is ready to go.
Add eslint.config.js with typescript-eslint, install required deps, remove unused MapInfo interface that triggered lint error. Co-Authored-By: Claude <noreply@anthropic.com>
- Add respondsToAll config flag to replace hardcoded agentName check - Include bot name in proactive nudge IDs to prevent deduplication - Remove hardcoded "Clawd" reference from Gremlin hint fallback - Update CLAUDE.md for dual-bot architecture and Cloud Run deployment Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review - PR #8 (Re-review)
Summary: Extracts dual-bot support into a clean config-driven architecture, enabling Clawd and Gremlin to run as separate processes from a single codebase.
Previous feedback status:
- ESLint flat config: Fixed —
eslint.config.jsadded, CI is green respondsToAllconfig flag: Fixed — routing is now config-driven instead of hardcodingagentName === "clawd"- Gremlin fallback referencing Clawd: Fixed — now says "try asking the other bot!"
- CLAUDE.md deployment docs: Fixed — updated for Cloud Run dual-bot setup
Quality Assessment:
| Aspect | Status | Notes |
|---|---|---|
| Code Quality | pass | Clean config threading, good doc comments, nullable prompts handled consistently |
| Tests | warning | No tests (acceptable for refactor scope, test-command: none in ship config) |
| Security | pass | No new secrets or external inputs |
| Performance | pass | Same runtime behavior, just parameterized |
| CI | pass | Build + lint green |
Issue found:
-
Chat response IDs missing bot name (non-blocking) —
src/index.tsline 178 uses`${messageId}-response`but the CLAUDE.md documents the format asmessage-id-botname-responseand states "Response IDs include the bot'sagentNameto prevent deduplication when both bots respond to the same message." The proactive nudge ID was correctly updated to includebotConfig.agentName(line 501), but the regular chat response ID (line 178) and error ID (line 205) were not. Today this only matters when a user mentions "gremlin" (triggering both bots to respond to the same message), but it's a latent dedup bug.Suggested fix:
// line 178 id: `${messageId}-${botConfig.agentName}-response`, // line 205 id: `${messageId}-${botConfig.agentName}-error`,
Verdict: APPROVE
All blocking feedback from the previous review has been addressed. The response ID inconsistency is real but non-blocking — it only manifests when both bots respond to the same message, and the fix is a one-liner. The overall refactor is clean: config extraction is thorough, prompts are well-separated, and the BotConfig interface is a solid foundation for adding more bots later.
- Add extractText() helper that safely handles empty content arrays from the Anthropic API (prevents crash on undefined access) - Guard help-request handler with isBusy check to prevent concurrent help requests from racing and corrupting walk/state Co-Authored-By: Claude <noreply@anthropic.com>
Summary
bot-config.tswith per-bot identity, prompts, and wandering settingsBotConfigthrough all handlers to replace hardcoded Clawd valuesecosystem.config.cjsfor dual-bot PM2 managementTest plan
npm run buildpassesStack: 1/3 — base PR
Generated with Claude Code