feat(skills): scheduled dashboard + run/new pages + [github] preflight gate + composio-only GitHub I/O

[M3gA-Mind]

Summary

Clean version of #2875 by @sanil-23 on current upstream/main.

What changed from #2875: This branch is remote-2631/run/codegraph-full (sanil-23's latest head — which already includes cargo fmt, prettier, and ESLint fixes pushed by the author) merged cleanly onto current upstream/main. The previous PR #2880 accumulated unrelated commits from other PRs due to branch pollution.

The previously failing classifies_embedding_api_invalid_token_401_as_session_expired test passes — it was a stale-base issue (missing #2869 from main).

Closes #2875, closes #2880.

---

Original description from @sanil-23:

1. Scheduled-skills dashboard at /skills → Runners tab
2. Focused runner at /skills/run with [[inputs]] form
3. Full-page authoring at /skills/new with inputs editor + skill.toml generation
4. [github] preflight gate — Composio connection, git install, git config, identity match
5. GitHub I/O via Composio everywhere (replaces gh CLI in bundled skills)

Summary by CodeRabbit

• New Features

  • Skill authoring UI with configurable inputs and a streamlined create flow; dedicated skill creation page and modal.
  • Skills Runner: ad-hoc runs, “Run now”, recurring schedule creation, enable/disable toggles, and run history with live/tailing log viewer.
  • Skills Dashboard: grouped scheduled skills, counts, and quick actions.
  • Improved repo/branch pickers and repo-aware smart picker; codegraph-powered repo indexing/search for better code navigation.

• Documentation

  • Design doc for unifying Dev Workflow into the Skills Runner.

• Localization

  • New/updated translations for Skills and Dev Workflow areas.

• Tests

  • Expanded coverage across skills, scheduling, run logs, pickers, and UI flows.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cba7d8d4-604e-470a-861b-2c6201fff239

📥 Commits

Reviewing files that changed from the base of the PR and between 4117fa8 and 7e99ea6.

📒 Files selected for processing (38)

• app/src-tauri/src/lib.rs
• app/src/components/settings/panels/DevWorkflowPanel.tsx
• app/src/components/settings/panels/DeveloperOptionsPanel.tsx
• app/src/components/settings/panels/__tests__/DevWorkflowPanel.test.tsx
• app/src/components/skills/CreateSkillForm.tsx
• app/src/components/skills/ScheduledCronCard.tsx
• app/src/components/skills/SkillsRunnerBody.tsx
• app/src/components/skills/SmartIssuePicker.tsx
• app/src/components/skills/inputs/BranchPicker.test.tsx
• app/src/components/skills/inputs/BranchPicker.tsx
• app/src/lib/i18n/chunks/ar-5.ts
• app/src/lib/i18n/chunks/bn-5.ts
• app/src/lib/i18n/chunks/de-5.ts
• app/src/lib/i18n/chunks/en-5.ts
• app/src/lib/i18n/chunks/es-5.ts
• app/src/lib/i18n/chunks/fr-5.ts
• app/src/lib/i18n/chunks/hi-5.ts
• app/src/lib/i18n/chunks/id-5.ts
• app/src/lib/i18n/chunks/it-5.ts
• app/src/lib/i18n/chunks/ko-5.ts
• app/src/lib/i18n/chunks/pt-5.ts
• app/src/lib/i18n/chunks/ru-5.ts
• app/src/lib/i18n/chunks/zh-CN-5.ts
• app/src/lib/i18n/en.ts
• app/src/pages/Settings.tsx
• src/core/jsonrpc.rs
• src/openhuman/agent/harness/subagent_runner/ops.rs
• src/openhuman/composio/identity.rs
• src/openhuman/cron/schemas.rs
• src/openhuman/desktop_companion/pipeline_tests.rs
• src/openhuman/desktop_companion/session.rs
• src/openhuman/desktop_companion/session_tests.rs
• src/openhuman/mod.rs
• src/openhuman/skills/ops_create.rs
• src/openhuman/skills/preflight.rs
• src/openhuman/skills/schemas.rs
• src/openhuman/tools/impl/agent/run_skill.rs
• src/openhuman/tools/impl/codegraph/mod.rs

✅ Files skipped from review due to trivial changes (4)

• app/src/lib/i18n/chunks/de-5.ts
• app/src/lib/i18n/chunks/ko-5.ts
• app/src/lib/i18n/chunks/hi-5.ts
• app/src/lib/i18n/en.ts

🚧 Files skipped from review as they are similar to previous changes (26)

• src/openhuman/mod.rs
• app/src/components/settings/panels/DeveloperOptionsPanel.tsx
• src/openhuman/agent/harness/subagent_runner/ops.rs
• app/src/components/skills/inputs/BranchPicker.tsx
• src/openhuman/composio/identity.rs
• app/src/lib/i18n/chunks/ru-5.ts
• app/src/lib/i18n/chunks/fr-5.ts
• app/src/components/skills/inputs/BranchPicker.test.tsx
• app/src/lib/i18n/chunks/pt-5.ts
• app/src/lib/i18n/chunks/id-5.ts
• src/openhuman/tools/impl/agent/run_skill.rs
• src/core/jsonrpc.rs
• app/src/lib/i18n/chunks/en-5.ts
• app/src/lib/i18n/chunks/zh-CN-5.ts
• src/openhuman/tools/impl/codegraph/mod.rs
• app/src/components/skills/CreateSkillForm.tsx
• app/src/lib/i18n/chunks/it-5.ts
• app/src/lib/i18n/chunks/es-5.ts
• src/openhuman/cron/schemas.rs
• app/src/components/skills/ScheduledCronCard.tsx
• app/src/components/skills/SmartIssuePicker.tsx
• app/src/components/settings/panels/tests/DevWorkflowPanel.test.tsx
• src/openhuman/skills/preflight.rs
• app/src/components/skills/SkillsRunnerBody.tsx
• app/src/components/settings/panels/DevWorkflowPanel.tsx
• src/openhuman/skills/schemas.rs

---

📝 Walkthrough

Walkthrough

Adds unified Skills UI (create/run/dashboard), cron-backed scheduling and Tauri wrappers, background skill-run spawning and run-log streaming, skill registry with default bundled skills, codegraph indexing/search storage and agent tools, GitHub preflight checks, translations, routing, and extensive tests.

Changes

Unified Skills Runner, Cron, and Codegraph

Layer / File(s)	Summary
Frontend pages, runner body, forms, pickers, and tests app/src/pages/*, app/src/components/skills/*, app/src/components/settings/*, app/src/components/skills/inputs/*, app/src/components/settings/panels/*, app/src/components/skills/__tests__/*	Adds SkillNew/SkillsRun/SkillsDashboard pages, unified SkillsRunnerBody, CreateSkillForm/CreateSkillModal, SmartIssuePicker, RepoPicker/BranchPicker, ScheduledCronCard, DevWorkflow panel migrated to cron-backed flows, route/settings wiring, and many frontend tests.
Skills core: registry, run-log, preflight, schemas src/openhuman/skills/*, src/openhuman/skills/schemas.rs, src/openhuman/skills/run_log.rs	Implements skill registry and defaults seeding, run-log header/footer streaming and slicing, GitHub preflight gate with probes, new JSON-RPC skills.* endpoints (run/describe/recent_runs/read_run_log) and background spawn helpers.
Cron controller & Tauri wrapper src/openhuman/cron/*, app/src/utils/tauriCommands/cron.ts, app/src/components/settings/panels/DevWorkflowPanel.tsx	Adds cron.add controller/handler, registers handler, and provides openhumanCronAdd client wrapper; frontend panels now create/update/remove/run scheduled agent jobs via Tauri RPC.
Codegraph: store, index, search and agent tools src/openhuman/codegraph/*, src/openhuman/tools/impl/codegraph/*	Adds content-addressed SQLite CodegraphStore, lexical+dense indexing (index_ref), structural-doc extraction, embeddings batching/normalization, BM25+dense fusion (RRF), plus codegraph_index/codegraph_search agent tools and tests.
Agent tools, run_skill, embeddings, composio identity src/openhuman/tools/impl/agent/run_skill.rs, src/openhuman/embeddings/*, src/openhuman/composio/identity.rs	Adds orchestrator-callable run_skill tool that spawns background skill runs, an embeddings provider-from-config helper, and composio connection identity resolver; re-exports and tool registrations updated.
I18n, routes, settings, Tauri startup tweak, tests app/src/lib/i18n/*, app/src/AppRoutes.tsx, app/src/pages/Settings.tsx, app/src-tauri/src/lib.rs, many tests	Adds new translation keys across locales for Skills/Dev Workflow UI, updates /skills routing to include runners tab and new routes, registers SkillsRunnerPanel in Settings, and extends Linux CEF no-sandbox debug-opt env gating.

Sequence Diagram(s)

sequenceDiagram
  participant UI as SkillsRunnerBody / Dashboard
  participant Cron as openhuman.cron_*
  participant SkillsAPI as openhuman.skills_*
  participant Orchestrator as orchestrator (run_skill / spawn)
  participant Logs as run_log

  UI->>SkillsAPI: skills.run(skill_id, inputs)
  UI->>Cron: openhuman.cron_add / openhuman.cron_run / openhuman.cron_list
  SkillsAPI->>Orchestrator: spawn_skill_run_background(skill_id, inputs)
  Orchestrator->>Logs: write_header + drain progress
  Orchestrator->>Logs: write_footer(status, output)
  UI->>SkillsAPI: skills.read_run_log(run_id, offset, max_bytes) (poll)

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• tinyhumansai/openhuman#2703 — Earlier DevWorkflowPanel implementation; this PR refactors it to cron/RPC-backed scheduling.
• tinyhumansai/openhuman#2258 — Overlaps developer menu changes to DeveloperOptionsPanel (skills-runner entry adjustments).
• tinyhumansai/openhuman#2103 — Also updates Tauri Linux CEF --no-sandbox handling in the same startup path.

Suggested reviewers

• sanil-23
• graycyrus

Poem

I hop through routes and cron-sown fields,
Seeds of skills beneath my heels.
Logs like carrots line the trail,
Codegraph maps each hidden detail.
I nudge the runner — run and done! 🥕🐰

[graycyrus]

@M3gA-Mind hey! the code looks clean overall — good tests, parameterized SQL throughout, proper sanitization on log paths, and the debug-assertions guard on the CEF sandbox escape is exactly right. CI is still pending across the board, so holding off on the formal approval until that's green. once it passes i'll come back and approve.

One functional issue worth fixing before merge:

DevWorkflowPanel: form doesn't repopulate from existing job on mount. The old implementation read back from localStorage on mount, so if you navigated away and returned, the repo/branch/schedule dropdowns reflected the current config. This PR moves persistence to the cron backend (good), but loadExistingJob only populates existingJob — it doesn't write back to selectedRepo, targetBranch, or schedule. The result: a user who navigates to the DevWorkflow panel and sees an active cron job running will still see blank dropdowns, making it impossible to know what the current config is without digging into the raw job prompt. The controls for run/pause/delete work fine; it's only the "read back into form" path that's missing.

A few minor notes below.

[graycyrus]

[major] loadExistingJob sets existingJob but never writes back selectedRepo, targetBranch, or schedule from the job's stored data. When a user returns to this panel with an active job, the form is blank — they can't see or edit the current configuration.

Suggestion: parse the job's prompt (or store the config fields separately in the cron job's name/tags) and restore form state on load. At minimum, consider reading existingJob.schedule.expr into the schedule dropdown so users can see the current cadence.

[graycyrus]

[minor] REPO_INPUT_NAMES and BRANCH_INPUT_NAMES routing by name convention is acknowledged tech debt (TODO(picker-schema)). Fine for now, but worth filing the follow-up issue so this doesn't linger — the schema-driven picker field would also clean up the SmartIssuePicker hard-coding.

[coderabbitai]

Actionable comments posted: 17

Note

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

🟡 Minor comments (14)

app/src/AppRoutes.tsx-85-93 (1)

85-93: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the /skills/new vs /skills route-order wording.

React Router v7 ranks route matches by specificity (ranked/most-specific matching) rather than relying on <Route> declaration order, so the comment saying “Order matters: keep /skills/new before /skills so it wins the prefix match” is misleading—adjust the comment to describe ranked/specific matching instead.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/AppRoutes.tsx` around lines 85 - 93, Update the explanatory comment
in AppRoutes.tsx to remove the misleading "Order matters" claim and instead
state that React Router v7 uses ranked/specific matching (most-specific routes
win), so `/skills/new` wins over `/skills` due to specificity rather than
declaration order; reference the two routes `/skills/new` and `/skills` and
clarify that the specificity of the route pattern determines which route
matches.

app/src/components/settings/panels/DevWorkflowPanel.tsx-713-716 (1)

713-716: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Localize the hard-coded connector text in branch note.

Line 715 hard-codes " on " inside a user-visible sentence, which bypasses translation.

💡 Proposed fix

-                  {t('settings.devWorkflow.targetBranchNote')}
-                  {forkInfo ? ` on ${forkInfo.upstreamFullName}` : ''}.
+                  {forkInfo
+                    ? t('settings.devWorkflow.targetBranchNoteWithRepo', {
+                        repo: forkInfo.upstreamFullName,
+                      })
+                    : t('settings.devWorkflow.targetBranchNote')}

As per coding guidelines: “Every user-visible string in app/src/** must use useT() … hard-coded literals in JSX or element props are not allowed.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/components/settings/panels/DevWorkflowPanel.tsx` around lines 713 -
716, The hard-coded connector " on " in DevWorkflowPanel.tsx should be moved
into the translation string and rendered via the t() function instead of string
concatenation; replace the expression
{t('settings.devWorkflow.targetBranchNote')}{forkInfo ? ` on
${forkInfo.upstreamFullName}` : ''} with a single t call that accepts a variable
(e.g. t('settings.devWorkflow.targetBranchNoteWithUpstream', { upstream:
forkInfo?.upstreamFullName })) or two translation keys for presence/absence,
using the existing t function and the forkInfo.upstreamFullName value so the
full user-visible sentence (including the connector and punctuation) is
localized.

app/src/components/skills/SkillsRunnerBody.tsx-537-537 (1)

537-537: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Clear the schedule success timeout on unmount (and cancel previous).

Line 537 starts a setTimeout that can call setScheduleSaved(false) after the component unmounts since there’s no corresponding clearTimeout cleanup. Add a useRef for the timeout handle (import useRef) and clear it in a useEffect unmount cleanup; also clear any existing timer before scheduling a new one.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/components/skills/SkillsRunnerBody.tsx` at line 537, The setTimeout
call that resets setScheduleSaved(false) can fire after unmount and multiple
timers can stack; introduce a ref (e.g., scheduleSavedTimerRef via useRef<number
| null>) and use it when scheduling: before calling setTimeout, clear any
existing timer in scheduleSavedTimerRef, assign the new timer id to it, and then
add a useEffect cleanup that clears scheduleSavedTimerRef on unmount; ensure you
import useRef and useEffect and replace the direct setTimeout(...) at the site
where setScheduleSaved is scheduled.

app/src/components/skills/scheduledCronFormat.ts-26-29 (1)

26-29: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Handle singular/plural and avoid 0 minutes output.

Line 27 can round to 0, and Line 28 always uses plural ("1 minutes"). Clamp to at least 1 and pluralize correctly.

Suggested fix

   if (s.kind === 'every' && s.every_ms) {
-    const minutes = Math.round(s.every_ms / 60_000);
-    return `Every ${minutes} minutes`;
+    const minutes = Math.max(1, Math.round(s.every_ms / 60_000));
+    return `Every ${minutes} ${minutes === 1 ? 'minute' : 'minutes'}`;
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/components/skills/scheduledCronFormat.ts` around lines 26 - 29, In
the branch that handles s.kind === 'every' (where minutes is computed from
s.every_ms), clamp the computed minutes to at least 1 (e.g. minutes =
Math.max(1, Math.round(s.every_ms / 60_000))) to avoid "0 minutes", and choose
the correct singular/plural word when building the return string (use "minute"
when minutes === 1, otherwise "minutes") so the return becomes "Every 1 minute"
or "Every N minutes" as appropriate.

app/src/lib/cron/cronToHuman.ts-54-85 (1)

54-85: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate cron numeric bounds before rendering friendly text.

The parser currently treats invalid cron numbers as valid (e.g., minute 99, step 0), producing misleading output instead of falling back to the raw expression.

Suggested fix

+function inRange(n: number, min: number, max: number): boolean {
+  return Number.isInteger(n) && n >= min && n <= max;
+}
+
   const stepMin = /^\*\/(\d+)$/.exec(min);
   if (stepMin && hour === '*' && allDays) {
     const n = Number(stepMin[1]);
+    if (!inRange(n, 1, 59)) return e;
     if (n === 1) return 'Every minute';
     return `Every ${n} minutes`;
   }
@@
   if (minLiteral && hour === '*' && allDays) {
     const m = Number(minLiteral[1]);
+    if (!inRange(m, 0, 59)) return e;
     if (m === 0) return 'Every hour';
     return `Hourly at :${pad2(m)}`;
   }
@@
   if (stepHour && minLiteral && allDays) {
     const n = Number(stepHour[1]);
     const m = Number(minLiteral[1]);
+    if (!inRange(n, 1, 23) || !inRange(m, 0, 59)) return e;
     const suffix = m === 0 ? '' : ` at :${pad2(m)}`;
     if (n === 1) return `Every hour${suffix}`;
     return `Every ${n} hours${suffix}`;
   }
@@
   if (minLiteral && hourLiteral && allDays) {
     const h = Number(hourLiteral[1]);
     const m = Number(minLiteral[1]);
+    if (!inRange(h, 0, 23) || !inRange(m, 0, 59)) return e;
     return `Daily at ${pad2(h)}:${pad2(m)}`;
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/lib/cron/cronToHuman.ts` around lines 54 - 85, The friendly-rendering
branches (stepMin, minLiteral, stepHour, hourLiteral) currently accept any
numeric matches; validate numeric bounds before returning by checking minute
literals are 0–59, hour literals are 0–23, and step values are positive and
within sensible bounds (minute step 1–59, hour step 1–23). In practice update
the guards around stepMin (/^\*\/(\d+)$/), minLiteral (/^(\d+)$/), stepHour
(/^\*\/(\d+)$/) and hourLiteral (/^(\d+)$/) inside cronToHuman.ts to parse the
captured values (n, m, h) and only produce the human strings in those branches
if the parsed numbers satisfy the ranges; otherwise fall back to returning the
raw cron expression. Ensure you check both the single-value branches
(minLiteral/hourLiteral) and the step branches (stepMin/stepHour) so invalid
values like minute 99 or step 0 do not produce misleading text.

app/src/lib/i18n/chunks/en-5.ts-838-839 (1)

838-839: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix duplicated phrase in panel description copy.

Line 839 repeats the verb phrase (“fire a fire-and-forget”), which reads as a typo in the English source string.

✏️ Suggested fix

   'settings.developerMenu.skillsRunner.panelDesc':
-    'Pick a bundled skill, fill in its declared inputs, and fire a fire-and-forget background run. Use Dev Workflow instead if you want a cron-scheduled recurring job.',
+    'Pick a bundled skill, fill in its declared inputs, and start a fire-and-forget background run. Use Dev Workflow instead if you want a cron-scheduled recurring job.',

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/lib/i18n/chunks/en-5.ts` around lines 838 - 839, The copy for
settings.developerMenu.skillsRunner.panelDesc contains a duplicated verb phrase
("fire a fire-and-forget"); update the string in en-5.ts to remove the
repetition by replacing "fire a fire-and-forget background run" with a clearer
phrasing such as "trigger a fire-and-forget background run" so the full value
reads: "Pick a bundled skill, fill in its declared inputs, and trigger a
fire-and-forget background run. Use Dev Workflow instead if you want a
cron-scheduled recurring job."

docs/skills-runner-unification.md-70-118 (1)

70-118: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language to the fenced block (markdownlint MD040).

Line 70 uses an unlabeled fenced code block; this triggers lint noise and is easy to fix.

🛠️ Suggested fix

-```
+```text
 /skills
   ├─ Library tab (existing)
   └─ Runners tab (existing — this is where SkillsRunnerBody lives)
@@
-```
+```

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/skills-runner-unification.md` around lines 70 - 118, The fenced code
block showing the SkillsRunnerBody ascii UI is unlabeled (markdownlint MD040);
update the opening fence to include a language like "text" (e.g., change ``` to
```text) so the block is labeled, leaving the content and the closing fence
intact; the block to change contains the drawn UI and references
"SkillsRunnerBody" and "/skills".

src/openhuman/agent/agents/code_executor/prompt.md-32-33 (1)

32-33: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Escape pipe characters in inline code inside table cells.

The | characters inside backticks are being parsed as extra table separators (MD056), so the table structure is malformed.

🛠️ Suggested fix

-| **Read** issues / PRs / review comments / check runs / labels / commit metadata | **Composio** | `composio_execute({ tool: "GITHUB_GET_PULL_REQUEST" | "GITHUB_LIST_REVIEW_COMMENTS" | "GITHUB_GET_COMBINED_STATUS" | "GITHUB_GET_ISSUE" | "GITHUB_LIST_ISSUES" | … })` |
-| **Write** PRs / comments / reviews / labels / branch as remote ref | **Composio** | `composio_execute({ tool: "GITHUB_CREATE_PULL_REQUEST" | "GITHUB_CREATE_ISSUE_COMMENT" | "GITHUB_CREATE_REVIEW" | "GITHUB_ADD_LABELS" | … })` |
+| **Read** issues / PRs / review comments / check runs / labels / commit metadata | **Composio** | `composio_execute({ tool: "GITHUB_GET_PULL_REQUEST" \| "GITHUB_LIST_REVIEW_COMMENTS" \| "GITHUB_GET_COMBINED_STATUS" \| "GITHUB_GET_ISSUE" \| "GITHUB_LIST_ISSUES" \| … })` |
+| **Write** PRs / comments / reviews / labels / branch as remote ref | **Composio** | `composio_execute({ tool: "GITHUB_CREATE_PULL_REQUEST" \| "GITHUB_CREATE_ISSUE_COMMENT" \| "GITHUB_CREATE_REVIEW" \| "GITHUB_ADD_LABELS" \| … })` |

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/agent/agents/code_executor/prompt.md` around lines 32 - 33, The
inline code spans in the table use unescaped pipe characters which break
Markdown tables (MD056); update the two composio_execute examples so any `|`
inside the backtick code spans are escaped (e.g., replace `...
"GITHUB_GET_PULL_REQUEST" | "GITHUB_LIST_REVIEW_COMMENTS" ...` with `...
"GITHUB_GET_PULL_REQUEST"\ |\ "GITHUB_LIST_REVIEW_COMMENTS" ...`) — apply this
to both rows referencing composio_execute to preserve the table cells and keep
the entire command strings intact.

src/openhuman/codegraph/search.rs-145-181 (1)

145-181: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Short-circuit empty queries before ranking or embedding.

Right now query.trim().is_empty() still produces a BM25 ranking over all-zero scores, and the dense arm will try to embed the blank string. That can return arbitrary hits in lexical mode and a provider error in dense mode.

[suggested fix]

🛠️ Proposed guard

     if docs.is_empty() {
         return Ok(SearchOutcome {
             hits: vec![],
             coverage,
             indexed: 0,
             total,
         });
     }
+    if query.trim().is_empty() {
+        return Ok(SearchOutcome {
+            hits: vec![],
+            coverage,
+            indexed: docs.len(),
+            total,
+        });
+    }

     let q_tokens = code_tokens(query);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/codegraph/search.rs` around lines 145 - 181, Detect empty
queries by checking if query.trim().is_empty() immediately after computing
coverage (and before calling code_tokens, bm25_rank, or embedder.embed). If
empty, return Ok(SearchOutcome { hits: vec![], coverage, indexed: 0, total }) to
avoid running code_tokens, bm25_rank, embedder.embed, dense_rank, or rrf and
prevent spurious lexical hits or provider errors in the dense arm; update the
early-return logic near the existing docs.is_empty() branch to short-circuit
when the trimmed query is empty.

src/openhuman/skills/defaults/github-issue-crusher/skill.toml-8-10 (1)

8-10: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix comment mismatch on push mechanism.

Line [9] says the fix is pushed “via the GitHub API,” but this skill’s workflow uses local git push to the fork. Please align the comment so it doesn’t conflict with the actual runbook.

Suggested patch

-# (via the GitHub API — no local push creds needed), and the PR is cross-repo.
+# (via local `git` push to a fork), and the PR is cross-repo.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/skills/defaults/github-issue-crusher/skill.toml` around lines 8
- 10, The comment above the skill id "github-issue-crusher" incorrectly states
the fix is pushed "via the GitHub API"; update that comment to reflect the
actual push mechanism used by this skill's workflow (local git push to the fork)
so the runbook and header are consistent—i.e., edit the comment starting with
"Fork-aware:" to mention a local `git push` to a FORK (or remove the
parenthetical reference to the GitHub API) while leaving id =
"github-issue-crusher" unchanged.

src/openhuman/skills/registry.rs-163-164 (1)

163-164: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Don’t silently ignore default-skill write failures.

seed_default_skills currently drops std::fs::write errors, which obscures partial-seed failures and complicates debugging.

🧩 Suggested fix

-        let _ = std::fs::write(dir.join("skill.toml"), skill_toml);
-        let _ = std::fs::write(dir.join("SKILL.md"), skill_md);
+        if let Err(e) = std::fs::write(dir.join("skill.toml"), skill_toml) {
+            log::warn!("[skills] seed {id}: write skill.toml failed: {e}");
+            continue;
+        }
+        if let Err(e) = std::fs::write(dir.join("SKILL.md"), skill_md) {
+            log::warn!("[skills] seed {id}: write SKILL.md failed: {e}");
+            continue;
+        }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/skills/registry.rs` around lines 163 - 164, In
seed_default_skills do not ignore std::fs::write errors for
dir.join("skill.toml") and dir.join("SKILL.md"); instead propagate or handle
them — update seed_default_skills to return a Result and replace the `let _ =
std::fs::write(...)` calls with fallible writes (use the ? operator or map
errors into a descriptive error) so failures surface to the caller (or log the
error with context including the filename and skill name) and ensure both writes
are validated before considering the seed successful.

src/openhuman/skills/run_log.rs-359-366 (1)

359-366: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Resolve run_id by exact header match, not only 8-char filename prefix.

Prefix-only matching can return the wrong log if two run IDs share the same first 8 chars.

🔎 Suggested hardening

-        let short = run_id.get(..8).unwrap_or(run_id);
-        if name.contains(&format!("_{short}.log")) {
-            return Some(path);
+        let short = run_id.get(..8).unwrap_or(run_id);
+        if name.contains(&format!("_{short}.log")) {
+            if let Ok(text) = std::fs::read_to_string(&path) {
+                if text
+                    .lines()
+                    .find_map(|l| l.strip_prefix("run_id :").map(str::trim))
+                    == Some(run_id)
+                {
+                    return Some(path);
+                }
+            }
         }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/skills/run_log.rs` around lines 359 - 366, The current
filename-prefix check using short = run_id.get(..8) can return wrong logs when
run IDs share the first 8 chars; change the logic so after the filename match
(name.contains(&format!("_{short}.log"))), open the candidate file (path), read
its header/metadata to extract the full run id stored in the log (the same
header field used elsewhere to identify runs), and only return Some(path) if
that header run id equals the provided run_id; otherwise continue searching.
Target the block using variables run_id, short, name, and path in this file
(run_log.rs) and ensure file reads handle errors gracefully and close files.

src/openhuman/tools/impl/agent/run_skill.rs-79-89 (1)

79-89: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Validate inputs as a required object at runtime.

The schema marks inputs as required/object, but execution currently forwards any value (or missing key) downstream. Add a local guard so callers get a deterministic tool-level error instead of a deeper spawn failure.

Suggested fix

-        let inputs = args.get("inputs").cloned();
+        let inputs = match args.get("inputs") {
+            Some(v) if v.is_object() => Some(v.clone()),
+            _ => {
+                return Ok(ToolResult::error(
+                    "run_skill: missing required argument `inputs` (object)",
+                ));
+            }
+        };

Also applies to: 111-113

src/openhuman/tools/impl/codegraph/mod.rs-23-24 (1)

23-24: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

auto_mode threshold is off by one vs its own contract.

Comment says dense indexing should start at/above the threshold, but the code currently requires strictly greater than (>).

Suggested fix

-        Ok(n) if n > dense_min_files() => IndexMode::Dense,
+        Ok(n) if n >= dense_min_files() => IndexMode::Dense,

Also applies to: 37-38

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/tools/impl/codegraph/mod.rs` around lines 23 - 24, The
auto_mode threshold check is off-by-one: adjust the comparison(s) that currently
use '>' to use '>=' so that dense indexing starts at or above the threshold per
the comment; specifically update the conditional(s) in the codepath that
reference auto_mode (the comparison near the comment at the top and the similar
check around lines 37-38) to use >= against the same threshold variable so the
contract ("at/above") is honored.

🧹 Nitpick comments (10)

app/src/components/skills/SkillsRunnerBody.tsx (1)

351-368: ⚡ Quick win

Use async/await in effect-side async flows for consistency.

This promise-chain style is repeated in the file; prefer async/await with try/catch/finally for consistency with repo conventions.

As per coding guidelines: "Always use async/await for promises in JavaScript/TypeScript code (Rust uses async with tokio; respect language conventions)".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/components/skills/SkillsRunnerBody.tsx` around lines 351 - 368,
Replace the promise .then/.catch/.finally chain for skillsApi.listSkills() with
an async function using try/catch/finally: call await skillsApi.listSkills(),
check the cancelled flag after await, filter out the 'codegraph-smoke' skill and
call setSkills(filtered) and log the loaded count, in catch build the error
message (err instanceof Error ? err.message : String(err)) and call log(...) and
setSkillsError(msg), and in finally unset loading with setSkillsLoading(false)
only if not cancelled; reference the existing symbols skillsApi.listSkills,
setSkills, setSkillsError, setSkillsLoading, cancelled, and log when making the
change.

src/core/jsonrpc.rs (1)

1367-1370: ⚡ Quick win

Add explicit [boot][skills] diagnostics around default-skill seeding.

This startup path is new, but there’s no callsite-level debug trace around seed execution. Adding stable boot logs here improves startup correlation when seeding is slow or fails silently.

🧭 Suggested instrumentation

+                log::debug!(
+                    "[boot][skills] seeding default skills (workspace={})",
+                    cfg.workspace_dir.display()
+                );
                 crate::openhuman::skills::registry::seed_default_skills(&cfg.workspace_dir);
+                log::debug!("[boot][skills] default skills seed complete");

As per coding guidelines, “Rust code: use log / tracing crate for logging at debug or trace level; add substantial, development-oriented logs on new/changed flows … with stable prefixes.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/jsonrpc.rs` around lines 1367 - 1370, Add explicit trace/debug
instrumentation around the call to
crate::openhuman::skills::registry::seed_default_skills: before calling it emit
a trace/debug log with a stable prefix like "[boot][skills] starting
seed_default_skills" (include the workspace path from cfg.workspace_dir), then
measure elapsed time and after the call emit another trace/debug log
"[boot][skills] finished seed_default_skills" with the duration; if the call
returns a Result or can error, log the error at debug/error level with the same
"[boot][skills]" prefix and include error details to aid correlation. Ensure you
use the project logging crate (tracing::debug/trace or log::debug) and place
these logs immediately around the existing seed_default_skills invocation in the
code path.

app/src/pages/Skills.tsx (1)

953-963: 🏗️ Heavy lift

Split Skills.tsx before adding more tab-specific surface.

This file is already far beyond the module-size target, and this new Runners panel adds more page-level branching/state into the same component. Please extract tab panels into dedicated components to keep this file maintainable and aligned with repo constraints.

As per coding guidelines, “{src,app/src,app/src-tauri}/**/*.{rs,ts,tsx}: File size: prefer ≤ ~500 lines per source file; split modules when growing to maintain readability and single responsibility.”

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/pages/Skills.tsx` around lines 953 - 963, The Skills.tsx file is
growing too large and the new "runners" tab adds more page-level branching;
extract the runners panel into its own component to reduce module size and
improve single responsibility. Create a new React component (e.g., RunnersPanel)
that contains the markup currently rendered when activeTab === 'runners' (the
div wrapper, comment/notes, and <SkillsDashboard /> usage), export it and
replace the in-file JSX with <RunnersPanel /> (or dynamically import it) inside
Skills.tsx where activeTab and the 'runners' branch are checked; ensure the new
component receives any required props or reads shared context/state the same way
(e.g., the activeTab prop or parent context) so behavior is unchanged and file
length for Skills.tsx is reduced.

src/openhuman/agent/harness/subagent_runner/ops.rs (1)

1233-1239: ⚡ Quick win

Add debug diagnostics when autonomous cap changes iteration budget.

This new cap-merge branch should log the original limit, autonomous cap, and effective limit so budget changes are traceable during skill-run debugging.

🛠️ Suggested fix

-    let max_iterations = super::autonomous::autonomous_iter_cap()
-        .map(|cap| cap.max(max_iterations))
-        .unwrap_or(max_iterations)
-        .max(1);
+    let configured_max_iterations = max_iterations.max(1);
+    let max_iterations = if let Some(cap) = super::autonomous::autonomous_iter_cap() {
+        let effective = cap.max(configured_max_iterations);
+        tracing::debug!(
+            agent_id = %agent_id,
+            autonomous_cap = cap,
+            configured_max_iterations,
+            effective_max_iterations = effective,
+            "[subagent_runner] applied autonomous iteration cap"
+        );
+        effective
+    } else {
+        configured_max_iterations
+    };

As per coding guidelines: "Rust code: use log / tracing crate for logging at debug or trace level; add substantial, development-oriented logs on new/changed flows at entry/exit points, branch decisions, external calls, retries/timeouts, state transitions, and error paths."

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/agent/harness/subagent_runner/ops.rs` around lines 1233 - 1239,
Compute and log the autonomous cap merge decision around the max_iterations
calculation: capture the original iteration budget (e.g., bind original_max =
max_iterations), call super::autonomous::autonomous_iter_cap() into a variable
(e.g., autonomous_cap_opt), compute the resulting effective limit as you already
do, and emit a debug/trace log (using tracing::debug! or log::debug!) that
reports original_max, the autonomous_cap (or "None"), and the effective limit;
only log when autonomous_cap_opt.is_some() or when the effective limit differs
from original_max to avoid noise, and place this immediately before/after
computing max_iterations in ops.rs so the branch decision is traceable.

src/openhuman/codegraph/index.rs (2)

241-359: ⚡ Quick win

Add phase-level tracing to index_ref.

This new indexing path has several important branches (cached, skipped, embed batches, manifest rewrite), but there’s no structured debug output to explain why a ref ended up partial or slow. A few stable [codegraph] debug events around phase entry/exit and batch counts would make this much easier to operate.

As per coding guidelines, src/**/*.rs: Rust code should use log / tracing at debug or trace level on new/changed flows with grep-friendly context at entry/exit points, branch decisions, external calls, and error paths.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/codegraph/index.rs` around lines 241 - 359, Add structured
debug/tracing calls in index_ref to mark phase boundaries and key branch
decisions: log at debug the function entry with repo_id and git_ref, Phase 1
start with total blobs and counts as you loop, and for each skipped/cached
increment (from store.has_blob, file size check, read_to_string) emit a debug
line identifying sha/path and reason; log Phase 2 start with
pend_docs.len()/pend_sha.len() and each embedder.batch call (include chunk size
and model via mode.model_key/embedder) plus any mismatch error before bailing;
log Phase 3 start before store.put_blobs and after set_manifest including
computed/cached/skipped totals; use the tracing/log crate macros (debug/trace)
so messages are grep-friendly (e.g., prefix “[codegraph]”) and reference
functions/vars like index_ref, IndexMode, pend_docs, pend_sha, embedder, store,
put_blobs, set_manifest.

---

361-829: 🏗️ Heavy lift

Split the benchmark/live-test bulk out of index.rs.

The production indexing code is now sharing one file with test fakes, ignored benchmarks, and live cloud probes, which pushes this new module well past the repo’s size target. Moving the long test/probe sections into sibling test modules would keep the runtime path much easier to navigate.

As per coding guidelines, **/*.{ts,tsx,rs}: Keep React component files and Rust modules at ≤ ~500 lines; split growing modules into multiple files.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/codegraph/index.rs` around lines 361 - 829, The file index.rs
contains a very large #[cfg(test)] tests module (including FakeEmbedder,
StrictEmbedder, NoEmbed, BenchEmbedder, cloud_embed_probe, index_e2e_cloud,
bench_index_speed, and many integration tests) which pushes the module past the
size guideline; extract these test structs and long ignored benchmarks/probes
into one or more sibling test modules/files (e.g., create
tests/codegraph_fakes.rs and tests/codegraph_bench.rs or similar) and left a
small, focused #[cfg(test)] mod in index.rs that re-exports or references the
new test modules; move the FakeEmbedder, StrictEmbedder, NoEmbed, BenchEmbedder,
and the long tokio::test functions
(index_ref_is_content_addressed_and_incremental,
index_ref_never_embeds_empty_doc,
lexical_mode_indexes_and_searches_without_embedding, bench_index_speed,
cloud_embed_probe, index_e2e_cloud) out of index.rs, update mod declarations so
tests compile, and ensure functions that reference CodegraphStore, index_ref,
search_ref, and EmbeddingProvider keep the same names/signatures so callers need
no changes.

src/openhuman/skills/defaults/dev-workflow/SKILL.md (1)

18-137: ⚡ Quick win

Add language identifiers to fenced code blocks.

Lines [18-137] include multiple unlabeled fenced blocks. Please annotate them (for example, json for composio_execute(...) payloads and bash for git commands) to clear MD040 warnings and improve rendering.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/skills/defaults/dev-workflow/SKILL.md` around lines 18 - 137,
The markdown contains unlabeled fenced code blocks (notably the composio_execute
payload examples and the git commands such as the git clone / git -C checkout
snippets and the composio_execute blocks for GITHUB_CREATE_A_BLOB / TREE /
COMMIT / UPDATE_A_REFERENCE / CREATE_A_PULL_REQUEST), which triggers MD040; edit
SKILL.md to add appropriate language identifiers to each fence (e.g., use json
for composio_execute payloads and bash for shell/git snippets) so every ```
block becomes ```json or ```bash as appropriate, keeping the existing content
unchanged.

src/openhuman/skills/defaults/pr-review-shepherd/SKILL.md (1)

20-126: ⚡ Quick win

Label fenced code blocks with languages.

Lines [20-126] include several unlabeled fenced blocks. Adding language tags (json, bash) will resolve MD040 warnings and improve copy/paste clarity.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/skills/defaults/pr-review-shepherd/SKILL.md` around lines 20 -
126, The markdown in SKILL.md contains unlabeled fenced code blocks (the
composio_execute JSON call blocks and the git/bash snippets such as the
composio_execute({...}) sequences and the git clone/git -C ... examples); fix by
adding appropriate language tags to each fenced block (e.g., ```json for
composio_execute JSON blocks and ```bash for shell/git snippets) so linters stop
flagging MD040 and copy/paste behavior is improved.

src/openhuman/skills/defaults/github-issue-crusher/SKILL.md (1)

13-95: ⚡ Quick win

Specify fence languages for command examples.

Lines [13-95] contain unlabeled fenced blocks. Please mark them (json, bash, etc.) to satisfy markdownlint MD040 and keep examples easier to scan.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/skills/defaults/github-issue-crusher/SKILL.md` around lines 13
- 95, The markdown in SKILL.md has unlabeled fenced code blocks (the
composio_execute JSON examples and the shell/git snippets) which trigger
markdownlint MD040; update each triple-backtick fence in the section containing
the composio_execute calls and the git/CLI examples to include an explicit
language tag (e.g., json for composio_execute blocks and bash/sh for shell/git
commands) so the examples are properly highlighted and lint-clean; look for
occurrences of composio_execute({ ... }) and the git clone/config/checkout
snippets and add the appropriate fence language labels around those blocks.

src/openhuman/embeddings/rpc.rs (1)

384-402: ⚡ Quick win

Add debug diagnostics to the new provider-construction path.

Lines [384-402] introduce a cross-domain helper with no trace/debug instrumentation. Please log entry/exit context (provider, model, dims, custom endpoint presence) to match existing observability standards.

Suggested patch

 pub fn provider_from_config(config: &Config) -> anyhow::Result<Box<dyn super::EmbeddingProvider>> {
     let provider_name = &config.memory.embedding_provider;
     let model = &config.memory.embedding_model;
     let dims = config.memory.embedding_dimensions;
     let api_key = resolve_api_key(config, provider_name);
     let custom_endpoint = provider_name.strip_prefix("custom:").map(|s| s.to_string());
     let provider_slug = if provider_name.starts_with("custom:") {
         "custom"
     } else {
         provider_name.as_str()
     };
-    create_embedding_provider_with_credentials(
+    tracing::debug!(
+        provider = provider_slug,
+        model = model.as_str(),
+        dims,
+        has_custom_endpoint = custom_endpoint.is_some(),
+        "{LOG_PREFIX} provider_from_config start"
+    );
+    let provider = create_embedding_provider_with_credentials(
         provider_slug,
         model,
         dims,
         &api_key,
         custom_endpoint.as_deref(),
-    )
+    )?;
+    tracing::debug!(
+        provider = provider_slug,
+        model = model.as_str(),
+        dims,
+        "{LOG_PREFIX} provider_from_config ready"
+    );
+    Ok(provider)
 }

As per coding guidelines: “Rust code: use log / tracing crate for logging at debug or trace level; add substantial, development-oriented logs on new/changed flows…”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/embeddings/rpc.rs` around lines 384 - 402, Add debug/tracing
instrumentation to provider_from_config: at function entry emit a debug/trace
log that includes provider_name, model, dims, whether custom_endpoint is Some
(but not the raw API key), and the resolved provider_slug; after calling
create_embedding_provider_with_credentials emit a debug/trace log indicating
success or include the error on failure. Use the tracing/log crate consistent
with project usage, call resolve_api_key only as now but ensure the API key is
never logged, and reference provider_from_config, resolve_api_key,
create_embedding_provider_with_credentials, and the local variables
provider_name, model, dims, custom_endpoint, provider_slug when adding the logs.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 280c4acd-dcef-420f-84a5-03038c6d683a

📥 Commits

Reviewing files that changed from the base of the PR and between 6ec0a83 and 4f955a1.

📒 Files selected for processing (87)

• .claude/memory.md
• app/src-tauri/src/lib.rs
• app/src/AppRoutes.tsx
• app/src/components/settings/panels/DevWorkflowPanel.tsx
• app/src/components/settings/panels/DeveloperOptionsPanel.tsx
• app/src/components/settings/panels/SkillsRunnerPanel.tsx
• app/src/components/settings/panels/__tests__/DevWorkflowPanel.test.tsx
• app/src/components/skills/CreateSkillForm.tsx
• app/src/components/skills/CreateSkillModal.tsx
• app/src/components/skills/ScheduledCronCard.test.tsx
• app/src/components/skills/ScheduledCronCard.tsx
• app/src/components/skills/SkillsRunnerBody.tsx
• app/src/components/skills/SmartIssuePicker.tsx
• app/src/components/skills/__tests__/CreateSkillForm.test.tsx
• app/src/components/skills/__tests__/CreateSkillModal.test.tsx
• app/src/components/skills/__tests__/SkillsRunnerBody.test.tsx
• app/src/components/skills/inputs/BranchPicker.tsx
• app/src/components/skills/inputs/RepoPicker.tsx
• app/src/components/skills/preflightGate.test.ts
• app/src/components/skills/preflightGate.ts
• app/src/components/skills/scheduledCronFormat.ts
• app/src/lib/cron/cronToHuman.test.ts
• app/src/lib/cron/cronToHuman.ts
• app/src/lib/i18n/chunks/ar-5.ts
• app/src/lib/i18n/chunks/bn-5.ts
• app/src/lib/i18n/chunks/de-5.ts
• app/src/lib/i18n/chunks/en-5.ts
• app/src/lib/i18n/chunks/es-5.ts
• app/src/lib/i18n/chunks/fr-5.ts
• app/src/lib/i18n/chunks/hi-5.ts
• app/src/lib/i18n/chunks/id-5.ts
• app/src/lib/i18n/chunks/it-5.ts
• app/src/lib/i18n/chunks/ko-5.ts
• app/src/lib/i18n/chunks/pl-5.ts
• app/src/lib/i18n/chunks/pt-5.ts
• app/src/lib/i18n/chunks/ru-5.ts
• app/src/lib/i18n/chunks/zh-CN-5.ts
• app/src/lib/i18n/en.ts
• app/src/pages/Settings.tsx
• app/src/pages/SkillNew.test.tsx
• app/src/pages/SkillNew.tsx
• app/src/pages/Skills.tsx
• app/src/pages/SkillsDashboard.test.tsx
• app/src/pages/SkillsDashboard.tsx
• app/src/pages/SkillsRun.tsx
• app/src/services/api/skillsApi.ts
• app/src/utils/tauriCommands/cron.ts
• docs/skills-runner-unification.md
• src/core/jsonrpc.rs
• src/openhuman/agent/agents/code_executor/agent.toml
• src/openhuman/agent/agents/code_executor/prompt.md
• src/openhuman/agent/agents/orchestrator/agent.toml
• src/openhuman/agent/agents/orchestrator/prompt.md
• src/openhuman/agent/agents/tools_agent/agent.toml
• src/openhuman/agent/harness/subagent_runner/autonomous.rs
• src/openhuman/agent/harness/subagent_runner/mod.rs
• src/openhuman/agent/harness/subagent_runner/ops.rs
• src/openhuman/codegraph/index.rs
• src/openhuman/codegraph/mod.rs
• src/openhuman/codegraph/search.rs
• src/openhuman/codegraph/store.rs
• src/openhuman/composio/identity.rs
• src/openhuman/composio/mod.rs
• src/openhuman/cron/schemas.rs
• src/openhuman/embeddings/mod.rs
• src/openhuman/embeddings/rpc.rs
• src/openhuman/mod.rs
• src/openhuman/skills/defaults/dev-workflow/SKILL.md
• src/openhuman/skills/defaults/dev-workflow/skill.toml
• src/openhuman/skills/defaults/github-issue-crusher/SKILL.md
• src/openhuman/skills/defaults/github-issue-crusher/skill.toml
• src/openhuman/skills/defaults/pr-review-shepherd/SKILL.md
• src/openhuman/skills/defaults/pr-review-shepherd/skill.toml
• src/openhuman/skills/mod.rs
• src/openhuman/skills/ops.rs
• src/openhuman/skills/ops_create.rs
• src/openhuman/skills/ops_tests.rs
• src/openhuman/skills/preflight.rs
• src/openhuman/skills/registry.rs
• src/openhuman/skills/run_log.rs
• src/openhuman/skills/schemas.rs
• src/openhuman/tools/impl/agent/mod.rs
• src/openhuman/tools/impl/agent/run_skill.rs
• src/openhuman/tools/impl/codegraph/mod.rs
• src/openhuman/tools/impl/mod.rs
• src/openhuman/tools/impl/network/url_guard.rs
• src/openhuman/tools/ops.rs

[graycyrus]

@M3gA-Mind / @sanil-23 — CI is still failing on this push. The step that's tripping is Prettier formatting on the new test files added in the last two commits (SkillsRunnerPanel.test.tsx, SmartIssuePicker.test.tsx, BranchPicker.test.tsx, RepoPicker.test.tsx, SkillsRun.test.tsx, skillsApi.test.ts, cron.test.ts). Run pnpm prettier --write on those and push again.

Also flagging that the major issue from my last review is still open:

The loadExistingJob function in DevWorkflowPanel.tsx sets existingJob but never writes back selectedRepo, targetBranch, or schedule from the stored job data. Users who return to this panel with an active cron job see a blank form — there's no way to inspect or edit the current configuration without nuking and recreating it. That regression needs to be fixed before this merges.

Fix the Prettier failures and the DevWorkflowPanel form state restoration, and I'll do a full pass.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

app/src/components/skills/SmartIssuePicker.test.tsx (1)

31-69: ⚡ Quick win

Scenario tests are too weak for the behaviors they claim.

These cases currently all pass on combobox presence, so they don’t prove empty-state, active-connection repo loading, or graceful error handling behavior. Please assert the scenario outputs (e.g., loaded repo options, empty-state text, or that execute was/wasn’t called as expected).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/components/skills/SmartIssuePicker.test.tsx` around lines 31 - 69,
Update the SmartIssuePicker.test.tsx assertions to verify specific scenario
outputs rather than only combobox presence: in the "shows loading and then empty
state" test assert that mockListConnections resolves to no connections and the
component displays the empty-state text or specific option like "No repositories
found" after render; in "renders repos when GitHub connection is active" assert
mockExecute is called (mockExecute) and that the repo option "testuser/myrepo"
(or a rendered option element) appears in the dropdown/options list; in
"pre-selects repo from values prop" assert the combobox value or selected option
equals "owner/repo"; and in "handles listConnections error gracefully" assert
mockExecute was not called and the UI shows the expected error or fallback
empty-state indicator. Use the existing mocks mockListConnections and
mockExecute and the component SmartIssuePicker to locate the tests and update
only the expect(...) assertions accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/src/components/skills/inputs/BranchPicker.test.tsx`:
- Around line 27-31: Test "reflects a pre-selected value" currently only checks
the combobox exists; update it to assert the selected branch is actually "main".
After rendering BranchPicker with value="main" (using baseProps and
repo="owner/repo"), get the combobox via screen.findByRole('combobox') and
assert its value (e.g., expect(select).toHaveValue('main') or verify the option
for "main" is selected) so the test fails if the preselection is ignored.

---

Nitpick comments:
In `@app/src/components/skills/SmartIssuePicker.test.tsx`:
- Around line 31-69: Update the SmartIssuePicker.test.tsx assertions to verify
specific scenario outputs rather than only combobox presence: in the "shows
loading and then empty state" test assert that mockListConnections resolves to
no connections and the component displays the empty-state text or specific
option like "No repositories found" after render; in "renders repos when GitHub
connection is active" assert mockExecute is called (mockExecute) and that the
repo option "testuser/myrepo" (or a rendered option element) appears in the
dropdown/options list; in "pre-selects repo from values prop" assert the
combobox value or selected option equals "owner/repo"; and in "handles
listConnections error gracefully" assert mockExecute was not called and the UI
shows the expected error or fallback empty-state indicator. Use the existing
mocks mockListConnections and mockExecute and the component SmartIssuePicker to
locate the tests and update only the expect(...) assertions accordingly.

🪄 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: 1f66f95e-86e2-40ec-8cf9-116456baf7af

📥 Commits

Reviewing files that changed from the base of the PR and between 4f955a1 and d5bf384.

📒 Files selected for processing (7)

• app/src/components/settings/panels/SkillsRunnerPanel.test.tsx
• app/src/components/skills/SmartIssuePicker.test.tsx
• app/src/components/skills/inputs/BranchPicker.test.tsx
• app/src/components/skills/inputs/RepoPicker.test.tsx
• app/src/pages/SkillsRun.test.tsx
• app/src/services/api/skillsApi.test.ts
• app/src/utils/tauriCommands/cron.test.ts

[graycyrus]

@M3gA-Mind (and @sanil-23) — still a couple of things blocking this:

Two new test commits since last look. The expanded BranchPicker/RepoPicker async paths and SkillsRun back-button test are good additions. But CI is now failing at TypeScript type check — the previous blocker was a Prettier format issue, now tsc itself is erroring before Prettier even runs. That's a regression from the test commits. Run pnpm tsc --noEmit locally and fix whatever the new test files introduced.

The major issue from Review 1 is still open: loadExistingJob in DevWorkflowPanel populates existingJob state but never writes back selectedRepo, targetBranch, or schedule from the stored job data. Users who return to the panel with an active cron job see a blank form. That needs to be fixed before this ships.

Once tsc is clean and the form-restore is fixed, I'll approve.

[coderabbitai]

🧹 Nitpick comments (2)

app/src/components/skills/inputs/BranchPicker.test.tsx (1)

27-33: ⚡ Quick win

Verify branches are actually displayed, not just that the API was called.

The test title claims it "loads and displays branches," but it only asserts mockExecute was called. This doesn't verify the branches appear in the UI or that users can interact with them.

Suggested enhancement

   it('loads and displays branches when repo is set', async () => {
     render(<BranchPicker {...baseProps} repo="owner/repo" />);
-    await waitFor(() => {
-      expect(screen.getByRole('combobox')).toBeInTheDocument();
-    });
     expect(mockExecute).toHaveBeenCalled();
+    // Verify branches are actually available/rendered
+    const select = await screen.findByRole('combobox');
+    expect(select).not.toBeDisabled();
   });

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/components/skills/inputs/BranchPicker.test.tsx` around lines 27 - 33,
The test "loads and displays branches when repo is set" currently only asserts
that mockExecute was called; update it to also verify UI rendering and
interactivity by checking that the fetched branch names are present in the DOM
and selectable. After rendering <BranchPicker {...baseProps} repo="owner/repo"
/> and awaiting the combobox (getByRole('combobox')), assert that specific
branch text nodes returned by the mocked API (e.g., "main" or other fixture
branch names) are rendered (getByText / queryByText) and simulate/select one to
ensure user interaction works; retain the mockExecute assertion to ensure the
API was invoked. Include references to BranchPicker, mockExecute, and the
combobox role in the updated assertions.

app/src/components/skills/__tests__/SkillsRunnerBody.test.tsx (1)

1-618: ⚖️ Poor tradeoff

Consider splitting this test file as it grows beyond 618 lines.

The test suite is well-organized with five distinct test suites, but exceeds the ~500 line guideline. As the component evolves, consider splitting into focused test files (e.g., SkillsRunnerBody.toggle.test.tsx, SkillsRunnerBody.history.test.tsx, etc.) to maintain readability.

As per coding guidelines: **/*.{ts,tsx,rs}: Keep React component files and Rust modules at ≤ ~500 lines; split growing modules into multiple files

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/components/skills/__tests__/SkillsRunnerBody.test.tsx` around lines 1
- 618, The test file has grown past the ~500 line guideline and should be split
into smaller, focused test files; extract related describe blocks into separate
files (e.g., the "saved-schedule toggle" suite, the "per-job history viewer"
suite, the "SmartIssuePicker conditional mount" suite, the "URL ?skill=
preselect" suite, and the "Run Now flow" suite). Create new test modules that
each import the helper utilities used here (importBody, renderBody, hoisted
mocks, stableT) or refactor those helpers into a shared test-utils file, then
move the corresponding describe(...) blocks (search for
describe('SkillsRunnerBody — saved-schedule toggle', describe('SkillsRunnerBody
— per-job history viewer', describe('SkillsRunnerBody — SmartIssuePicker
conditional mount', describe('SkillsRunnerBody — URL ?skill= preselect', and
describe('SkillsRunnerBody — Run Now flow')) into their own files and update
imports accordingly so each new file remains under ~500 lines.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/src/components/skills/__tests__/SkillsRunnerBody.test.tsx`:
- Around line 1-618: The test file has grown past the ~500 line guideline and
should be split into smaller, focused test files; extract related describe
blocks into separate files (e.g., the "saved-schedule toggle" suite, the
"per-job history viewer" suite, the "SmartIssuePicker conditional mount" suite,
the "URL ?skill= preselect" suite, and the "Run Now flow" suite). Create new
test modules that each import the helper utilities used here (importBody,
renderBody, hoisted mocks, stableT) or refactor those helpers into a shared
test-utils file, then move the corresponding describe(...) blocks (search for
describe('SkillsRunnerBody — saved-schedule toggle', describe('SkillsRunnerBody
— per-job history viewer', describe('SkillsRunnerBody — SmartIssuePicker
conditional mount', describe('SkillsRunnerBody — URL ?skill= preselect', and
describe('SkillsRunnerBody — Run Now flow')) into their own files and update
imports accordingly so each new file remains under ~500 lines.

In `@app/src/components/skills/inputs/BranchPicker.test.tsx`:
- Around line 27-33: The test "loads and displays branches when repo is set"
currently only asserts that mockExecute was called; update it to also verify UI
rendering and interactivity by checking that the fetched branch names are
present in the DOM and selectable. After rendering <BranchPicker {...baseProps}
repo="owner/repo" /> and awaiting the combobox (getByRole('combobox')), assert
that specific branch text nodes returned by the mocked API (e.g., "main" or
other fixture branch names) are rendered (getByText / queryByText) and
simulate/select one to ensure user interaction works; retain the mockExecute
assertion to ensure the API was invoked. Include references to BranchPicker,
mockExecute, and the combobox role in the updated assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 21c49661-953f-4039-b0cd-9e164643fdab

📥 Commits

Reviewing files that changed from the base of the PR and between d5bf384 and 4117fa8.

📒 Files selected for processing (8)

• app/src/components/skills/SmartIssuePicker.test.tsx
• app/src/components/skills/__tests__/CreateSkillModal.test.tsx
• app/src/components/skills/__tests__/SkillsRunnerBody.test.tsx
• app/src/components/skills/inputs/BranchPicker.test.tsx
• app/src/components/skills/inputs/RepoPicker.test.tsx
• app/src/pages/SkillsRun.test.tsx
• app/src/services/api/skillsApi.test.ts
• app/src/utils/tauriCommands/cron.test.ts

🚧 Files skipped from review as they are similar to previous changes (2)

• app/src/pages/SkillsRun.test.tsx
• app/src/components/skills/SmartIssuePicker.test.tsx

[graycyrus]

@M3gA-Mind the new test commits look good — coverage is expanding in the right places. But the two blockers from my first review are still standing:

1. The DevWorkflowPanel form state regression (my inline thread on line 93) — loadExistingJob sets existingJob but never writes back selectedRepo, targetBranch, or schedule. Users navigating back to that panel see a blank form even with an active cron job. This needs to be fixed before we can ship.

2. CI is still pending across several checks (Rust tests, Frontend unit tests, Rust quality, build jobs). Nothing's failing, but I need everything green before I can approve.

Fix the form state issue and let CI finish, and I'll get this over the line.

[senamakel]

Deferred (not addressed in this pass) — gc() model-variant reclamation.

This is a real gap, but it's a heavy lift: making GC model-aware requires a schema change (store the active model per ref, or a manifest_refs(sha, model) table) plus a migration and updates to every manifest writer. That's larger than the surgical review-fix scope and carries migration risk, so I'm deferring it rather than landing a rushed partial.

Impact today is bounded: obsolete (sha, model) rows only accumulate when a repo is reindexed across embedding signatures (lexical→dense), and they're inert (never queried). Suggest tracking as a follow-up issue.

[senamakel]

Deferred (not addressed in this pass) — full-file scans in scan_runs / footer detection.

Valid perf concern, but the fix is an I/O redesign (drive listing from metadata(), seek+read only header/tail byte windows instead of read_to_string, derive size/mtime from DirEntry::metadata) touching scan_runs, footer detection (~405-414, ~440-443) and ScannedRun construction. That's broader and riskier than the surgical review-fix scope here, so deferring to a dedicated follow-up rather than reworking the read path under time pressure.

Current impact is the runner-panel poll re-reading whole .runs logs; bounded for normal history sizes but worth optimizing.

[senamakel]

Acknowledged — the REPO_INPUT_NAMES / BRANCH_INPUT_NAMES name-convention routing (and the SmartIssuePicker hard-coding it implies) is left as-is for this PR per the existing TODO(picker-schema). Flagging here so it's tracked for the schema-driven picker field follow-up rather than silently lingering.

[senamakel]

Addressed the read-back: loadExistingJob now restores schedule (from schedule.expr/expression), selectedRepo (decoded from the dev-workflow-<owner>-<repo> job name), and targetBranch (parsed from the prompt's PRs target `<branch>` line), all undefined-guarded, with a debug log of what was restored.

Two caveats worth your eyes (flagging rather than expanding scope):

1. Restored state isn't visible while a job is active. The form (repo/branch/schedule controls) only renders under {!existingJob}, so the restore currently only takes effect once the job is removed — or if an edit affordance is added that renders the form with an active job. If you intended an in-place edit flow, the component needs that affordance.
2. Repo decode is best-effort. It replaces the first - after the prefix with /, which mis-splits owners/repos that contain - (e.g. M3gA-Mind). A robust round-trip would store the owner/repo in job metadata rather than encoding it in the name. Left as-is to stay within the review-fix scope.