Skip to content

Add GITHUB_NOTIFY_CHAT_ID to make config prompts#184

Merged
dcellison merged 2 commits intomainfrom
feature/config-github-notify-chat-id
Mar 26, 2026
Merged

Add GITHUB_NOTIFY_CHAT_ID to make config prompts#184
dcellison merged 2 commits intomainfrom
feature/config-github-notify-chat-id

Conversation

@dcellison
Copy link
Copy Markdown
Owner

Summary

Add GITHUB_NOTIFY_CHAT_ID to the interactive make config prompts so it persists in install.conf and survives make apply.

Without this, the env var had to be manually added to /etc/kai/env, where it would be silently dropped on the next make apply (which regenerates the env file from install.conf).

Changes

Change Detail
_validate_chat_id() New validator accepting any non-zero integer (Telegram group IDs are negative)
_cmd_config() prompt New "GitHub notifications" section between issue triage and optional features
Env dict entry GITHUB_NOTIFY_CHAT_ID added to env dict when non-empty
Test update test_writes_install_conf input list updated for new prompt

Test plan

  • test_writes_install_conf updated with new prompt input
  • test_reads_existing_defaults passes (empty default accepted)
  • All 1147 tests pass, make check clean
  • Manual: run make config, enter a group chat ID, verify it appears in install.conf

Fixes #183

Add interactive prompt for the GitHub notification group chat ID in
_cmd_config(), between the issue triage section and optional features.
Validates as a non-zero integer (accepts negative Telegram group IDs).
Empty input disables the feature.

Without this, the env var would be silently dropped on every make
apply since _generate_env_file writes from install.conf only.

Fixes #183
@dcellison
Copy link
Copy Markdown
Owner Author

Review by Kai

This is a clean, focused PR. No critical issues or bugs found.

One minor suggestion:

install.py:176_validate_chat_id only catches ValueError, but that's actually correct here: int() on a string raises only ValueError for non-numeric input (no OverflowError risk since Python's int() handles arbitrary precision). No issue.

Suggestion (low priority): tests/test_install.py:294 — The test only exercises the empty/disabled path. A second test case with a valid negative group ID (e.g., "-1001234567890") and one with "0" (should loop/reject) would give fuller coverage of _validate_chat_id. Not a blocker.

Everything else looks correct:

  • Validator correctly accepts any non-zero integer, including negative Telegram group IDs
  • Loop logic properly accepts empty (feature disabled) or valid ID
  • Env dict only populated when non-empty
  • Placement in the prompt sequence matches the PR description
  • No injection risk (integer string stored in a config file)

Ready to merge.

@dcellison
Copy link
Copy Markdown
Owner Author

Review by Kai

The PR is clean. The prior review's suggestion about _validate_chat_id test coverage has been addressed — the new assertions in test_validates_required_fields (lines 354-357) cover negative IDs, positive IDs, zero, and non-numeric strings.

No new issues to raise:

  • Validator (install.py:176): Correct. int() handles the full valid range; empty-string case is handled at the call site, not inside the validator.
  • Prompt loop (install.py:382): Pattern is consistent with the rest of _cmd_config. Empty accepted, non-zero integer accepted, anything else re-prompts.
  • Env dict (install.py:448): Guarded by if github_notify_chat_id — the "0"-rejection in the validator means a zero ID can never reach this point, but the guard is still correct on its own terms.
  • Test input list (tests/test_install.py:294): Position of the new "" entry matches the new prompt's position in the function.

Ready to merge.

@dcellison dcellison merged commit 48761f9 into main Mar 26, 2026
1 check passed
@dcellison dcellison deleted the feature/config-github-notify-chat-id branch March 26, 2026 17:57
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.

Add GITHUB_NOTIFY_CHAT_ID to make config prompts

2 participants