Skip to content

fix: resolve SonarCloud code quality issues across codebase#99

Open
sugat009 wants to merge 9 commits intomainfrom
fix/sonarcloud-code-quality
Open

fix: resolve SonarCloud code quality issues across codebase#99
sugat009 wants to merge 9 commits intomainfrom
fix/sonarcloud-code-quality

Conversation

@sugat009
Copy link
Copy Markdown
Member

Summary

  • Resolve ~50 SonarCloud findings across 12 files: cognitive complexity, deprecated APIs, unsafe patterns, and CHT coding guideline violations
  • Cognitive complexity: extracted helpers in research.ts, research-supervisor-example.ts, research-supervisor.ts, context-analysis-agent.ts, domain-inference.ts, ticket-parser.ts, context-loader.ts
  • Deprecated APIs: modelNamemodel in LangChain ChatAnthropic, .match()RegExp.exec()
  • Unsafe patterns: response.content.toString() → safe type check, anyRecord<string, unknown>, regex backtracking fixes, String.raw for regex escapes
  • CHT style alignment: function declarations → arrow functions, inline unions → type aliases (IssueType, Priority, Complexity), early returns
  • Config: bump tsconfig to ES2023 for toSorted(), add coverage/ to .eslintignore, add no-require-imports: off for tests
  • Uncommented loadDomainIndices in domain-inference.ts (was dead commented-out code, now wired up)

Address ~50 SonarCloud findings: reduce cognitive complexity by
extracting helpers, replace deprecated APIs, fix unsafe patterns,
and align with CHT coding guidelines (arrow functions, small
functions, early returns). Bump tsconfig to ES2023 for toSorted.
SonarCloud flags bare imports like 'fs' and 'path' as non-conventional
(typescript:S7772). Updated all source and test files to use 'node:fs'
and 'node:path'. Updated proxyquire mock keys in tests to match.
research.ts and research-supervisor-example.ts shared ~90 lines of
identical display functions. Extracted into cli/display-helpers.ts.
Moves remaining duplicated code (loadTicket, supervisor init, run loop)
into display-helpers.ts. Entry points now only define their unique
getTicketPath logic and help hints.
@sugat009 sugat009 moved this from Todo to In Review in CHT Multi-Agent System (cht-agent) Apr 17, 2026
@sugat009 sugat009 assigned Hareet and sugat009 and unassigned Hareet Apr 30, 2026
# Conflicts:
#	.eslintrc
#	src/agents/documentation-search-agent.ts
#	src/cli/research.ts
#	src/utils/context-loader.ts
Ajay9704 added a commit to Ajay9704/cht-agent that referenced this pull request May 1, 2026
Revert files not related to ticket validation feature:
- .eslintignore
- .mocharc.json
- test/helpers.ts
- test/utils/context-loader.spec.ts
- test/utils/ticket-parser.spec.ts

These files were modified during SonarLint cleanup but are outside
the scope of PR medic#53 (ticket validation). They will be addressed
separately in PR medic#99.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Hareet Hareet self-requested a review May 6, 2026 15:28
Hareet
Hareet previously approved these changes May 6, 2026
Copy link
Copy Markdown
Member

@Hareet Hareet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

small node consistency if you want to grab it during this pass:

node 18 in conventional-commits.yml
node 22 in unit_tests.yml
and node >= 22 in engines.node

# Conflicts:
#	.github/workflows/conventional-commits.yml
@sugat009
Copy link
Copy Markdown
Member Author

sugat009 commented May 6, 2026

@Hareet bumped conventional-commits.yml to Node 22 and pulled in main (resolved a small conflict). Could you take another look when you get a chance?

@sugat009 sugat009 requested a review from Hareet May 6, 2026 16:03
Copy link
Copy Markdown
Member

@Hareet Hareet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified:

  • ✅ PR #98's script-injection mitigation preserved (env: PR_TITLE + <<< "$PR_TITLE").
  • ✅ PR #98's --ignore-scripts hardening preserved.
  • ✅ PR #98's action version bumps (@v6) preserved.
  • ✅ Node 22 (PR #99's intent) preserved.
  • ✅ Now consistent with unit_tests.yml and engines.node.
  • ✅ cache: npm is a small bonus that's safe — package-lock.json exists at the branch root, so the cache key has something to hash and npm ci won't fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

2 participants