feat(api): add LabelBadge type for two-part pill badges Introduce LabelBadge struct that renders a muted label followed by an emphasized value, supporting customizable colors, text colors, shapes, and optional icons. Implements Textable interface with String(), ANSI(), HTML(), and Markdown() methods. Add HTML React formatter support to convert LabelBadge to clicky-ui Badge component. Also add DetailLevel enum and Detailable interface to text.go for multi-section detail views with context-aware rendering.#99
Conversation
…nd completions Implement Filter interface and lookup functions to enable dynamic filter metadata exposure through the RPC executor. This allows UI components to display available and selected filter options with rich formatting (styles, tooltips) and supports dependent filter narrowing based on other filter selections. Key additions: - Filter[T] interface for defining filter behavior with Key(), Label(), Lookup(), and Options() methods - buildLookupFunc() generates lookup metadata from filter definitions - buildFilterCompletionBinder() registers flag completion functions for shell and UI - resolveEntityOpts() applies filters to transform raw flag values into backend-canonical forms - LookupFunc field on EntityOperation and BulkActionInfo for RPC exposure - BindCompletions field for registering completion functions on generated commands - New clicky_lookup.go with entityLookupResponse and clickyNode types for rich filter metadata - RPC executor support for HEAD and GET __lookup=filters requests - Comprehensive test coverage in entity_filters_test.go and rpc/filter_lookup_test.go - End-to-end example in examples/entity/ demonstrating filters, admin entities, and executor-backed UI Breaking changes: buildOpts() now returns (T, error) instead of T; callers must handle error return value.
…and text input Introduce comprehensive prompt functionality for interactive terminal UIs: - Vendor gumchoose from charmbracelet/gum for single and multi-select prompts - Add PromptSelect, PromptMultiSelect, and PromptText APIs for interactive user input - Implement terminal detection, PTY handling, and screen management - Support customizable rendering, validation, and pagination - Include extensive test coverage with PTY-based integration tests - Handle terminal state restoration and cursor positioning This enables rich interactive CLI experiences with proper terminal handling across macOS and Linux.
… ownership management Implement exclusive ANSI terminal ownership control to allow prompts to safely take over the terminal from task renderers. Add new task-prompt demo command showcasing interactive select, multi-select, and text input prompts integrated with concurrent task execution. Key changes: - Add AcquirePromptTerminal() for prompt-to-task renderer handoff - Implement ansiTerminalController to coordinate terminal access - Add SetNoRender() to suppress all task output including final summaries - Add StopTask(id) to cancel specific tasks by immutable UUID - Add Task.ID() returning stable UUID independent of task name - Update go.mod dependencies (Go 1.26.1, commons v1.50.3, gomplate v3.24.77) - Add comprehensive tests for no-render and stop-task functionality This enables building interactive CLI applications where prompts can safely interrupt task rendering and restore it afterward.
…/json+clicky Change the primary content type for clicky JSON format from application/clicky+json to application/json+clicky to follow standard IANA media type conventions. The HTTP layer now accepts both the new standard format and the legacy alias for backward compatibility. BREAKING CHANGE: The Content-Type header for clicky JSON responses is now application/json+clicky instead of application/clicky+json. Clients should update their expectations, though the legacy format is still accepted in Accept headers.
… navigation Implement time range filtering with from/to parameters replacing the previous window-based approach. Add entity detail page with row action support and response body viewing. Update API to support both application/clicky+json and application/json+clicky content types. Add StopTask function to global task API for canceling specific tasks. Upgrade dependencies including Go 1.26.1 and charmbracelet UI libraries. BREAKING CHANGE: stackWindowOpts now uses From and To time.Time fields instead of UpdatedSince and Window duration fields. Content-Type header for clicky-json format changed from application/clicky+json to application/json+clicky. Refs: entity demo tests, example app routing, API response handling
…p-linking support Introduce a new /links page demonstrating Link and LinkCommand usage patterns with various browser targets (dialog, hover, expand, _clicky, _self, _window, _tab). Add team entity with list/get operations. Refactor webapp from domain-based OperationCatalog to metadata-driven EntityExplorerApp. Implement deep-link command pages that accept query params as initialValues and support autoRun. Update stack and cluster entities to use new Columns/Row API with LinkCommand rendering. Add serveLinkExamples handler to serve Clicky document with link examples. Update vite config to resolve clicky-ui from local monorepo path.
… for OpenAPI Introduce Link and LinkCommand types in the API package to support hyperlink rendering across multiple formats (HTML, Markdown, ANSI). Add comprehensive entity annotation system to capture semantic metadata about CLI commands, enabling the RPC/OpenAPI layer to expose entity operations without path guessing. This includes: - New Link and LinkCommand types with fluent builder API supporting multiple output formats - Entity annotation framework with CommandOpenAPIMeta for storing operation semantics - OpenAPI x-clicky extension to expose entity surfaces and operation metadata - Enhanced command generation to annotate all entity operations with verb, scope, and capability flags - Support for lookup and filter mode capabilities in operations The annotation system allows downstream tools to understand entity relationships and operation semantics from generated CLI commands.
Add a callback system that allows registering before/after hooks around formatting operations. This enables transforming input before formatting and output after rendering, with context forwarding for request-aware formatting. Add Link and LinkCommand types with target options (Dialog, Hover, Expand, Clicky, Self, Window, Tab) to support interactive links and command execution in formatted output. Update HTML React formatter to convert these new types to clickyNode structures with proper serialization. Add FormatWithContext variants throughout the formatting pipeline to pass caller context (e.g., *http.Request) to registered callbacks, enabling request-scoped data enrichment and transformation. Breaking change: FormatWithOptions now delegates to FormatWithContext internally; custom FormatManager implementations should override FormatWithContext instead.
…an task rendering Implement output capture and log serialization to prevent interleaving between task renderer frames and direct stdout/stderr writes. Key changes: - Add StartCapturingOutput/StopCapturingOutput to buffer bare fmt.Print writes while keeping the live renderer unaffected - Install logSerializingWriter during render loop to serialize logger output against tick renders using shared mutex - Add direct-stdout linter rule to flag fmt.Println/Printf and os.Stdout/Stderr writes that bypass the renderer - Add Println/Printf/Fprintln convenience functions that route through the active logger output - Widen ClearLines to account for log lines emitted between ticks, preventing stale frame lines from stacking - Add SetForceInteractive/IsForceInteractive to enable interactive rendering in test environments when needed - Guard all terminal writes with bufferMutex to prevent log serializer writes from splitting renderer output This fixes the bug where direct writes to stdout/stderr would shift the cursor without updating lastLines, causing the next ClearLines to undercount and leave stale summary lines stacked above the live region.
Introduce LabelBadge struct that renders a muted label followed by an emphasized value, supporting customizable colors, text colors, shapes, and optional icons. Implements Textable interface with String(), ANSI(), HTML(), and Markdown() methods. Add HTML React formatter support to convert LabelBadge to clicky-ui Badge component. Also add DetailLevel enum and Detailable interface to text.go for multi-section detail views with context-aware rendering.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (26)
WalkthroughThis pull request introduces a comprehensive interactive prompt system with filtering support for entities, adds rich text rendering for links and badges, implements context-aware formatting callbacks with HTTP integration, enhances task management with cancellation and output capture, adds direct stdout linting, and includes a complete entity demo application with E2E tests and a web UI. Changes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lint/analyzer_test.go (1)
25-53:⚠️ Potential issue | 🟠 MajorAssertions don't validate the behavior the test names claim.
analysistest.Runalways returns one*Resultper analyzed package, regardless of whether any diagnostics were produced. SoExpect(results).NotTo(BeEmpty())is essentially a tautology — it only proves the analyzer ran. Concretely:
- The
directstdout/good,directstdout/allowed, anddirectstdout/mainpkgspecs would pass even if the analyzer wrongly flagged every line in those fixtures.- The
directstdout/badspec would pass even if the analyzer produced zero diagnostics on disallowed code — i.e., if the rule is silently broken.This same weakness already affects the pre-existing
bad/goodapi.Textspecs (lines 19–29), so this PR is propagating the pattern rather than introducing it, but the new fixtures make it more pronounced.The idiomatic approach with
analysistestis to embed// want "..."comments in the testdata for expected diagnostics and letanalysistest.Runitself report mismatches (it already callst.Errorfinternally). For "allowed" cases, assert eachResult.Diagnosticsslice is empty:♻️ Suggested assertion shape
- It("flags direct stdout/stderr writes", func() { - testdata := analysistest.TestData() - results := analysistest.Run(GinkgoT(), testdata, lint.Analyzer, "directstdout/bad") - Expect(results).NotTo(BeEmpty()) - }) + It("flags direct stdout/stderr writes", func() { + testdata := analysistest.TestData() + // `// want "..."` markers in directstdout/bad/bad.go drive validation; + // also assert at least one diagnostic was produced. + results := analysistest.Run(GinkgoT(), testdata, lint.Analyzer, "directstdout/bad") + var total int + for _, r := range results { + total += len(r.Diagnostics) + } + Expect(total).To(BeNumerically(">", 0), "expected directstdout/bad to produce diagnostics") + }) + + It("allows legitimate os.Stdin / inspection / buffer writes", func() { + testdata := analysistest.TestData() + results := analysistest.Run(GinkgoT(), testdata, lint.Analyzer, "directstdout/good") + for _, r := range results { + Expect(r.Diagnostics).To(BeEmpty(), "directstdout/good should not be flagged") + } + })Apply the same
BeEmpty()shape todirectstdout/allowedanddirectstdout/mainpkg.Note:
analysistest.Runinvokes the test object'sErrorfon// wantmismatches.GinkgoT()adapts to that, so embedding// wantlines in the bad fixtures is the most precise approach.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lint/analyzer_test.go` around lines 25 - 53, The tests currently assert only that analysistest.Run(GinkgoT(), testdata, lint.Analyzer, ...) returned a non-empty results slice, which merely proves the analyzer ran; instead, rely on analysistest's embedded "// want" comments for bad fixtures and explicitly assert that Result.Diagnostics are empty for allowed fixtures. Update the specs that call analysistest.Run (using GinkgoT() and receiving results) so that "bad" cases rely on // want comments in the testdata (remove Expect(results).NotTo(BeEmpty()) for those), and for the allowed/skip cases (e.g., directstdout/good, directstdout/allowed, directstdout/mainpkg and the api.Text good/allowed cases) assert each returned results[i].Diagnostics slice is empty (iterate results and check Result.Diagnostics for emptiness) rather than asserting only that results is non-empty.
🟡 Minor comments (18)
task/manager_output_test.go-44-45 (1)
44-45:⚠️ Potential issue | 🟡 MinorTime-based wait may be flaky; no public flush mechanism available.
The 20ms
time.Sleepcreates a race condition. Under heavy CI load or with-race, the reader goroutines spawned byStartCapturingOutput()may not finish reading and buffering all three written lines into the manager's output buffer beforeStopCapturingOutput()is called. This can cause intermittent test failures where "charlie" or other lines are missing.The implementation has an internal
Flush()method on the buffering writer, but it is not exposed via the public API. To robustly wait for capture to complete, consider:
- Extending the public API with a sync/flush method (e.g.,
FlushCapturedOutput()) that waits for reader goroutines to drain.- Alternatively, refactor the test to poll the captured output buffer (if made accessible) with a deadline, instead of a fixed sleep.
For now, if keeping the sleep approach, document the risk and consider increasing the timeout or adding comments explaining the race condition.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@task/manager_output_test.go` around lines 44 - 45, The time.Sleep(20 * time.Millisecond) is flaky because reader goroutines started by StartCapturingOutput() may not finish before StopCapturingOutput(); remove the fixed sleep and either (A) add a public FlushCapturedOutput() method that calls the buffering writer's internal Flush() and blocks until readers drain, then call FlushCapturedOutput() in the test between writes and StopCapturingOutput(), or (B) change the test to poll the captured output (via an accessor on the manager or by exposing the buffer) with a deadline until all expected lines (alice, bob, charlie) appear before calling StopCapturingOutput(); update references to StartCapturingOutput(), StopCapturingOutput(), and the buffering writer's Flush() in the implementation/test accordingly.examples/README.md-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorLikely typo:
enitity→entity.The directory name
enitity/(referenced in the README,examples/enitity/webapp/..., e2e specs, etc.) appears to be a misspelling ofentity. Since this propagates into import paths, server URLs, and documentation, it's worth fixing once now (rename the directory and update references) rather than after consumers start linking to it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/README.md` at line 8, Rename the misspelled examples/enitity directory to examples/entity and update all references: change README.md line entries referencing "enitity/" and any paths like examples/enitity/webapp/... to examples/entity/..., update import paths in source files, e2e spec file references and any server URLs or documentation strings that include "enitity", and run/verify tests and example scripts to ensure no broken imports or paths remain (search for the token "enitity" and replace with "entity" across the repo).examples/enitity/webapp/tsconfig.tsbuildinfo-1-1 (1)
1-1:⚠️ Potential issue | 🟡 MinorDon't commit
tsconfig.tsbuildinfo— it's a generated build artifact.
*.tsbuildinfois the TS incremental build cache. Committing it causes noisy diffs on every build, can leak absolute paths from contributors' machines, and serves no purpose for other developers (TypeScript regenerates it locally). Please remove the file from the PR and add an ignore entry.🛠 Suggested `.gitignore` addition
+# TypeScript incremental build cache +*.tsbuildinfoAlso run:
git rm --cached examples/enitity/webapp/tsconfig.tsbuildinfo🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/enitity/webapp/tsconfig.tsbuildinfo` at line 1, Remove the generated TypeScript build artifact examples/enitity/webapp/tsconfig.tsbuildinfo from the commit, add a gitignore entry for *.tsbuildinfo (or tsconfig.tsbuildinfo) so it isn’t tracked, and unstage/remove the cached file from the repo (e.g., run git rm --cached for the file) before committing; ensure the updated .gitignore is committed so future builds won’t add tsbuildinfo files.clicky_lookup.go-51-61 (1)
51-61:⚠️ Potential issue | 🟡 MinorAdd explicit cases for
Link/LinkCommand/LabelBadgeintoClickyNodeto matchhtml_react_formatter.go.All filter
Options()implementations currently return onlyapi.Text, but the newapi.LabelBadgetype introduced in this PR (and existingapi.Link/api.LinkCommand) lack explicit handling intoClickyNode. Without them, any future filter that includes badges/links will silently degrade to plain text nodes, losingHref,Target, command args, and badge styling. Add cases mirroringhtml_react_formatter.go'sconvertLink,convertLinkCommand, andconvertLabelBadgeto preserve metadata in lookup responses.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clicky_lookup.go` around lines 51 - 61, The toClickyNode function currently falls through to a plain text node for unknown api types, causing api.Link, api.LinkCommand, and api.LabelBadge to lose metadata; add explicit cases in toClickyNode to handle api.Link (preserve Href, Target, Text), api.LinkCommand (preserve command name/args and display text), and api.LabelBadge (preserve badge text and style) following the conversions implemented in html_react_formatter.go (see convertLink, convertLinkCommand, convertLabelBadge) so lookup responses retain link hrefs/targets, command args, and badge styling instead of degrading to plain text.examples/enitity/webapp/src/index.css-2-3 (1)
2-3:⚠️ Potential issue | 🟡 MinorFragile cross-repo path assumption affects both CSS and JS imports.
The relative paths
../../../../../clicky-ui/...inindex.css(and similarly inmain.tsx) assumeclicky-uiis checked out as a sibling directory toclicky. This is confirmed by thevite.config.tsaliases pointing to the same sibling location, but the CSS imports bypass the package manager entirely, making the configuration brittle for CI builds and non-standard directory layouts.The
vite.config.tsalready establishes@flanksource/clicky-uialiases; consider using those in CSS via Tailwind configuration, or migrate both CSS and JS imports to reference a workspace dependency or published package so the paths are stable and environment-agnostic.The stylelint warning on
@sourceis a false positive — it's a valid Tailwind v4 directive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/enitity/webapp/src/index.css` around lines 2 - 3, The CSS uses brittle repo-relative imports in index.css (`@import "../../../../../clicky-ui/...";` and `@source ...`) that bypass package resolution; replace those with the package alias already defined in vite.config.ts (use `@flanksource/clicky-ui` in both CSS and JS) or import the published/workspace package entry instead, and update main.tsx to import styles from the package rather than a relative path; keep the Tailwind v4 `@source` directive as-is but ensure any CSS tooling (postcss/tailwind config) can resolve the `@flanksource/clicky-ui` alias so CI and alternate repo layouts work reliably.lint/direct_stdout.go-28-33 (1)
28-33:⚠️ Potential issue | 🟡 MinorDoc comment overpromises vs. implementation.
Two scope claims here are not actually implemented:
- "clicky's own module (package path prefix github.com/flanksource/clicky)" —
skipDirectStdoutFilenever inspectspass.Pkg.Path(), so this skip does not happen. Either add the check or drop the bullet.- "on the enclosing file, function, or statement" —
hasAllowStdoutDirectiveonly walksfile.Commentsat file scope (and lines 167-169 acknowledge this). The function/statement granularity advertised here misleads readers writing the directive next to a single statement.Reconcile the doc with the implementation (or extend the implementation to match), so users don't expect inline opt-outs that silently don't work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lint/direct_stdout.go` around lines 28 - 33, The docstring claims two scopes that the code doesn't implement: update skipDirectStdoutFile to actually check the package path by reading pass.Pkg.Path() and skip files whose package path has the prefix "github.com/flanksource/clicky", or remove that bullet from the comment; and either extend hasAllowStdoutDirective to search for comment groups attached to function and statement AST nodes (not just file.Comments) so inline `//clicky:allow-stdout` next to a function or statement is honored, or change the doc to state the directive only works at file scope (update the comment near hasAllowStdoutDirective and lines acknowledging file-scope behavior). Ensure you reference the functions skipDirectStdoutFile and hasAllowStdoutDirective and the use of pass.Pkg.Path() and file.Comments when making the change.examples/enitity/webapp/src/api.ts-11-25 (1)
11-25:⚠️ Potential issue | 🟡 Minor
delete remaining.argsquietly drops a flag namedargs.Inside the substitution loop,
delete remaining.argsfires every time any path segment is replaced — not just when the substituted key isargs. This is presumably a workaround for the cobra "args" fallback positional name introduced byrpc/converter.go(see theextractParameterName(...) == ""branch in that file), so that when a real positional name likepolicyNumberis substituted into the path, the redundantargsquery param isn't also sent.But:
- It silently drops a user-defined flag legitimately named
args.- It only fires when some path segment was substituted, and only for the first iteration that happens to match — readers can't see the cobra-coupling rationale from this file.
At minimum, a comment explaining the coupling would help; better, scope the deletion to the case where the substituted key was the positional name (i.e., the converter's "single positional → path param" path), or move this normalization server-side.
♻️ Suggested clarification
for (const [key, value] of Object.entries(params)) { if (resolved.includes(`{${key}}`)) { resolved = resolved.replace(`{${key}}`, encodeURIComponent(value)); delete remaining[key]; - delete remaining.args; + // The Go converter falls back to the synthetic "args" positional + // when no real name is extractable from cobra `Use`. Once a real + // positional has been substituted into the path, "args" is the + // redundant fallback and must not also be sent as a query param. + delete remaining.args; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/enitity/webapp/src/api.ts` around lines 11 - 25, The loop in substitutePath currently unconditionally deletes remaining.args whenever any path param is substituted, which accidentally drops a legitimate user flag named "args"; change the deletion so it only removes remaining.args when the substituted key is literally "args" (i.e., inside the if (resolved.includes(`{${key}}`)) branch, replace the unconditional delete with a conditional delete when key === "args"), and add a short comment referencing the cobra/rpc converter fallback (the extractor that uses "args" as a positional fallback) so future readers understand the coupling.lint/direct_stdout.go-170-179 (1)
170-179:⚠️ Potential issue | 🟡 MinorTighten directive matching beyond
strings.Contains.
strings.Contains(c.Text, allowStdoutDirective)will silence the rule for any comment in the file that mentions the directive substring — including prose such as// don't reach for //clicky:allow-stdout hereor doc strings discussing the directive (e.g., this very file once it's merged into a docs blob). Go directive conventions are usually//directivewith no leading space and the directive token at column 0 of the comment text.♻️ Suggested tightening
func hasAllowStdoutDirective(file *ast.File) bool { for _, cg := range file.Comments { for _, c := range cg.List { - if strings.Contains(c.Text, allowStdoutDirective) { + // Match `//clicky:allow-stdout` as a directive, not a substring + // of arbitrary prose. Mirrors how `//go:` directives are parsed. + text := strings.TrimPrefix(c.Text, "//") + text = strings.TrimSpace(text) + if text == allowStdoutDirective || + strings.HasPrefix(text, allowStdoutDirective+" ") { return true } } } return false }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lint/direct_stdout.go` around lines 170 - 179, Replace the loose substring check in hasAllowStdoutDirective: instead of strings.Contains(c.Text, allowStdoutDirective) require the directive to appear as a top-level comment token. For line comments check strings.HasPrefix(c.Text, "//"+allowStdoutDirective) and for block comments split c.Text into lines and check each trimmed line for a prefix of "//"+allowStdoutDirective (and treat directive end-of-token by ensuring the next rune is whitespace or EOF); use the allowStdoutDirective constant and the hasAllowStdoutDirective function to locate where to change the logic.task/force_interactive_test.go-41-55 (1)
41-55:⚠️ Potential issue | 🟡 MinorTest does not actually verify the env-var → force-interactive path.
This test sets
CLICKY_FORCE_INTERACTIVE=1, checks viaos.Getenvthat it is set, then manually callsSetForceInteractive(true). The success ofIsForceInteractive()at line 52 is therefore due to the explicitSetForceInteractive(true)call, not the env var. Sinceinit()runs before any test, you cannot retroactively re-trigger it from inside a test.To actually exercise the env-var path, factor the env-var read out of
init()into a helper (e.g.applyForceInteractiveFromEnv()) that the test can invoke aftert.Setenv. As written, this test is a no-op tautology.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@task/force_interactive_test.go` around lines 41 - 55, The test currently sets CLICKY_FORCE_INTERACTIVE and then calls SetForceInteractive(true) directly, so it never exercises the env-var reading done in init(); extract the env-var logic from init() into a new helper (e.g. applyForceInteractiveFromEnv()) that reads CLICKY_FORCE_INTERACTIVE and calls SetForceInteractive accordingly, update init() to call that helper, and change TestForceInteractiveEnvVarSetsFlag to call t.Setenv("CLICKY_FORCE_INTERACTIVE","1") followed by applyForceInteractiveFromEnv() and then assert IsForceInteractive(); reference IsForceInteractive, SetForceInteractive and init() when making the change.rpc/filter_lookup_test.go-79-104 (1)
79-104:⚠️ Potential issue | 🟡 MinorGlobal entity registration should use unique test-specific names to avoid interference.
clicky.RegisterEntitymutates package-global state. Whilerpc/entities_test.gocorrectly mitigates this by registering a uniquely-named entity ("rpc-entities-test"), this test uses the generic name"rpc-filter-entity"which could collide if the test runs in parallel or if other tests in the same package register with the same name. Adopt the same mitigation strategy: use a unique identifier in the entity name, such as"test-filter-lookup-rpc-filter-entity"or similar.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rpc/filter_lookup_test.go` around lines 79 - 104, TestSwaggerServer_FilterLookupRoutes registers a package-global entity via clicky.RegisterEntity using the non-unique Name "rpc-filter-entity", which can collide with other tests; update the Name in the registration inside TestSwaggerServer_FilterLookupRoutes to a test-scoped unique string (e.g. prefix with "test-filter-lookup-") so the clicky.RegisterEntity call registers a distinct entity, leaving the Filters, List, and BulkActions (and the function names/closures) unchanged.examples/enitity/main.go-909-913 (1)
909-913:⚠️ Potential issue | 🟡 MinorUnknown
/api/*paths fall through to the SPA handler.
mux.Handle("/", uiHandler)is the catch-all, so any request that does not match a more specific registered API pattern (e.g. a typo in/api/v1/..., or an executor route that fails to register) will be servedindex.htmlwith HTTP 200 instead of returning a proper 404. Clients (and tests) will then have to disambiguate API errors from SPA HTML by content-sniffing. Consider scoping the SPA handler under a non-/apiprefix or short-circuiting requests whose path starts with/api/before falling back toserveIndex.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/enitity/main.go` around lines 909 - 913, The SPA catch-all mux.Handle("/", uiHandler) lets unknown /api/* paths fall through to index.html; update routing so API paths are not swallowed by the SPA: either register the SPA under a non-/api prefix (e.g. mount uiHandler at "/" but only after explicitly registering all API routes) or add a short-circuit before uiHandler to detect req.URL.Path starting with "/api/" and return a 404 (or pass to mux.NotFound) instead; modify the main routing logic where newWebappHandler, uiHandler, and mux are used to implement this check so unknown API endpoints no longer get served the SPA.prompt_test.go-213-229 (1)
213-229:⚠️ Potential issue | 🟡 MinorTime-based sync is likely to flake in CI.
Several tests use fixed
time.Sleepto wait for the helper to reach the prompt before sending keystrokes (e.g.time.Sleep(500*time.Millisecond)here,time.Sleep(800*time.Millisecond)inTestPromptStopsTaskRenderingBeforePrompt). On a loaded CI runner the helper may not have rendered the prompt yet, causing the arrow key / Enter to be consumed before the bubbletea program is reading input — the test then fails or hangs.
TestPromptTextValidationalready demonstrates the right pattern viawaitForCapturedText. Switching the other PTY tests to wait for a prompt-specific marker (e.g. the prompt title or>cue) before writing keystrokes will make them deterministic.Also applies to: 309-320
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@prompt_test.go` around lines 213 - 229, Replace the fragile time.Sleep waits in the PTY test that uses startPromptHelper("choose_confirm") with deterministic waiting: use the existing waitForCapturedText helper to block until the prompt-specific marker (e.g. the prompt title or the prompt cue like ">" or the option text that appears before you send keystrokes) appears in capture.String(), then send the arrow key and Enter. Update the test that currently writes to ptmx after time.Sleep(500*time.Millisecond) (and the similar case at lines 309-320) to call waitForCapturedText(capture, "expected prompt marker") before ptmx.Write so the keystrokes are delivered only after the bubbletea prompt is rendered; keep the rest of the assertions (requireExitWithin, getTerminalFlags, assertFlagsMatch, and checking clickytext.StripANSI output) unchanged.prompt_test.go-415-435 (1)
415-435:⚠️ Potential issue | 🟡 Minor
requireExitWithinsilently passes on unexpectedWaiterrors.If
cmd.Wait()returns a non-*exec.ExitError(e.g. an I/O error while reading the child’s exit), the type assertion at line 428 fails and the function falls through to the end without callingt.Fatalf, so the test reports success despite the helper crashing. Recommend tightening:♻️ Proposed change
case err := <-done: if err == nil { return } - if exitErr, ok := err.(*exec.ExitError); ok && exitErr.ExitCode() != 0 { - t.Fatalf("helper exited with error: %v", err) - } + t.Fatalf("helper exited with error: %v", err)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@prompt_test.go` around lines 415 - 435, The helper function requireExitWithin silently ignores non-*exec.ExitError failures from cmd.Wait(); update requireExitWithin to treat any non-nil error from cmd.Wait() as a test failure by calling t.Fatalf with the error (and context) when err != nil, rather than only handling *exec.ExitError; reference cmd.Wait(), the done channel handling, and the exitErr type assertion so you replace the current fall-through behavior with a generic failure path that logs unexpected error types.rpc/serve.go-622-631 (1)
622-631:⚠️ Potential issue | 🟡 MinorHeaders already flushed before encode error path runs.
w.Header().Set(...)+w.WriteHeader(http.StatusOK)commits the response, so the subsequenthttp.Error(...)on encode failure cannot change the status code or content-type — it will just append a plaintext error line after a partial JSON body, producing a malformed response that still claims200 application/json+clicky. Either encode into a buffer first, or drop thehttp.Errorand just log the error.♻️ Proposed change
- w.Header().Set("Content-Type", "application/json+clicky") - w.WriteHeader(http.StatusOK) - - if strings.EqualFold(r.Method, http.MethodHead) { - return - } - - if err := json.NewEncoder(w).Encode(data); err != nil { - http.Error(w, fmt.Sprintf("failed to encode lookup response: %v", err), http.StatusInternalServerError) - } + body, err := json.Marshal(data) + if err != nil { + http.Error(w, fmt.Sprintf("failed to encode lookup response: %v", err), http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json+clicky") + w.WriteHeader(http.StatusOK) + if strings.EqualFold(r.Method, http.MethodHead) { + return + } + _, _ = w.Write(body)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rpc/serve.go` around lines 622 - 631, The response is committed before json encoding, so on encode failure the handler can’t change status or headers; fix by encoding into a buffer (e.g., use json.Marshal or encode into a bytes.Buffer) and only call w.Header().Set(...) and w.WriteHeader(http.StatusOK) and write the buffer to w after a successful encode, keeping the HEAD check in place; remove or replace the http.Error(w, ...) call with logging if encode fails so you don't attempt to mutate a committed response.examples/enitity/webapp/src/LinkExamplesPage.tsx-224-238 (1)
224-238:⚠️ Potential issue | 🟡 MinorComma-joined
argscan corrupt positional values that contain,.
commandInitialValuesjoins remaining positional args with","intovalues.args(line 228), which is then placed verbatim into a single query parameter. If any positional argument contains a comma, the receiver (LinkExampleCommandPage'sinitialValues) cannot recover the original tokens. Consider repeating the parameter (URLSearchParams.append) or using a delimiter that cannot appear in arg values, and document the contract on both sides.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/enitity/webapp/src/LinkExamplesPage.tsx` around lines 224 - 238, The current assembly of remaining positional args into a comma-joined string (values.args) corrupts tokens that contain commas; change the serialization so the receiver can reconstruct the original tokens—either (preferred) emit repeated query params by appending each remaining arg under the same key (use URLSearchParams.append for key "args" when building the link) or else serialize the array safely (e.g., JSON.stringify + encodeURIComponent) and update LinkExampleCommandPage's initialValues to decode/JSON.parse; locate and update the code that builds values.args (the loop over positionalParams and the assignment to values.args) and update LinkExampleCommandPage.initialValues decoding to match the chosen format.rpc/openapi.go-450-452 (1)
450-452:⚠️ Potential issue | 🟡 MinorAdmin description reads awkwardly.
When
Admin == true,entry.surface.Titleis"Admin — Foo"; the fallback then produces"Manage admin — foo resources.". Build the fallback fromentry.baseKey(or the un-prefixed title) and prepend the admin qualifier separately, e.g."Manage Foo resources (admin).".🛠️ Suggested fix
- if entry.surface.Description == "" { - entry.surface.Description = fmt.Sprintf("Manage %s resources.", strings.ToLower(entry.surface.Title)) - } + if entry.surface.Description == "" { + noun := strings.ToLower(clickyTitleCase(entry.baseKey)) + if entry.surface.Admin { + entry.surface.Description = fmt.Sprintf("Manage %s resources (admin).", noun) + } else { + entry.surface.Description = fmt.Sprintf("Manage %s resources.", noun) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rpc/openapi.go` around lines 450 - 452, The fallback description generation should use the un-prefixed resource name (e.g. entry.baseKey or strip the "Admin — " prefix from entry.surface.Title) instead of entry.surface.Title so you don't get "admin — foo" inside the sentence; update the block that sets entry.surface.Description to build the base name from entry.baseKey (or remove the admin prefix) and then output something like "Manage Foo resources (admin)." when the entry denotes an admin surface (check the Admin flag or detect the "Admin" prefix on entry.surface.Title) so the admin qualifier is appended separately.internal/gumchoose/choose.go-370-390 (1)
370-390:⚠️ Potential issue | 🟡 MinorTrailing newline is always emitted;
i != m.heightcondition is never false.In the loop at line 370,
iiterates overm.items[start:end]where the slice size is determined bypaginator.GetSliceBounds()withPerPage = m.height(line 179). Thusiranges from0toheight-1, and the conditionif i != m.heightat line 387 is alwaystrue. This means a newline is written after every item, including the visually last one on the page. The intended behavior is likely to omit the trailing newline after the final item on the page.Suggested fix
- if i != m.height { - s.WriteRune('\n') - } + if i < (end-start)-1 { + s.WriteRune('\n') + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/gumchoose/choose.go` around lines 370 - 390, The loop always emits a trailing newline because the condition checks i != m.height while i is an index into the paged slice; change the newline check to compare against the last index of the current page slice instead (e.g., compute the slice := m.items[start:end] or use the length of m.items[start:end] and only write '\n' when i != len(slice)-1). Update the loop around m.items[start:end], m.index, and m.height so the newline is omitted after the final item on the current page.prompt.go-454-475 (1)
454-475:⚠️ Potential issue | 🟡 MinorStrip ANSI escapes in
promptRenderedLineCountto avoid miscalculating line height.The function measures
len([]rune(line))directly, so ANSI SGR sequences in the title inflate the visible width and overestimatetitleHeight, which then shrinks the page size viaclampPromptPageSizeon narrow terminals.The
text.StripANSIhelper is available and already used throughout the codebase. Use it to strip sequences before measuring:text.StripANSI(line).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@prompt.go` around lines 454 - 475, In promptRenderedLineCount, strip ANSI sequences before measuring visible width to avoid inflated lengths: for each line (in the loop over strings.Split(text, "\n")), call text.StripANSI(line) (or assign stripped := text.StripANSI(line)) and base the empty check and the rune length calculation on the stripped string (use len([]rune(stripped)) when computing (lineLength-1)/width + 1); keep the rest of the logic unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7b2d9f0d-de2e-4aaf-965e-9dd67174f743
⛔ Files ignored due to path filters (3)
examples/go.sumis excluded by!**/*.sumexamples/uber_demo/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sum
📒 Files selected for processing (81)
api/labelbadge.goapi/labelbadge_test.goapi/link.goapi/parser.goapi/text.goclicky_lookup.gocobra_command.goentity.goentity_annotations.goentity_filters_test.goexamples/README.mdexamples/enitity/e2e/.gitignoreexamples/enitity/e2e/playwright.config.tsexamples/enitity/e2e/tests/entity-demo.spec.tsexamples/enitity/main.goexamples/enitity/main_test.goexamples/enitity/webapp/.gitignoreexamples/enitity/webapp/src/App.tsxexamples/enitity/webapp/src/LinkExamplesPage.tsxexamples/enitity/webapp/src/api.tsexamples/enitity/webapp/src/index.cssexamples/enitity/webapp/src/main.tsxexamples/enitity/webapp/tsconfig.tsbuildinfoexamples/enitity/webapp/vite.config.tsexamples/go.modexamples/uber_demo/go.modexamples/uber_demo/main.goformat.goformat_callback_test.goformat_callbacks.goformatters/callbacks.goformatters/html_react_formatter.goformatters/html_react_formatter_test.goformatters/http/http.goformatters/http/http_test.goformatters/manager.goglobal_task_api.gogo.modinternal/gumchoose/LICENSEinternal/gumchoose/choose.gointernal/gumchoose/choose_test.golint/analyzer.golint/analyzer_test.golint/direct_stdout.golint/testdata/src/directstdout/allowed/allowed.golint/testdata/src/directstdout/bad/bad.golint/testdata/src/directstdout/good/good.golint/testdata/src/directstdout/mainpkg/main.golog.golog_test.goprompt.goprompt_terminal_ioctl_darwin_test.goprompt_terminal_ioctl_linux_test.goprompt_test.gorpc/converter.gorpc/executor.gorpc/filter_lookup_test.gorpc/openapi.gorpc/serve.gorpc/serve_test.gorpc/types.gosub_command.gosub_command_test.gotask/force_interactive_test.gotask/group.gotask/log_serializer.gotask/log_serializer_test.gotask/manager.gotask/manager_lifecycle.gotask/manager_output.gotask/manager_output_test.gotask/manager_wait.gotask/no_render_test.gotask/options.gotask/render.gotask/snapshot.gotask/stop_test.gotask/task.gotask/terminal_owner.gotask/worker.gotestdata/prompt_test_helper.go
| // Register the <iconify-icon> web component so clicky-ui's <Icon> glyphs | ||
| // render (it loads icons on-demand from the Iconify CDN at runtime). | ||
| import "iconify-icon"; | ||
| import "../../../../../clicky-ui/packages/ui/src/styles/tokens.css"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether `@flanksource/clicky-ui` exports tokens.css (or a styles entry).
fd -t f 'package.json' -E 'node_modules' | xargs rg -l '"@flanksource/clicky-ui"' 2>/dev/null
echo '---'
# Check if a local copy of tokens.css exists in the example we could point at instead.
fd -t f 'tokens.css'Repository: flanksource/clicky
Length of output: 66
🏁 Script executed:
# Check the package.json in the examples/enitity/webapp directory
cat "examples/enitity/webapp/package.json"Repository: flanksource/clicky
Length of output: 131
🏁 Script executed:
# Check the directory structure at the repo root to see what exists
ls -la | head -20Repository: flanksource/clicky
Length of output: 1238
🏁 Script executed:
# Look for importmap or vite/build config
fd -t f -E 'node_modules' 'vite.config|importmap|index.html'Repository: flanksource/clicky
Length of output: 124
🏁 Script executed:
# Check if there's any reference to tokens.css in the entire repo (not just package.json)
rg 'tokens\.css' --type-list | head -5 && rg 'tokens\.css' -E 'node_modules|dist|build'Repository: flanksource/clicky
Length of output: 513
🏁 Script executed:
# List the examples directory structure
find examples -type d | head -20Repository: flanksource/clicky
Length of output: 327
🏁 Script executed:
# Read the vite.config.ts file to see how the build is configured
cat "examples/enitity/webapp/vite.config.ts"Repository: flanksource/clicky
Length of output: 1450
🏁 Script executed:
# Search for tokens.css properly
rg 'tokens\.css' -E 'node_modules|dist|build' --invert-matchRepository: flanksource/clicky
Length of output: 123
🏁 Script executed:
# Read the actual main.tsx to confirm the import line
cat "examples/enitity/webapp/src/main.tsx"Repository: flanksource/clicky
Length of output: 1076
🏁 Script executed:
# Check if there's a build/dist output and how it handles this
fd -t f 'package.json' examples/enitity/webappRepository: flanksource/clicky
Length of output: 44
🏁 Script executed:
# Verify if the path with 5 "../" actually resolves to a real file
test -f "../../../../../clicky-ui/packages/ui/src/styles/tokens.css" && echo "Path exists" || echo "Path does not exist"Repository: flanksource/clicky
Length of output: 82
🏁 Script executed:
# Check from the webapp directory specifically where the import happens
cd "examples/enitity/webapp/src" && test -f "../../../../../clicky-ui/packages/ui/src/styles/tokens.css" && echo "Path resolves from src" || echo "Path does NOT resolve from src"Repository: flanksource/clicky
Length of output: 93
🏁 Script executed:
# Check what the correct depth should be - count from webapp/src to repo root
cd "examples/enitity/webapp/src" && pwd && cd ../../../../../.. && pwdRepository: flanksource/clicky
Length of output: 115
🏁 Script executed:
# Verify the actual path from root to the tokens.css in clicky-ui
ls -la "clicky-ui/packages/ui/src/styles/tokens.css" 2>/dev/null || echo "clicky-ui sibling doesn't exist in this checkout"Repository: flanksource/clicky
Length of output: 111
🏁 Script executed:
# Check if using the configured alias would work by verifying the alias target
test -f "clicky-ui/packages/ui/dist/index.mjs" && echo "clicky-ui dist exists" || echo "clicky-ui dist does not exist"Repository: flanksource/clicky
Length of output: 92
🏁 Script executed:
# Check the README to understand how this example is meant to be run
cat README.md | grep -A 20 -B 5 "example" | head -50Repository: flanksource/clicky
Length of output: 1309
🏁 Script executed:
# Look for any CI/build configuration that might set up clicky-ui
find .github -type f -name "*.yaml" -o -name "*.yml" | xargs grep -l "clicky-ui" 2>/dev/null || echo "No clicky-ui references in CI"Repository: flanksource/clicky
Length of output: 92
🏁 Script executed:
# Check if there's a monorepo setup (package.json in root with workspaces)
cat package.json 2>/dev/null | head -50Repository: flanksource/clicky
Length of output: 44
🏁 Script executed:
# Check if there are any other imports of the alias in the codebase
rg '@flanksource/clicky-ui' examples/enitity/webapp/src --type ts --type tsxRepository: flanksource/clicky
Length of output: 90
🏁 Script executed:
# Verify the vite.config alias one more time to understand the exact alias key
grep -A 2 '@flanksource/clicky-ui/styles' examples/enitity/webapp/vite.config.tsRepository: flanksource/clicky
Length of output: 205
🏁 Script executed:
# Check if this is meant to be built/deployed or only used in dev mode
grep -r "serve\|dev" examples/enitity/webapp/package.json 2>/dev/null || echo "No package.json found"Repository: flanksource/clicky
Length of output: 84
🏁 Script executed:
# Look for how the example is actually run/built in the Go code
rg "enitity|webapp" --type go | head -20Repository: flanksource/clicky
Length of output: 841
🏁 Script executed:
# Check the exact alias syntax one more time and look for any other style imports
cat examples/enitity/webapp/vite.config.ts | grep -A 5 -B 5 'styles'Repository: flanksource/clicky
Length of output: 741
🏁 Script executed:
# Look for how other files in the webapp import styles or use the alias
rg 'import.*@flanksource' examples/enitity/webapp/srcRepository: flanksource/clicky
Length of output: 172
Use the configured @flanksource/clicky-ui/styles.css alias instead of a broken relative import.
The current import ../../../../../clicky-ui/packages/ui/src/styles/tokens.css uses five ../ segments and doesn't resolve (it reaches outside the repo entirely). The vite.config.ts already defines an alias for this file:
"@flanksource/clicky-ui/styles.css" → "../../../../clicky-ui/packages/ui/src/styles/tokens.css"
Use the configured alias instead:
♻️ Suggested fix
-import "../../../../../clicky-ui/packages/ui/src/styles/tokens.css";
+import "@flanksource/clicky-ui/styles.css";
import "./index.css";This matches how other imports in the file use @flanksource/clicky-ui and works both in dev (via vite alias resolution) and build (since vite resolves the alias during bundling).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import "../../../../../clicky-ui/packages/ui/src/styles/tokens.css"; | |
| import "@flanksource/clicky-ui/styles.css"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/enitity/webapp/src/main.tsx` at line 9, Replace the broken relative
import in main.tsx that reads
"../../../../../clicky-ui/packages/ui/src/styles/tokens.css" with the configured
Vite alias "@flanksource/clicky-ui/styles.css"; locate the import statement at
the top of main.tsx and change it to use the alias so Vite's resolver (as
defined in vite.config.ts) handles the path during dev and build.
| module examples | ||
|
|
||
| go 1.25.1 | ||
| go 1.26.1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check root go.mod and any CI workflow files for Go version alignment
fd -t f 'go.mod$' --exec sh -c 'echo "=== {} ==="; head -5 "{}"'
fd -t f -e yml -e yaml . .github 2>/dev/null | xargs -I{} sh -c 'echo "=== {} ==="; rg -n "go-version|setup-go" {} 2>/dev/null'Repository: flanksource/clicky
Length of output: 890
🏁 Script executed:
# Search for GO_VERSION environment variable definition in workflow files
rg -n "GO_VERSION\s*:" .github/workflows/ -A 1 -B 1
# Check if there's an env block at the workflow level
rg -n "env:" .github/workflows/ -A 5
# Check root go.mod for toolchain directive
head -10 ./go.mod
# Search for any toolchain references
rg -n "toolchain" .Repository: flanksource/clicky
Length of output: 2545
Update release.yml workflow to Go 1.26 or later, or downgrade examples/go.mod to match.
The examples/go.mod specifies go 1.26.1, but the release.yml CI workflow is configured with GO_VERSION: "1.25". This mismatch will cause build failures in the release pipeline. The lint.yml workflow correctly uses GO_VERSION: "1.26", which is compatible with Go 1.26.1.
Either update release.yml to use GO_VERSION: "1.26" (or later) to align with the required Go version, or downgrade the go directive in examples/go.mod to 1.25 or compatible version.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/go.mod` at line 3, The Go version mismatch between examples/go.mod
(go 1.26.1) and the release workflow (GO_VERSION: "1.25") will break CI; fix by
either updating the release workflow's GO_VERSION to "1.26" (or later) in the
release.yml CI config, or by lowering the go directive in examples/go.mod to
"1.25" to match release.yml — ensure the chosen change keeps lint.yml's
GO_VERSION compatible and update only the GO_VERSION variable in release.yml or
the "go" directive in examples/go.mod accordingly.
| package clicky | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "io" | ||
| "os" | ||
| "reflect" | ||
| "strings" | ||
| "time" | ||
|
|
||
| tea "github.com/charmbracelet/bubbletea" | ||
| "github.com/charmbracelet/huh" | ||
| "github.com/flanksource/clicky/api" | ||
| "github.com/flanksource/clicky/internal/gumchoose" | ||
| "github.com/flanksource/clicky/task" | ||
| "golang.org/x/sys/unix" | ||
| "golang.org/x/term" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the repo claims Windows support and whether sibling
# platform-specific files / build tags exist for the prompt code path.
fd -t f 'prompt.*\.go$'
echo '---'
rg -nP '^//go:build' -g '*.go' | head -n 50
echo '---'
# Look for any existing GOOS=windows handling or stubs.
rg -nP 'GOOS|//go:build\s+(windows|!windows|unix)' -g '*.go' | head -n 50
echo '---'
# Confirm /dev/tty and unix package usage are confined to this file.
rg -nP '/dev/tty|golang\.org/x/sys/unix' -g '*.go'Repository: flanksource/clicky
Length of output: 1761
Add build constraint to prompt.go to prevent Windows compilation failure.
This file imports golang.org/x/sys/unix and opens /dev/tty, both unavailable on Windows. It lacks a //go:build !windows constraint at the top. The test files (prompt_terminal_ioctl_linux_test.go, prompt_terminal_ioctl_darwin_test.go) and test logic in prompt_test.go confirm the feature is Unix-only, yet other Unix-specific code in the repo (e.g., exec/process_group_unix.go) already follows this pattern correctly. Add the build constraint to the top of the file to match the repo's conventions and prevent compilation failures on unsupported platforms.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@prompt.go` around lines 1 - 19, This file (prompt.go) is Unix-only but lacks
a build constraint, causing Windows builds to fail due to imports like
"golang.org/x/sys/unix" and opening "/dev/tty"; add a build tag at the very top
of the file (before the package declaration) such as "//go:build !windows" (and
optionally the legacy "// +build !windows" line directly beneath it) so the
compiler excludes prompt.go from Windows builds and matches the repo's Unix-only
pattern used by other files like exec/process_group_unix.go.
| func TestFormatToContentType_ClickyJSON(t *testing.T) { | ||
| assert.Equal(t, "application/clicky+json", formatToContentType("clicky-json")) | ||
| assert.Equal(t, "application/json+clicky", formatToContentType("clicky-json")) | ||
| assert.Equal(t, "text/html; charset=utf-8", formatToContentType("html-react")) | ||
| assert.Equal(t, "application/json", formatToContentType("json")) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
RFC 6839 structured syntax suffix +json convention application/something+json
💡 Result:
RFC 6839 defines the "+json" structured syntax suffix for media types whose representation follows the JSON format established by application/json (RFC 4627, updated by RFC 8259). The convention for "application/something+json" is to append "+json" to the subtype name (e.g., application/problem+json), indicating that the media type uses JSON syntax. This allows generic JSON processors to handle the underlying representation while specific processors use the full type semantics. Use the +json suffix when: - The media type identifies specific semantics (e.g., application/problem+json for HTTP error details per RFC 7807 / RFC 9457). - Generic JSON parsing is possible without special knowledge beyond standard JSON. Examples include: - application/problem+json (RFC 7807/9457): Problem details for HTTP APIs. - application/ld+json: JSON-LD. - application/vnd.api+json: JSON:API. - application/geo+json: GeoJSON. - application/activity+json: Activity Streams. Processors seeing "xxx/yyy+json" can parse as JSON generically (per RFC 6839 Section 3.1 fragment considerations). It is not a subtype of application/json for content negotiation; specific types like application/problem+json are distinct. The suffix is registered in IANA's Structured Syntax Suffix Registry. New media types register via RFC 6838 procedures.
Citations:
- 1: https://datatracker.ietf.org/doc/html/rfc6839
- 2: http://www.rfc-editor.org/rfc/rfc6839
- 3: https://tools.ietf.org/html/rfc6839
- 4: https://www.iana.org/assignments/media-type-structured-suffix
- 5: https://stackoverflow.com/questions/79687556/should-content-type-application-problemjson-be-considered-a-sub-subtype-of
- 6: https://iana.org/assignments/media-types
- 7: https://www.iana.org/assignments/media-type-structured-suffix/
- 8: https://rfc-editor.org/rfc/rfc7807.html
- 9: https://http.dev/media-types
🏁 Script executed:
# First, let's check the file structure and locate the test file
find . -name "serve_test.go" -type fRepository: flanksource/clicky
Length of output: 82
🏁 Script executed:
# Read the relevant portions of the test file
wc -l ./rpc/serve_test.goRepository: flanksource/clicky
Length of output: 86
🏁 Script executed:
# Read the entire test file
cat -n ./rpc/serve_test.goRepository: flanksource/clicky
Length of output: 15070
🏁 Script executed:
# Now let's look at the serve.go file to find the formatToContentType function
find . -name "serve.go" -type f | head -5Repository: flanksource/clicky
Length of output: 77
🏁 Script executed:
# Let's search for the formatToContentType function in serve.go
rg -n "formatToContentType" ./rpc/serve.go -A 10Repository: flanksource/clicky
Length of output: 669
🏁 Script executed:
# Let's see the full formatToContentType function
rg -n "formatToContentType" ./rpc/serve.go -A 20 | tail -30Repository: flanksource/clicky
Length of output: 865
🏁 Script executed:
# Let's also check if there are any other references to these media types in the codebase
rg -n "clicky\+json|json\+clicky" --type goRepository: flanksource/clicky
Length of output: 2060
🏁 Script executed:
# Let's check the git history or comments around the change to understand the reasoning
rg -n "json\+clicky|clicky\+json" ./rpc/serve.go -B 3 -A 3Repository: flanksource/clicky
Length of output: 732
Non-standard outbound media type — application/clicky+json is the standards-compliant form per RFC 6839.
The structured syntax suffix +json (RFC 6839 / RFC 9110) follows the convention application/<subtype>+json (e.g., application/ld+json, application/problem+json). The current implementation outputs application/json+clicky, which does not match any registered suffix scheme and breaks content-negotiation libraries that specifically match the +json suffix.
Inbound parsing correctly accepts both forms (lines 401-417), but the outbound Content-Type header should use the RFC-compliant form. While the code comments refer to application/clicky+json as "legacy", the standards expect the opposite.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@rpc/serve_test.go` around lines 442 - 446, The test reveals the outbound
Content-Type is using a non-standard suffix order; update the
formatToContentType implementation so that the "clicky-json" case returns the
RFC-compliant structured-syntax-suffix form "application/clicky+json" (change
the mapping in formatToContentType) and adjust the test
TestFormatToContentType_ClickyJSON to expect "application/clicky+json" instead
of "application/json+clicky"; ensure only the outbound mapping is changed
(inbound parsing can remain as-is).
Remove outdated example files (example-data.json, example-data-200.json, go.mod, go.sum, main.go, order-schema.yaml, output.html) from the example directory. Update examples/go.mod to use commons v1.51.0 and clean up unused dependencies. Minor formatting adjustments to order-schema.yaml for consistency.
…ollapsing Refactor task rendering to prioritize problem tasks (failed/warning/cancelled) and running tasks, ensuring they are never collapsed into summaries. Success tasks are collapsed when they exceed available terminal height, while pending tasks remain limited to 5 visible items. This change improves visibility of critical task states that require user attention while still managing screen real estate for less critical tasks. Also updates dependencies: flanksource/commons v1.50.3 → v1.51.0, adds charmbracelet/x/conpty, charmbracelet/x/errors, charmbracelet/x/termios, charmbracelet/x/xpty, erikgeiser/coninput, and mattn/go-localereader. Removes unused dependencies including Snawoot/go-http-digest-auth-client and aws/aws-sdk-go-v2/service/route53. Adds .gavel/ to .gitignore and includes JSX type definitions for iconify-icon component.
- prompt: set explicit PTY winsize and TERM=screen-256color so huh's
text input renders under PTY without leaking OSC11 query response
into stdin as user input.
- text/tokenizer: rewrite StripANSI to terminate CSI sequences on any
byte in 0x40-0x7E (not just 'm') so cursor-movement codes like \x1b[3A
no longer swallow text up to the next 'm' character.
- rpc/converter: skip the {id}/action path rewrite for top-level commands
whose only "parent" is the cobra root, so commands like 'pretty <file>'
resolve to /api/v1/pretty instead of /api/v1/{file}/pretty.
- format_callbacks: MustFormatWithContext now panics on error to match
its documented Must* contract (was silently swallowing the error).
- api/labelbadge: HTML() now honors Shape (rounded/pill/square) and
Icon fields the struct documents.
- log: loggerOutput() falls back to os.Stderr when the commons logger
has no writer configured, preventing a nil-writer panic.
- task/manager: convert isInteractive to atomic.Bool so SetForceInteractive
doesn't race with the render loop.
- rpc/openapi: drop duplicate Surface-key assignment loop.
- examples/go.mod: bump go-jose/v3 to v3.0.5 (GHSA-78h2-9frx-2jm8).
- release.yml: align GO_VERSION with examples/go.mod (1.26).
- e2e: update OpenAPI path expectation to match converter fix.
## [1.21.6](v1.21.5...v1.21.6) (2026-04-27) ### ⚠ BREAKING CHANGES * **formatters:** FormatWithOptions now delegates to FormatWithContext internally; custom FormatManager implementations should override FormatWithContext instead. * **api,ui,example:** stackWindowOpts now uses From and To time.Time fields instead of UpdatedSince and Window duration fields. Content-Type header for clicky-json format changed from application/clicky+json to application/json+clicky. Refs: entity demo tests, example app routing, API response handling * **formatters:** The Content-Type header for clicky JSON responses is now application/json+clicky instead of application/clicky+json. Clients should update their expectations, though the legacy format is still accepted in Accept headers. ### ✨ Features * **api,rpc,entity:** Add Link types and entity operation annotations for OpenAPI ([3bf7d81](3bf7d81)) * **api,ui,example:** add time range filtering and entity detail page navigation ([0b78123](0b78123)) * **api:** add LabelBadge type for two-part pill badges ([8874efb](8874efb)) * **entity-example:** add link and linkcommand examples page with deep-linking support ([d6d89b6](d6d89b6)) * **entity:** Add filter lookup support for dynamic filter metadata and completions ([168e9b2](168e9b2)) * **formatters:** add format callbacks and link/command support ([b19923a](b19923a)) * **formatters:** standardize clicky JSON content type to application/json+clicky ([a899d50](a899d50)) * **prompt:** Add interactive prompt APIs with select, multi-select, and text input ([ad6a55d](ad6a55d)) * **task,examples:** Add interactive prompt support and task terminal ownership management ([5218f72](5218f72)) * **task,lint,log:** add output capture and log serialization for clean task rendering ([6dc5e67](6dc5e67)) * **task:** improve task rendering with priority-based ordering and collapsing ([aaa5043](aaa5043)) ### 🐛 Bug Fixes * address PR [#99](#99) test failures and CodeRabbit review ([db4f748](db4f748)) ### 🔧 Maintenance * **examples:** remove legacy example files and update dependencies ([190395f](190395f))
|
🎉 This PR is included in version 1.21.6 |
2 participants
Summary by CodeRabbit
New Features
Improvements
Tooling