Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
416 changes: 416 additions & 0 deletions .claude/CLAUDE.md

Large diffs are not rendered by default.

239 changes: 239 additions & 0 deletions .claude/plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,239 @@
# Phase 2 PR #1: Flatten Account Model - IMPLEMENTATION COMPLETE

## Status: βœ… COMPLETED

**Implementation Date:** February 13, 2026
**All tests passing:** 120/120 βœ…
**TypeScript compilation:** Success βœ…

---

## Summary

Successfully implemented Phase 2 PR #1: Flatten Account Model. The Account model has been simplified from 177 lines (51 + 126 inherited) to 51 lines, removing the BaseAccount inheritance and flattening nested structures into direct properties.

## Changes Implemented

### Files Modified (7 files)

1. **`jslib/common/src/enums/stateVersion.ts`**
- Added `StateVersion.Five` for the flattened Account structure
- Updated `StateVersion.Latest = Five`

2. **`src/models/account.ts`**
- Removed `extends BaseAccount` inheritance
- Removed `ClientKeys` class (redundant)
- Flattened 6 authentication fields to top level:
- `userId`, `entityId`, `apiKeyClientId`
- `accessToken`, `refreshToken`, `apiKeyClientSecret`
- Kept `DirectoryConfigurations` and `DirectorySettings` unchanged
- Added compatibility fields with FIXME comment for jslib infrastructure:
- `data?`, `keys?`, `profile?`, `settings?`, `tokens?` (optional, unused)
- Simplified constructor without Object.assign

3. **`src/services/stateMigration.service.ts`**
- Added `migrateStateFrom3To4()` placeholder migration
- Added `migrateStateFrom4To5()` to flatten nested β†’ flat Account structure
- Updated `migrate()` method with new case statements for v3β†’v4 and v4β†’v5
- Updated `migrateStateFrom1To2()` to use flattened structure (removed `account.profile`, `account.clientKeys`)

4. **`src/services/auth.service.ts`**
- Removed imports: `AccountKeys`, `AccountProfile`, `AccountTokens`
- Simplified account creation from 26 lines to 10 lines (62% reduction)
- Direct property assignment instead of nested objects with spread operators

5. **`src/services/state.service.ts`**
- Changed `account.profile.userId` β†’ `account.userId`
- Removed `account.settings` from `scaffoldNewAccountDiskStorage`
- Added `settings` back to `resetAccount` for base class compatibility (unused but required)

6. **`src/services/authService.spec.ts`**
- Removed imports: `AccountKeys`, `AccountProfile`, `AccountTokens`
- Updated test expectations to match new flat Account structure

### Files Created (1 file)

7. **`src/services/stateMigration.service.spec.ts`**
- Comprehensive migration test suite (5 tests, 210 lines)
- Tests flattening nested account structure
- Tests handling missing nested objects gracefully
- Tests empty account list
- Tests preservation of directory configurations and settings
- Tests state version update

## Code Reduction Achieved

- **Account model:** 177 lines (51 + 126 inherited) β†’ 51 lines (71% reduction)
- **AuthService account creation:** 26 lines β†’ 10 lines (62% reduction)
- **Import statements removed:** 5 jslib imports across multiple files

## Migration Logic

### State Version v4 β†’ v5 Migration

The `migrateStateFrom4To5()` method handles conversion from nested to flat structure:

```typescript
// OLD (nested structure):
{
profile: {
userId: "CLIENT_ID",
entityId: "CLIENT_ID",
apiKeyClientId: "organization.CLIENT_ID"
},
tokens: {
accessToken: "token",
refreshToken: "refresh"
},
keys: {
apiKeyClientSecret: "secret"
}
}

// NEW (flat structure):
{
userId: "CLIENT_ID",
entityId: "CLIENT_ID",
apiKeyClientId: "organization.CLIENT_ID",
accessToken: "token",
refreshToken: "refresh",
apiKeyClientSecret: "secret"
}
```

**Migration Safety:**

- Null-safe property access with `??` operator
- Preserves all directory configurations and settings
- Falls back to userId if profile.userId doesn't exist
- Handles empty account lists gracefully

## Test Results

### Unit Tests: βœ… PASS

```
Test Suites: 14 passed, 14 total
Tests: 120 passed, 120 total
```

New tests added:

- `should flatten nested account structure` βœ…
- `should handle missing nested objects gracefully` βœ…
- `should handle empty account list` βœ…
- `should preserve directory configurations and settings` βœ…
- `should update state version after successful migration` βœ…

### TypeScript Compilation: βœ… PASS

```
npm run test:types
```

All type checks pass with zero errors.

## Technical Notes

### Compatibility Fields

Added optional compatibility fields to Account model to satisfy jslib infrastructure type constraints:

```typescript
// FIXME: Remove these compatibility fields after StateServiceVNext migration (PR #990) is merged
// These fields are unused but required for type compatibility with jslib's StateService infrastructure
data?: any;
keys?: any;
profile?: any;
settings?: any;
tokens?: any;
```

These will be removed after PR #990 (StateServiceVNext) merges and old StateService is deleted.

### Key Architectural Decision

Chose to add compatibility fields rather than refactor entire jslib infrastructure because:

1. PR #990 (StateServiceVNext) will eventually replace this infrastructure
2. Minimizes changes needed in this PR
3. Avoids conflicts with PR #990
4. Can be cleaned up later

## What This Enables

### Immediate Benefits

- βœ… Simplified Account model (71% code reduction)
- βœ… Clearer authentication field structure
- βœ… Easier debugging (no nested property access)
- βœ… Self-documenting code (obvious what DC needs)

### Enables Future Work

- **Phase 2 PR #2:** Remove StateFactory infrastructure
- **Phase 2 PR #3:** Delete ~90 unused jslib files including:
- EncString (only used by old nested Account)
- SymmetricCryptoKey (only used by old nested Account)
- OrganizationData (completely unused)
- ProviderData (completely unused)
- AccountKeys, AccountProfile, AccountTokens, AccountData, AccountSettings

## Merge Strategy

**Conflict Management:**

- This PR targets current codebase (with old StateService)
- Will conflict with PR #990 (StateServiceVNext) when it merges
- Plan: Rebase this PR after #990 merges
- Expected conflicts: StateService files, Account model structure
- Resolution: Keep StateServiceVNext changes, apply Account flattening to new structure

## Next Steps

1. **Review & Test:** Thorough code review and manual testing
2. **Create PR:** Open PR with comprehensive description and test results
3. **Manual Testing Scenarios:**
- Fresh installation β†’ authentication flow
- Existing installation β†’ migration runs successfully
- All directory types β†’ configuration persists correctly
- CLI authentication β†’ flat structure works
4. **After Merge:**
- Begin Phase 2 PR #2: Remove StateFactory Infrastructure
- Monitor for any migration issues in production

## Related Work

- **Depends On:** None (can merge independently)
- **Blocks:** Phase 2 PR #2 (Remove StateFactory), Phase 2 PR #3 (Delete Unused jslib Files)
- **Conflicts With:** PR #990 (StateServiceVNext) - plan to rebase after #990 merges
- **Part Of:** Phase 2 tech debt cleanup (see CLAUDE.md)

---

## Original Implementation Plan

[The original detailed step-by-step plan from the conversation has been preserved below for reference]

### Context

Directory Connector's Account model currently extends jslib's BaseAccount, inheriting 126 lines of complex nested structures designed for multi-account password manager features that DC doesn't use. This inheritance creates unnecessary coupling and blocks cleanup of unused jslib dependencies.

**Current State:**

- Account extends BaseAccount with nested objects: `profile.userId`, `tokens.accessToken`, `keys.apiKeyClientSecret`
- Only 6 fields from BaseAccount are actually used by DC
- 120+ lines of inherited code (AccountData, AccountKeys, AccountProfile, AccountSettings, AccountTokens) are unused
- Creates dependencies on EncString, SymmetricCryptoKey, OrganizationData, ProviderData that DC never uses

**Problem:**

- Unnecessary complexity for a single-account application
- Blocks deletion of unused jslib models (Phase 2 goal)
- Verbose account creation code (26 lines to set 6 fields)
- Difficult to understand what DC actually needs

**Goal:**
Flatten Account model to contain only the 8 fields DC uses, removing BaseAccount inheritance. This enables Phase 2 PR #2 and PR #3 to delete ~90 unused jslib files.

[Rest of original plan preserved in conversation transcript]
130 changes: 130 additions & 0 deletions .claude/skills/commonjs-to-esm/skill.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
---
userInvocable: true
---

# CommonJS to ESM Conversion

Convert a file (or files) from CommonJS module syntax to ECMAScript Modules (ESM).

## Usage

```
/commonjs-to-esm <file-path> [additional-file-paths...]
```

## Parameters

- `file-path` - Path to the file(s) to convert from CommonJS to ESM

## Examples

```
/commonjs-to-esm src/services/auth.service.ts
/commonjs-to-esm src/utils/helper.ts src/utils/parser.ts
```

## Process

This skill performs a comprehensive analysis and planning process:

### 1. Analyze Target File(s)

For each file to convert:

- Read the file contents
- Identify its purpose and functionality
- Catalog all CommonJS patterns used:
- `require()` statements
- `module.exports` assignments
- `exports.x = ...` assignments
- Dynamic requires
- `__dirname` and `__filename` usage

### 2. Find Dependents

- Search for all files that import/require the target file(s)
- Identify the import patterns used by dependents
- Map the dependency tree to understand impact scope

### 3. Analyze Dependencies

- List all modules the target file(s) depend on
- Determine if dependencies support ESM
- Identify potential blocking dependencies (CommonJS-only packages)
- Check for dynamic imports that may need special handling

### 4. Identify Conversion Challenges

Common issues to flag:

- `__dirname` and `__filename` (need `import.meta.url` conversion)
- Dynamic `require()` calls (need `import()` conversion)
- Conditional requires (need refactoring)
- JSON imports (need `assert { type: 'json' }`)
- CommonJS-only dependencies (may block conversion)
- Circular dependencies (may need restructuring)

### 5. Generate Conversion Plan

Create a step-by-step plan that includes:

**Target File Changes:**

- Convert `require()` to `import` statements
- Convert `module.exports` to `export` statements
- Update `__dirname`/`__filename` to use `import.meta.url`
- Handle dynamic imports appropriately
- Update file extensions if needed (e.g., `.js` to `.mjs`)

**Dependent File Changes:**

- Update all import statements in dependent files
- Ensure consistent naming (default vs named exports)
- Update path references if extensions change

**Configuration Changes:**

- `package.json`: Add `"type": "module"` or use `.mjs` extension
- `tsconfig.json`: Update `module` and `moduleResolution` settings
- Build tools: Update bundler/compiler configurations

**Testing Strategy:**

- Run unit tests after conversion
- Verify no runtime errors from import changes
- Check that all exports are accessible
- Test dynamic import scenarios

### 6. Risk Assessment

Evaluate:

- Number of files affected
- Complexity of CommonJS patterns used
- Presence of blocking dependencies
- Potential for breaking changes

### 7. Present Plan

Output a structured plan with:

- Summary of changes needed
- Ordered steps for execution
- List of files to modify
- Configuration changes required
- Testing checkpoints
- Risk factors and mitigation strategies
- Estimated scope (small/medium/large change)

## Notes

- ESM is **not** compatible with CommonJS in all cases - ESM can import CommonJS, but CommonJS **cannot** require ESM
- This means conversions should generally proceed from leaf dependencies upward
- Some packages remain CommonJS-only and may block full conversion
- The skill generates a plan but does NOT automatically execute the conversion - review and approve first

## References

- [Pure ESM package guide](https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c)
- [Node.js ESM documentation](https://nodejs.org/api/esm.html)
- [TypeScript ESM support](https://www.typescriptlang.org/docs/handbook/esm-node.html)
Loading
Loading