Conversation
…or-url) Shared utilities for integration wizards: - ensureCredentials(): check for existing auth, prompt for API key if missing - askBrowseOrUrl(): common "browse or paste URL" prompt - askForUrl(): URL input with domain validation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Interactive wizard for working with Notion pages: - Auth check (Notion API token) - Search pages by query with "search again" loop, or paste URL - Extracts page titles from Notion's property structure - Delegates to runCommand() for execution Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add notion wizard to CLI as top-level command - Add wizard fallback in run.ts for --from notion without project Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add interactive wizard section to Notion source docs - Add ralph-starter notion to README commands table Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Issue Linking ReminderThis PR doesn't appear to have a linked issue. Consider linking to:
Using If this PR doesn't need an issue, you can ignore this message. |
github-actions
bot
added
candidate-release
✔️ Bundle Size Analysis
Bundle breakdown |
🔗 Docs PreviewPreview URL: https://feat-notion-integration-wiza.ralph-starter-docs.pages.dev This preview was deployed from the latest commit on this PR. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| headers: { | ||
| Authorization: `Bearer ${token}`, | ||
| 'Notion-Version': NOTION_API_VERSION, | ||
| 'Content-Type': 'application/json', | ||
| }, |
Check warning
Code scanning / CodeQL
File data in outbound network request Medium
Greptile SummaryThis PR introduces an interactive Notion wizard command ( Key changes and findings:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant CLI as cli.ts
participant Run as run.ts
participant Notion as notion.ts (wizard)
participant Shared as shared.ts
participant Config as sources/config.ts
participant API as Notion API
User->>CLI: ralph-starter notion [opts]
CLI->>Notion: notionCommand(opts)
Notion->>Shared: ensureCredentials('notion', ...)
Shared->>Config: getSourceCredentials('notion')
Config-->>Shared: creds or null
alt No credentials found
Shared->>User: Prompt for token
User-->>Shared: token input
Shared->>Config: setSourceCredential('notion', 'token', value)
end
Shared-->>Notion: token string
Notion->>Shared: askBrowseOrUrl('Notion')
Shared-->>Notion: 'browse' | 'url'
alt URL mode
Notion->>Shared: askForUrl('Notion', pattern)
Shared-->>Notion: url
Notion->>Run: runCommand(undefined, {from:'notion', project:url, ...})
else Browse mode
Notion->>Config: getSourceCredentials('notion')
Config-->>Notion: creds
loop Search loop
Notion->>User: Prompt search query
User-->>Notion: query
Notion->>API: POST /search {query}
API-->>Notion: results[]
Notion->>User: Select page or Search again
User-->>Notion: selectedUrl or SEARCH_AGAIN
end
Notion->>Run: runCommand(undefined, {from:'notion', project:selectedUrl, ...})
end
Note over User,Run: Fallback path
User->>CLI: ralph-starter run --from notion
CLI->>Run: runCommand(task, {from:'notion', ...})
Run->>Notion: notionCommand(opts) [dynamic import]
Last reviewed commit: 249a6d2 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
src/commands/notion.ts
Outdated
| export interface NotionWizardOptions { | ||
| commit?: boolean; | ||
| push?: boolean; | ||
| pr?: boolean; | ||
| validate?: boolean; | ||
| maxIterations?: number; | ||
| agent?: string; | ||
| } | ||
|
|
||
| const NOTION_API_BASE = 'https://api.notion.com/v1'; | ||
| const NOTION_API_VERSION = '2022-06-28'; | ||
|
|
||
| interface NotionSearchResult { |
There was a problem hiding this comment.
Use type instead of interface for simple data structures
Per the project style guide, type should be used by default unless you specifically need interface features (extension / implementation). Both NotionWizardOptions and NotionSearchResult are plain data structures with no inheritance needs.
| export interface NotionWizardOptions { | |
| commit?: boolean; | |
| push?: boolean; | |
| pr?: boolean; | |
| validate?: boolean; | |
| maxIterations?: number; | |
| agent?: string; | |
| } | |
| const NOTION_API_BASE = 'https://api.notion.com/v1'; | |
| const NOTION_API_VERSION = '2022-06-28'; | |
| interface NotionSearchResult { | |
| export type NotionWizardOptions = { | |
| commit?: boolean; | |
| push?: boolean; | |
| pr?: boolean; | |
| validate?: boolean; | |
| maxIterations?: number; | |
| agent?: string; | |
| }; |
The same applies to interface NotionSearchResult at line 28 — it should be declared as type NotionSearchResult = { ... }.
This pattern is also violated in src/integrations/wizards/shared.ts:12 where export interface CredentialOptions should be export type CredentialOptions = { ... }.
Rule Used: Use type by default in TypeScript unless you spe... (source)
Learnt From
cytonic-network/ai-frontend#48
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/notion.ts
Line: 16-28
Comment:
**Use `type` instead of `interface` for simple data structures**
Per the project style guide, `type` should be used by default unless you specifically need `interface` features (extension / implementation). Both `NotionWizardOptions` and `NotionSearchResult` are plain data structures with no inheritance needs.
```suggestion
export type NotionWizardOptions = {
commit?: boolean;
push?: boolean;
pr?: boolean;
validate?: boolean;
maxIterations?: number;
agent?: string;
};
```
The same applies to `interface NotionSearchResult` at line 28 — it should be declared as `type NotionSearchResult = { ... }`.
This pattern is also violated in `src/integrations/wizards/shared.ts:12` where `export interface CredentialOptions` should be `export type CredentialOptions = { ... }`.
**Rule Used:** Use `type` by default in TypeScript unless you spe... ([source](https://app.greptile.com/review/custom-context?memory=c862f053-5655-4b41-be69-c840e3c9f280))
**Learnt From**
[cytonic-network/ai-frontend#48](https://github.com/cytonic-network/ai-frontend/pull/48)
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
src/commands/notion.ts
Outdated
| // Browse mode — search for pages | ||
| // Get the actual token for API calls (ensureCredentials may have returned '__cli_auth__') | ||
| const creds = await import('../sources/config.js').then((m) => m.getSourceCredentials('notion')); | ||
| const token = process.env.NOTION_API_KEY || creds?.token; |
There was a problem hiding this comment.
Dynamic import used to break circular dependency
notion.ts statically imports runCommand from run.ts (line 14), while run.ts dynamically imports notionCommand from notion.ts (line 347). This is a circular dependency handled at runtime via the dynamic import. While it works in Node.js/ESM, it is a recognised architecture smell and can cause hard-to-debug issues as the codebase grows (e.g., if the static import graph is analysed by bundlers or test runners).
Consider extracting the shared NotionWizardOptions type and/or a thin launcher helper into a separate module (e.g., src/commands/notion-wizard.ts) so that run.ts can import the wizard without creating a cycle. At a minimum, a short comment explaining the intentional cycle would help future maintainers.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/notion.ts
Line: 143-146
Comment:
**Dynamic import used to break circular dependency**
`notion.ts` statically imports `runCommand` from `run.ts` (line 14), while `run.ts` dynamically imports `notionCommand` from `notion.ts` (line 347). This is a circular dependency handled at runtime via the dynamic import. While it works in Node.js/ESM, it is a recognised architecture smell and can cause hard-to-debug issues as the codebase grows (e.g., if the static import graph is analysed by bundlers or test runners).
Consider extracting the shared `NotionWizardOptions` type and/or a thin launcher helper into a separate module (e.g., `src/commands/notion-wizard.ts`) so that `run.ts` can import the wizard without creating a cycle. At a minimum, a short comment explaining the intentional cycle would help future maintainers.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
src/commands/notion.ts
Outdated
| // Browse mode — search for pages | ||
| // Get the actual token for API calls (ensureCredentials may have returned '__cli_auth__') | ||
| const creds = await import('../sources/config.js').then((m) => m.getSourceCredentials('notion')); | ||
| const token = process.env.NOTION_API_KEY || creds?.token; |
There was a problem hiding this comment.
Missing apiKey fallback when retrieving Notion token
getSourceCredentials('notion') can return credentials stored under either the token key (saved by ensureCredentials) or the apiKey key (populated from the environment-variable mapping defined in sources/config.ts). The current line only checks creds?.token, so if the credential object only carries an apiKey property — for example, when it was previously set through a non-wizard config path — the token will be undefined and the check at line 148 will show the "Could not obtain Notion API token" error even though a valid credential exists.
Add a final || creds?.apiKey fallback to cover this case, matching the same fallback chain already used in shared.ts (existing?.token || existing?.apiKey).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/commands/notion.ts
Line: 146
Comment:
**Missing `apiKey` fallback when retrieving Notion token**
`getSourceCredentials('notion')` can return credentials stored under either the `token` key (saved by `ensureCredentials`) or the `apiKey` key (populated from the environment-variable mapping defined in `sources/config.ts`). The current line only checks `creds?.token`, so if the credential object only carries an `apiKey` property — for example, when it was previously set through a non-wizard config path — the token will be `undefined` and the check at line 148 will show the "Could not obtain Notion API token" error even though a valid credential exists.
Add a final `|| creds?.apiKey` fallback to cover this case, matching the same fallback chain already used in `shared.ts` (`existing?.token || existing?.apiKey`).
How can I resolve this? If you propose a fix, please make it concise.
rubenmarcus
changed the title
- Change interface to type for plain data structures (Greptile) - Anchor Notion URL regex pattern (CodeQL fix) - Add apiKey fallback when retrieving Notion token (Greptile) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
ralph-starter notiontop-level command for interactive Notion page selectionsrc/integrations/wizards/shared.tsralph-starter run --from notionis used without--projectChanges
src/commands/notion.ts— Notion wizard with search API integrationsrc/cli.ts— Registerralph-starter notioncommandsrc/commands/run.ts— Wizard fallback for--from notiondocs/docs/sources/notion.md— Wizard section + examplesREADME.md— Commands table entryTest plan
ralph-starter notionlaunches wizard and completes flowralph-starter run --from notion(no --project) redirects to wizardralph-starter run --from notion --project "Product Specs"still works (existing behavior)pnpm build— no new errors (pre-existing pixelmatch/sharp errors only)🤖 Generated with Claude Code