Repository navigation
Conversation
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
[Low impact] The change affects stream-error messages in the hidden agent command. The PR appears safe to merge, with a non-blocking fix recommended for the time-dependent test. Findings
Fix with agent prompt### Issue 1
internal/agent/stream_test.go:67
The HTTP-date test uses real time but expects exactly 17 seconds. `http.TimeFormat` drops fractions of a second, so crossing the next second before `describeError` runs can reduce the countdown to 16 seconds. A busy test runner can then fail even when the code is correct. Use a fixed clock, such as `testing/synctest`, for this case.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryReplaces error-string parsing with
Reviews (1) · Last reviewed commit: "feat(agent): use typed rootly-go stream ..." · Reviewed by Greptile |
| } | ||
|
|
||
| func TestDescribeError(t *testing.T) { | ||
| retryDate := time.Now().Add(17 * time.Second).UTC().Format(http.TimeFormat) |
There was a problem hiding this comment.
Countdown test can fail randomly
The HTTP-date test uses real time but expects exactly 17 seconds. http.TimeFormat drops fractions of a second, so crossing the next second before describeError runs can reduce the countdown to 16 seconds. A busy test runner can then fail even when the code is correct. Use a fixed clock, such as testing/synctest, for this case.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/agent/stream_test.go
Line: 67
Comment:
**Countdown test can fail randomly**
The HTTP-date test uses real time but expects exactly 17 seconds. `http.TimeFormat` drops fractions of a second, so crossing the next second before `describeError` runs can reduce the countdown to 16 seconds. A busy test runner can then fail even when the code is correct. Use a fixed clock, such as `testing/synctest`, for this case.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
rootly agent AI chat mode with terminal UX
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Real-terminal E2E: local Rails + real LLMI ran these tests on frozen binaries built from Native metric charts
Selection, scroll, paste
Earlier, on Not verified
All QA sessions are archived and the local Rails state is restored. |
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
There was a problem hiding this comment.
✅ Devin resolved all 13 findings on 21de752
Fixed by Devin (13)
- Line chart loses its final drop
- Step charts omit later measurements
- Indented code examples become charts
- Step charts show sloped transitions
- Bar charts omit numeric values
- Line charts conceal missing samples
- Skill lookup failures discard prompts
- Chat transcript misses the terminal
- Redirected chats lose their transcript
- Approved activities appear as failures
- Missing approval fields disappear without preview fields
- Truncated chat streams report success
- Debug logs expose chat prompts
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
…pacing Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
…rected Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
…aced turns, full-screen pickers Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
… and null gaps in metric charts Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Co-Authored-By: Quentin Rousseau <quentin@rootly.com>
Summary
Adds the opt-in
rootly agent(aliaschat) AI chat mode, hidden for now, plus the terminal UX on top of it. This now includes the phase-1 work from #73 (closed as superseded), so phases 1 and 2 land together.Core (phase 1)
/v1/ai/chat/streamthrough rootly-goStreamAiChatEvents, rendering into a Bubble Tea chat with Glamour Markdown and a status spinner.confirmation_requiredpauses for y/N and resumes withcheckpoint_id+decision. An expired card is re-checked before resuming.-p "q"or piped stdin runs one turn and prints to stdout.rootlyand all existing subcommands are unchanged.Builds on rootly-go
v0.15.0(typed stream errors from rootlyhq/rootly-go#206, sessiontitle).Errors
*rootly.AiChatStreamStatusErroris mapped to friendly messages for 401, OAuth scope 403 and 429. A 429 saysretry in Ns, usingRetry-After.AI chat is not enabled for this team, or/users/mereturningfeature_flags["ai-chat"] == false, gives: Rootly AI chat isn't enabled for your organization. Contact your Rootly account team or Rootly support… This check runs before the first turn. If the key is missing, the CLI proceeds as before.Layout (full screen, fixed footer)
tea.View.AltScreen, no mouse capture so native select/Cmd+C work; scroll with PgUp/PgDn, Ctrl+Home/End). Print mode, piped stdin and non-TTY output are unchanged.ESC[2J ESC[3J ESC[H). Turn it off with--no-clear.bubbles/viewportover an in-model transcript (m.printlnappends a block; the wrapped text is cached per width) plus the live tail. Footer = rule,❯composer, rule, hint; it's always on the last rows. The/menu and the delete prompt are overlays between body and footer./resume//sessionsand/skillstake over the whole screen; Esc clears the filter, then returns to the chat unchanged.↓ new output · ctrl+end to jump. While an overlay is open, it gets the paging keys.exit,/exit, double Ctrl+C, errors),Transcript()is printed to the normal screen so the chat stays in terminal scrollback, followed byResume with: ….rootly agent | tee chat.log): the UI draws on stderr, and stdout gets only the plain ANSI-free transcript at exit; the terminal gets the coloured copy too. If neither stream is a TTY, it errors with ause -phint.Header
/new,/clearand/resume, and scrolls away. Each new header replays the ECG animation (draw-in, then beat, about 1.6s; static with--no-color).welcome.go,#7C5CE6) with 4 lines beside it:✦ Rootly AI vX, user · team, API host, then the session (INC-8 · title, alert, the resumed title, orNew session). Below 50 columns it drops the logo.--no-coloror low-colour terminals.Transcript
❯plus plain text, with no background band. The answer starts with●and has noAssistant:label. One blank line separates the prompt, the answer and the turn footer.✓ Searched incidents (1.2s).Triaging…,Paging the hamsters…). The footer reuses it:✻ Triaged for 2s · done 10:04 AM(the session id is in/statusandResume with:).Resume with: rootly agent --resume <uuid>.Approve? [y/N].tea.PasteMsg) goes to the composer (idle or streaming) or the open picker filter, never to a y/n prompt.metric-chartfence fromAi::SurfaceRenderers::MetricResult(api surface) is drawn natively — pie/bar as eighth-block bars (13 · 52%), line/step viaasciigraph(plotted at width (n-1)·k+1 so every point and peak is exact; step as real steps; nulls as gaps) — instead of raw JSON; the Markdown table still follows.[Add to dashboard](#ai-add-to-dashboard:…)becomesAdd to dashboard: open in web→<web>/account?chat=<session id>. JSON/stream-JSON output stays raw.--incident/--alertbinding shows once, on the header's 4th line./statusshows it asbinding INC-8: ….Auth
rootly logoutalso clears the cached OAuth dynamic client registration (client_id/scopes), so the nextloginre-registers with the current scopes,ai.*included. A stale registration was getting "requested scope is invalid" from prod.Flags
-r/--resume <uuid>and-c/--continue.--continuepicks the latest non-archived session. History is loaded from the server session APIs, with no local storage.-r <uuid>shows the session title on the header's 4th line, from thetitlefield ofGET /v1/ai/chat/sessions/:id/messages(rootlyhq/rootly#25957);/statusshows it too. Empty orUntitledfalls back to the short id.--incident <id>/--alert <id>bind the first turn. These four flags are mutually exclusive.--output-format text|json|stream-jsonworks in print/stdin mode only.Slash commands (
/helplists them in this order)/exit,/quit, or plainexit/quit./newand/clear./resume(alias/sessions): abubbles/listpicker with filtering. Load more pages throughbefore=next_before, so it handles 1,000+ sessions./rename [title],/pin,/unpinand/archivecall PATCH on the session./deleteasks y/n first./copyuses OSC-52 viatea.SetClipboard./export [file]writes a Markdown transcript and never overwrites an existing file./skills [query]: a picker overGET /v1/ai_skills(25 per page withpage[number]/page[size], sorted by name, Load more; the query goes tofilter[search]). Rows look like✔ /incident-retrowithname · v3 · you + model. Enter only inserts/<slug>into the composer and never toggles a skill. A disabled or model-only skill says why. With AI Skills off: AI Skills isn't enabled for this organization./incident <id>,/alert <id>,/statusand/help./opens a filtered command menu: Tab completes, Esc closes. After the built-ins, it suggests enabled user-invocable skills, fetched with a 150ms debounce and cached per prefix./<slug>, the form the server'sLoadSkill#user_named_skill?matches, so the text is sent unchanged. Built-ins win: a skill whose slug equals a built-in (status,new, …) is hidden and can't be typed. Any other/tokenis checked withfilter[slug]. A matching enabled user-invocable skill is sent as a normal prompt; otherwise you getUnknown command /foo.Existing commands (
rootly incidents list, etc.) and barerootlyare unchanged.Risk: low.
agentis still a hidden command. Rollback: revert this PR.Depends on (all merged): rootlyhq/rootly-go#206, rootlyhq/rootly#25887 (
ai-chatin/users/me), rootlyhq/rootly#25957 (sessiontitle).Link to Devin session: https://rootly.devinenterprise.com/sessions/28ad93d126254ea18116565e125bb089
Open in Devin Desktop: https://rootly.devinenterprise.com/desktop/session/28ad93d126254ea18116565e125bb089?variant=devin
Requested by: @kwent