Skip to content
Merged
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
300 changes: 129 additions & 171 deletions docs/gigamind/pr-analysis-workflow.md

Large diffs are not rendered by default.

116 changes: 106 additions & 10 deletions docs/templates/pr-action-items-template.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,42 @@
<!--
INSTRUCTIONS FOR AGENTS:
- Use giga_read_pr to fetch PR data and CodeRabbit comments
- Focus on unresolved comments without '✅ Addressed' status
- Group related issues by root cause (not just severity)
- Include specific file paths and line numbers
- Provide clear, actionable fixes
- Add code examples for complex issues
- Prioritize by "blocking merge" vs "architectural" vs "polish"
- Create file as PR-{{NUMBER}}-ACTION-ITEMS.md in project root

CRITICAL FILTERING RULES - MUST FOLLOW EXACTLY:
1. ONLY include comments that meet ALL criteria:
✅ NO "✅ Addressed" status (must be truly unresolved)
✅ NO "nitpick" or "suggestion" labels (medium+ severity only)
✅ MEDIUM to CRITICAL impact (affects functionality, security, or architecture)
✅ NOT cosmetic/style issues (unless security/performance related)
✅ DO NOT include secrets, access tokens, API keys, private keys, MRZ data, or any PII. Redact sensitive values and replace with placeholders.

2. VERIFICATION PROCESS - MANDATORY:
- Read EACH comment's status field carefully
- Check for "✅ Addressed in commits X to Y" text
- Verify comment type: "🛠️ Refactor suggestion", "⚠️ Potential issue", etc.
- Exclude: style, formatting, naming, documentation-only suggestions
- Include: security, memory leaks, breaking changes, API inconsistencies, platform compatibility

3. SEVERITY CLASSIFICATION:
- CRITICAL: Security vulnerabilities, memory leaks, breaking platform compatibility
- HIGH: API inconsistencies, type safety issues, significant architectural problems
- MEDIUM: Test coverage gaps, minor architectural improvements, performance concerns
- LOW/NITPICK: Style, naming, documentation, minor suggestions (EXCLUDE THESE)

4. DOUBLE-CHECK PROCESS:
- After initial analysis, re-read ALL comments
- Verify each "unresolved" issue is actually unresolved
- Remove any that have been addressed in subsequent commits
- If NO unresolved medium+ issues exist, state "All issues resolved ✅"
- Run a final pass to ensure no credentials, secrets, or PII are present in examples, logs, or screenshots.

5. EXECUTION:
- Use giga_read_pr to fetch PR data and CodeRabbit comments
- Group related issues by root cause (not just severity)
- Include specific file paths and line numbers from CodeRabbit metadata
- Provide clear, actionable fixes with code examples
- Prioritize by "blocking merge" vs "architectural" vs "polish"
- Create file as PR-{{NUMBER}}-ACTION-ITEMS.md in project root
- Follow gitignore pattern: PR-*-ACTION*.md
-->

# PR {{PR_NUMBER}} Action Items
Expand All @@ -21,23 +50,52 @@ INSTRUCTIONS FOR AGENTS:

{{PR_SUMMARY}}

## Analysis Summary

<!-- IF ALL ISSUES RESOLVED, USE THIS SECTION: -->
**After thorough review of all {{TOTAL_COMMENTS}} CodeRabbit comments, ALL issues have been resolved in subsequent commits. The PR is ready for merge.**

### Resolved Comments ✅ ({{TOTAL_COMMENTS}}/{{TOTAL_COMMENTS}})
All comments show ✅ "Addressed" status, indicating they were fixed in commits:
- {{RESOLVED_CATEGORY_1}} - Fixed in commits {{COMMIT_RANGE}}
- {{RESOLVED_CATEGORY_2}} - Fixed in commits {{COMMIT_RANGE}}

### Unresolved Comments 🔴 (0/{{TOTAL_COMMENTS}})
**None** - All comments have been addressed.

## Conclusion
**This PR is ready for merge.** All CodeRabbit issues have been resolved.

<!-- IF UNRESOLVED ISSUES EXIST, USE SECTIONS BELOW INSTEAD: -->

## Critical Issues (Blocking Merge)

<!-- ONLY include if there are ACTUAL unresolved medium+ severity issues -->

### 1. {{ISSUE_TITLE}}
**Files:** `{{FILE_PATH}}:{{LINE_NUMBER}}`
**CodeRabbit Comment:** {{COMMENT_ID}}
**Problem:** {{PROBLEM_DESCRIPTION}}
**Fix:** {{SPECIFIC_FIX_OR_ACTION}}

**Code Example:**
```{{LANGUAGE}}
{{CODE_EXAMPLE}}
```

### 2. {{ISSUE_TITLE}}
**Files:** `{{FILE_PATH}}:{{LINE_NUMBER}}`
**CodeRabbit Comment:** {{COMMENT_ID}}
**Problem:** {{PROBLEM_DESCRIPTION}}
**Fix:** {{SPECIFIC_FIX_OR_ACTION}}

## Required Actions

### Issue 1: {{GROUPED_ISSUE_TITLE}}
**Files:** `{{FILE_PATH}}:{{LINE_NUMBER}}`, `{{FILE_PATH}}:{{LINE_NUMBER}}`
**Root Cause:** {{ROOT_CAUSE_DESCRIPTION}}
**Files Affected:**
- `{{FILE_PATH}}:{{LINE_NUMBER}}` - {{ISSUE_DESCRIPTION}}
- `{{FILE_PATH}}:{{LINE_NUMBER}}` - {{ISSUE_DESCRIPTION}}

**Actions:**
- [ ] {{SPECIFIC_ACTION_1}}
Expand All @@ -50,13 +108,29 @@ INSTRUCTIONS FOR AGENTS:
```

### Issue 2: {{GROUPED_ISSUE_TITLE}}
**Files:** `{{FILE_PATH}}:{{LINE_NUMBER}}`
**Root Cause:** {{ROOT_CAUSE_DESCRIPTION}}
**Files Affected:**
- `{{FILE_PATH}}:{{LINE_NUMBER}}` - {{ISSUE_DESCRIPTION}}

**Actions:**
- [ ] {{SPECIFIC_ACTION_1}}
- [ ] {{SPECIFIC_ACTION_2}}

## CodeRabbit Analysis Summary

### Resolved Comments ✅
- {{RESOLVED_COMMENT_1}}
- {{RESOLVED_COMMENT_2}}

### Unresolved Comments 🔴
- {{UNRESOLVED_COMMENT_1}} - {{STATUS}}
- {{UNRESOLVED_COMMENT_2}} - {{STATUS}}

### Comment Categories
- **Critical:** {{COUNT}} comments (React Native compatibility, security, memory leaks)
- **Architecture:** {{COUNT}} comments (API design, type safety)
- **Code Quality:** {{COUNT}} comments (testing, imports, documentation)

## Testing Checklist

### Before Merge
Expand Down Expand Up @@ -101,4 +175,26 @@ INSTRUCTIONS FOR AGENTS:

---

## Agent Verification Checklist

**BEFORE FINALIZING - VERIFY EACH ITEM:**
- [ ] ✅ Read ALL {{TOTAL_COMMENTS}} CodeRabbit comments thoroughly
- [ ] ✅ Checked each comment for "✅ Addressed" status
- [ ] ✅ Excluded all nitpick/style/documentation-only suggestions
- [ ] ✅ Only included MEDIUM+ severity issues (security, architecture, functionality)
- [ ] ✅ Verified unresolved count is accurate
- [ ] ✅ If 0 unresolved issues, used "All issues resolved" template
- [ ] ✅ Double-checked that each "unresolved" issue is actually unresolved

**SEVERITY VERIFICATION:**
- [ ] ✅ CRITICAL: Security, memory leaks, platform compatibility ({{CRITICAL_COUNT}})
- [ ] ✅ HIGH: API inconsistencies, type safety, architecture ({{HIGH_COUNT}})
- [ ] ✅ MEDIUM: Test coverage, performance, minor architecture ({{MEDIUM_COUNT}})
- [ ] ❌ LOW/NITPICK: Style, naming, docs (EXCLUDED - {{EXCLUDED_COUNT}})

---

**Last Updated:** {{DATE}}
**CodeRabbit Comments Analyzed:** {{TOTAL_COMMENTS}}
**Unresolved Medium+ Issues:** {{UNRESOLVED_COUNT}}
**Excluded Low/Nitpick Issues:** {{EXCLUDED_COUNT}}
15 changes: 13 additions & 2 deletions packages/mobile-sdk-alpha/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@selfxyz/mobile-sdk-alpha",
"version": "0.1.0",
"version": "2.6.4",
"description": "Self SDK (alpha) for registering and proving. Adapters-first, RN-first with web shims.",
"keywords": [
"self",
Expand All @@ -14,7 +14,6 @@
"exports": {
".": {
"types": "./dist/esm/index.d.ts",
"react-native": "./dist/esm/index.js",
"browser": "./dist/esm/browser.js",
"import": "./dist/esm/index.js",
"require": "./dist/cjs/index.cjs",
Expand Down Expand Up @@ -54,7 +53,15 @@
"@selfxyz/common": "workspace:*",
"tslib": "^2.6.2"
},
"peerDependencies": {
"react": "^18.3.1",
"react-native": "^0.75.4",
"tamagui": "^1.126.0"
},
"devDependencies": {
"@testing-library/react": "^14.1.2",
"@types/react": "^18.3.4",
"@types/react-dom": "^18.3.0",
"@typescript-eslint/eslint-plugin": "^8.0.0",
"@typescript-eslint/parser": "^8.0.0",
"eslint": "^8.57.0",
Expand All @@ -63,7 +70,11 @@
"eslint-plugin-prettier": "^5.5.4",
"eslint-plugin-simple-import-sort": "^12.1.1",
"eslint-plugin-sort-exports": "^0.8.0",
"jsdom": "^24.0.0",
"prettier": "^3.5.3",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-native": "0.75.4",
"tsup": "^8.0.1",
"typescript": "^5.9.2",
"vitest": "^1.6.0"
Expand Down
7 changes: 7 additions & 0 deletions packages/mobile-sdk-alpha/src/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,15 @@ export type { QRProofOptions } from './qr';
export type { SdkErrorCategory } from './errors';

export { SCANNER_ERROR_CODES, notImplemented, sdkError } from './errors';
export { SelfClientContext, SelfClientProvider, useSelfClient } from './context';
// Browser-only high-level component (DOM-based)
Copy link
Contributor

Choose a reason for hiding this comment

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

ah. this isnt true theres nothing about SelfMobileSdk that is browser or dom based. it supposed to be react native based

export { SelfMobileSdk as SelfMobileSdkHighLevel } from './components/SelfMobileSdk';

export { createSelfClient } from './client';

export { defaultConfig } from './config/defaults';

/** @deprecated Use createSelfClient().extractMRZInfo or import from './mrz' */
export { extractMRZInfo, formatDateToYYMMDD, scanMRZ } from './mrz';

// Core functions
Expand Down
19 changes: 19 additions & 0 deletions packages/mobile-sdk-alpha/src/client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { defaultConfig } from './config/defaults';
import { mergeConfig } from './config/merge';
import { notImplemented } from './errors';
import { extractMRZInfo as parseMRZInfo } from './processing/mrz';
import type {
Adapters,
Config,
Expand All @@ -19,6 +20,11 @@ import type {
ValidationResult,
} from './types/public';

/**
* Optional adapter implementations used when a consumer does not provide their
* own. These defaults are intentionally minimal no-ops suitable for tests and
* non-production environments.
*/
const optionalDefaults: Partial<Adapters> = {
storage: {
get: async () => null,
Expand All @@ -36,6 +42,13 @@ const optionalDefaults: Partial<Adapters> = {
},
};

/**
* Creates a fully configured {@link SelfClient} instance.
*
* The function validates that all required adapters are supplied and merges the
* provided configuration with sensible defaults. Missing optional adapters are
* filled with benign no-op implementations.
*/
export function createSelfClient({ config, adapters }: { config: Config; adapters: Partial<Adapters> }): SelfClient {
const cfg = mergeConfig(defaultConfig, config);
const required: (keyof Adapters)[] = ['scanner', 'network', 'crypto'];
Expand Down Expand Up @@ -77,6 +90,10 @@ export function createSelfClient({ config, adapters }: { config: Config; adapter
return { registered: false, reason: 'SELF_REG_STATUS_STUB' };
}

async function registerDocument(_input: RegistrationInput): Promise<RegistrationStatus> {
return { registered: false, reason: 'SELF_REG_STATUS_STUB' };
}

async function generateProof(
_req: ProofRequest,
opts: {
Expand All @@ -101,7 +118,9 @@ export function createSelfClient({ config, adapters }: { config: Config; adapter
scanDocument,
validateDocument,
checkRegistration,
registerDocument,
generateProof,
extractMRZInfo: parseMRZInfo,
on,
emit,
};
Expand Down
64 changes: 64 additions & 0 deletions packages/mobile-sdk-alpha/src/components/SelfMobileSdk.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import type { ComponentType, ReactNode } from 'react';

import { SelfClientProvider } from '../context';
import { useDocumentManager } from '../hooks/useDocumentManager';
import type { Adapters, Config } from '../types/public';
import type { ExternalAdapter, PassportCameraProps, ScreenProps } from '../types/ui';
import { OnboardingFlow } from './flows/OnboardingFlow';
import { QRCodeScreen } from './screens/QRCodeScreen';

interface SelfMobileSdkProps {
config: Config;
adapters?: Partial<Adapters>;
external: ExternalAdapter;
children?: ReactNode;
// Optional custom components
customScreens?: {
PassportCamera?: ComponentType<PassportCameraProps>;
NFCScanner?: ComponentType<ScreenProps>;
QRScanner?: ReactNode;
};
}

const SelfMobileSdkContent = ({
external,
customScreens = {},
}: {
external: ExternalAdapter;
customScreens?: SelfMobileSdkProps['customScreens'];
}) => {
const { documents, isLoading, hasRegisteredDocuments } = useDocumentManager(external);

if (isLoading) {
return <div>Loading documents...</div>;
}

// Check if user has any registered documents
const hasDocuments = Object.keys(documents).length > 0 && hasRegisteredDocuments();

if (!hasDocuments) {
return (
<OnboardingFlow
external={external}
setDocument={external.setDocument}
PassportCamera={customScreens.PassportCamera}
NFCScanner={customScreens.NFCScanner}
/>
);
}

// Show disclosure flow
return (
customScreens.QRScanner || (
<QRCodeScreen onSuccess={external.onDisclosureSuccess} onFailure={external.onDisclosureFailure} />
)
);
};

export const SelfMobileSdk = ({ config, adapters = {}, external, children, customScreens }: SelfMobileSdkProps) => {
return (
<SelfClientProvider config={config} adapters={adapters}>
{children || <SelfMobileSdkContent external={external} customScreens={customScreens} />}
</SelfClientProvider>
);
};
Loading
Loading