feat(gastown): add manual container token refresh in town settings#1102
Merged
feat(gastown): add manual container token refresh in town settings#1102
Conversation
Contributor
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Other Observations (not in diff)N/A Files Reviewed (5 files)
Reviewed by gpt-5.4-20260305 · 823,449 tokens |
chrarnoldus
approved these changes
Mar 15, 2026
The $timeFilter macro already bounds the result set sufficiently. Hardcoded LIMITs were silently truncating data in 9 panels.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| "nullifySparse": false, | ||
| "query": "SELECT blob5 AS error_message, blob1 AS event, SUM(_sample_interval) AS count FROM gastown_events WHERE $timeFilter AND blob5 != '' GROUP BY error_message, event ORDER BY count DESC LIMIT 30", | ||
| "rawSql": "SELECT blob5 AS error_message, blob1 AS event, SUM(_sample_interval) AS count FROM gastown_events WHERE $timeFilter AND blob5 != '' GROUP BY error_message, event ORDER BY count DESC LIMIT 30", | ||
| "query": "SELECT blob5 AS error_message, blob1 AS event, SUM(_sample_interval) AS count FROM gastown_events WHERE $timeFilter AND blob5 != '' GROUP BY error_message, event ORDER BY count DESC", |
Contributor
There was a problem hiding this comment.
WARNING: Keep a row cap on this high-cardinality panel
Removing the LIMIT turns the "Top Error Messages" table into an unbounded GROUP BY error_message, event over the whole time window. On production data that can return thousands of rows, which is likely to slow the ClickHouse query and make the dashboard much heavier to load.
| "nullifySparse": false, | ||
| "query": "SELECT blob2 AS user_id, SUM(_sample_interval) AS total_events, SUM(IF(blob5 != '', _sample_interval, 0)) AS error_count, SUM(IF(blob5 != '', _sample_interval, 0)) / SUM(_sample_interval) AS error_rate, SUM(_sample_interval * double1) / SUM(_sample_interval) AS avg_latency_ms, COUNT(DISTINCT blob6) AS town_count FROM gastown_events WHERE $timeFilter AND blob2 != '' GROUP BY user_id ORDER BY total_events DESC LIMIT 25", | ||
| "rawSql": "SELECT blob2 AS user_id, SUM(_sample_interval) AS total_events, SUM(IF(blob5 != '', _sample_interval, 0)) AS error_count, SUM(IF(blob5 != '', _sample_interval, 0)) / SUM(_sample_interval) AS error_rate, SUM(_sample_interval * double1) / SUM(_sample_interval) AS avg_latency_ms, COUNT(DISTINCT blob6) AS town_count FROM gastown_events WHERE $timeFilter AND blob2 != '' GROUP BY user_id ORDER BY total_events DESC LIMIT 25", | ||
| "query": "SELECT blob2 AS user_id, SUM(_sample_interval) AS total_events, SUM(IF(blob5 != '', _sample_interval, 0)) AS error_count, SUM(IF(blob5 != '', _sample_interval, 0)) / SUM(_sample_interval) AS error_rate, SUM(_sample_interval * double1) / SUM(_sample_interval) AS avg_latency_ms, COUNT(DISTINCT blob6) AS town_count FROM gastown_events WHERE $timeFilter AND blob2 != '' GROUP BY user_id ORDER BY total_events DESC", |
Contributor
There was a problem hiding this comment.
WARNING: Bound this per-user query before shipping
This panel now groups every distinct user_id in the selected window with no LIMIT. On busy towns that can explode to thousands of rows and materially increase both query time and Grafana render time; the previous top-N cap kept the panel useful without making it unbounded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
forceRefreshContainerToken()public RPC method on TownDO andrefreshContainerTokentRPC mutation with ownership verificationCloses #1101
Verification
pnpm typecheck— all 28 workspace projects pass cleanlyVisual Changes
New "Container" section at the bottom of the town settings page with a "Refresh Token" button (spinning icon while pending, success/error toasts).
Reviewer Notes
refreshContainerTokenin Town.do is private and throttled to 1h. The newforceRefreshContainerTokenbypasses the throttle but still updateslastContainerTokenRefreshAtso the next alarm-driven refresh doesn't fire unnecessarily.router.d.tstype bridge file was updated manually to include the new procedure in both the innergastownRouterand outerwrappedGastownRoutertypes.