Skip to content

fix(agent): address skill review feedback - #2009

Merged
zhangmo8 merged 3 commits into
devfrom
fix/skill-review-followup
Jul 23, 2026
Merged

fix(agent): address skill review feedback#2009
zhangmo8 merged 3 commits into
devfrom
fix/skill-review-followup

Conversation

@zhangmo8

@zhangmo8 zhangmo8 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Addresses follow-up review findings from #2007:

  • Gate session lifecycle operations with resolved canonical agent IDs.
  • Redact queue-drain error logs across runtime and interaction paths.
  • Tighten skill tool/catalog assertions.

Validation: targeted Vitest suite, node/web typechecks, format, i18n, and lint passed.

Summary by CodeRabbit

  • Bug Fixes
    • Session creation now consistently uses the canonical assignment/runtime configuration across session types.
    • Runtime errors are now redacted in logs and reported with clearer stage context during pending queue draining and abort flows.
  • Tests
    • Expanded and updated coverage for canonical agent handling, built-in settings tool recognition, skills catalog refresh parameters, and refined runtime/error expectations.
  • Chores
    • Improved icon collection subset generation to remove stale modification metadata.

@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2940ae5e-53ee-493b-8f48-fc028a0f076c

📥 Commits

Reviewing files that changed from the base of the PR and between 0ebd392 and 192c827.

⛔ Files ignored due to path filters (1)
  • src/renderer/src/lib/icons/icon-collections.generated.ts is excluded by !**/*.generated.*
📒 Files selected for processing (1)
  • scripts/generate-icon-collections.mjs

📝 Walkthrough

Walkthrough

Changes

The PR redacts runtime errors in structured logs, passes resolved session assignments through lifecycle gates, strengthens related test expectations and fixtures, and removes inherited lastModified metadata from generated icon subsets.

Runtime error logging

Layer / File(s) Summary
Redacted runtime error contract
src/main/agent/deepchat/runtime/runtimeErrorLogging.ts
Adds a minimal redacted error shape that classifies values as Error or UnknownError.
Coordinator error-path logging
src/main/agent/deepchat/runtime/*Coordinator.ts, test/main/agent/deepchat/runtime/deepChatRuntimeCoordinator.test.ts
Uses structured logger calls with redacted payloads and stage, session, or reason context.

Session assignment threading

Layer / File(s) Summary
Lifecycle-gated assignment resolution
src/main/session/lifecycle.ts, test/main/session/lifecycle.test.ts
Resolves assignments before lifecycle-gated session creation and passes canonical agent IDs into lifecycle operations.

Supporting contract and generation updates

Layer / File(s) Summary
Behavioral test contracts and fixtures
test/main/session/assignment.test.ts, test/main/skill/toolNameMapping.test.ts, test/renderer/stores/skillsStore.test.ts, test/main/tool/agentTools/*
Strengthens retry, tool-list, catalog-refresh, and skills-directory assertions and mocks.
Icon subset metadata cleanup
scripts/generate-icon-collections.mjs
Removes inherited lastModified metadata from generated icon subsets.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the PR’s focus on addressing review feedback in agent skill handling and related runtime/session fixes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/skill-review-followup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zhangmo8
zhangmo8 merged commit 011407a into dev Jul 23, 2026
8 checks passed
@zhangmo8
zhangmo8 deleted the fix/skill-review-followup branch July 23, 2026 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant