Skip to content

fix: allow enabling an agent without Telegram credentials - #60

Open
Jeremy2026AI wants to merge 1 commit into
noogalabs:mainfrom
Jeremy2026AI:fix/enable-without-telegram
Open

Jeremy2026AI wants to merge 1 commit into
noogalabs:mainfrom
Jeremy2026AI:fix/enable-without-telegram

Conversation

@Jeremy2026AI

Copy link
Copy Markdown

Summary

  • The `enable` CLI preflight hard-blocked every agent without BOT_TOKEN/CHAT_ID, but agent-manager.ts already treats Telegram as fully optional at the daemon level (it's simply skipped when BOT_TOKEN is absent — see the existing comment at agent-manager.ts:578).
  • This aligns the CLI check with existing daemon behavior: enable now succeeds when both BOT_TOKEN and CHAT_ID are blank, and still fails loud on a partial config (only one of the two set), since that's a real misconfiguration rather than an intentional no-Telegram setup.

Test plan

  • npm test — full suite passes (3646 passed)
  • New integration test tests/integration/enable-no-telegram-cli.test.ts covers: both blank → exit 0 + enabled, only BOT_TOKEN set → exit 1, only CHAT_ID set → exit 1
  • Manually verified against a temp fixture directory

🤖 Generated with Claude Code

The `enable` CLI preflight hard-blocked every agent without BOT_TOKEN/CHAT_ID,
but agent-manager.ts already treats Telegram as fully optional at the daemon
level (Telegram is simply skipped when BOT_TOKEN is absent). This aligns the
CLI check with existing daemon behavior: enabling now succeeds when both
BOT_TOKEN and CHAT_ID are blank, and still fails loud on a partial config
(only one of the two set), since that's a real misconfiguration rather than
an intentional no-Telegram setup.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@noogalabs

Copy link
Copy Markdown
Owner

Thanks for the contribution, @Jeremy2026AI. We’ve acknowledged PR #60 and queued it for maintainer review. We’ll post a substantive verdict by Monday, August 10.

@noogalabs

Copy link
Copy Markdown
Owner

Reopening to re-emit the pull_request event because GitHub did not materialize the first-time-contributor workflow approval run for exact head 86c7b62.

@noogalabs noogalabs closed this Aug 9, 2026
@noogalabs noogalabs reopened this Aug 9, 2026

@noogalabs noogalabs left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 — unreadable .env now fails open as intentional no-Telegram configuration.

parseEnvFile() catches every read error and returns {} (src/cli/enable-agent.ts:33-47). This change then treats both absent values as telegramConfigured === false and continues enabling (src/cli/enable-agent.ts:182-197). On exact head 86c7b62190775c66796ed901fc026bce570cbc69, I made the discovered .env path a directory so readFileSync throws EISDIR; cortextos enable exited 0, wrote enabled-agents.json, and reported “enabling without a Telegram channel.” The predecessor failed closed because the same empty parse result hit the required-value error. A permissions/read I/O failure has the same outcome.

Please distinguish a successful readable env with both Telegram values blank/absent from an env read failure. Fail before writing the registry on read failure, and add the unreadable/non-file .env casualty. The new positive and partial-pair cases can remain as-is. I also independently verified that a configured pair still enters live validation (an invalid configured token exited 1 without writing the registry), but a committed configured-pair regression would strengthen the moved branch.

@noogalabs

Copy link
Copy Markdown
Owner

Evidence closure for exact head 86c7b62190775c66796ed901fc026bce570cbc69:

  • Hosted CI: all jobs green — CI run 31318951830
  • Hosted safety gates: leak guard and scope reconcile green — run 31318951835
  • Isolated checkout: typecheck/build green; focused new integration test 3/3 green; configured-pair invalid-token casualty exited 1 without registry write; unreadable/non-file .env casualty exited 0 and wrote the registry, which is the blocking REQUEST_CHANGES finding.

The full isolated root suite was not used as authority because it was run before dashboard dependencies were installed and also hit an unrelated timing test; hosted CI installed both dependency sets and completed green. The review remains blocked only on the fail-open repair and its regression test.

@noogalabs

Copy link
Copy Markdown
Owner

Thanks for this - it's a clean fix. You're right that the daemon already treats Telegram as optional; aligning the enable preflight with that behavior (and keeping the fail-loud path when exactly one of the two vars is set) is exactly the right shape, and the integration test is appreciated. Verdict: adopting. It's mergeable against current main, so we'll route it through our review process as-is. Thanks for contributing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants