Conversation
bash /tmp/v090/audit.sh . → verdict: COMPLIANT (all 7 dimensions zero). Co-authored-by: Codex <noreply@openai.com> Co-Authored-By: Virgil <virgil@lethean.io>
📝 WalkthroughWalkthroughThe PR consolidates Go dependencies by replacing explicit Changes
🚥 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. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.4)level=error msg="[linters_context] typechecking error: dappco.re/go@v0.9.0: replacement directory ../go does not exist" Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
pkg/help/server.go (1)
1-1:⚠️ Potential issue | 🟡 MinorTypo in SPDX header: "Licence" should be "License".
The SPDX identifier uses American English spelling "License", not "Licence".
Proposed fix
-// SPDX-Licence-Identifier: EUPL-1.2 +// SPDX-License-Identifier: EUPL-1.2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/help/server.go` at line 1, Update the SPDX header string at the top of the file by correcting the typo "Licence" to the standard SPDX identifier spelling "License" so the header reads "SPDX-License-Identifier: EUPL-1.2"; locate and edit the existing header comment in pkg/help/server.go (the top-of-file SPDX comment) to perform this single-word fix.pkg/help/templates_test.go (1)
1-1:⚠️ Potential issue | 🟡 MinorTypo in SPDX header.
Same "Licence" vs "License" typo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/help/templates_test.go` at line 1, Fix the SPDX header typo by changing the token "SPDX-Licence-Identifier: EUPL-1.2" to the correct "SPDX-License-Identifier: EUPL-1.2" so the SPDX identifier uses "License" (update the string that currently reads "SPDX-Licence-Identifier" in the file).pkg/help/layout.go (2)
413-414:⚠️ Potential issue | 🟠 MajorSnippet not escaped in search results.
On line 414,
r.Snippetis inserted directly into HTML without escaping. If the snippet contains user-provided content or markdown-derived text with HTML entities, this could lead to rendering issues or XSS if the search index ever indexes untrusted content.content.WriteString(fmt.Sprintf(`<p>%s</p>`, r.Snippet))Proposed fix
if r.Snippet != "" { - content.WriteString(fmt.Sprintf(`<p>%s</p>`, r.Snippet)) + content.WriteString(fmt.Sprintf(`<p>%s</p>`, html.EscapeString(r.Snippet))) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/help/layout.go` around lines 413 - 414, The snippet r.Snippet is written directly into HTML via content.WriteString(fmt.Sprintf(`<p>%s</p>`, r.Snippet)) which can lead to XSS or broken rendering; fix by HTML-escaping the snippet before insertion (e.g., use html.EscapeString on r.Snippet or render with text/template/htmlescape utilities) and then write the escaped value into content (update the code around content.WriteString and the use of r.Snippet accordingly so the output is safe).
1-1:⚠️ Potential issue | 🟡 MinorTypo in SPDX header: "Licence" should be "License".
Same issue as in server.go.
Proposed fix
-// SPDX-Licence-Identifier: EUPL-1.2 +// SPDX-License-Identifier: EUPL-1.2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/help/layout.go` at line 1, The SPDX header at the top of pkg/help/layout.go currently reads "SPDX-Licence-Identifier: EUPL-1.2" (typo "Licence"); update that top-of-file header string to "SPDX-License-Identifier: EUPL-1.2" and make the identical fix in server.go where the same header appears so both files use the correct SPDX-License-Identifier spelling.pkg/help/ingest_test.go (1)
1-1:⚠️ Potential issue | 🟡 MinorTypo in SPDX header.
Same "Licence" vs "License" typo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/help/ingest_test.go` at line 1, The SPDX header contains a typo "SPDX-Licence-Identifier" — update it to the correct token "SPDX-License-Identifier" in pkg/help/ingest_test.go (the file-level header string) so the SPDX header reads "SPDX-License-Identifier: EUPL-1.2".pkg/help/search_test.go (1)
1-1:⚠️ Potential issue | 🟡 MinorTypo in SPDX header.
Same "Licence" vs "License" typo present.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/help/search_test.go` at line 1, Update the SPDX header string "SPDX-Licence-Identifier: EUPL-1.2" to the correct spelling "SPDX-License-Identifier: EUPL-1.2" (replace "Licence" with "License") so the SPDX identifier is valid across the repository; ensure the same correction is applied wherever the exact token "SPDX-Licence-Identifier: EUPL-1.2" appears.pkg/help/parser_test.go (1)
1-1:⚠️ Potential issue | 🟡 MinorTypo in SPDX header.
Same "Licence" vs "License" typo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/help/parser_test.go` at line 1, The SPDX header string "SPDX-Licence-Identifier: EUPL-1.2" contains a typo ("Licence"); replace it with the correct identifier "SPDX-License-Identifier: EUPL-1.2" wherever it appears (e.g., at the top of parser_test.go) so the SPDX tag is valid and parsers/tools recognize the license.go.mod (1)
1-1:⚠️ Potential issue | 🟠 MajorModule name does not match coding guidelines.
The Go module name must be
forge.lthn.ai/core/docsas specified in the coding guidelines, but is currently set todappco.re/go/core/docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@go.mod` at line 1, The module declaration in go.mod is incorrect; open go.mod and replace the module path string "dappco.re/go/core/docs" with the required module name "forge.lthn.ai/core/docs" so the module directive (the module line at the top of go.mod) matches the coding guidelines and the rest of the project.
🧹 Nitpick comments (3)
pkg/help/parser_test.go (2)
641-646: Redundant assertions checking the same condition.Lines 643-645 all verify that
sectionsis empty using three different assertions. One assertion is sufficient.Proposed simplification
func TestExtractSections_Bad_EmptyString(t *T) { sections := ExtractSections("") AssertEmpty(t, sections) - AssertLen(t, sections, 0) - AssertFalse(t, len(sections) > 0) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/help/parser_test.go` around lines 641 - 646, The test TestExtractSections_Bad_EmptyString redundantly asserts the same emptiness condition three times; simplify by keeping a single clear assertion (e.g., AssertEmpty) and removing the duplicate AssertLen and AssertFalse checks. Update the test body for TestExtractSections_Bad_EmptyString to call ExtractSections("") once and assert emptiness via AssertEmpty(t, sections) (or a single AssertLen(t, sections, 0)) and remove AssertLen and AssertFalse to avoid duplicated checks.
675-680: Similar redundant assertions.
AssertEqual(t, "", result)andAssertEmpty(t, result)andAssertNotContains(t, result, "-")are checking overlapping conditions. An empty string cannot contain "-".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/help/parser_test.go` around lines 675 - 680, In TestGenerateID_Ugly_EmptyString, remove the redundant assertions so the test only asserts emptiness: drop AssertEqual(t, "", result) and AssertNotContains(t, result, "-") and keep a single AssertEmpty(t, result) (references: TestGenerateID_Ugly_EmptyString, AssertEqual, AssertEmpty, AssertNotContains).pkg/help/ax7_test.go (1)
39-636: Please avoid a second omnibus suite for behaviour already covered elsewhere.This file overlaps
catalog_test.go,generate_test.go,layout_test.go,render_test.go, andserver_test.goalmost one-for-one. Keeping both layers will double the maintenance burden and make failures harder to localise; I’d fold the extra edge cases into the existing focused test files instead of carrying a parallel AX7 suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/help/ax7_test.go` around lines 39 - 636, This AX7 omnibus test suite duplicates tests already present in catalog_test.go, generate_test.go, layout_test.go, render_test.go, and server_test.go; remove this redundant file (or prune it) and move any unique edge-case tests into the focused test files that cover the relevant area: e.g. move Topic/Index/Search edge cases to catalog_test.go (tests referencing Catalog, newSearchIndex, Tokens, Search, Add), move generation/output edge cases to generate_test.go (tests referencing Generate, RenderIndexPage, RenderTopicPage, RenderSearchPage, Render404Page, RenderMarkdown), and move HTTP/server edge cases to server_test.go (tests referencing NewServer, Server.ListenAndServe, ServeHTTP); ensure each moved test keeps its assertions and uses the existing helper funcs (ax7Catalog, ax7Topic) so there is no behavioral duplication across test files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/help/ax7_test.go`:
- Around line 1-10: This new test file in package help is missing the required
SPDX EUPL-1.2 header; add the standard SPDX header comment block (EUPL-1.2
license identifier and any required copyright line) at the very top of
ax7_test.go immediately before the "package help" declaration so the file
complies with the pkg/**/*.go SPDX header guideline.
In `@pkg/help/catalog.go`:
- Around line 1-2: Add the required SPDX license header for EUPL-1.2 at the top
of the source file so the package declaration (package help) is preceded by the
SPDX comment block; update the header to match the project's standard SPDX
format for pkg/**/*.go files and ensure it is placed before any package or
import statements to satisfy licensing checks.
In `@pkg/help/server_test.go`:
- Around line 4-9: The tests in pkg/help/server_test.go (and the other migrated
files) use a dot-import of dappco.re/go instead of the repo-standard testify;
revert the import to use "github.com/stretchr/testify/assert" and
"github.com/stretchr/testify/require" (remove . "dappco.re/go"), then update
assertion calls in functions like those in server_test.go to call assert.* and
require.* as appropriate; apply the same replacement across the other 12
migrated test files to restore consistent use of testify.
In `@pkg/help/server.go`:
- Around line 135-141: Replace the current double-write status flow when query
== "" so you don't call http.Error after already calling WriteHeader: either (A)
use http.Error to send the 400 with a plain text message (e.g. http.Error(w,
"missing query parameter 'q'", http.StatusBadRequest)) and remove
WriteHeader/Encode, or (B) keep the JSON response path but do not call
WriteHeader before a potential fallback http.Error — instead set the status only
when you know encoding succeeded and on encode failure log the error and write a
500 (e.g. call w.WriteHeader(http.StatusInternalServerError) before writing the
error body). Update the branch that uses json.NewEncoder(w).Encode(...)
accordingly so a second http.Error is never invoked after WriteHeader.
- Around line 117-122: The current handler calls
w.WriteHeader(http.StatusNotFound) and then uses json.NewEncoder(w).Encode(...)
and on encode error calls http.Error, which cannot change the already-sent
status; instead either (A) marshal/encode the JSON into a buffer first (use
json.Marshal or json.NewEncoder(&buf).Encode) and only after successful encoding
set w.Header().Set("Content-Type", "application/json"), call
w.WriteHeader(http.StatusNotFound), and write the buffer to w, or (B) if you
prefer streaming with json.NewEncoder(w).Encode, remove the http.Error call and
log the encoding error (e.g., log.Printf or your package logger) because the 404
has already been sent; update the code paths around json.NewEncoder(w).Encode,
w.WriteHeader, and http.Error accordingly.
In `@pkg/help/stemmer_test.go`:
- Around line 5-6: Remove the dot-import of "dappco.re/go" in
pkg/help/stemmer_test.go and add explicit imports for
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require";
then update all test and benchmark functions (those using *T, *B and the
dot-imported helpers like Assert* and Require*) to use the standard testify
calls (assert.* and require.*) and the local testing objects (t *testing.T, b
*testing.B) instead of the dot-imported helpers so the file follows the
repository guideline.
---
Outside diff comments:
In `@go.mod`:
- Line 1: The module declaration in go.mod is incorrect; open go.mod and replace
the module path string "dappco.re/go/core/docs" with the required module name
"forge.lthn.ai/core/docs" so the module directive (the module line at the top of
go.mod) matches the coding guidelines and the rest of the project.
In `@pkg/help/ingest_test.go`:
- Line 1: The SPDX header contains a typo "SPDX-Licence-Identifier" — update it
to the correct token "SPDX-License-Identifier" in pkg/help/ingest_test.go (the
file-level header string) so the SPDX header reads "SPDX-License-Identifier:
EUPL-1.2".
In `@pkg/help/layout.go`:
- Around line 413-414: The snippet r.Snippet is written directly into HTML via
content.WriteString(fmt.Sprintf(`<p>%s</p>`, r.Snippet)) which can lead to XSS
or broken rendering; fix by HTML-escaping the snippet before insertion (e.g.,
use html.EscapeString on r.Snippet or render with text/template/htmlescape
utilities) and then write the escaped value into content (update the code around
content.WriteString and the use of r.Snippet accordingly so the output is safe).
- Line 1: The SPDX header at the top of pkg/help/layout.go currently reads
"SPDX-Licence-Identifier: EUPL-1.2" (typo "Licence"); update that top-of-file
header string to "SPDX-License-Identifier: EUPL-1.2" and make the identical fix
in server.go where the same header appears so both files use the correct
SPDX-License-Identifier spelling.
In `@pkg/help/parser_test.go`:
- Line 1: The SPDX header string "SPDX-Licence-Identifier: EUPL-1.2" contains a
typo ("Licence"); replace it with the correct identifier
"SPDX-License-Identifier: EUPL-1.2" wherever it appears (e.g., at the top of
parser_test.go) so the SPDX tag is valid and parsers/tools recognize the
license.
In `@pkg/help/search_test.go`:
- Line 1: Update the SPDX header string "SPDX-Licence-Identifier: EUPL-1.2" to
the correct spelling "SPDX-License-Identifier: EUPL-1.2" (replace "Licence" with
"License") so the SPDX identifier is valid across the repository; ensure the
same correction is applied wherever the exact token "SPDX-Licence-Identifier:
EUPL-1.2" appears.
In `@pkg/help/server.go`:
- Line 1: Update the SPDX header string at the top of the file by correcting the
typo "Licence" to the standard SPDX identifier spelling "License" so the header
reads "SPDX-License-Identifier: EUPL-1.2"; locate and edit the existing header
comment in pkg/help/server.go (the top-of-file SPDX comment) to perform this
single-word fix.
In `@pkg/help/templates_test.go`:
- Line 1: Fix the SPDX header typo by changing the token
"SPDX-Licence-Identifier: EUPL-1.2" to the correct "SPDX-License-Identifier:
EUPL-1.2" so the SPDX identifier uses "License" (update the string that
currently reads "SPDX-Licence-Identifier" in the file).
---
Nitpick comments:
In `@pkg/help/ax7_test.go`:
- Around line 39-636: This AX7 omnibus test suite duplicates tests already
present in catalog_test.go, generate_test.go, layout_test.go, render_test.go,
and server_test.go; remove this redundant file (or prune it) and move any unique
edge-case tests into the focused test files that cover the relevant area: e.g.
move Topic/Index/Search edge cases to catalog_test.go (tests referencing
Catalog, newSearchIndex, Tokens, Search, Add), move generation/output edge cases
to generate_test.go (tests referencing Generate, RenderIndexPage,
RenderTopicPage, RenderSearchPage, Render404Page, RenderMarkdown), and move
HTTP/server edge cases to server_test.go (tests referencing NewServer,
Server.ListenAndServe, ServeHTTP); ensure each moved test keeps its assertions
and uses the existing helper funcs (ax7Catalog, ax7Topic) so there is no
behavioral duplication across test files.
In `@pkg/help/parser_test.go`:
- Around line 641-646: The test TestExtractSections_Bad_EmptyString redundantly
asserts the same emptiness condition three times; simplify by keeping a single
clear assertion (e.g., AssertEmpty) and removing the duplicate AssertLen and
AssertFalse checks. Update the test body for TestExtractSections_Bad_EmptyString
to call ExtractSections("") once and assert emptiness via AssertEmpty(t,
sections) (or a single AssertLen(t, sections, 0)) and remove AssertLen and
AssertFalse to avoid duplicated checks.
- Around line 675-680: In TestGenerateID_Ugly_EmptyString, remove the redundant
assertions so the test only asserts emptiness: drop AssertEqual(t, "", result)
and AssertNotContains(t, result, "-") and keep a single AssertEmpty(t, result)
(references: TestGenerateID_Ugly_EmptyString, AssertEqual, AssertEmpty,
AssertNotContains).
🪄 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: 790dd869-29b0-49bf-a4de-f02efcc5fa90
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
go.modpkg/help/ax7_test.gopkg/help/catalog.gopkg/help/catalog_test.gopkg/help/generate_test.gopkg/help/ingest_test.gopkg/help/integration_test.gopkg/help/layout.gopkg/help/layout_test.gopkg/help/parser_test.gopkg/help/render_test.gopkg/help/search_bench_test.gopkg/help/search_test.gopkg/help/server.gopkg/help/server_test.gopkg/help/stemmer_test.gopkg/help/templates_test.go
Comment on lines
+1
to
+10
| package help | ||
|
|
||
| import ( | ||
| . "dappco.re/go" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "os" | ||
| "path/filepath" | ||
| "slices" | ||
| ) |
There was a problem hiding this comment.
Add the SPDX header to this new file.
pkg/help/ax7_test.go is new, but it does not carry the required EUPL SPDX header at the top.
📌 Proposed fix
+// SPDX-License-Identifier: EUPL-1.2
package helpAs per coding guidelines, pkg/**/*.go: Use SPDX headers (EUPL-1.2 license) in source files.
📝 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
| package help | |
| import ( | |
| . "dappco.re/go" | |
| "net/http" | |
| "net/http/httptest" | |
| "os" | |
| "path/filepath" | |
| "slices" | |
| ) | |
| // SPDX-License-Identifier: EUPL-1.2 | |
| package help | |
| import ( | |
| . "dappco.re/go" | |
| "net/http" | |
| "net/http/httptest" | |
| "os" | |
| "path/filepath" | |
| "slices" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/help/ax7_test.go` around lines 1 - 10, This new test file in package help
is missing the required SPDX EUPL-1.2 header; add the standard SPDX header
comment block (EUPL-1.2 license identifier and any required copyright line) at
the very top of ax7_test.go immediately before the "package help" declaration so
the file complies with the pkg/**/*.go SPDX header guideline.
Comment on lines
1
to
2
| package help | ||
|
|
There was a problem hiding this comment.
Missing SPDX license header.
As per coding guidelines, source files in pkg/**/*.go should include an SPDX header with the EUPL-1.2 licence.
Proposed fix
+// SPDX-License-Identifier: EUPL-1.2
package help📝 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
| package help | |
| // SPDX-License-Identifier: EUPL-1.2 | |
| package help |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/help/catalog.go` around lines 1 - 2, Add the required SPDX license header
for EUPL-1.2 at the top of the source file so the package declaration (package
help) is preceded by the SPDX comment block; update the header to match the
project's standard SPDX format for pkg/**/*.go files and ensure it is placed
before any package or import statements to satisfy licensing checks.
Comment on lines
4
to
9
| import ( | ||
| . "dappco.re/go" | ||
| "encoding/json" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify which Go test files under pkg/help switched to dappco.re/go helpers
# without retaining the repo-standard testify imports.
fd -e go . pkg/help | while read -r f; do
[[ "$f" == *_test.go ]] || continue
if rg -q '^\s*\. "dappco\.re/go"' "$f" && ! rg -q 'github\.com/stretchr/testify/(assert|require)' "$f"; then
printf '%s\n' "$f"
fi
doneRepository: dAppCore/docs
Length of output: 384
Restore testify to Go test files.
This change migrates 13 test files in pkg/help/ from the repo-standard testify/assert and testify/require to dot-imported dappco.re/go helpers, violating the coding guideline: **/*_test.go: Use testify/assert and testify/require for test assertions in Go tests. Either restore testify across all migrated test files or update the guideline in this PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/help/server_test.go` around lines 4 - 9, The tests in
pkg/help/server_test.go (and the other migrated files) use a dot-import of
dappco.re/go instead of the repo-standard testify; revert the import to use
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require"
(remove . "dappco.re/go"), then update assertion calls in functions like those
in server_test.go to call assert.* and require.* as appropriate; apply the same
replacement across the other 12 migrated test files to restore consistent use of
testify.
Comment on lines
117
to
122
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(http.StatusNotFound) | ||
| _ = json.NewEncoder(w).Encode(map[string]string{"error": "topic not found"}) | ||
| if err := json.NewEncoder(w).Encode(map[string]string{"error": "topic not found"}); err != nil { | ||
| http.Error(w, "Internal server error", http.StatusInternalServerError) | ||
| } | ||
| return |
There was a problem hiding this comment.
http.Error after WriteHeader will not set status code correctly.
Once WriteHeader(http.StatusNotFound) is called on line 118, subsequent calls to http.Error on line 120 cannot change the status code to 500. The response will still have status 404 but with an "Internal server error" body, which is confusing. If JSON encoding fails after headers are sent, consider logging the error instead.
Proposed fix
if err != nil {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusNotFound)
- if err := json.NewEncoder(w).Encode(map[string]string{"error": "topic not found"}); err != nil {
- http.Error(w, "Internal server error", http.StatusInternalServerError)
- }
+ _ = json.NewEncoder(w).Encode(map[string]string{"error": "topic not found"})
return
}📝 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
| w.Header().Set("Content-Type", "application/json") | |
| w.WriteHeader(http.StatusNotFound) | |
| _ = json.NewEncoder(w).Encode(map[string]string{"error": "topic not found"}) | |
| if err := json.NewEncoder(w).Encode(map[string]string{"error": "topic not found"}); err != nil { | |
| http.Error(w, "Internal server error", http.StatusInternalServerError) | |
| } | |
| return | |
| w.Header().Set("Content-Type", "application/json") | |
| w.WriteHeader(http.StatusNotFound) | |
| _ = json.NewEncoder(w).Encode(map[string]string{"error": "topic not found"}) | |
| return |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/help/server.go` around lines 117 - 122, The current handler calls
w.WriteHeader(http.StatusNotFound) and then uses json.NewEncoder(w).Encode(...)
and on encode error calls http.Error, which cannot change the already-sent
status; instead either (A) marshal/encode the JSON into a buffer first (use
json.Marshal or json.NewEncoder(&buf).Encode) and only after successful encoding
set w.Header().Set("Content-Type", "application/json"), call
w.WriteHeader(http.StatusNotFound), and write the buffer to w, or (B) if you
prefer streaming with json.NewEncoder(w).Encode, remove the http.Error call and
log the encoding error (e.g., log.Printf or your package logger) because the 404
has already been sent; update the code paths around json.NewEncoder(w).Encode,
w.WriteHeader, and http.Error accordingly.
Comment on lines
135
to
141
| if query == "" { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(http.StatusBadRequest) | ||
| _ = json.NewEncoder(w).Encode(map[string]string{"error": "missing query parameter 'q'"}) | ||
| if err := json.NewEncoder(w).Encode(map[string]string{"error": "missing query parameter 'q'"}); err != nil { | ||
| http.Error(w, "Internal server error", http.StatusInternalServerError) | ||
| } | ||
| return |
There was a problem hiding this comment.
Same issue: http.Error after WriteHeader is ineffective.
The WriteHeader(http.StatusBadRequest) on line 137 prevents http.Error on line 139 from setting status 500. The client will receive a 400 response with "Internal server error" text.
Proposed fix
if query == "" {
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusBadRequest)
- if err := json.NewEncoder(w).Encode(map[string]string{"error": "missing query parameter 'q'"}); err != nil {
- http.Error(w, "Internal server error", http.StatusInternalServerError)
- }
+ _ = json.NewEncoder(w).Encode(map[string]string{"error": "missing query parameter 'q'"})
return
}📝 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
| if query == "" { | |
| w.Header().Set("Content-Type", "application/json") | |
| w.WriteHeader(http.StatusBadRequest) | |
| _ = json.NewEncoder(w).Encode(map[string]string{"error": "missing query parameter 'q'"}) | |
| if err := json.NewEncoder(w).Encode(map[string]string{"error": "missing query parameter 'q'"}); err != nil { | |
| http.Error(w, "Internal server error", http.StatusInternalServerError) | |
| } | |
| return | |
| if query == "" { | |
| w.Header().Set("Content-Type", "application/json") | |
| w.WriteHeader(http.StatusBadRequest) | |
| _ = json.NewEncoder(w).Encode(map[string]string{"error": "missing query parameter 'q'"}) | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/help/server.go` around lines 135 - 141, Replace the current double-write
status flow when query == "" so you don't call http.Error after already calling
WriteHeader: either (A) use http.Error to send the 400 with a plain text message
(e.g. http.Error(w, "missing query parameter 'q'", http.StatusBadRequest)) and
remove WriteHeader/Encode, or (B) keep the JSON response path but do not call
WriteHeader before a potential fallback http.Error — instead set the status only
when you know encoding succeeded and on encode failure log the error and write a
500 (e.g. call w.WriteHeader(http.StatusInternalServerError) before writing the
error body). Update the branch that uses json.NewEncoder(w).Encode(...)
accordingly so a second http.Error is never invoked after WriteHeader.
Comment on lines
+5
to
6
| . "dappco.re/go" | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify this file does not rely on dappco.re/go test helpers.
rg -n '^\s*\.\s*"dappco\.re/go"|func\s+\w+\(.*\*T\)|func\s+\w+\(.*\*B\)|\bAssert[A-Z]|\bRequire[A-Z]' pkg/help/stemmer_test.go
# Verify testify imports/assertions are present.
rg -n 'github\.com/stretchr/testify/(assert|require)|\bassert\.|\brequire\.' pkg/help/stemmer_test.goRepository: dAppCore/docs
Length of output: 1874
Restore testify/assert and testify/require in this test file.
The code uses dot-imported dappco.re/go helpers (*T, *B, Assert*, Require*) which conflicts with the repository's coding guideline requiring testify/assert and testify/require for **/*_test.go files.
Representative fix pattern
import (
- . "dappco.re/go"
+ "testing"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
-func TestStem_Good(t *T) {
+func TestStem_Good(t *testing.T) {
@@
- t.Run(tt.name, func(t *T) {
+ t.Run(tt.name, func(t *testing.T) {
result := stem(tt.input)
- AssertEqual(t, tt.expected, result)
+ assert.Equal(t, tt.expected, result)
})
}
}Also applies to lines: 12–221 (all test and benchmark functions).
📝 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
| . "dappco.re/go" | |
| ) | |
| import ( | |
| "testing" | |
| "github.com/stretchr/testify/assert" | |
| "github.com/stretchr/testify/require" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pkg/help/stemmer_test.go` around lines 5 - 6, Remove the dot-import of
"dappco.re/go" in pkg/help/stemmer_test.go and add explicit imports for
"github.com/stretchr/testify/assert" and "github.com/stretchr/testify/require";
then update all test and benchmark functions (those using *T, *B and the
dot-imported helpers like Assert* and Require*) to use the standard testify
calls (assert.* and require.*) and the local testing objects (t *testing.T, b
*testing.B) instead of the dot-imported helpers so the file follows the
repository guideline.
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
1 participant
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.
Brings this repo to verdict: COMPLIANT against the v0.9.0 audit.
🤖 Generated with Claude Code + Codex
Co-Authored-By: Codex noreply@openai.com
Co-Authored-By: Virgil virgil@lethean.io
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
Refactor