Skip to content

fix(api): log every Telegram callback press - #195

Merged
mashkovd merged 2 commits into
mainfrom
fix/log-telegram-callbacks
Oct 28, 2026
Merged

mashkovd merged 2 commits into
mainfrom
fix/log-telegram-callbacks

Conversation

@mashkovd

Copy link
Copy Markdown
Member

Why

Owner presses of the scout Create spike N button produce nothing visible and no line in the mctl-agent log. Three paths in handleTelegramCallback were silent: callback data that does not match scout:create:N, a message without a keyboard ("message unavailable"), and success. That leaves no way to tell a press that reached the agent from one Telegram never delivered.

What

  • An Info line telegram callback received for every press, with the callback data (bounded to 64 bytes, the Bot API limit) and whether the message is present. No chat or user id is logged, which TestScoutCallbackAuthRefusals still pins.
  • A telegram callback refused reason for unrecognised data and for a missing keyboard.
  • scout spike created on success.
  • A failed answerCallbackQuery is now logged instead of discarded.

Tests

TestScoutCallbackEveryPressIsLogged covers the three previously silent paths. Mutation-checked: dropping the arrival line, or the unrecognised-data refusal, fails it. go test ./... passes.

🤖 Generated with Claude Code

A scout Create spike press left no trace when its callback data did not
parse, when the message carried no keyboard, or when it succeeded, so a
press that did nothing on screen could not be told apart from one
Telegram never delivered. Log each press on arrival and each outcome.
@claude

claude Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Claude finished @mashkovd's task in 12s —— View job


Review of PR #195

  • Read diff and touched files
  • Post findings and verdict

Verdict: approved. P1: 0, P2: 0, P3: 1.

  • P3: the handleTelegramCallback doc comment now sits above callbackDataForLog, so it attaches to the wrong declaration (internal/api/router.go:453).

The logging is bounded and gives every press a line. Failed answerCallbackQuery calls are now logged. The new tests cover the previously silent paths. This was a static review only, so I did not run the tests.

Comment thread internal/api/router.go
// handleTelegramCallback handles an inline-button press. Refusals log only a
// reason and chat type, never chat or user ids. A press from a chat or user
// that is not allowed gets no Telegram call at all.
// callbackDataForLog bounds callback data for logging. The Bot API caps it at

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3 — The handleTelegramCallback doc comment (lines 450-452) is now directly above callbackDataForLog's doc, so Go attaches it to the wrong declaration and handleTelegramCallback has none. Move the two helpers above that comment, or move the comment back down to the function.

claude[bot]
claude Bot previously approved these changes Oct 28, 2026

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No P1/P2 findings (1 P3: misplaced doc comment). Good to merge.

@github-actions

Copy link
Copy Markdown
Contributor

Antigravity (agy) review — informational

Model: gemini-3.8-flash-high · commit 0bfbb01.

No significant issues found.

@claude

claude Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Claude finished @mashkovd's task in 22s —— View job


Review of PR #195 (commit 0104425)

  • Read diff and touched files
  • Post findings and verdict

Verdict: approved. P1: 0, P2: 0, P3: 1.

  • The earlier P3 about the misplaced handleTelegramCallback doc comment is fixed. The helpers now sit above it.
  • P3: const max in callbackDataForLog shadows the builtin max, and data[:max] can cut a multi-byte rune. Neither matters in practice because slog quotes the value.

The arrival line is bounded and carries no chat or user ids. Every previously silent path now logs a reason, and failed answerCallbackQuery calls are logged. The tests cover the three silent paths and the truncation bound. This was a static review, so I did not run the tests.

Comment thread internal/api/router.go
}
}

// callbackDataForLog bounds callback data for logging. The Bot API caps it at

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3 — const max shadows the Go 1.21+ builtin max, and data[:max] can cut a multi-byte rune in half. Neither matters here (slog quotes the value), but a rename and a rune-safe cut would be tidier.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No P1/P2 findings (1 P3). Good to merge.

@github-actions

Copy link
Copy Markdown
Contributor

Antigravity (agy) review — informational

Model: gemini-3.8-flash-high · commit 0104425.

No significant issues found.

@mashkovd
mashkovd merged commit c6037c7 into main Oct 28, 2026
10 checks passed
@mashkovd
mashkovd deleted the fix/log-telegram-callbacks branch October 28, 2026 17:46
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.

1 participant