feat(exec): detect compiler activity during process startup#119
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 21 minutes and 2 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (131)
WalkthroughAdds compiler detection and a ChangesCompiler-aware supervision
Aichat chat/runtime/tooling
Entity package split and dynamic filters
Markdown parsing and rendering
RPC, OpenAPI, and MCP transport
Lint severity and entity checks
Docs, examples, and build tooling
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Contributor
Gavel summary
Totals: 2130 passed · 0 failed · 63 skipped · 1m42s |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@exec/compiler.go`:
- Around line 34-50: The detectCompilers function silently ignores all errors
from gopsutil probes (gops.NewProcess, proc.Name, proc.Exe, proc.CmdlineSlice)
by using continue statements and always returns nil error regardless of
inspection failures. Track errors during the iteration through collectPids
results, and when compiler detection has no visibility into the process tree
(all probes fail), return an appropriate error instead of nil so callers can
distinguish between "no compilers found" and "couldn't inspect processes". This
ensures the caller doesn't prematurely advance startup state when inspection
failures occur.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 21e3e8df-63fe-40e8-b4b9-6c478b1c7cd0
📒 Files selected for processing (4)
exec/compiler.goexec/supervise_loop.goexec/supervised.goexec/supervised_test.go
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
moshloop
force-pushed
the
feat/pluggable-live-renderer
branch
from
fc1dda6 to
31009b5
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
moshloop
added a commit
that referenced
this pull request
Jun 23, 2026
… comments - rpc: pass a non-nil args slice to the executed cobra command so an empty request no longer falls back to os.Args[1:] and leaks the server's --port flag - aichat: implement missing provider-credential helpers (configuredProviders, credentialMap, resolveProviderKey, firstNonEmptyString) and drop the unused strings import so the module compiles - lint(e2e): update fixtures/expectations for severity levels — warnings are advisory, only error-severity violations fail the run; summary reports errors/warnings - api: build footnote HTML via strings.Builder to clear the CodeQL unsafe-quoting alert without changing output - exec: surface compiler-probe failures when detection has no visibility into the process tree
Contributor
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/enitity/main.go (1)
949-969: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
--devfailure can leave the API server running unexpectedly.The HTTP server starts before
startViteDevServer(...). If Vite startup fails,RunEreturns early, but the already-startedListenAndServegoroutine is not shut down first.Suggested ordering fix
- errCh := make(chan error, 1) - go func() { - ... - if err := httpSrv.ListenAndServe(); err != nil && err != http.ErrServerClosed { - errCh <- err - } - }() - if dev { vite, err := startViteDevServer(ctx, cmd, host, port, uiPort) if err != nil { return err } defer func() { _ = vite.Wait() }() } + +errCh := make(chan error, 1) +go func() { + ... + if err := httpSrv.ListenAndServe(); err != nil && err != http.ErrServerClosed { + errCh <- err + } +}()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/enitity/main.go` around lines 949 - 969, The HTTP server (httpSrv.ListenAndServe) is started in a goroutine before the Vite dev server is initialized, so if startViteDevServer fails and returns an error, the RunE function exits early while the HTTP server goroutine continues running. To fix this, reorder the code so that the dev mode check and Vite server startup happen before the HTTP server goroutine is launched. This way, if Vite initialization fails, the HTTP server is never started in the first place, preventing the server from running unexpectedly after an early return.rpc/openapi.go (1)
325-332: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPropagate the resolved operation path before OpenAPI conversion
When
op.Pathis empty, you computepathfor routing but still pass anopwith emptyPathintoconvertOperationToOpenAPI. Downstream,filterLookupMetaemitsx-clicky-lookup.urlfromop.Path, so lookup URLs can be blank even though the route exists.Suggested change
for _, op := range operations { method := strings.ToLower(op.Method) if method == "" { method = "post" } + if op.Path == "" { + op.Path = path + } openAPIPath[method] = g.convertOperationToOpenAPI(op) }Also applies to: 486-489
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rpc/openapi.go` around lines 325 - 332, The operation object being passed to convertOperationToOpenAPI has an empty Path field, which causes downstream functions like filterLookupMeta to generate blank lookup URLs. Before calling convertOperationToOpenAPI on the op object within the loop that iterates over operations, update the op.Path field with the computed path value that is determined when op.Path is empty, ensuring that the resolved path is propagated through the OpenAPI conversion process.
🧹 Nitpick comments (13)
api/blocks.go (1)
61-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the
api.Textbuilder path instead of a struct literal.Line 61 builds
Textwith a struct literal; switch to the builder-style construction used by the project conventions.As per coding guidelines, "Build
api.Textviaclicky.Text(...)orapi.Text{}.Append(...)chaining rather thanapi.Text{...}struct literals orChildren:slice literals."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/blocks.go` at line 61, The return statement on line 61 constructs Text using a struct literal with Content and Style fields. Replace this struct literal construction Text{Content: h.Content.String(), Style: "font-bold"} with the builder-style pattern by using either clicky.Text() or api.Text{}.Append() chaining methods, ensuring the same Content value (h.Content.String()) and Style value ("font-bold") are properly set through the builder methods instead of direct struct initialization.Source: Coding guidelines
aichat/threads_http.go (1)
16-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftRegister this endpoint through entity/RPC surfaces instead of raw
ServeMux.Line 16 adds another direct
mux.HandleFuncroute, which bypasses the generated CLI/REST/MCP pipeline used elsewhere.As per coding guidelines, "Do not directly register net/http handlers via http.HandleFunc, http.Handle, or (*http.ServeMux).HandleFunc/.Handle" and "Prefer a command + filter struct ... rather than a raw mux.HandleFunc." Based on learnings, register operations via
clicky.NewEntity(...).Register()so surfaces are generated consistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@aichat/threads_http.go` at line 16, Remove the direct mux.HandleFunc registration for the DELETE endpoint in the handleDeleteThread handler setup. Instead, register this operation through the entity/RPC surface pattern by using clicky.NewEntity(...).Register() to ensure the endpoint is consistently exposed across CLI/REST/MCP pipelines. This aligns with the coding guidelines that require operations to be registered through command and filter structs rather than raw http.ServeMux handlers.Sources: Coding guidelines, Learnings
entity/dynamic.go (1)
175-179: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBuild
PrettyRowcells with the approved text builders.
PrettyRowcurrently constructs cells withapi.Text{...}literals. Please useclicky.Text(...)orapi.Text{}.Append(...)for consistency with clicky render-builder rules.As per coding guidelines, build
api.Textviaclicky.Text(...)orapi.Text{}.Append(...)chaining rather than struct literals. Based on learnings,clicky lint ./...enforces these API-usage rules.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@entity/dynamic.go` around lines 175 - 179, In the dynamicItem.PrettyRow method, replace the api.Text struct literal construction with the approved text builders. Instead of using api.Text{Content: toStringValue(v)} when assigning to out[k], use either clicky.Text(...) or api.Text{}.Append(...) to build the text object. This ensures consistency with clicky render-builder rules and passes the linting requirements enforced by clicky lint.Sources: Coding guidelines, Learnings
lint/summary.go (1)
85-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse approved text builders in lint summary render nodes.
These changed render branches still construct output with
api.Text{...}literals. Please switch toclicky.Text(...)orapi.Text{}.Append(...)to match the repository render-builder contract.As per coding guidelines, build
api.Textviaclicky.Text(...)orapi.Text{}.Append(...)chaining rather than struct literals. Based on learnings,clicky lint ./...enforces these API-usage rules.Also applies to: 167-176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lint/summary.go` around lines 85 - 93, Replace all direct api.Text struct literal constructions with the approved text builder functions. In the render branches that return api.Text objects (including the ones that construct text with "text-red-600", "text-green-600", and "text-yellow-600" styles), switch from using api.Text{Content: ..., Style: ...} syntax to using either clicky.Text(...) or api.Text{}.Append(...) chaining methods. This change should be applied to all occurrences in the file, including both the section at lines 85-93 and the additional section at lines 167-176 mentioned in the comment.Sources: Coding guidelines, Learnings
entity/attach_test.go (1)
56-59: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse render builders for
api.Texttest fixtures.The
StaticOptionsfixture currently usesapi.Text{...}literals. Please switch toclicky.Text(...)orapi.Text{}.Append(...)so tests follow the same render-construction contract as production code.As per coding guidelines, build
api.Textviaclicky.Text(...)orapi.Text{}.Append(...)chaining rather than struct literals. Based on learnings,clicky lint ./...enforces these API-usage rules.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@entity/attach_test.go` around lines 56 - 59, The RegisterFilter call in the StaticOptions is using api.Text struct literals (api.Text{Content: "..."}) instead of render builders. Replace these struct literals with builder methods by using either clicky.Text(...) for creating the text objects or api.Text{}.Append(...) for chaining. Update the map values in the StaticOptions call to use the appropriate builder pattern instead of direct struct initialization to align with the coding guidelines and production code contracts.Sources: Coding guidelines, Learnings
entity/filter_test.go (1)
39-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse supported
api.Textbuilders in fixtures instead of struct literals.Please replace these
api.Text{Content: ...}values withclicky.Text(...)orapi.Text{}.Append(...)to keep test code aligned with renderer API rules.As per coding guidelines, “Build
api.Textviaclicky.Text(...)orapi.Text{}.Append(...)chaining rather thanapi.Text{...}struct literals.” Based on learnings, “Use theclicky lint ./...bundled analyzer to enforce API-usage rules when building clicky render primitives.”Also applies to: 75-75, 119-119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@entity/filter_test.go` around lines 39 - 42, The test fixture uses api.Text struct literals (api.Text{Content: "..."}) which violates the coding guidelines for building render primitives. Replace all instances of api.Text{Content: "..."} struct literals in the options map (at lines 40-41) and other locations (also at lines 75 and 119) with the proper builder methods using either clicky.Text(...) or api.Text{}.Append(...) to maintain consistency with the renderer API rules and pass the clicky lint analyzer checks.Sources: Coding guidelines, Learnings
entity/dynamic_test.go (1)
61-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace
api.Text{Content: ...}literals with supported text builders.Use
clicky.Text(...)orapi.Text{}.Append(...)for these fixture values to match the render-builder contract and avoid lint drift.As per coding guidelines, “Build
api.Textviaclicky.Text(...)orapi.Text{}.Append(...)chaining rather thanapi.Text{...}struct literals.” Based on learnings, “Use theclicky lint ./...bundled analyzer to enforce API-usage rules when building clicky render primitives.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@entity/dynamic_test.go` around lines 61 - 64, Replace the api.Text{Content: ...} struct literal values in the StaticOptions map for the "open" and "closed" keys with proper text builders. Instead of using struct literals like api.Text{Content: "Open"}, use either clicky.Text("Open") or api.Text{}.Append(...) to build these text values. This aligns with the coding guidelines that require building api.Text objects through builder methods rather than direct struct instantiation, ensuring consistency with the render-builder contract.Sources: Coding guidelines, Learnings
entity/sources.go (1)
175-175: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse clicky text builders instead of
api.Textstruct literals.Line 175 and Line 231 construct labels with
api.Text{Content: ...}. Please switch toclicky.Text(...)orapi.Text{}.Append(...)to align with render-builder conventions.As per coding guidelines, "Build
api.Textviaclicky.Text(...)orapi.Text{}.Append(...)chaining rather thanapi.Text{...}struct literals orChildren:slice literals".Also applies to: 231-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@entity/sources.go` at line 175, Replace the api.Text struct literal syntax api.Text{Content: item.GetName()} on line 175 and the similar pattern on line 231 with the builder method pattern using either clicky.Text(...) function or api.Text{}.Append(...) chaining method. Update both occurrences to use these builder approaches instead of struct literal constructors to align with the render-builder conventions.Source: Coding guidelines
entity/spec.go (1)
49-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid
api.Text{...}literals in static filter option conversion.Line 51 should use a text builder (
clicky.Text(...)orapi.Text{}.Append(...)) instead of direct struct literal construction.As per coding guidelines, "Build
api.Textviaclicky.Text(...)orapi.Text{}.Append(...)chaining rather thanapi.Text{...}struct literals orChildren:slice literals".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@entity/spec.go` around lines 49 - 52, In the loop that iterates over s.Source.Options and builds the options map, replace the direct api.Text struct literal api.Text{Content: label} with the appropriate text builder function. Instead of constructing the api.Text object with api.Text{Content: label}, use either clicky.Text(label) or api.Text{}.Append(label) to conform with the coding guidelines for building api.Text objects. This change should be applied in the assignment within the for loop where value and label are being processed.Source: Coding guidelines
formatters/clicky_document.go (1)
19-21: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDefault provider documents to schema version 1.
This fast-path returns provider payloads verbatim; if a provider returns a zero-value document, output becomes
version: 0while the non-provider path defaults toversion: 1. Normalizing here keeps the envelope contract stable.Suggested fix
func NewClickyDocument(t api.Textable) ClickyDocument { if t == nil { return ClickyDocument{Version: 1, Node: clickyTextNode("")} } if provider, ok := t.(ClickyDocumentProvider); ok { - return provider.ClickyDocument() + doc := provider.ClickyDocument() + if doc.Version == 0 { + doc.Version = 1 + } + return doc } return ClickyDocument{Version: 1, Node: convertTextable(t)} }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@formatters/clicky_document.go` around lines 19 - 21, The ClickyDocumentProvider fast-path in the conditional block checks if the type assertion succeeds and returns the provider.ClickyDocument() result directly without normalizing the schema version. When a provider returns a zero-value document, this results in version: 0 instead of the expected default version: 1 that the non-provider path uses. After obtaining the document from provider.ClickyDocument(), apply the same schema version normalization logic used in the non-provider path to ensure consistency in the output envelope contract.rpc/executor.go (1)
323-332: 🔒 Security & Privacy | 🔵 TrivialUnbounded
io.ReadAllon request body.Buffering the whole body with
io.ReadAllhas no size cap, so a large/malicious payload can drive memory pressure. Consider wrapping withhttp.MaxBytesReader(or enforcing a configured limit) before reading.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rpc/executor.go` around lines 323 - 332, The io.ReadAll call on r.Body has no size limit, which can cause memory exhaustion from large or malicious payloads. Wrap r.Body with http.MaxBytesReader before calling io.ReadAll to enforce a configured maximum size limit. This ensures the request body reading respects a bounded memory constraint and prevents unbounded buffering.rpc/openapi_reflect_test.go (1)
8-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a pointer-receiver
SchemaDescribertest caseCurrent coverage only validates value-receiver describers. Add one type with
func (*T) JSONSchema() map[string]anyand assertSchemaForStructdoes not panic and emits the expected schema.Also applies to: 114-129
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rpc/openapi_reflect_test.go` around lines 8 - 16, Add test coverage for pointer-receiver SchemaDescriber implementations. Create a new type similar to envVarLike (which has a value-receiver JSONSchema method) but with a pointer-receiver func (*T) JSONSchema() map[string]any instead. Then add test assertions that call SchemaForStruct on this new type to verify it does not panic and correctly emits the expected schema, ensuring SchemaForStruct properly handles both value-receiver and pointer-receiver SchemaDescriber implementations.rpc/filter_ref_openapi_test.go (1)
47-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
clicky.Text(...)instead ofapi.Text{...}literalReplace the struct literal in the static options map with the clicky text builder to match project rendering conventions.
Suggested change
- Source: entity.StaticOptions(map[string]api.Textable{"u1": api.Text{Content: "Alice"}}), + Source: entity.StaticOptions(map[string]api.Textable{"u1": clicky.Text("Alice")}),As per coding guidelines, “Build
api.Textviaclicky.Text(...)orapi.Text{}.Append(...)chaining rather thanapi.Text{...}struct literals.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rpc/filter_ref_openapi_test.go` around lines 47 - 51, In the RegisterFilter call that registers the "ref-users" filter with Label "Owner", replace the api.Text struct literal in the Source parameter's StaticOptions map with the clicky.Text builder function. Instead of using api.Text{Content: "Alice"} directly in the map for the "u1" key, use clicky.Text to construct the text value according to the project's coding conventions for building api.Text objects.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@aichat/server.go`:
- Around line 208-218: Move the enforceRuntimeSettings call to execute
immediately after the runtimeSettings check completes and before the runtime
call. Currently s.runtime is called before enforceRuntimeSettings, which causes
provider initialization failures to return 503 status instead of the
deterministic local policy rejections (402/413). Reorder these calls so that
local guardrails are checked first, preventing unnecessary runtime setup and
ensuring proper error precedence.
In `@aichat/settings.go`:
- Around line 44-49: The modelIDForRequest function checks if the trimmed
request model is non-empty on the emptiness check, but returns the untrimmed
req.Model value, which causes downstream failures when the model contains
leading or trailing whitespace. Apply strings.TrimSpace to req.Model before
returning it on the line where req.Model is returned, ensuring consistency
between the emptiness check and the actual value returned to the caller.
In `@aichat/threads_http.go`:
- Around line 82-84: The store.Delete call in the delete handler currently maps
all errors to HTTP 404, which incorrectly hides actual backend failures. Define
or use a typed/sentinel error in the store package to distinguish "not found"
errors from other failures. In the error handling block after store.Delete,
check if the returned error matches the not-found error type and return
http.StatusNotFound only in that case, while returning
http.StatusInternalServerError for all other non-nil errors. This ensures actual
failures are reported correctly to clients.
In `@cmd/clicky/main.go`:
- Around line 1388-1397: The issue is that when outputPattern contains a
wildcard (*), the ReplaceAll call in this code block replaces the asterisk with
only the base filename without appending the format extension. The ext variable
is calculated by calling getOutputExtension(format) but is not used when
handling the wildcard case. To fix this, modify the strings.ReplaceAll call that
handles the wildcard pattern to use base+ext instead of just base, so that the
format-derived extension is included in the final output path.
In `@entity/dynamic_spec.go`:
- Around line 126-130: The validation of DynamicFilter.Options is currently
happening inside a loop during the lookup execution path, which is too late and
should be caught at registration time instead. Move the nil check for df.Options
from within the loop in the current location to the RegisterDynamicEntity
function where filters are being registered, so that invalid configurations are
rejected immediately when the entity is registered rather than causing a panic
during live request processing.
In `@entity/entity.go`:
- Around line 1178-1179: The RenderResult function calls at lines 1178-1179 and
the other locations mentioned (1250-1251, 1337-1338, 1383-1384) are ignoring
errors returned by RenderResult using the blank identifier. Instead of
discarding the error, capture the return value from RenderResult and check if it
is non-nil; if an error occurs during rendering, return that error instead of
returning nil. This ensures that rendering failures are properly propagated and
the command does not exit successfully when output rendering fails.
In `@entity/schema.go`:
- Around line 103-108: The code currently allows multiple properties to be
marked with IsID or IsName by silently overwriting the previous selection when
assigning to ps.IDKey and ps.NameKey. Add validation checks before each
assignment to detect if ps.IDKey or ps.NameKey has already been set by a
previous property. If either has been set and you're attempting to set it again,
return a validation error instead of allowing the overwrite. This will ensure
schemas with duplicate ID or name markers are rejected rather than silently
accepted and resolved by iteration order.
- Around line 85-99: The variable `p` retrieved from `doc.Properties[name]` can
be nil when a property is explicitly null, but the code does not validate this
before dereferencing its fields (such as `p.Type`, `p.Format`, `p.Label`,
`p.PrettyFmt`, `p.Short`, and `p.Filter`). Add a nil check immediately after the
assignment to `p` and before constructing the schemaField. If `p` is nil, return
an appropriate schema-validation error to handle the case gracefully instead of
allowing a panic to occur when accessing nil pointer fields.
In `@examples/enitity/main.go`:
- Line 901: The markdown-preview handler is being registered directly via
mux.HandleFunc at line 901, which bypasses the entity-first routing model and
prevents auto-generation of CLI/REST/MCP/OpenAPI integration. Remove the direct
mux.HandleFunc call for serveMarkdownPreview and instead define the markdown
preview functionality as a registered entity using the project's fluent builder
pattern. Register this entity with the RPC layer so it will automatically expose
the functionality through the generated REST, CLI, and web UI surfaces rather
than wiring it directly to the HTTP mux.
- Around line 1121-1139: Invalid format values passed by clients are currently
causing internal server errors (500) when clicky.Format() fails, but should
instead return bad request errors (400) since format validation is a client
request issue. After normalizing the format using
normalizeMarkdownPreviewFormat(), validate that it is a supported format before
attempting to use it in clicky.Format() or serveMarkdownPreviewExcel(). Return
http.Error with http.StatusBadRequest when the format is not in the allowlist of
supported formats, rather than allowing clicky.Format() to fail with a 500 error
later.
In `@lint/summary.go`:
- Around line 119-124: The ruleGroup severity is initialized only once when the
group is first created (when checking if key exists in byRule map), but it is
never updated when subsequent violations are appended to that group. When
appending a violation to an existing ruleGroup at the location where
g.violations = append(g.violations, v) is called, add logic to compare the
incoming violation's severity (v.Severity) with the current group's severity
(g.severity) and update g.severity to the higher/more severe value if the new
violation is more severe than what was previously recorded.
In `@markdown/document.go`:
- Around line 182-194: The href and src attributes in the "link" and "image"
cases accept URLs without validating their schemes, allowing dangerous schemes
like javascript: to be included in the HTML output, creating an XSS
vulnerability. Before constructing the HTML attributes in both the "link" case
(using n.Href) and the "image" case (using n.Href), validate that the URL starts
with a safe scheme such as http, https, or mailto. If the URL scheme is not in a
whitelist of safe schemes, either omit the href/src attribute entirely or
replace it with a safe default to prevent the XSS vector from being executed.
- Around line 669-689: The code generates ClickyColumn names using
slugColumnName() but does not handle cases where multiple headers slug to the
same name, causing duplicate keys in the ClickyRow.Cells map and silent data
loss. To fix this, track already-used column names in a map as you populate the
columns slice, and when a name collision is detected, append the index or
another unique suffix to slugColumnName() result to ensure each column has a
distinct name. This ensures that when populating clickyRow.Cells in the loop
over rows, each cell is stored with its correct corresponding column name
without overwriting previous values.
In `@markdown/options.go`:
- Around line 17-25: The defaultOptions() function currently sets PreserveHTML
to true, which is an unsafe default that allows raw HTML to pass through
unfiltered and creates XSS exposure. Change the PreserveHTML field in the
defaultOptions() function from true to false to use a safe default that prevents
raw HTML from being preserved by default.
In `@rpc/cobra_command.go`:
- Around line 80-87: The cobra.Command initialization for execCmd does not
silence usage and error output, causing errors to be reported twice - once
through cobra's automatic error handling and once through the returned error.
Add SilenceUsage and SilenceErrors fields set to true in the execCmd struct
initialization to prevent cobra from automatically printing errors and usage
information to stderr, allowing the error to be handled only through the
returned cmdErr value.
In `@rpc/executor.go`:
- Line 356: The Content-Type header check using strict equality comparison with
"application/json" will not match valid values that include additional
parameters such as charset (e.g., "application/json; charset=utf-8"), causing
JSON bodies with these headers to be silently ignored. Replace the strict
equality check on r.Header.Get("Content-Type") == "application/json" with a more
robust approach that matches the media type prefix regardless of parameters. You
can either use strings.HasPrefix to check if the header value starts with
"application/json", or use mime.ParseMediaType to properly parse the header and
extract only the media type portion before comparison.
In `@rpc/request_context.go`:
- Around line 21-23: The RequestFromContext function currently returns (nil,
true) when a typed-nil *http.Request is stored in the context, which violates
the helper's contract and can cause downstream errors when callers trust the ok
value. After the type assertion in RequestFromContext, add a check to verify
that r is not nil; if r is nil despite ok being true, set ok to false before
returning to ensure the function correctly indicates when no valid request is
available.
---
Outside diff comments:
In `@examples/enitity/main.go`:
- Around line 949-969: The HTTP server (httpSrv.ListenAndServe) is started in a
goroutine before the Vite dev server is initialized, so if startViteDevServer
fails and returns an error, the RunE function exits early while the HTTP server
goroutine continues running. To fix this, reorder the code so that the dev mode
check and Vite server startup happen before the HTTP server goroutine is
launched. This way, if Vite initialization fails, the HTTP server is never
started in the first place, preventing the server from running unexpectedly
after an early return.
In `@rpc/openapi.go`:
- Around line 325-332: The operation object being passed to
convertOperationToOpenAPI has an empty Path field, which causes downstream
functions like filterLookupMeta to generate blank lookup URLs. Before calling
convertOperationToOpenAPI on the op object within the loop that iterates over
operations, update the op.Path field with the computed path value that is
determined when op.Path is empty, ensuring that the resolved path is propagated
through the OpenAPI conversion process.
---
Nitpick comments:
In `@aichat/threads_http.go`:
- Line 16: Remove the direct mux.HandleFunc registration for the DELETE endpoint
in the handleDeleteThread handler setup. Instead, register this operation
through the entity/RPC surface pattern by using clicky.NewEntity(...).Register()
to ensure the endpoint is consistently exposed across CLI/REST/MCP pipelines.
This aligns with the coding guidelines that require operations to be registered
through command and filter structs rather than raw http.ServeMux handlers.
In `@api/blocks.go`:
- Line 61: The return statement on line 61 constructs Text using a struct
literal with Content and Style fields. Replace this struct literal construction
Text{Content: h.Content.String(), Style: "font-bold"} with the builder-style
pattern by using either clicky.Text() or api.Text{}.Append() chaining methods,
ensuring the same Content value (h.Content.String()) and Style value
("font-bold") are properly set through the builder methods instead of direct
struct initialization.
In `@entity/attach_test.go`:
- Around line 56-59: The RegisterFilter call in the StaticOptions is using
api.Text struct literals (api.Text{Content: "..."}) instead of render builders.
Replace these struct literals with builder methods by using either
clicky.Text(...) for creating the text objects or api.Text{}.Append(...) for
chaining. Update the map values in the StaticOptions call to use the appropriate
builder pattern instead of direct struct initialization to align with the coding
guidelines and production code contracts.
In `@entity/dynamic_test.go`:
- Around line 61-64: Replace the api.Text{Content: ...} struct literal values in
the StaticOptions map for the "open" and "closed" keys with proper text
builders. Instead of using struct literals like api.Text{Content: "Open"}, use
either clicky.Text("Open") or api.Text{}.Append(...) to build these text values.
This aligns with the coding guidelines that require building api.Text objects
through builder methods rather than direct struct instantiation, ensuring
consistency with the render-builder contract.
In `@entity/dynamic.go`:
- Around line 175-179: In the dynamicItem.PrettyRow method, replace the api.Text
struct literal construction with the approved text builders. Instead of using
api.Text{Content: toStringValue(v)} when assigning to out[k], use either
clicky.Text(...) or api.Text{}.Append(...) to build the text object. This
ensures consistency with clicky render-builder rules and passes the linting
requirements enforced by clicky lint.
In `@entity/filter_test.go`:
- Around line 39-42: The test fixture uses api.Text struct literals
(api.Text{Content: "..."}) which violates the coding guidelines for building
render primitives. Replace all instances of api.Text{Content: "..."} struct
literals in the options map (at lines 40-41) and other locations (also at lines
75 and 119) with the proper builder methods using either clicky.Text(...) or
api.Text{}.Append(...) to maintain consistency with the renderer API rules and
pass the clicky lint analyzer checks.
In `@entity/sources.go`:
- Line 175: Replace the api.Text struct literal syntax api.Text{Content:
item.GetName()} on line 175 and the similar pattern on line 231 with the builder
method pattern using either clicky.Text(...) function or api.Text{}.Append(...)
chaining method. Update both occurrences to use these builder approaches instead
of struct literal constructors to align with the render-builder conventions.
In `@entity/spec.go`:
- Around line 49-52: In the loop that iterates over s.Source.Options and builds
the options map, replace the direct api.Text struct literal api.Text{Content:
label} with the appropriate text builder function. Instead of constructing the
api.Text object with api.Text{Content: label}, use either clicky.Text(label) or
api.Text{}.Append(label) to conform with the coding guidelines for building
api.Text objects. This change should be applied in the assignment within the for
loop where value and label are being processed.
In `@formatters/clicky_document.go`:
- Around line 19-21: The ClickyDocumentProvider fast-path in the conditional
block checks if the type assertion succeeds and returns the
provider.ClickyDocument() result directly without normalizing the schema
version. When a provider returns a zero-value document, this results in version:
0 instead of the expected default version: 1 that the non-provider path uses.
After obtaining the document from provider.ClickyDocument(), apply the same
schema version normalization logic used in the non-provider path to ensure
consistency in the output envelope contract.
In `@lint/summary.go`:
- Around line 85-93: Replace all direct api.Text struct literal constructions
with the approved text builder functions. In the render branches that return
api.Text objects (including the ones that construct text with "text-red-600",
"text-green-600", and "text-yellow-600" styles), switch from using
api.Text{Content: ..., Style: ...} syntax to using either clicky.Text(...) or
api.Text{}.Append(...) chaining methods. This change should be applied to all
occurrences in the file, including both the section at lines 85-93 and the
additional section at lines 167-176 mentioned in the comment.
In `@rpc/executor.go`:
- Around line 323-332: The io.ReadAll call on r.Body has no size limit, which
can cause memory exhaustion from large or malicious payloads. Wrap r.Body with
http.MaxBytesReader before calling io.ReadAll to enforce a configured maximum
size limit. This ensures the request body reading respects a bounded memory
constraint and prevents unbounded buffering.
In `@rpc/filter_ref_openapi_test.go`:
- Around line 47-51: In the RegisterFilter call that registers the "ref-users"
filter with Label "Owner", replace the api.Text struct literal in the Source
parameter's StaticOptions map with the clicky.Text builder function. Instead of
using api.Text{Content: "Alice"} directly in the map for the "u1" key, use
clicky.Text to construct the text value according to the project's coding
conventions for building api.Text objects.
In `@rpc/openapi_reflect_test.go`:
- Around line 8-16: Add test coverage for pointer-receiver SchemaDescriber
implementations. Create a new type similar to envVarLike (which has a
value-receiver JSONSchema method) but with a pointer-receiver func (*T)
JSONSchema() map[string]any instead. Then add test assertions that call
SchemaForStruct on this new type to verify it does not panic and correctly emits
the expected schema, ensuring SchemaForStruct properly handles both
value-receiver and pointer-receiver SchemaDescriber implementations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df9b9111-1130-4d05-ad7f-1a8085e57543
⛔ Files ignored due to path filters (6)
aichat/go.sumis excluded by!**/*.sumexamples/enitity/go.sumis excluded by!**/*.sumexamples/enitity/webapp/dist/index.htmlis excluded by!**/dist/**examples/go.sumis excluded by!**/*.sumexamples/uber_demo/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sum
📒 Files selected for processing (127)
.gitignoreCLAUDE.mdCONTRIBUTING.mdMakefileaichat/approval.goaichat/approval_test.goaichat/genkit.goaichat/go.modaichat/messages.goaichat/messages_test.goaichat/models.goaichat/server.goaichat/settings.goaichat/settings_test.goaichat/threads.goaichat/threads_http.goaichat/tool_group_test.goaichat/tool_registry.goaichat/tool_registry_test.goaichat/tools_clicky.goapi/blocks.goapi/blocks_test.goargs_parser.gocmd/clicky/input_format_test.gocmd/clicky/main.goe2e_integration_test.goentity/annotations.goentity/attach.goentity/attach_test.goentity/builder.goentity/command.goentity/command_test.goentity/doc.goentity/dynamic.goentity/dynamic_spec.goentity/dynamic_test.goentity/entity.goentity/entity_suite_test.goentity/entity_test.goentity/filter.goentity/filter_test.goentity/filters_test.goentity/lookup_response.goentity/multi_filter.goentity/operation.goentity/pagination.goentity/registry.goentity/render.goentity/response_meta.goentity/schema.goentity/searchable_filter_test.goentity/sources.goentity/spec.goentity/sub_command.goentity/sub_command_test.goentity/toolgroup_test.goentity/types.goentity_aliases.goexamples/ANSI-FIXTURES.mdexamples/date-formatting-tests.mdexamples/enitity/Procfileexamples/enitity/README.mdexamples/enitity/Taskfile.yamlexamples/enitity/go.modexamples/enitity/main.goexamples/enitity/main_test.goexamples/enitity/webapp/package.jsonexamples/enitity/webapp/pnpm-workspace.yamlexamples/enitity/webapp/src/App.tsxexamples/enitity/webapp/src/MarkdownEditorPage.tsxexamples/enitity/webapp/src/main.tsxexamples/enitity/webapp/tsconfig.tsbuildinfoexamples/enitity/webapp/vite.config.tsexamples/go.modexamples/kitchen-sink.mdexamples/simple-test.mdexamples/uber_demo/README.mdexamples/uber_demo/go.modexec/compiler.goexec/supervise_loop.goexec/supervised.goexec/supervised_test.goformat.goformatters/clicky_document.goformatters/html_react_formatter.goformatters/html_react_formatter_test.gogo.modlint/analyzer.golint/analyzer_test.golint/direct_stdout.golint/entity_registration.golint/render_builder.golint/runner.golint/severity.golint/severity_test.golint/summary.golint/summary_test.golint/testdata/src/bad/bad.golint/testdata/src/entityreg/bad/bad.golint/testdata/src/entityreg/good/good.golint/testdata/src/github.com/flanksource/clicky/api/stub.golint/testdata/src/github.com/flanksource/clicky/stub.golint/testdata/src/github.com/spf13/cobra/stub.golint/testdata/src/good/good.gomarkdown.gomarkdown/document.gomarkdown/markdown_test.gomarkdown/options.gomarkdown/parse.gomcp/registry.gomcp/server.gorpc/cobra_command.gorpc/context_datafunc_test.gorpc/converter.gorpc/converter_toolgroup_test.gorpc/executor.gorpc/executor_test.gorpc/filter_ref_openapi_test.gorpc/openapi.gorpc/openapi_reflect.gorpc/openapi_reflect_test.gorpc/openapi_test.gorpc/request_context.gorpc/request_context_test.gorpc/types.gotask/render.gotask/render_test.go
💤 Files with no reviewable changes (2)
- args_parser.go
- examples/go.mod
✅ Files skipped from review due to trivial changes (24)
- examples/enitity/Procfile
- examples/enitity/webapp/tsconfig.tsbuildinfo
- lint/testdata/src/github.com/spf13/cobra/stub.go
- entity/doc.go
- entity/filters_test.go
- rpc/converter_toolgroup_test.go
- lint/testdata/src/entityreg/good/good.go
- lint/summary_test.go
- examples/date-formatting-tests.md
- examples/kitchen-sink.md
- aichat/tool_group_test.go
- CONTRIBUTING.md
- format.go
- entity/multi_filter.go
- examples/enitity/README.md
- entity/sub_command_test.go
- entity/types.go
- exec/supervised.go
- CLAUDE.md
- entity/pagination.go
- examples/uber_demo/README.md
- lint/testdata/src/good/good.go
- examples/enitity/webapp/src/main.tsx
- lint/testdata/src/entityreg/bad/bad.go
🚧 Files skipped from review as they are similar to previous changes (3)
- exec/compiler.go
- exec/supervised_test.go
- exec/supervise_loop.go
Comment on lines
+82
to
+84
| if err := store.Delete(r.Context(), r.PathValue("id")); err != nil { | ||
| http.Error(w, err.Error(), http.StatusNotFound) | ||
| return |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not map all delete failures to 404.
Line 82 currently treats every store.Delete error as not-found. That masks real backend failures and returns the wrong status to clients. Use a typed/sentinel not-found error for thread stores and return 500 for all non-not-found failures.
Suggested status split
- if err := store.Delete(r.Context(), r.PathValue("id")); err != nil {
- http.Error(w, err.Error(), http.StatusNotFound)
- return
- }
+ if err := store.Delete(r.Context(), r.PathValue("id")); err != nil {
+ if errors.Is(err, ErrThreadNotFound) {
+ http.Error(w, err.Error(), http.StatusNotFound)
+ return
+ }
+ http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+ return
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@aichat/threads_http.go` around lines 82 - 84, The store.Delete call in the
delete handler currently maps all errors to HTTP 404, which incorrectly hides
actual backend failures. Define or use a typed/sentinel error in the store
package to distinguish "not found" errors from other failures. In the error
handling block after store.Delete, check if the returned error matches the
not-found error type and return http.StatusNotFound only in that case, while
returning http.StatusInternalServerError for all other non-nil errors. This
ensures actual failures are reported correctly to clients.
Comment on lines
+17
to
+25
| func defaultOptions() Options { | ||
| return Options{ | ||
| GFM: true, | ||
| Footnotes: true, | ||
| Frontmatter: true, | ||
| Admonitions: true, | ||
| PreserveHTML: true, | ||
| SourceSpans: true, | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Use a safe default for PreserveHTML to reduce XSS exposure.
Defaulting to PreserveHTML: true passes raw HTML through by default. If caller input is user-controlled, this is an unsafe default for HTML output paths.
Suggested fix
func defaultOptions() Options {
return Options{
GFM: true,
Footnotes: true,
Frontmatter: true,
Admonitions: true,
- PreserveHTML: true,
+ PreserveHTML: false,
SourceSpans: true,
}
}📝 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.
Suggested change
| func defaultOptions() Options { | |
| return Options{ | |
| GFM: true, | |
| Footnotes: true, | |
| Frontmatter: true, | |
| Admonitions: true, | |
| PreserveHTML: true, | |
| SourceSpans: true, | |
| } | |
| func defaultOptions() Options { | |
| return Options{ | |
| GFM: true, | |
| Footnotes: true, | |
| Frontmatter: true, | |
| Admonitions: true, | |
| PreserveHTML: false, | |
| SourceSpans: true, | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@markdown/options.go` around lines 17 - 25, The defaultOptions() function
currently sets PreserveHTML to true, which is an unsafe default that allows raw
HTML to pass through unfiltered and creates XSS exposure. Change the
PreserveHTML field in the defaultOptions() function from true to false to use a
safe default that prevents raw HTML from being preserved by default.
Comment on lines
+21
to
+23
| func RequestFromContext(ctx context.Context) (*http.Request, bool) { | ||
| r, ok := ctx.Value(requestContextKey{}).(*http.Request) | ||
| return r, ok |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return ok=false when stored request is nil
A typed-nil *http.Request in context currently yields (nil, true). That violates the helper’s contract and can lead to downstream dereferences when callers trust ok.
Suggested change
func RequestFromContext(ctx context.Context) (*http.Request, bool) {
r, ok := ctx.Value(requestContextKey{}).(*http.Request)
- return r, ok
+ if !ok || r == nil {
+ return nil, false
+ }
+ return r, true
}📝 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.
Suggested change
| func RequestFromContext(ctx context.Context) (*http.Request, bool) { | |
| r, ok := ctx.Value(requestContextKey{}).(*http.Request) | |
| return r, ok | |
| func RequestFromContext(ctx context.Context) (*http.Request, bool) { | |
| r, ok := ctx.Value(requestContextKey{}).(*http.Request) | |
| if !ok || r == nil { | |
| return nil, false | |
| } | |
| return r, true | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rpc/request_context.go` around lines 21 - 23, The RequestFromContext function
currently returns (nil, true) when a typed-nil *http.Request is stored in the
context, which violates the helper's contract and can cause downstream errors
when callers trust the ok value. After the type assertion in RequestFromContext,
add a check to verify that r is not nil; if r is nil despite ok being true, set
ok to false before returning to ensure the function correctly indicates when no
valid request is available.
This was referenced Jun 23, 2026
moshloop
added a commit
that referenced
this pull request
Jun 23, 2026
… on PR #119 Apply 12 validated correctness/stability findings, with regression tests: - aichat: trim explicit request model IDs; enforce runtime limits (402/413) before provider runtime init so they aren't masked by a 503 - entity: validate DynamicFilter.Options at registration; propagate RenderResult errors in generated commands; reject null property schemas and duplicate x-clicky-id/x-clicky-name markers - markdown: deduplicate colliding column names so duplicate header labels don't silently drop cells - rpc: silence usage/errors on the captured exec command; tolerate Content-Type charset params when parsing JSON bodies - cmd/clicky: append the format extension to wildcard output paths - lint: escalate a rule group's severity to the highest of its violations Deferred items filed as #127 (thread-store sentinel error) and #128 (markdown HTML sanitization).
Add compiler and linker detection to distinguish compilation phase from startup phase. Introduces StatusCompiling to track when build tools are actively running, preventing premature promotion to StatusRunning. The watchPorts function now accepts a compilationDetector callback and manages three startup states: compiling → starting → running. Compiler activity resets the grace period timer, allowing long builds to complete without timing out. A detected port immediately promotes to running regardless of compilation state. Refs: compiler detection for improved startup lifecycle management
…authoring guidance Add a `make docs` target that serves Go package documentation locally using pkgsite, with configurable port (default 8089). This enables developers to browse package docs without publishing. Enhance entity authoring guidance by adding documentation references to RegisterEntity and NewEntity functions, directing users to the entity package for best practices and examples. Refactor example app to use RouterAdapter pattern from clicky-ui, consolidating router integration logic and improving separation of concerns. This replaces the previous renderLink prop with a unified adapter that handles link rendering, pathname tracking, and navigation. Rename clicky_lookup.go to entity_lookup.go for better semantic clarity. IMPACT: Developers can now easily browse local package documentation. Entity registration functions provide clearer guidance. Example app demonstrates improved router integration pattern.
Stop ignoring markdown files globally to allow committing documentation. Add CLAUDE.md (API guide for Claude Code), CONTRIBUTING.md (development guide), and example test fixtures (ANSI-FIXTURES.md, date-formatting-tests.md, simple-test.md, uber_demo/README.md, enitity/README.md). Also ignore build artifacts (.todos/, todos-bak/, specs/) that should not be committed.
…ped provider credentials Introduce per-request tool filtering via ToolPreferences (enabled/ask/disabled modes), runtime settings for model defaults and token/cost budgets, and request-scoped provider credentials to support org-owned connection stores. Key changes: - ToolPreferences map allows clients to control tool availability per request - ToolApprovalPolicy provides metadata-aware approval decisions with tool operation details - RuntimeSettings and RuntimeSettingsProvider enable app-owned defaults and budget enforcement - ProviderCredentialsProvider allows per-request upstream API keys instead of relying only on environment - ChatRequest now carries context (serialized + structured) and tool preferences - Custom tools via ToolDefinition for app-owned operations outside Cobra commands - Thread deletion support in ThreadStore interface - Tool registry refactored to track ToolInfo metadata alongside ToolRef for filtering and approval BREAKING CHANGE: ApprovalPolicy signature unchanged but ToolApprovalPolicy takes precedence; initGenkit now accepts optional ProviderCredential args; ThreadStore.Delete method added to interface; ClickyToolset.DefineTools now wraps DefineRegisteredTools which returns registeredTool structs.
…te, footnotes) Introduce new Textable types for markdown block-level elements: - Heading: renders markdown-style section headings (h1-h6) - Blockquote: renders quoted document blocks - FootnoteRef: inline reference to footnote definitions - Footnote: single GFM-style footnote definition - Footnotes: ordered collection of footnote definitions Each type implements the Textable interface with String, ANSI, HTML, and Markdown renderers. Helper functions added to format.go for convenient construction. HTML React formatter updated with conversion functions for all new types. Lint analyzer and test data updated to recognize these as helper-backed types.
Introduce comprehensive markdown parsing capabilities using Goldmark with support for: - Parsing markdown source into structured semantic trees - Exporting parsed documents as Clicky JSON format - Support for GitHub Flavored Markdown, footnotes, frontmatter, and admonitions - Preserving source spans (line numbers) and original markdown snippets - Converting markdown nodes to Clicky's document format via ClickyDocumentProvider interface Enhance ClickyDocument to support metadata and add ClickyDocumentProvider interface for rich producers to provide fully structured documents without flattening through PrettyData. Extend ClickyNode with additional fields: Checked, Attributes, SourceMarkdown, LineStart, LineEnd for better semantic preservation. Add goldmark v1.7.13 dependency for markdown parsing. Refs: markdown parsing feature
Add comprehensive test case that validates markdown parsing, roundtrip conversion, and clicky JSON formatting. Includes verification of frontmatter metadata, all major markdown node types (headings, lists, tables, code blocks, etc.), task list state preservation, and table alignment. Also adds helper functions countKind and findNode for traversing and querying the document tree.
…y support Introduce a new entity package that enables defining filters once and reusing them across multiple entities. Named filters are registered centrally and attached to static entities via the typed Use/As adapter or to dynamic (schema-driven) entities via x-clicky-filter annotations. Key additions: - FilterSource abstraction for type-agnostic option resolution (StaticOptions, FuncOptions, EntityOptions) - NamedFilter registry with RegisterFilter/GetFilter APIs - attachedFilter adapter to bind named filters to typed ListOpts structs - DynamicEntityBuilder and DynamicFilter for schema-driven entities - boundFilter and resolveLookupCore to unify lookup response building - OpenAPI support: emit filter definitions in components.x-clicky-filters and reference them via x-clicky-lookup.$ref on filter parameters - GetEntity helper to look up registered entities by name or alias - entityFilterRefs tracking to map parameter keys to named filter names This enables cascading filters, shared filter definitions across entity types, and proper OpenAPI documentation of filter relationships without duplicating definitions.
Move all entity-related source files from root directory into a new `entity` package subdirectory. This improves code organization and modularity by grouping related functionality together. Files are renamed to remove redundant prefixes now that they reside in the entity package namespace.
…ubpackage Reorganize the codebase by moving the entity model, CLI generation, and command registry from the root clicky package to a dedicated entity subpackage. This establishes a clean one-way dependency: entity → clicky-subpackages (api, flags, formatters, task), never the reverse. The root clicky package now imports entity and re-exports its types via aliases (entity_aliases.go), preserving the public API so callers continue using clicky.Entity, clicky.RegisterEntity, etc. unchanged. Key changes: - Rename all package declarations from 'package clicky' to 'package entity' - Add Icon and Title fields to EntityInfo, DynamicEntitySpec, and CommandOpenAPIMeta for UI surface customization - Add ToolGroup field to EntityInfo, ActionInfo, and ActionSpec for AI tool-preference grouping - Introduce context-aware handlers: GetWithContext, CreateWithContext, UpdateWithContext, DeleteWithContext - Rename actionSpec/bulkActionSpec to ActionSpec/BulkActionSpec (exported) - Add WithToolGroup() builder methods for Entity and Action - Extract render/parse hooks (RenderResult, ParseArgs) to render.go for host-owned globals injection - Add operation.go with RPCOperation, ClickyOperationMeta, and related types - Add types.go with ExecutableCommand interface and ExecuteOptions - Update all internal imports to use entity package types directly - Add PrettyRow() to dynamicItem for table formatter support - Simplify command result rendering to use RenderResult hook BREAKING CHANGE: Internal package reorganization; public API preserved via root clicky re-exports.
…mmand adapter Move command execution logic from CommandExecutor into a new CobraExecutableCommand adapter that implements entity.ExecutableCommand. This decouples the RPC layer from cobra specifics and enables reuse across different command frameworks. Key changes: - New cobra_command.go with CobraExecutableCommand wrapper and captureGlobal function - Consolidate global stdout/stderr capture into globalCaptureMu (replaces per-executor mutex) - Move command state reset and flag handling into Execute method - Update converter to wrap cobra.Command in NewCobraExecutableCommand - Add support for Icon and Title metadata in ClickyOperationMeta and ClickySurface - Add ToolGroup propagation to RPCOperation.Group and ClickyOperationMeta.Group - Extract request context handling into request_context.go for nested JSON body preservation - Move type definitions to entity package aliases in types.go - Update all tests to use NewCobraExecutableCommand wrapper BREAKING CHANGE: RPCOperation.Command is now entity.ExecutableCommand instead of *cobra.Command. Callers must wrap cobra commands with NewCobraExecutableCommand or use the new interface methods (Path, Name, RootName, Runnable, Hidden, IsBoolFlag, Execute).
Add a new markdown editor page to the example application that allows users to edit markdown and preview it through multiple formats (JSON, HTML, CSV, Excel, Markdown). Implements a new `/api/examples/markdown-preview` endpoint that parses markdown and renders it in the requested format. Includes comprehensive test coverage for different preview formats and Excel export functionality. The markdown preview feature integrates with the Clicky formatting stack used by the CLI and executor API, providing a unified formatting experience across the application.
Introduce tool grouping functionality that allows tools to be organized into logical groups in the preferences UI. When a tool belongs to a group, the group's preference governs all member tools, and individual members are hidden from the UI. Changes: - Add Group field to ToolInfo and ToolDefinition structs - Implement effectivePreference() to resolve tool mode considering group membership - Add ToolEntry struct and ListToolEntries() to collapse grouped tools for UI presentation - Update toolsForRequest() and shouldRequireApproval() to use group-aware preference resolution - Add comprehensive test coverage for tool grouping behavior - Fix cmd.Hidden property access in toolableOperation() - Update go.mod/go.sum with goldmark v1.7.16 dependency
Introduce error and warning severity levels to distinguish structural violations from style preferences. Error-severity violations fail the lint run; warnings are advisory. Add three new checks: - checkManualCobraCommand: flags hand-rolled cobra.Command with Run/RunE handlers that should be registered entities - checkHTTPHandlerRegistration: flags direct net/http handler registration that should use the entity/rpc layer - checkEntityTableProvider: warns when registered entities lack api.TableProvider implementation for table rendering Refactor all existing diagnostics to use the new report() function with explicit severity. Update Result to track error vs warning counts separately, with HasErrors() (not HasIssues()) driving exit status. Update summary views to display severity labels and color-code by error/warning/success. Breaking change: Violation struct now includes Severity field; Result.Success now depends on HasErrors() rather than HasIssues().
…aml, and text Add --input-format flag to enable parsing and rendering of markdown, JSON, YAML, and plain text files without requiring a schema. Supports auto-detection of input format based on file extension or content analysis. Key changes: - New renderInput() function handles schema-less input processing - Auto-detection logic for markdown, JSON, YAML, and text formats - Input format normalization and validation - Output path resolution for multiple input files - Stdin input detection for automatic format selection - Remove parser provenance fields (sourceMarkdown, lineStart, lineEnd) from clicky-json output to reduce noise - Update markdown document generation to include frontmatter in output - Add comprehensive test coverage for input format handling BREAKING CHANGE: Removed SourceMarkdown field from markdown Node and Document structures; removed PreserveSource option from markdown parser. Parser provenance fields are no longer included in clicky-json output.
…ky-ui development Add --dev flag to serve-ui command that launches the Vite dev server alongside the Go API, enabling live development against a local clicky-ui checkout without rebuilding the embedded bundle. Changes include: - New serve-ui --dev flag and --ui-port option - startViteDevServer() function to launch pnpm dev with proper process group handling - webappDevDir() helper to locate webapp/ from source checkout - Updated Vite config to exclude @flanksource/clicky-ui from prebundling and allow filesystem access to sibling clicky-ui dist - Added react-grab dev dependency for development tooling - New task dev and webapp:dev in Taskfile.yaml - Procfile for convenient local development - Comprehensive documentation in README Requires source checkout with pnpm installed and `pnpm install` run in webapp/.
… unify command execution Move the entity/operation registration model to a dedicated entity/ subpackage to reduce circular dependencies and improve modularity. Add entity_aliases.go to re-export the public API from clicky root, maintaining backward compatibility. Introduce ExecutableCommand interface in entity package to abstract cobra.Command execution, enabling transport-neutral invocation (CLI, REST, MCP) with unified argument/flag handling and output capture. Simplify MCP server tool execution by delegating to this interface instead of manual flag manipulation and stdout/stderr redirection. Enhance OpenAPI schema generation with SchemaDescriber interface for types that need custom JSON-schema representation (e.g., commons-db EnvVar), and add clicky:"..." struct tags to control form widget rendering (component, title, description, format, source, property, order, required). Merge vendor extensions (x-clicky-*) into marshalled schema. Fix surface key generation to use singular entity name (no pluralization) so clicky-ui row-click routes resolve correctly against /api/v1/<entity> paths. Improve task renderer's line-wrapping accounting: count physical terminal rows (accounting for soft-wrap) instead of logical newlines, preventing frame smear when output exceeds terminal width. Update .gitignore to exclude examples/src and allow cmd/clicky/ and its test files. Add aichat and examples/enitity to GO_MODULES. Refactor Makefile to separate tidy and fmt targets. Update CLAUDE.md and CONTRIBUTING.md to document the new render-code rules (errors vs warnings) and entity-registration linter behavior.
… comments - rpc: pass a non-nil args slice to the executed cobra command so an empty request no longer falls back to os.Args[1:] and leaks the server's --port flag - aichat: implement missing provider-credential helpers (configuredProviders, credentialMap, resolveProviderKey, firstNonEmptyString) and drop the unused strings import so the module compiles - lint(e2e): update fixtures/expectations for severity levels — warnings are advisory, only error-severity violations fail the run; summary reports errors/warnings - api: build footnote HTML via strings.Builder to clear the CodeQL unsafe-quoting alert without changing output - exec: surface compiler-probe failures when detection has no visibility into the process tree
… on PR #119 Apply 12 validated correctness/stability findings, with regression tests: - aichat: trim explicit request model IDs; enforce runtime limits (402/413) before provider runtime init so they aren't masked by a 503 - entity: validate DynamicFilter.Options at registration; propagate RenderResult errors in generated commands; reject null property schemas and duplicate x-clicky-id/x-clicky-name markers - markdown: deduplicate colliding column names so duplicate header labels don't silently drop cells - rpc: silence usage/errors on the captured exec command; tolerate Content-Type charset params when parsing JSON bodies - cmd/clicky: append the format extension to wildcard output paths - lint: escalate a rule group's severity to the highest of its violations Deferred items filed as #127 (thread-store sentinel error) and #128 (markdown HTML sanitization).
Publish nested Go sub-modules (aichat, valkey, etc.) by pushing path-prefixed tags after semantic-release. Updates sub-module dependencies to the released version before tagging to ensure downstream consumers can install them via go get.
moshloop
force-pushed
the
feat/pluggable-live-renderer
branch
from
0836dd7 to
fdd53f1
Compare
…g for reader drain Add a done channel to capturedPTY that signals when the reader goroutine has finished draining the PTY to EOF. This ensures that final output written by the helper process before exit is captured before assertions snapshot the buffer, eliminating a race condition observed on slower CI hosts. Update requireExitWithin to wait for the reader to drain before returning.
moshloop
force-pushed
the
feat/pluggable-live-renderer
branch
from
c2bd972 to
ddb09f9
Compare
moshloop
added a commit
that referenced
this pull request
Jun 26, 2026
… comments - rpc: pass a non-nil args slice to the executed cobra command so an empty request no longer falls back to os.Args[1:] and leaks the server's --port flag - aichat: implement missing provider-credential helpers (configuredProviders, credentialMap, resolveProviderKey, firstNonEmptyString) and drop the unused strings import so the module compiles - lint(e2e): update fixtures/expectations for severity levels — warnings are advisory, only error-severity violations fail the run; summary reports errors/warnings - api: build footnote HTML via strings.Builder to clear the CodeQL unsafe-quoting alert without changing output - exec: surface compiler-probe failures when detection has no visibility into the process tree
moshloop
added a commit
that referenced
this pull request
Jun 26, 2026
… on PR #119 Apply 12 validated correctness/stability findings, with regression tests: - aichat: trim explicit request model IDs; enforce runtime limits (402/413) before provider runtime init so they aren't masked by a 503 - entity: validate DynamicFilter.Options at registration; propagate RenderResult errors in generated commands; reject null property schemas and duplicate x-clicky-id/x-clicky-name markers - markdown: deduplicate colliding column names so duplicate header labels don't silently drop cells - rpc: silence usage/errors on the captured exec command; tolerate Content-Type charset params when parsing JSON bodies - cmd/clicky: append the format extension to wildcard output paths - lint: escalate a rule group's severity to the highest of its violations Deferred items filed as #127 (thread-store sentinel error) and #128 (markdown HTML sanitization).
flankbot
pushed a commit
that referenced
this pull request
Jun 26, 2026
## [1.21.29](v1.21.28...v1.21.29) (2026-06-26) ### ⚠ BREAKING CHANGES * **cli:** Removed SourceMarkdown field from markdown Node and Document structures; removed PreserveSource option from markdown parser. Parser provenance fields are no longer included in clicky-json output. * **lint:** Violation struct now includes Severity field; Result.Success now depends on HasErrors() rather than HasIssues(). * **rpc:** RPCOperation.Command is now entity.ExecutableCommand instead of *cobra.Command. Callers must wrap cobra commands with NewCobraExecutableCommand or use the new interface methods (Path, Name, RootName, Runnable, Hidden, IsBoolFlag, Execute). * **entity:** Internal package reorganization; public API preserved via root clicky re-exports. * **aichat:** ApprovalPolicy signature unchanged but ToolApprovalPolicy takes precedence; initGenkit now accepts optional ProviderCredential args; ThreadStore.Delete method added to interface; ClickyToolset.DefineTools now wraps DefineRegisteredTools which returns registeredTool structs. ### ♻️ Code Refactoring * **entity,rpc,mcp,task:** extract entity model to subpackage and unify command execution ([074db76](074db76)) * **entity:** move entity package from root clicky to dedicated subpackage ([4a7a96b](4a7a96b)) * **entity:** reorganize entity-related files into dedicated package ([042e4f6](042e4f6)) * **rpc:** extract cobra command execution to entity.ExecutableCommand adapter ([de23c99](de23c99)) ### ✅ Tests * **markdown:** add kitchen sink roundtrip and clicky JSON test ([0b9045a](0b9045a)) * **prompt_test:** Fix race condition in PTY output capture by waiting for reader drain ([812891a](812891a)) ### ✨ Features * **aichat:** Add tool grouping support for preferences UI ([da7cc53](da7cc53)) * **aichat:** Add tool preferences, runtime settings, and request-scoped provider credentials ([c800642](c800642)) * **api:** add support for markdown block elements (heading, blockquote, footnotes) ([ebbb9ac](ebbb9ac)) * **cli:** add schema-less input format support for markdown, json, yaml, and text ([89e6d71](89e6d71)) * **docs,entity,examples:** Add local docs server and improve entity authoring guidance ([9679a4a](9679a4a)) * **entity:** add reusable named filters with typed and dynamic entity support ([7a830f3](7a830f3)) * **examples:** add markdown editor page with preview API ([c8076ea](c8076ea)) * **examples/entity:** add Vite HMR dev server support for local clicky-ui development ([9912ca1](9912ca1)) * **exec:** detect compiler activity during process startup ([dc6ebb8](dc6ebb8)) * **lint:** add severity levels and entity registration checks ([6c5ce9b](6c5ce9b)) * **markdown:** Add markdown parsing with structured document support ([656e8e4](656e8e4)) ### 🐛 Bug Fixes * **aichat,entity,markdown,rpc,cmd,lint:** address CodeRabbit findings on PR [#119](#119) ([1742312](1742312)), closes [#127](#127) [#128](#128) * **rpc,aichat,lint,api,exec:** address PR [#119](#119) CI failures and review comments ([e5e0aea](e5e0aea)) ### 👷 CI/CD * **release:** add sub-module release workflow ([e273b40](e273b40)) ### 📚 Documentation * **gitignore:** Remove *.md from gitignore and add documentation files ([71862e0](71862e0))
|
🎉 This PR is included in version 1.21.29 |
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
3 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.
What
StatusCompilingto track when build tools are actively runningwatchPortsfunction to acceptcompilationDetectorcallbackWhy
StatusRunningduring buildsNotes
Summary by CodeRabbit
--input-formatsupport for stdin/files, plus Markdown parsing/rendering upgrades (new block primitives; improved HTML-React).make docs.