[sync] agent: 58 commits from Forge

[Snider]

Forge → GitHub Sync

Commits: 58
Files changed: 329

Large sync — may need splitting for CodeRabbit review.

---

Co-Authored-By: Virgil virgil@lethean.io

Summary by CodeRabbit

• New Features

  • Consolidated plugin architecture into a unified core platform with enhanced agent dispatch, verification, and review capabilities.
  • Introduced local development stack with Docker setup and automated environment configuration.
  • Added comprehensive agent orchestration system with workspace management, status tracking, and automated verification.
  • Implemented brain subsystem for persistent knowledge management and memory recall.
  • Added PR creation, review queue, and automated merge workflows.

• Documentation

  • New onboarding guides and setup documentation.
  • Updated architecture and tooling guidance.

• Chores

  • Removed legacy plugin and command implementations.
  • Updated dependencies and configuration files.

[coderabbitai]

Actionable comments posted: 8

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

🟡 Minor comments (18)

pkg/agentic/verify.go-331-338 (1)

331-338: ⚠️ Potential issue | 🟡 Minor

Make PR number extraction robust to trailing slash/query.

Current parsing fails for URLs like /pulls/123/ or /pulls/123?foo=bar.

Suggested fix

 func extractPRNumber(prURL string) int {
-	parts := strings.Split(prURL, "/")
+	clean := strings.TrimSpace(prURL)
+	clean = strings.SplitN(clean, "?", 2)[0]
+	clean = strings.TrimRight(clean, "/")
+	parts := strings.Split(clean, "/")
 	if len(parts) == 0 {
 		return 0
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/verify.go` around lines 331 - 338, The extractPRNumber function
fails on URLs with trailing slashes or query strings; update it to robustly
obtain the last path segment and parse the integer: parse the URL (using
net/url.Parse) or at minimum strip any query string after '?' and any trailing
'/' before splitting, then take the final path segment (or use path.Base on
parsed URL.Path) and Atoi it, returning 0 on parse errors; modify
extractPRNumber to use these steps so inputs like "/pulls/123/",
"/pulls/123?foo=bar" and full URLs all work.

pkg/agentic/verify.go-61-72 (1)

61-72: ⚠️ Potential issue | 🟡 Minor

Use the retry outcome when flagging for review.

flagForReview(..., result) uses the first attempt result even if the second attempt fails differently, so the posted reason can be wrong.

Suggested fix

-	result := s.attemptVerifyAndMerge(srcDir, org, st.Repo, st.Branch, prNum)
-	if result == mergeSuccess {
+	result := s.attemptVerifyAndMerge(srcDir, org, st.Repo, st.Branch, prNum)
+	if result == mergeSuccess {
 		markMerged()
 		return
 	}
@@
-	if result == mergeConflict || result == testFailed {
+	finalResult := result
+	if result == mergeConflict || result == testFailed {
 		if s.rebaseBranch(srcDir, st.Branch) {
-			if s.attemptVerifyAndMerge(srcDir, org, st.Repo, st.Branch, prNum) == mergeSuccess {
+			finalResult = s.attemptVerifyAndMerge(srcDir, org, st.Repo, st.Branch, prNum)
+			if finalResult == mergeSuccess {
 				markMerged()
 				return
 			}
 		}
 	}
@@
-	s.flagForReview(org, st.Repo, prNum, result)
+	s.flagForReview(org, st.Repo, prNum, finalResult)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/verify.go` around lines 61 - 72, The code currently passes the
original result into flagForReview even if the second attempt (rebase +
attemptVerifyAndMerge) produced a different outcome; update the logic to capture
the outcome of the second attempt and pass that final result to flagForReview
instead of the initial result. Concretely, call attemptVerifyAndMerge and assign
its return value to a variable (or overwrite result) when doing the rebase
branch flow (referencing s.rebaseBranch, s.attemptVerifyAndMerge), then use that
final outcome when invoking s.flagForReview(org, st.Repo, prNum, ...). Ensure
flagForReview receives the second attempt's result when the second attempt
fails.

pkg/agentic/review_queue.go-187-188 (1)

187-188: ⚠️ Potential issue | 🟡 Minor

Keep training verdict detection in lockstep with the runtime verdict.

Line 187 recognises LGTM as clean, but Line 336 does not. A clean LGTM review will therefore be recorded in reviews.jsonl as findings, which will skew the training corpus. Please share one isCleanReviewOutput() helper between both paths.

Also applies to: 329-337

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/review_queue.go` around lines 187 - 188, Create a single helper
function isCleanReviewOutput(output string) bool that encapsulates the
clean-detection logic (checking for "No findings", "no issues", "LGTM", etc.)
and replace the duplicated inline checks at the two call sites (the one setting
result.Verdict = "clean" and the other around the reviews.jsonl path currently
duplicated at the 329–337/336 area) to call this helper so both runtime verdict
and training-record logic stay in lockstep; ensure the helper is used by the
code that currently inspects output and that it treats "LGTM" the same as the
other clean indicators.

docs/plans/2026-03-16-issue-tracker.md-76-83 (1)

76-83: ⚠️ Potential issue | 🟡 Minor

Specify a fenced code language for the routes snippet.

This currently triggers markdownlint MD040 and hurts rendering/tooling consistency.

🔧 Suggested fix

-```
+```php
 Route::apiResource('issues', IssueController::class);
 Route::post('issues/{issue}/comments', [IssueController::class, 'addComment']);
 Route::get('issues/{issue}/comments', [IssueController::class, 'listComments']);
 Route::apiResource('sprints', SprintController::class)->only(['index', 'store']);
 Route::post('sprints/{sprint}/start', [SprintController::class, 'start']);
 Route::post('sprints/{sprint}/complete', [SprintController::class, 'complete']);

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @docs/plans/2026-03-16-issue-tracker.md around lines 76 - 83, The fenced code
block showing routing definitions lacks a language tag which triggers MD040;
update the block to use a PHP language fence by adding "php" after the opening
triple backticks so the snippet containing Route::apiResource('issues',
IssueController::class), Route::post('issues/{issue}/comments',
[IssueController::class, 'addComment']), Route::get('issues/{issue}/comments',
[IssueController::class, 'listComments']), Route::apiResource('sprints',
SprintController::class)->only(['index', 'store']),
Route::post('sprints/{sprint}/start', [SprintController::class, 'start']), and
Route::post('sprints/{sprint}/complete', [SprintController::class, 'complete'])
is fenced as PHP.

</details>

</blockquote></details>
<details>
<summary>claude/core/commands/review.md-14-15 (1)</summary><blockquote>

`14-15`: _⚠️ Potential issue_ | _🟡 Minor_

**Use `CORE_WORKSPACE` env var for path consistency with production scripts.**

Documentation should reference `${CORE_WORKSPACE:-$HOME/Code/.core}` like other workspace-aware scripts in the codebase (check-inbox.sh, workspace-status.sh). Hardcoding `.core/workspace` bypasses the established env var convention. The `main` branch reference is safe; the codebase consistently uses `main` as the default branch.

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @claude/core/commands/review.md around lines 14 - 15, Replace the hardcoded
".core/workspace/{workspace}/src" path in the two git commands with the
CORE_WORKSPACE-aware expansion used elsewhere; update the occurrences of "git -C
.core/workspace/{workspace}/src log --oneline main..HEAD" and "git -C
.core/workspace/{workspace}/src diff --stat main" to use
"${CORE_WORKSPACE:-$HOME/Code/.core}/workspace/{workspace}/src" so the docs
follow the same environment variable convention as check-inbox.sh and
workspace-status.sh.

</details>

</blockquote></details>
<details>
<summary>claude/core/scripts/workspace-status.sh-24-25 (1)</summary><blockquote>

`24-25`: _⚠️ Potential issue_ | _🟡 Minor_

**Handle missing log files gracefully.**

When no `agent-*.log` files exist, `$LOG` is empty, causing `basename "" .log` to output `.log` and `sed` to produce `.log` as the agent name. This results in confusing output like `workspace | .log | completed | 0b log | 0 commits`.


<details>
<summary>🛠️ Proposed fix</summary>

```diff
   # Check for agent log
   LOG=$(ls "$ws"/agent-*.log 2>/dev/null | head -1)
-  AGENT=$(basename "$LOG" .log 2>/dev/null | sed 's/agent-//')
+  if [ -n "$LOG" ]; then
+    AGENT=$(basename "$LOG" .log | sed 's/agent-//')
+  else
+    AGENT="none"
+  fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@claude/core/scripts/workspace-status.sh` around lines 24 - 25, The code
computes LOG and then unconditionally runs basename/sed which yields ".log" when
LOG is empty; update workspace-status.sh to handle missing log files by checking
LOG before deriving AGENT: if LOG is non-empty, set AGENT using basename "$LOG"
.log and sed 's/agent-//', otherwise set AGENT to an empty string or a sensible
placeholder (e.g., "none") so that downstream output doesn't show ".log"; ensure
the new check uses the existing LOG and AGENT symbols and avoids calling
basename with an empty argument.

pkg/agentic/remote_client.go-64-72 (1)

64-72: ⚠️ Potential issue | 🟡 Minor

Errors silently ignored during initialized notification.

Line 64 ignores the error from http.NewRequestWithContext, and lines 67-70 silently succeed even if the notification request fails. If the notification fails, the MCP session may be in an inconsistent state.

🛠️ Proposed fix

-	notifReq, _ := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(notifBody))
+	notifReq, err := http.NewRequestWithContext(ctx, "POST", url, bytes.NewReader(notifBody))
+	if err != nil {
+		return "", coreerr.E("mcpInitialize", "create notification request", err)
+	}
 	setHeaders(notifReq, token, sessionID)
 
 	notifResp, err := client.Do(notifReq)
-	if err == nil {
-		notifResp.Body.Close()
+	if err != nil {
+		return "", coreerr.E("mcpInitialize", "notification failed", err)
 	}
+	notifResp.Body.Close()
 
 	return sessionID, nil

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/remote_client.go` around lines 64 - 72, Check and propagate
errors when building and sending the initialized notification in
remote_client.go: do not ignore the error returned by http.NewRequestWithContext
when creating notifReq (validate err and return it), and check the result of
client.Do(notifReq) for both network errors and non-2xx HTTP status codes;
ensure notifResp.Body is closed only if notifResp != nil and return a
descriptive error (rather than silently succeeding) so the caller can detect a
failed notification; reference notifReq, setHeaders, client.Do, notifResp, and
sessionID when making these changes.

claude/core/agents/agent-task-code-simplifier.md-13-13 (1)

13-13: ⚠️ Potential issue | 🟡 Minor

Clarify the Git range semantics in the prompt.

origin/main..HEAD shows commits in HEAD not in origin/main (branch-relative changes), not "the last commit." The semantics are:

• HEAD^..HEAD — last commit only (fails in this repository; no parent commit)
• origin/main..HEAD — changes in HEAD relative to origin/main (works; appropriate for PR review)
• origin/main...HEAD — symmetric difference between branches (fails; no merge base)

The command in the prompt is correct for showing PR changes, but the prose "since the last commit" is misleading. Reword to clarify that this shows files changed relative to the main branch, not relative to the immediately preceding commit.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@claude/core/agents/agent-task-code-simplifier.md` at line 13, Update the
prose in agent-task-code-simplifier.md to remove "since the last commit" and
instead clarify that the command `git diff --name-only origin/main..HEAD` lists
files changed in HEAD relative to the main branch (branch-relative changes), not
the immediately preceding commit; mention the correct semantics and examples
(`HEAD^..HEAD` for the last commit, `origin/main..HEAD` for PR/branch changes,
and `origin/main...HEAD` for symmetric difference) so the prompt accurately
describes which range each form shows.

docker/scripts/setup.sh-89-89 (1)

89-89: ⚠️ Potential issue | 🟡 Minor

Avoid printing secrets to terminal output.

Echoing the Brain API key to stdout may expose it in terminal scrollback, CI logs, or screen recordings. Consider writing it to a file or instructing the user where to find it instead.

🛡️ Suggested improvement

-echo "Brain API key: $(grep CORE_BRAIN_KEY "$DOCKER_DIR/.env" | cut -d= -f2)"
+echo "Brain API key: See $DOCKER_DIR/.env (CORE_BRAIN_KEY)"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docker/scripts/setup.sh` at line 89, The script setup.sh currently echoes the
Brain API key via the line that reads CORE_BRAIN_KEY from "$DOCKER_DIR/.env" and
prints it to stdout; instead stop printing secrets by removing that echo and
either write the full key to a secure file with restrictive permissions (e.g.,
create a file like "$DOCKER_DIR/brain_key.txt" and use umask/chmod 600 before
saving the cut output) or print an instruction pointing users to the .env
location (without showing the value) or print a masked value; update the code
referencing CORE_BRAIN_KEY/DOCKER_DIR in setup.sh accordingly so the key is
never logged to stdout or CI logs.

CLAUDE.md-45-58 (1)

45-58: ⚠️ Potential issue | 🟡 Minor

The MCP tool count in this heading is off by one.

The table below lists 34 tools, not 33. Small issue, but this file is used as runtime context and the mismatch makes the guidance less trustworthy.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@CLAUDE.md` around lines 45 - 58, Update the heading that currently reads "MCP
Tools (33)" to reflect the actual count "MCP Tools (34)" so the header matches
the 34 items listed; locate the heading text "MCP Tools (33)" in the file (near
the tools table) and change the number to 34.

cmd/core-agent/main.go-84-85 (1)

84-85: ⚠️ Potential issue | 🟡 Minor

Handle os.UserHomeDir failure before building the PID path.

If HOME is unset, filepath.Join("", ".core", "core-agent.pid") resolves to a relative path, so PID tracking silently moves into the current working directory.

💡 Proposed fix

-		home, _ := os.UserHomeDir()
+		home, err := os.UserHomeDir()
+		if err != nil {
+			return cli.Wrap(err, "resolve home directory")
+		}
 		pidFile := filepath.Join(home, ".core", "core-agent.pid")

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/core-agent/main.go` around lines 84 - 85, The code calls os.UserHomeDir()
and ignores the error, which can produce a relative pidFile path; check the
returned error from os.UserHomeDir() before using the home value and handle it
(return error/exit or fall back to a safe absolute directory), then build
pidFile with filepath.Join(home, ".core", "core-agent.pid"); update the block
around the os.UserHomeDir call in main (where home and pidFile are set) to
validate the error and ensure pidFile is always an absolute, well-defined path.

pkg/brain/direct.go-133-151 (1)

133-151: ⚠️ Potential issue | 🟡 Minor

Missing fields in remember request payload.

The RememberInput struct (from pkg/brain/tools.go) includes Confidence, Supersedes, and ExpiresIn fields, but these are not forwarded to the API call. This means callers cannot use these features when storing memories via the direct subsystem.

🔧 Proposed fix to include missing fields

 result, err := s.apiCall(ctx, "POST", "/v1/brain/remember", map[string]any{
 	"content":  input.Content,
 	"type":     input.Type,
 	"tags":     input.Tags,
 	"project":  input.Project,
 	"agent_id": agentName(),
+	"confidence": input.Confidence,
+	"supersedes": input.Supersedes,
+	"expires_in": input.ExpiresIn,
 })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/brain/direct.go` around lines 133 - 151, The remember handler is not
forwarding three fields from RememberInput to the API; update the payload in
DirectSubsystem.remember to include input.Confidence, input.Supersedes and
input.ExpiresIn in the map passed to s.apiCall (use the same snake_case keys
style as the existing payload, e.g. "confidence", "supersedes", "expires_in") so
the API receives those values when creating a memory; keep existing behavior for
other fields and return values unchanged.

pkg/agentic/mirror.go-260-274 (1)

260-274: ⚠️ Potential issue | 🟡 Minor

Fragile JSON field extraction.

extractJSONField uses simple string manipulation that doesn't handle escaped quotes, nested objects, or arrays. For example, a URL containing \" or a field value with "url" as a substring could cause incorrect parsing. Consider using encoding/json for reliable extraction.

🔧 More robust approach using json.Unmarshal

-// extractJSONField extracts a simple string field from JSON array output.
-func extractJSONField(jsonStr, field string) string {
-	// Quick and dirty — works for gh CLI output like [{"url":"https://..."}]
-	key := fmt.Sprintf(`"%s":"`, field)
-	idx := strings.Index(jsonStr, key)
-	if idx < 0 {
-		return ""
-	}
-	start := idx + len(key)
-	end := strings.Index(jsonStr[start:], `"`)
-	if end < 0 {
-		return ""
-	}
-	return jsonStr[start : start+end]
-}
+// extractJSONField extracts a simple string field from JSON array output.
+func extractJSONField(jsonStr, field string) string {
+	var items []map[string]any
+	if err := json.Unmarshal([]byte(jsonStr), &items); err != nil || len(items) == 0 {
+		return ""
+	}
+	if val, ok := items[0][field].(string); ok {
+		return val
+	}
+	return ""
+}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/mirror.go` around lines 260 - 274, The current extractJSONField
function is brittle because it uses string searches to parse JSON; replace its
implementation to parse jsonStr with encoding/json (e.g., json.Unmarshal into
[]map[string]interface{} or a defined struct) and then safely read the requested
field from the first element, returning "" if the JSON is invalid or the field
is missing; update extractJSONField to handle escaped characters and nested
structures by using the parsed value (type-assert to string) instead of manual
substring operations.

pkg/agentic/prep.go-191-193 (1)

191-193: ⚠️ Potential issue | 🟡 Minor

Git branch creation error ignored.

If git checkout -b fails (e.g., branch already exists, or git error), the error is silently ignored. The workspace preparation continues but may be in an unexpected state.

🔧 Consider logging or handling the error

 branchCmd := exec.CommandContext(ctx, "git", "checkout", "-b", branchName)
 branchCmd.Dir = srcDir
-branchCmd.Run()
+if err := branchCmd.Run(); err != nil {
+	// Branch may already exist, try switching to it
+	switchCmd := exec.CommandContext(ctx, "git", "checkout", branchName)
+	switchCmd.Dir = srcDir
+	switchCmd.Run()
+}
 out.Branch = branchName

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/prep.go` around lines 191 - 193, The git branch creation error is
currently ignored; update the code around the exec.CommandContext call that
creates branchCmd (exec.CommandContext(ctx, "git", "checkout", "-b",
branchName)) to capture and handle the returned error from branchCmd.Run():
check the error value, log a clear message including the branchName and
underlying error (using the existing logger or processLogger), and return or
propagate the error (or fallback behavior) so workspace preparation does not
continue silently on failure.

pkg/agentic/scan.go-102-127 (1)

102-127: ⚠️ Potential issue | 🟡 Minor

Ignored errors in HTTP request creation and JSON decoding.

At line 104, http.NewRequestWithContext error is discarded. At line 120, json.Decode error is silently ignored, which could result in an empty slice being returned without any indication of parse failure. This pattern repeats in listRepoIssues.

🔧 Proposed fix for listOrgRepos

 func (s *PrepSubsystem) listOrgRepos(ctx context.Context, org string) ([]string, error) {
 	url := fmt.Sprintf("%s/api/v1/orgs/%s/repos?limit=50", s.forgeURL, org)
-	req, _ := http.NewRequestWithContext(ctx, "GET", url, nil)
+	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
+	if err != nil {
+		return nil, coreerr.E("scan.listOrgRepos", "failed to create request", err)
+	}
 	req.Header.Set("Authorization", "token "+s.forgeToken)

 	resp, err := s.client.Do(req)
 	// ...

 	var repos []struct {
 		Name string `json:"name"`
 	}
-	json.NewDecoder(resp.Body).Decode(&repos)
+	if err := json.NewDecoder(resp.Body).Decode(&repos); err != nil {
+		return nil, coreerr.E("scan.listOrgRepos", "failed to decode response", err)
+	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/scan.go` around lines 102 - 127, In listOrgRepos (and similarly
in listRepoIssues) stop ignoring errors from http.NewRequestWithContext and
json.NewDecoder.Decode: assign the returned error from NewRequestWithContext
(req, err := http.NewRequestWithContext(...)) and if non-nil return
coreerr.E("scan.listOrgRepos", "failed to create request", err); after reading
the response check the JSON decode error (err =
json.NewDecoder(resp.Body).Decode(&repos)) and if non-nil return
coreerr.E("scan.listOrgRepos", "failed to decode response", err); apply the same
pattern and error messages in listRepoIssues to surface request/parse failures
instead of silently returning empty results.

pkg/agentic/prep.go-176-189 (1)

176-189: ⚠️ Potential issue | 🟡 Minor

Task slug could become empty after sanitisation.

The slug sanitisation replaces invalid characters with -, then truncates to 40 chars, then trims - from both ends. If the task contains only invalid characters (e.g., all spaces or symbols), the result would be an empty string, leading to a branch name of just "agent/".

🔧 Add fallback for empty slug

 taskSlug = strings.Trim(taskSlug, "-")
+if taskSlug == "" {
+	taskSlug = fmt.Sprintf("task-%d", time.Now().Unix())
+}
 branchName := fmt.Sprintf("agent/%s", taskSlug)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/prep.go` around lines 176 - 189, After sanitising input.Task into
taskSlug (in the taskSlug mapping and trimming logic) add a fallback if taskSlug
is empty: generate a non-empty slug (e.g., "untitled" or a short deterministic
fallback such as "task-<short-hash-of-input.Task>") and assign it to taskSlug
before building branchName; update the code around taskSlug and branchName
(fmt.Sprintf("agent/%s", taskSlug)) so branchName is never "agent/" even when
input.Task contains only invalid characters.

pkg/agentic/plan.go-230-232 (1)

230-232: ⚠️ Potential issue | 🟡 Minor

Normalise phase fields on update, not only on create.

When phases are replaced in Line 230, defaults applied in create flow (status/number) are skipped. This allows inconsistent phase data after updates.

♻️ Suggested fix

 	if input.Phases != nil {
 		plan.Phases = input.Phases
+		for i := range plan.Phases {
+			if plan.Phases[i].Status == "" {
+				plan.Phases[i].Status = "pending"
+			}
+			if plan.Phases[i].Number == 0 {
+				plan.Phases[i].Number = i + 1
+			}
+		}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/plan.go` around lines 230 - 232, The code currently replaces
plan.Phases with input.Phases but skips the create-flow normalization
(defaulting fields like status and number), causing inconsistent data after
updates; after assigning plan.Phases = input.Phases ensure you run the same
normalization used on create — e.g., iterate plan.Phases and for each phase set
default Status (e.g., "pending") if empty and set Number to its index+1 if zero,
or invoke the existing normalizePhases/normalizePhase helper used during
creation so updated phases get the same defaults.

pkg/agentic/plan.go-344-346 (1)

344-346: ⚠️ Potential issue | 🟡 Minor

Handle rand.Read error in ID generation.

The rand.Read call at line 345 ignores errors. If entropy reading fails, the generated ID will use uninitialized bytes, degrading suffix quality and increasing collision risk.

🛠️ Suggested fix

 	// Append short random suffix for uniqueness
 	b := make([]byte, 3)
-	rand.Read(b)
+	if _, err := rand.Read(b); err != nil {
+		return fmt.Sprintf("%s-%d", slug, time.Now().UnixNano())
+	}
 	return slug + "-" + hex.EncodeToString(b)

Note: The fmt package import must be added if not already present.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/plan.go` around lines 344 - 346, The call to rand.Read(b) must
check for errors: replace the ignored call with n, err := rand.Read(b) and if
err != nil log the error (e.g., using fmt.Printf or process logger) and return a
safe fallback suffix (for example hex.EncodeToString([]byte{0,0,0}) or another
deterministic value) to avoid using uninitialized bytes; otherwise use
hex.EncodeToString(b) as before — update imports to include fmt if you use
fmt.Printf. Ensure you edit the rand.Read, b, and hex.EncodeToString usage in
this function accordingly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 01b2d38d-df4a-49f9-8aad-ee7daf8ec958

📥 Commits

Reviewing files that changed from the base of the PR and between b317064 and 85bdf26.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (299)

• .claude-plugin/marketplace.json
• .gitignore
• .mcp.json
• CLAUDE.md
• CODEX.md
• GEMINI.md
• agents/examples/README.md
• agents/examples/nexus-spatial-discovery.md
• agents/examples/workflow-landing-page.md
• agents/examples/workflow-startup-mvp.md
• agents/examples/workflow-with-memory.md
• claude/code/.claude-plugin/plugin.json
• claude/code/hooks/hooks.json
• claude/code/hooks/prefer-core.sh
• claude/code/scripts/block-docs.sh
• claude/code/scripts/capture-context.sh
• claude/code/scripts/extract-actionables.sh
• claude/code/scripts/post-commit-check.sh
• claude/code/scripts/pr-created.sh
• claude/code/scripts/session-start.sh
• claude/code/scripts/suggest-compact.sh
• claude/core/.claude-plugin/plugin.json
• claude/core/agents/agent-task-code-review.md
• claude/core/agents/agent-task-code-simplifier.md
• claude/core/collection/HOOKS.md
• claude/core/collection/collect-whitepaper.sh
• claude/core/collection/dispatch.sh
• claude/core/collection/hooks.json
• claude/core/collection/update-index.sh
• claude/core/commands/code-review.md
• claude/core/commands/dispatch.md
• claude/core/commands/pipeline.md
• claude/core/commands/ready.md
• claude/core/commands/recall.md
• claude/core/commands/remember.md
• claude/core/commands/review-pr.md
• claude/core/commands/review.md
• claude/core/commands/scan.md
• claude/core/commands/security.md
• claude/core/commands/status.md
• claude/core/commands/sweep.md
• claude/core/commands/tests.md
• claude/core/commands/verify.md
• claude/core/commands/yes.md
• claude/core/hooks.json
• claude/core/mcp.json
• claude/core/scripts/auto-approve.sh
• claude/core/scripts/check-completions.sh
• claude/core/scripts/check-debug.sh
• claude/core/scripts/check-inbox.sh
• claude/core/scripts/check-notify.sh
• claude/core/scripts/ensure-commit.sh
• claude/core/scripts/go-format.sh
• claude/core/scripts/local-dispatch.sh
• claude/core/scripts/php-format.sh
• claude/core/scripts/post-pr-create.sh
• claude/core/scripts/pre-compact.sh
• claude/core/scripts/pre-push-check.sh
• claude/core/scripts/session-save.sh
• claude/core/scripts/session-start.sh
• claude/core/scripts/workspace-status.sh
• claude/core/skills/architecture-review.md
• claude/core/skills/bitcointalk/SKILL.md
• claude/core/skills/bitcointalk/collect.sh
• claude/core/skills/block-explorer/SKILL.md
• claude/core/skills/block-explorer/generate-jobs.sh
• claude/core/skills/coinmarketcap/SKILL.md
• claude/core/skills/coinmarketcap/generate-jobs.sh
• claude/core/skills/coinmarketcap/process.sh
• claude/core/skills/community-chat/SKILL.md
• claude/core/skills/cryptonote-discovery/SKILL.md
• claude/core/skills/cryptonote-discovery/discover.sh
• claude/core/skills/cryptonote-discovery/registry.json
• claude/core/skills/github-history/SKILL.md
• claude/core/skills/github-history/collect.sh
• claude/core/skills/job-collector/SKILL.md
• claude/core/skills/job-collector/generate-jobs.sh
• claude/core/skills/job-collector/process.sh
• claude/core/skills/ledger-papers/SKILL.md
• claude/core/skills/ledger-papers/archive/00-genesis/README.md
• claude/core/skills/ledger-papers/archive/01-cryptonote/README.md
• claude/core/skills/ledger-papers/archive/02-mrl/README.md
• claude/core/skills/ledger-papers/archive/03-privacy/README.md
• claude/core/skills/ledger-papers/archive/04-smart-contracts/README.md
• claude/core/skills/ledger-papers/archive/05-layer2/README.md
• claude/core/skills/ledger-papers/archive/06-consensus/README.md
• claude/core/skills/ledger-papers/archive/07-cryptography/README.md
• claude/core/skills/ledger-papers/archive/08-defi/README.md
• claude/core/skills/ledger-papers/archive/09-storage/README.md
• claude/core/skills/ledger-papers/archive/10-identity/README.md
• claude/core/skills/ledger-papers/archive/11-dag/README.md
• claude/core/skills/ledger-papers/archive/12-mev/README.md
• claude/core/skills/ledger-papers/archive/13-standards-btc/README.md
• claude/core/skills/ledger-papers/archive/14-standards-eth/README.md
• claude/core/skills/ledger-papers/archive/15-p2p/README.md
• claude/core/skills/ledger-papers/archive/16-zk-advanced/README.md
• claude/core/skills/ledger-papers/archive/17-oracles/README.md
• claude/core/skills/ledger-papers/archive/18-bridges/README.md
• claude/core/skills/ledger-papers/archive/19-attacks/README.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/README.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/graft/README.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/graft/RFC-001-GSD-general-supernode-design.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/graft/RFC-002-SLS-supernode-list-selection.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/graft/RFC-003-RTVF-rta-transaction-validation.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/graft/RFC-005-DF-disqualification-flow.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/graft/auth-sample-selection-algorithm.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/graft/blockchain-based-list-selection-analysis.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/graft/communication-options-p2p-design.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/graft/rta-double-spend-attack-vectors.md
• claude/core/skills/ledger-papers/archive/20-cryptonote-projects/graft/udht-implementation.md
• claude/core/skills/ledger-papers/archive/README.md
• claude/core/skills/ledger-papers/discover.sh
• claude/core/skills/ledger-papers/registry.json
• claude/core/skills/mining-pools/SKILL.md
• claude/core/skills/mining-pools/generate-jobs.sh
• claude/core/skills/orchestrate.md
• claude/core/skills/project-archaeology/SKILL.md
• claude/core/skills/project-archaeology/digs/graftnetwork/SALVAGE-REPORT.md
• claude/core/skills/project-archaeology/excavate.sh
• claude/core/skills/project-archaeology/templates/LESSONS.md
• claude/core/skills/project-archaeology/templates/SALVAGE-REPORT.md
• claude/core/skills/prompts.md
• claude/core/skills/reality-check.md
• claude/core/skills/review-pipeline.md
• claude/core/skills/security-review.md
• claude/core/skills/senior-dev-fix.md
• claude/core/skills/test-analysis.md
• claude/core/skills/wallet-releases/SKILL.md
• claude/core/skills/whitepaper-archive/SKILL.md
• claude/review/.claude-plugin/plugin.json
• claude/review/hooks.json
• claude/verify/.claude-plugin/plugin.json
• claude/verify/hooks.json
• cmd/agent/cmd.go
• cmd/core-agent/main.go
• cmd/dispatch/cmd.go
• cmd/dispatch/ratelimit.go
• cmd/mcp/core_cli.go
• cmd/mcp/core_cli_test.go
• cmd/mcp/ethics.go
• cmd/mcp/ethics_test.go
• cmd/mcp/main.go
• cmd/mcp/marketplace.go
• cmd/mcp/marketplace_test.go
• cmd/mcp/plugin_info.go
• cmd/mcp/server.go
• cmd/mcp/types.go
• cmd/mcp/util.go
• cmd/taskgit/cmd.go
• cmd/tasks/cmd.go
• cmd/tasks/updates.go
• cmd/workspace/cmd.go
• cmd/workspace/cmd_agent.go
• cmd/workspace/cmd_agent_test.go
• cmd/workspace/cmd_prep.go
• cmd/workspace/cmd_task.go
• cmd/workspace/cmd_task_test.go
• cmd/workspace/cmd_workspace.go
• cmd/workspace/config.go
• config/agents.yaml
• docker/.env.example
• docker/Dockerfile
• docker/config/octane.ini
• docker/config/supervisord.conf
• docker/config/traefik-tls.yml
• docker/docker-compose.yml
• docker/scripts/entrypoint.sh
• docker/scripts/setup.sh
• docs/CHARON-ONBOARDING.md
• docs/github-app-setup.md
• docs/plans/2026-03-15-local-stack.md
• docs/plans/2026-03-16-issue-tracker.md
• go.mod
• google/gemini-cli/GEMINI.md
• google/gemini-cli/gemini-extension.json
• google/mcp/README.md
• google/mcp/main.go
• google/mcp/main_test.go
• google/mcp/testdata/core
• pkg/agentic/auto_pr.go
• pkg/agentic/dispatch.go
• pkg/agentic/epic.go
• pkg/agentic/events.go
• pkg/agentic/ingest.go
• pkg/agentic/mirror.go
• pkg/agentic/paths.go
• pkg/agentic/paths_test.go
• pkg/agentic/plan.go
• pkg/agentic/pr.go
• pkg/agentic/prep.go
• pkg/agentic/queue.go
• pkg/agentic/remote.go
• pkg/agentic/remote_client.go
• pkg/agentic/remote_status.go
• pkg/agentic/resume.go
• pkg/agentic/review_queue.go
• pkg/agentic/scan.go
• pkg/agentic/status.go
• pkg/agentic/verify.go
• pkg/agentic/watch.go
• pkg/brain/brain.go
• pkg/brain/brain_test.go
• pkg/brain/direct.go
• pkg/brain/messaging.go
• pkg/brain/provider.go
• pkg/brain/tools.go
• pkg/jobrunner/coverage_boost_test.go
• pkg/jobrunner/forgejo/signals.go
• pkg/jobrunner/forgejo/signals_test.go
• pkg/jobrunner/forgejo/source.go
• pkg/jobrunner/forgejo/source_extra_test.go
• pkg/jobrunner/forgejo/source_phase3_test.go
• pkg/jobrunner/forgejo/source_supplementary_test.go
• pkg/jobrunner/forgejo/source_test.go
• pkg/jobrunner/handlers/completion.go
• pkg/jobrunner/handlers/completion_test.go
• pkg/jobrunner/handlers/coverage_boost_test.go
• pkg/jobrunner/handlers/dispatch.go
• pkg/jobrunner/handlers/dispatch_test.go
• pkg/jobrunner/handlers/enable_auto_merge.go
• pkg/jobrunner/handlers/enable_auto_merge_test.go
• pkg/jobrunner/handlers/handlers_extra_test.go
• pkg/jobrunner/handlers/integration_test.go
• pkg/jobrunner/handlers/publish_draft.go
• pkg/jobrunner/handlers/publish_draft_test.go
• pkg/jobrunner/handlers/resolve_threads.go
• pkg/jobrunner/handlers/resolve_threads_test.go
• pkg/jobrunner/handlers/send_fix_command.go
• pkg/jobrunner/handlers/send_fix_command_test.go
• pkg/jobrunner/handlers/testhelper_test.go
• pkg/jobrunner/handlers/tick_parent.go
• pkg/jobrunner/handlers/tick_parent_test.go
• pkg/jobrunner/journal.go
• pkg/jobrunner/journal_replay_test.go
• pkg/jobrunner/journal_test.go
• pkg/jobrunner/poller.go
• pkg/jobrunner/poller_test.go
• pkg/jobrunner/types.go
• pkg/jobrunner/types_test.go
• pkg/lifecycle/allowance.go
• pkg/lifecycle/allowance_edge_test.go
• pkg/lifecycle/allowance_error_test.go
• pkg/lifecycle/allowance_redis.go
• pkg/lifecycle/allowance_redis_test.go
• pkg/lifecycle/allowance_service.go
• pkg/lifecycle/allowance_sqlite.go
• pkg/lifecycle/allowance_sqlite_test.go
• pkg/lifecycle/allowance_test.go
• pkg/lifecycle/brain.go
• pkg/lifecycle/brain_test.go
• pkg/lifecycle/client.go
• pkg/lifecycle/client_test.go
• pkg/lifecycle/completion.go
• pkg/lifecycle/completion_git_test.go
• pkg/lifecycle/completion_test.go
• pkg/lifecycle/config.go
• pkg/lifecycle/config_test.go
• pkg/lifecycle/context.go
• pkg/lifecycle/context_git_test.go
• pkg/lifecycle/context_test.go
• pkg/lifecycle/coverage_boost_test.go
• pkg/lifecycle/dispatcher.go
• pkg/lifecycle/dispatcher_test.go
• pkg/lifecycle/embed.go
• pkg/lifecycle/embed_test.go
• pkg/lifecycle/events.go
• pkg/lifecycle/events_integration_test.go
• pkg/lifecycle/events_test.go
• pkg/lifecycle/lifecycle_test.go
• pkg/lifecycle/logs.go
• pkg/lifecycle/logs_test.go
• pkg/lifecycle/plan_dispatcher.go
• pkg/lifecycle/plans.go
• pkg/lifecycle/prompts/commit.md
• pkg/lifecycle/registry.go
• pkg/lifecycle/registry_redis.go
• pkg/lifecycle/registry_redis_test.go
• pkg/lifecycle/registry_sqlite.go
• pkg/lifecycle/registry_sqlite_test.go
• pkg/lifecycle/registry_test.go
• pkg/lifecycle/router.go
• pkg/lifecycle/router_test.go
• pkg/lifecycle/score.go
• pkg/lifecycle/score_test.go
• pkg/lifecycle/service.go
• pkg/lifecycle/service_test.go
• pkg/lifecycle/sessions.go
• pkg/lifecycle/status.go
• pkg/lifecycle/status_test.go
• pkg/lifecycle/submit.go
• pkg/lifecycle/submit_test.go
• pkg/lifecycle/types.go
• pkg/loop/engine.go
• pkg/loop/engine_test.go
• pkg/loop/parse.go
• pkg/loop/parse_test.go
• pkg/loop/prompt.go
• pkg/loop/prompt_test.go
• pkg/loop/tools_eaas.go

💤 Files with no reviewable changes (58)

• claude/code/scripts/pr-created.sh
• claude/code/hooks/hooks.json
• google/mcp/testdata/core
• pkg/jobrunner/forgejo/source_extra_test.go
• google/mcp/README.md
• cmd/mcp/ethics.go
• google/mcp/main_test.go
• claude/verify/hooks.json
• pkg/jobrunner/forgejo/source_phase3_test.go
• cmd/tasks/cmd.go
• agents/examples/workflow-startup-mvp.md
• cmd/mcp/main.go
• claude/code/scripts/suggest-compact.sh
• agents/examples/nexus-spatial-discovery.md
• cmd/taskgit/cmd.go
• cmd/mcp/marketplace_test.go
• pkg/jobrunner/handlers/completion.go
• cmd/mcp/ethics_test.go
• claude/code/scripts/extract-actionables.sh
• cmd/tasks/updates.go
• pkg/jobrunner/forgejo/source_test.go
• cmd/mcp/util.go
• claude/code/scripts/block-docs.sh
• pkg/jobrunner/handlers/completion_test.go
• agents/examples/README.md
• agents/examples/workflow-with-memory.md
• claude/code/scripts/capture-context.sh
• cmd/mcp/marketplace.go
• google/mcp/main.go
• cmd/mcp/plugin_info.go
• cmd/mcp/types.go
• claude/code/scripts/post-commit-check.sh
• pkg/jobrunner/forgejo/signals_test.go
• claude/code/hooks/prefer-core.sh
• claude/code/.claude-plugin/plugin.json
• agents/examples/workflow-landing-page.md
• cmd/dispatch/ratelimit.go
• cmd/agent/cmd.go
• cmd/mcp/core_cli.go
• cmd/workspace/cmd_agent.go
• claude/code/scripts/session-start.sh
• cmd/workspace/cmd_task_test.go
• cmd/workspace/cmd_agent_test.go
• pkg/jobrunner/coverage_boost_test.go
• cmd/workspace/cmd.go
• pkg/jobrunner/forgejo/signals.go
• pkg/jobrunner/forgejo/source.go
• cmd/mcp/core_cli_test.go
• claude/verify/.claude-plugin/plugin.json
• cmd/workspace/cmd_task.go
• claude/review/.claude-plugin/plugin.json
• claude/review/hooks.json
• cmd/mcp/server.go
• pkg/jobrunner/forgejo/source_supplementary_test.go
• cmd/dispatch/cmd.go
• cmd/workspace/config.go
• cmd/workspace/cmd_prep.go
• cmd/workspace/cmd_workspace.go

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Block path traversal by validating plan IDs before path join.

Line 315 joins id directly into the filesystem path. Because input.ID is user-controlled in read/update/delete flows, values like ../ can escape PlansRoot() and allow arbitrary file read/delete.

🔐 Suggested fix

+func validatePlanID(id string) error {
+	if id == "" || strings.Contains(id, "..") || strings.ContainsRune(id, '/') || strings.ContainsRune(id, '\\') {
+		return coreerr.E("validatePlanID", "invalid plan id", nil)
+	}
+	return nil
+}
+
 func planPath(dir, id string) string {
 	return filepath.Join(dir, id+".json")
 }

 func (s *PrepSubsystem) planRead(_ context.Context, _ *mcp.CallToolRequest, input PlanReadInput) (*mcp.CallToolResult, PlanReadOutput, error) {
 	if input.ID == "" {
 		return nil, PlanReadOutput{}, coreerr.E("planRead", "id is required", nil)
 	}
+	if err := validatePlanID(input.ID); err != nil {
+		return nil, PlanReadOutput{}, err
+	}

Apply the same validatePlanID check in planUpdate and planDelete.

📝 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 planPath(dir, id string) string {
	return filepath.Join(dir, id+".json")
	}
	func validatePlanID(id string) error {
	if id == "" || strings.Contains(id, "..") || strings.ContainsRune(id, '/') || strings.ContainsRune(id, '\\') {
	return coreerr.E("validatePlanID", "invalid plan id", nil)
	}
	return nil
	}
	func planPath(dir, id string) string {
	return filepath.Join(dir, id+".json")
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/plan.go` around lines 315 - 317, The planPath helper currently
joins the raw id into a filesystem path, allowing path traversal; add and use
validatePlanID to reject IDs containing path separators or traversal patterns
before constructing paths and apply this validation in planUpdate and planDelete
flows (and any other callers) so planPath only receives a validated id, and
return/handle an error when validatePlanID fails to prevent
reading/updating/deleting files outside PlansRoot.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, find and examine the relevant file
find . -name "pr.go" -type f | head -20

Repository: dAppCore/agent

Length of output: 77

---

🏁 Script executed:

# Check the complete createPR function implementation
rg -A 30 "func.*createPR" pkg/agentic/

Repository: dAppCore/agent

Length of output: 1774

---

🏁 Script executed:

# Look for WorkspaceRoot definition
rg -B 2 -A 5 "func.*WorkspaceRoot" .

Repository: dAppCore/agent

Length of output: 1023

---

🏁 Script executed:

# Check for existing path validation on input.Workspace
rg -B 5 -A 5 "input\.Workspace" pkg/agentic/

Repository: dAppCore/agent

Length of output: 4481

---

🏁 Script executed:

# Check how git commands are executed with srcDir
rg -B 3 -A 3 "git.*rev-parse\|git.*push" pkg/agentic/

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Find the git push command in pr.go
rg -B 5 -A 10 "git.*push" pkg/agentic/pr.go

Repository: dAppCore/agent

Length of output: 642

---

🏁 Script executed:

# Check if there's any path validation utility in the codebase
rg -i "filepath.*clean\|validate.*path\|secure.*path" pkg/agentic/

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Look for tests to understand intended behaviour
fd -e "*test.go" pkg/agentic/ -exec grep -l "Workspace" {} \;

Repository: dAppCore/agent

Length of output: 302

---

🏁 Script executed:

# Check status.go to see how it handles workspace names
cat -n pkg/agentic/status.go | head -60

Repository: dAppCore/agent

Length of output: 2818

---

🏁 Script executed:

# Check input struct definitions for CreatePRInput
rg -B 5 -A 5 "type CreatePRInput" pkg/agentic/

Repository: dAppCore/agent

Length of output: 793

---

🏁 Script executed:

# Check if there's any validation of workspace names in the subsystem initialization
rg -B 10 -A 10 "type PrepSubsystem" pkg/agentic/

Repository: dAppCore/agent

Length of output: 918

---

🏁 Script executed:

# Look for any tests that verify workspace path safety
rg "createPR\|Workspace.*\\.\\." pkg/agentic/ -t go

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Check how workspace names are set/managed - look at dispatch or creation logic
rg -B 3 -A 8 "NewWorkspace\|CreateWorkspace" pkg/agentic/

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Check if this is an MCP-exposed interface and where workspace names come from
rg -B 5 -A 15 "RegisterTool.*createPR\|agentic_create_pr" pkg/agentic/

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Look for where workspace names are generated/validated at creation time
rg -B 5 -A 10 "dispatch\|workspace.*name" pkg/agentic/ -i | head -80

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Verify filepath.Join behaviour with .. components to be absolutely sure
python3 << 'EOF'
import os
import posixpath

# Simulate Go's filepath.Join behavior on Unix
root = "/tmp/workspace"
workspace = "../../etc"
result = posixpath.normpath(posixpath.join(root, workspace))
print(f"Root: {root}")
print(f"Input: {workspace}")
print(f"Result: {result}")
print(f"Escaped root? {not result.startswith(root)}")
print()

# More realistic examples
examples = [
    ("workspace1", "/tmp/workspace/workspace1"),
    ("../../../etc", "/etc"),
    ("ws/../../../etc", "/etc"),
    ("ws/normal", "/tmp/workspace/ws/normal"),
]

for inp, expected in examples:
    result = posixpath.normpath(posixpath.join(root, inp))
    print(f"Input: {inp:20} → {result:30} (Expected: {expected}, Match: {result == expected})")
EOF

Repository: dAppCore/agent

Length of output: 533

---

🏁 Script executed:

# Look for dispatch function and how workspace names are generated
rg -B 10 -A 20 "func.*dispatch\|Workspace.*=" pkg/agentic/ | head -120

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Check the MCP tool definitions to see what fields are exposed
rg -B 2 -A 8 "\"workspace\".*json" pkg/agentic/pr.go

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Verify if workspace names are user-controlled input via MCP or system-generated
rg -B 5 -A 10 "dispatch.*Input\|DispatchInput" pkg/agentic/

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Find all files in pkg/agentic to understand the codebase structure
fd . pkg/agentic/ -type f -name "*.go" | sort

Repository: dAppCore/agent

Length of output: 228

---

🏁 Script executed:

# Check dispatch.go or similar files
cat -n pkg/agentic/dispatch.go 2>/dev/null | head -100

Repository: dAppCore/agent

Length of output: 4690

---

🏁 Script executed:

# Look at the MCP tool registration and what inputs are exposed
rg "RegisterTool\|AddTool" pkg/agentic/ -A 5 | head -60

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Search for where input.Workspace is used - trace back to caller
rg "createPR\(" pkg/agentic/ -B 3 -A 3

Repository: dAppCore/agent

Length of output: 463

---

🏁 Script executed:

# Check the dispatch function to see how workspace directory is created
rg -B 5 -A 30 "func.*dispatch.*ctx.*context" pkg/agentic/dispatch.go | head -80

Repository: dAppCore/agent

Length of output: 1081

---

🏁 Script executed:

# Look for workspace directory creation and naming
rg "WorkspaceRoot\|wsDir.*=" pkg/agentic/dispatch.go -B 3 -A 3 | head -50

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Check if there's any input validation or sanitization applied to workspace names
rg -i "clean\|sanitize\|filepath.Base\|basename" pkg/agentic/

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Find the prepWorkspace function to see where workspace naming happens
rg -B 5 -A 40 "func.*prepWorkspace" pkg/agentic/

Repository: dAppCore/agent

Length of output: 2401

---

🏁 Script executed:

# Look for workspace name generation (likely using timestamp or random)
rg "workspace.*uuid\|workspace.*time\|workspace.*rand\|fmt.Sprintf.*workspace" pkg/agentic/ -i

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Check the full createPR function to understand the complete flow
sed -n '40,120p' pkg/agentic/pr.go

Repository: dAppCore/agent

Length of output: 2302

---

Validate input.Workspace against path traversal before using it in Git operations.

The createPR function accepts input.Workspace as untrusted caller input. Using filepath.Join(WorkspaceRoot(), input.Workspace) without validation allows path traversal: an attacker can pass "../../../tmp" or similar to escape WorkspaceRoot() and redirect Git operations (git rev-parse, git push) to arbitrary filesystem locations. The os.Stat() check only verifies path existence, not bounds.

Apply filepath.Base() to strip directory components, or validate that input.Workspace matches the expected workspace name format (e.g., {repo}-{timestamp}).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/pr.go` around lines 57 - 62, The code uses input.Workspace
directly when building wsDir/srcDir in createPR which allows path traversal;
sanitize and validate input.Workspace before joining with WorkspaceRoot: first
reduce it to a single segment (e.g., workspaceName :=
filepath.Base(input.Workspace)) or run filepath.Clean and then require it
matches an allowed pattern (e.g., regex for "{repo}-{timestamp}"), then
recompute wsDir := filepath.Join(WorkspaceRoot(), workspaceName) and (for extra
safety) resolve abs paths and verify the final wsDir has WorkspaceRoot as its
prefix before proceeding with os.Stat or any Git operations.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

find . -name "pr.go" -path "*/agentic/*" | head -5

Repository: dAppCore/agent

Length of output: 77

---

🏁 Script executed:

cat -n pkg/agentic/pr.go | sed -n '255,310p'

Repository: dAppCore/agent

Length of output: 1642

---

Validate and enforce limit consistently to prevent panics and honour user requests.

Two issues require fixing:

1. Negative limits cause panic: Negative input.Limit values bypass the zero-check at line 264 and reach line 294's slice operation (allPRs[:input.Limit]), causing a panic. This can crash the handler.

2. Requested limits above 10 are silently ignored: The listRepoPRs function hard-codes limit=10 in the API URL (line 305), regardless of the user's requested limit. For single-repo requests, users requesting 30 or 50 PRs receive at most 10 because the loop cannot fetch more from a single repository.

Required changes:

• Change line 264 condition from == 0 to <= 0 to reject negative values
• Pass the remaining limit to listRepoPRs instead of hard-coding 10
• Update the function signature to accept and use the limit parameter

Suggested fix

-	if input.Limit == 0 {
+	if input.Limit <= 0 {
 		input.Limit = 20
 	}
@@
-		prs, err := s.listRepoPRs(ctx, input.Org, repo, input.State)
+		remaining := input.Limit - len(allPRs)
+		prs, err := s.listRepoPRs(ctx, input.Org, repo, input.State, remaining)
 		if err != nil {
 			continue
 		}
@@
-func (s *PrepSubsystem) listRepoPRs(ctx context.Context, org, repo, state string) ([]PRInfo, error) {
-	url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls?state=%s&limit=10",
-		s.forgeURL, org, repo, state)
+func (s *PrepSubsystem) listRepoPRs(ctx context.Context, org, repo, state string, limit int) ([]PRInfo, error) {
+	url := fmt.Sprintf("%s/api/v1/repos/%s/%s/pulls?state=%s&limit=%d",
+		s.forgeURL, org, repo, state, limit)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/pr.go` around lines 261 - 266, The handler currently allows
negative input.Limit and always uses a hard-coded page size in listRepoPRs;
change the zero-check to "input.Limit <= 0" so negatives are normalized/rejected
(e.g., set to default 20 or treat as invalid) and update calls to listRepoPRs to
pass the remaining limit instead of 10; modify listRepoPRs' function signature
(e.g., listRepoPRs(..., limit int)) and ensure its URL/query uses that limit
when building the API request so slicing allPRs[:input.Limit] cannot panic and
the API fetch honors the requested limit.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cd /repo && find . -name "resume.go" -type f | head -20

Repository: dAppCore/agent

Length of output: 113

---

🏁 Script executed:

cd /repo && cat -n pkg/agentic/resume.go | head -100

Repository: dAppCore/agent

Length of output: 113

---

🏁 Script executed:

cd /repo && wc -l pkg/agentic/resume.go

Repository: dAppCore/agent

Length of output: 113

---

🏁 Script executed:

find . -name "resume.go" -type f

Repository: dAppCore/agent

Length of output: 81

---

🏁 Script executed:

cat -n pkg/agentic/resume.go | head -120

Repository: dAppCore/agent

Length of output: 4424

---

🏁 Script executed:

rg "filepath.Join.*WorkspaceRoot" -A 5 -B 5

Repository: dAppCore/agent

Length of output: 2362

---

🏁 Script executed:

cat > /tmp/test_filepath.go << 'EOF'
package main
import (
	"fmt"
	"path/filepath"
)
func main() {
	// Test 1: Normal join
	result1 := filepath.Join("/workspace", "myws")
	fmt.Printf("Test 1 (normal): %s\n", result1)
	
	// Test 2: Join with .. segment
	result2 := filepath.Join("/workspace", "../etc")
	fmt.Printf("Test 2 (with ..): %s\n", result2)
	
	// Test 3: Multiple .. segments
	result3 := filepath.Join("/workspace", "../../root")
	fmt.Printf("Test 3 (multiple ..): %s\n", result3)
	
	// Test 4: Absolute path in second argument (overrides first)
	result4 := filepath.Join("/workspace", "/etc/passwd")
	fmt.Printf("Test 4 (absolute second): %s\n", result4)
}
EOF
go run /tmp/test_filepath.go

Repository: dAppCore/agent

Length of output: 189

---

🏁 Script executed:

rg "input.Workspace" pkg/agentic/resume.go -B 2 -A 2

Repository: dAppCore/agent

Length of output: 826

---

🏁 Script executed:

rg "WorkspaceRoot" -A 2 -B 2 --type-add 'go:*.go' | head -50

Repository: dAppCore/agent

Length of output: 2670

---

🏁 Script executed:

rg "CallToolRequest" -B 5 -A 5 --type-add 'go:*.go' | head -80

Repository: dAppCore/agent

Length of output: 3998

---

🏁 Script executed:

rg "mcp.AddTool" -B 2 -A 10 | head -60

Repository: dAppCore/agent

Length of output: 3797

---

🏁 Script executed:

cat pkg/agentic/resume.go | grep -A 15 "registerResumeTool"

Repository: dAppCore/agent

Length of output: 746

---

Reject workspace names that escape the workspace root.

The code at line 46 uses filepath.Join(WorkspaceRoot(), input.Workspace) without validating the input. Go's filepath.Join normalises .. segments, so input.Workspace of ../etc/passwd would escape the workspace root entirely. This allows an attacker to write ANSWER.md to arbitrary directories (line 72–76) and spawn an agent process in an arbitrary location (line 96). Add validation to reject workspace names containing / or .. segments, or use filepath.IsLocal to ensure the resolved path remains under the workspace root.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/resume.go` around lines 46 - 52, Validate and reject workspace
names that escape the workspace root before joining paths: ensure
input.Workspace does not contain "/" or ".." segments and/or resolve the
absolute path of wsDir (using WorkspaceRoot() and input.Workspace) and verify it
is a child of WorkspaceRoot() (e.g., using filepath.Abs + filepath.Rel or
filepath.Clean) before creating srcDir; if validation fails return the existing
error instead of proceeding to create/write ANSWER.md or spawn the agent
process. Target the variables and calls wsDir, srcDir, WorkspaceRoot(), and
input.Workspace when adding the checks so the code never allows a path outside
the workspace root.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Use a leased push for github/dev.

Line 238 uses git push --force, which can overwrite commits added after candidate selection or by another queue worker. This is a non-idempotent external write on a shared branch; please use --force-with-lease at minimum, or avoid rewriting dev entirely.

Suggested fix

-	pushCmd := exec.CommandContext(ctx, "git", "push", "github", "HEAD:refs/heads/dev", "--force")
+	pushCmd := exec.CommandContext(
+		ctx,
+		"git", "push", "github", "HEAD:refs/heads/dev",
+		"--force-with-lease=refs/heads/dev",
+	)

📝 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 (s *PrepSubsystem) pushAndMerge(ctx context.Context, repoDir, repo string) error {
	// Push to dev
	pushCmd := exec.CommandContext(ctx, "git", "push", "github", "HEAD:refs/heads/dev", "--force")
	pushCmd.Dir = repoDir
	if out, err := pushCmd.CombinedOutput(); err != nil {
	return coreerr.E("pushAndMerge", "push failed: "+string(out), err)
	func (s *PrepSubsystem) pushAndMerge(ctx context.Context, repoDir, repo string) error {
	// Push to dev
	pushCmd := exec.CommandContext(
	ctx,
	"git", "push", "github", "HEAD:refs/heads/dev",
	"--force-with-lease=refs/heads/dev",
	)
	pushCmd.Dir = repoDir
	if out, err := pushCmd.CombinedOutput(); err != nil {
	return coreerr.E("pushAndMerge", "push failed: "+string(out), err)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/review_queue.go` around lines 236 - 241, In pushAndMerge, replace
the non-idempotent forced push that uses "git push ... --force" (constructed in
pushCmd) with a leased push to avoid overwriting concurrent commits: update the
git args to use "--force-with-lease" (or implement a safeguard that avoids
rewriting "dev" entirely) so the push command will fail if the remote branch has
new commits; ensure the change is applied to the exec.CommandContext invocation
that builds pushCmd in the pushAndMerge function.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Fail closed when verification cannot run.

Several “no runner / unreadable config” paths return passed: true, which allows auto-merge without executing any tests.

Suggested fix

-	return verifyResult{passed: true, testCmd: "none", output: "No test runner detected"}
+	return verifyResult{passed: false, testCmd: "none", output: "No supported test runner detected"}
@@
-		return verifyResult{passed: true, testCmd: "none", output: "Could not read package.json"}
+		return verifyResult{passed: false, testCmd: "none", output: "Could not read package.json"}
@@
-		return verifyResult{passed: true, testCmd: "none", output: "No test script in package.json"}
+		return verifyResult{passed: false, testCmd: "none", output: "No test script in package.json"}

Also applies to: 274-276, 281-283

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/verify.go` around lines 218 - 229, The verification paths
currently "fail open" by returning verifyResult{passed:true,...} when no test
runner or config is detected; change these to fail closed by returning
passed:false, set testCmd to "none" (or the attempted runner name), and set
output to a clear error message like "No test runner detected" or "Unreadable
config" so merges are blocked; update the runVerification function and any
similar branches in the related verification helpers (e.g., the
unreadable-config branches in the runGoTests/runPHPTests/runNodeTests flows) to
return passed:false instead of true.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Pest failures are currently reported as passes.

In the fallback path, any err2 (including test assertion failures with a non-zero exit code) returns passed: true.

Suggested fix

 			cmd2 := exec.Command("./vendor/bin/pest", "--no-interaction")
 			cmd2.Dir = srcDir
 			out2, err2 := cmd2.CombinedOutput()
 			if err2 != nil {
-				return verifyResult{passed: true, testCmd: "none", output: "No PHP test runner found"}
+				if exitErr, ok := err2.(*exec.ExitError); ok {
+					return verifyResult{
+						passed:   false,
+						output:   string(out2),
+						exitCode: exitErr.ExitCode(),
+						testCmd:  "vendor/bin/pest",
+					}
+				}
+				return verifyResult{passed: false, testCmd: "vendor/bin/pest", output: fmt.Sprintf("Failed to run pest: %v", err2), exitCode: 1}
 			}
 			return verifyResult{passed: true, output: string(out2), exitCode: 0, testCmd: "vendor/bin/pest"}

📝 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

	cmd2 := exec.Command("./vendor/bin/pest", "--no-interaction")
	cmd2.Dir = srcDir
	out2, err2 := cmd2.CombinedOutput()
	if err2 != nil {
	return verifyResult{passed: true, testCmd: "none", output: "No PHP test runner found"}
	}
	return verifyResult{passed: true, output: string(out2), exitCode: 0, testCmd: "vendor/bin/pest"}
	}
	cmd2 := exec.Command("./vendor/bin/pest", "--no-interaction")
	cmd2.Dir = srcDir
	out2, err2 := cmd2.CombinedOutput()
	if err2 != nil {
	if exitErr, ok := err2.(*exec.ExitError); ok {
	return verifyResult{
	passed: false,
	output: string(out2),
	exitCode: exitErr.ExitCode(),
	testCmd: "vendor/bin/pest",
	}
	}
	return verifyResult{passed: false, testCmd: "vendor/bin/pest", output: fmt.Sprintf("Failed to run pest: %v", err2), exitCode: 1}
	}
	return verifyResult{passed: true, output: string(out2), exitCode: 0, testCmd: "vendor/bin/pest"}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/verify.go` around lines 259 - 266, The fallback block in
verify.go treats any err2 from cmd2.CombinedOutput() as "no test runner" and
returns passed: true; instead detect whether err2 is an exec.ExitError (non-zero
test exit) and treat that as a failing test: set passed: false, include the
CombinedOutput() string as output and the actual exit code (from
ExitError.ExitCode()) and set testCmd to "vendor/bin/pest"; only treat the case
where the binary is missing (e.g., os.IsNotExist or other exec-related absence)
as "no PHP test runner found" with passed: true. Update the logic around cmd2,
err2, and verifyResult accordingly so test failures are reported correctly.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the watch.go file around the target lines and understand the full context
cat -n pkg/agentic/watch.go | head -200 | tail -50

Repository: dAppCore/agent

Length of output: 1673

---

🏁 Script executed:

# Get the full watch.go file to understand the broader context
wc -l pkg/agentic/watch.go

Repository: dAppCore/agent

Length of output: 82

---

🏁 Script executed:

# Search for all uses of resolveWorkspaceDir to see how it's called
rg "resolveWorkspaceDir" --context 3

Repository: dAppCore/agent

Length of output: 703

---

🏁 Script executed:

# Check how WorkspaceRoot is defined and what it returns
rg "func WorkspaceRoot" -A 5

Repository: dAppCore/agent

Length of output: 396

---

🏁 Script executed:

# Also check imports to understand filepath usage
head -30 pkg/agentic/watch.go

Repository: dAppCore/agent

Length of output: 906

---

🏁 Script executed:

# Find where 'remaining' is populated in the watch function
rg "remaining" pkg/agentic/watch.go -B 5 -A 2

Repository: dAppCore/agent

Length of output: 1083

---

🏁 Script executed:

# Check the full Watch function to understand input handling
cat -n pkg/agentic/watch.go | head -140

Repository: dAppCore/agent

Length of output: 4689

---

🏁 Script executed:

# Search for input validation on WatchInput
rg "WatchInput" -A 10 pkg/agentic/watch.go | head -50

Repository: dAppCore/agent

Length of output: 927

---

🏁 Script executed:

# Test filepath.Join behavior with .. segments in Go
python3 << 'EOF'
import subprocess
import json

# Test Go's filepath.Join behavior with .. segments
test_code = '''
package main
import (
	"fmt"
	"path/filepath"
)
func main() {
	wsRoot := "/home/user/workspace"
	
	// Test 1: absolute path
	test1 := filepath.Join(wsRoot, "/etc/passwd")
	fmt.Printf("Test 1 (absolute): %s\n", test1)
	
	// Test 2: relative path with ..
	test2 := filepath.Join(wsRoot, "../../../etc/passwd")
	fmt.Printf("Test 2 (relative with ..): %s\n", test2)
	
	// Test 3: relative path with ..
	test3 := filepath.Join(wsRoot, "myworkspace/../../etc/passwd")
	fmt.Printf("Test 3 (relative with nested ..): %s\n", test3)
	
	// Test 4: normal relative path
	test4 := filepath.Join(wsRoot, "myworkspace")
	fmt.Printf("Test 4 (normal relative): %s\n", test4)
	
	// Test 5: check if path traversal is possible
	test5 := filepath.Join(wsRoot, "workspace", "..", "..", "etc", "passwd")
	fmt.Printf("Test 5 (multiple ..): %s\n", test5)
}
'''

with open('/tmp/test.go', 'w') as f:
	f.write(test_code)

# Run the test
result = subprocess.run(['go', 'run', '/tmp/test.go'], capture_output=True, text=True, timeout=10)
print(result.stdout)
if result.stderr:
	print("STDERR:", result.stderr)
EOF

Repository: dAppCore/agent

Length of output: 847

---

🏁 Script executed:

# Find the readStatus function to see if it validates the path
rg "func readStatus" -A 15

Repository: dAppCore/agent

Length of output: 845

---

🏁 Script executed:

# Check if there's any path validation or cleaning in the codebase
rg "filepath.Clean" pkg/agentic/

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Look at readStatus more closely to understand what it does with the path
rg "readStatus" -B 2 -A 10 pkg/agentic/

Repository: dAppCore/agent

Length of output: 10408

---

🏁 Script executed:

# Check the Go filepath documentation behavior by examining actual usage patterns
# Let's test filepath.Join with .. manually using a simpler approach
cat > /tmp/filepath_test.go << 'EOF'
package main
import (
	"fmt"
	"path/filepath"
)
func main() {
	wsRoot := "/home/user/workspace"
	
	// Test with ..
	result := filepath.Join(wsRoot, "../../../etc/passwd")
	fmt.Println("filepath.Join with ..: ", result)
	
	// What if we check if a path is within root?
	abs, _ := filepath.Abs(result)
	fmt.Println("Absolute: ", abs)
}
EOF
go run /tmp/filepath_test.go

Repository: dAppCore/agent

Length of output: 112

---

🏁 Script executed:

# Test filepath.Join behaviour with Go code (without test file syntax)
cat > /tmp/test_join.go << 'EOF'
package main
import (
	"fmt"
	"path/filepath"
)
func main() {
	root := "/home/user/workspace"
	
	// Relative path with ..
	result1 := filepath.Join(root, "../../../../etc/passwd")
	fmt.Printf("filepath.Join with ..: %s\n", result1)
	
	// Absolute path check
	result2 := filepath.Join(root, "/etc/passwd")
	fmt.Printf("filepath.Join with absolute: %s\n", result2)
	
	// Normal relative
	result3 := filepath.Join(root, "myworkspace")
	fmt.Printf("filepath.Join normal: %s\n", result3)
}
EOF
cd /tmp && go run test_join.go

Repository: dAppCore/agent

Length of output: 208

---

🏁 Script executed:

# Check if there's any validation elsewhere in the codebase
rg "filepath.Clean|filepath.Abs|filepath.EvalSymlinks" pkg/agentic/

Repository: dAppCore/agent

Length of output: 40

---

🏁 Script executed:

# Check how the input flows from WatchInput to resolveWorkspaceDir
rg "WatchInput|input.Workspaces" pkg/agentic/watch.go

Repository: dAppCore/agent

Length of output: 296

---

Validate that resolved workspace paths remain within WorkspaceRoot().

resolveWorkspaceDir allows callers to escape the managed workspace tree: absolute paths are returned unchanged, and relative paths with .. segments can traverse upwards (e.g. "../../../../etc/passwd" resolves to /etc/passwd). Add validation to reject any path that does not resolve beneath WorkspaceRoot() before calling readStatus.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/agentic/watch.go` around lines 188 - 194, resolveWorkspaceDir currently
returns absolute or joined paths without checking they live under
WorkspaceRoot(), allowing escapes; change resolveWorkspaceDir (or add a
validation step before readStatus) to clean and make the path absolute, get the
absolute WorkspaceRoot(), compute filepath.Rel(root, resolved) and reject
(return an error) if the relative path begins with ".." or equals ".." (i.e. the
resolved path is outside the workspace); ensure callers such as readStatus
handle the new error instead of operating on an out-of-tree path.