Skip to content

Refactorsept2025#57

Merged
dhirmadi merged 6 commits intomainfrom
refactorsept2025
Sep 28, 2025
Merged

Refactorsept2025#57
dhirmadi merged 6 commits intomainfrom
refactorsept2025

Conversation

@dhirmadi
Copy link
Owner

Pull Request

Description

Brief description of the changes in this PR.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement
  • Test coverage improvement

Related Issues

Closes #(issue number)

Changes Made

  • Change 1
  • Change 2
  • Change 3

Documentation Impact

Testing

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have checked the Testing Documentation for guidance
  • I have run npm run docs:validate to check documentation links

Security Considerations

  • This change does not introduce security vulnerabilities
  • I have reviewed the security implications
  • Authentication/authorization is properly implemented
  • Input validation is in place where needed

Architecture Compliance

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Screenshots (if applicable)

Add screenshots to help explain your changes.

Additional Notes

Any additional information that reviewers should know.

- Add .cursorrules with repo summary, Node backend rules, security and workflow guidance
- Add .cursor/agents for: node_backend, openhands_migrated_agents, auth0, codereview, security, github, schema_generator, feature_lifecycle
- Preserve .openhands/ unchanged; mirror agents for Cursor discovery
- Ensure Cursor understands stack, layout, and conventions for MWAP backend
@dhirmadi dhirmadi requested a review from Copilot September 28, 2025 12:03
@dhirmadi dhirmadi self-assigned this Sep 28, 2025
@openhands-ai
Copy link

openhands-ai bot commented Sep 28, 2025

Looks like there are a few issues preventing this PR from being merged!

  • GitHub Actions are failing:
    • Documentation Validation
    • Documentation Validation

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #57 at branch `refactorsept2025`

Feel free to include any additional details that might help me get this PR into a better state.

You can manage your notification settings

@dhirmadi dhirmadi added bug Something isn't working documentation Improvements or additions to documentation maintenance labels Sep 28, 2025
@dhirmadi dhirmadi merged commit 1520108 into main Sep 28, 2025
4 of 7 checks passed
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This refactoring PR modernizes the codebase by updating file imports to use TypeScript extensions, implements input sanitization, adds minimal Heroku-optimized testing strategy, enhances logging with correlation IDs, and includes comprehensive documentation updates.

  • Updates file imports from .js to .ts extensions across test files
  • Implements input sanitization for tenant and project schemas
  • Adds minimal testing strategy optimized for Heroku deployments with fast release gates

Reviewed Changes

Copilot reviewed 39 out of 43 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/utils/*.test.ts New unit tests for TenantService and ProjectsService
tests/setupTests.ts Enhanced mock setup with multiple import specifier support
tests/middleware/auth.test.ts Updated imports and improved mock handling
tests/oauth/oauthCallbackSecurity.test.ts Modernized import syntax and test structure
src/utils/validate.ts Added sanitizeString utility function
src/utils/logger.ts Enhanced logging with correlation IDs
src/schemas/*.schema.ts Integrated input sanitization via transform
src/middleware/validateRequest.ts New request validation middleware
docs/* Updated documentation to reflect current architecture

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +50 to +51


Copy link

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

[nitpick] Remove the extra blank lines at the end of the file. The file should end with a single newline.

Suggested change

Copilot uses AI. Check for mistakes.
Comment on lines +28 to +29


Copy link

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

[nitpick] Remove the extra blank lines at the end of the file. The file should end with a single newline.

Suggested change

Copilot uses AI. Check for mistakes.
});

// Ensure mocks also match other specifiers used by tests/services
vi.mock('../../src/config/db', () => {
Copy link

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

There is duplicate mock configuration for the database module with different import specifiers. Consider consolidating these into a single mock definition to reduce code duplication.

Copilot uses AI. Check for mistakes.
return { getDB: () => db, db };
});

vi.mock('../../src/config/db.js', () => {
Copy link

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

There is duplicate mock configuration for the database module with different import specifiers. Consider consolidating these into a single mock definition to reduce code duplication.

Copilot uses AI. Check for mistakes.
Comment on lines +33 to +34


Copy link

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

[nitpick] Remove the extra blank lines at the end of the file. The file should end with a single newline.

Suggested change

Copilot uses AI. Check for mistakes.
Comment on lines +33 to +34


Copy link

Copilot AI Sep 28, 2025

Choose a reason for hiding this comment

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

[nitpick] Remove the extra blank lines at the end of the file. The file should end with a single newline.

Suggested change

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working documentation Improvements or additions to documentation maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant