This repository was archived by the owner on May 9, 2026. It is now read-only.
Conversation
…ript for Notion sync pipeline, including page discovery, content extraction, and memory ingestion. Enhance sync status tracking with new properties for phase, progress, and messages. Refactor sync logic to improve error handling and provide detailed progress updates during synchronization.
…ining sync status logging to include detailed progress, phase, and message information. Adjusted content sync progress reporting for better clarity on page fetching status. Improved error handling and logging consistency across sync operations.
… DB, removing content_text to prevent JSON serialization issues. Updated comments for clarity on data handling.
…nd list-users calls. Introduce logging wrapper for tool execution to enhance debugging with entry/exit logs, including execution time and error handling.
…m content length requirement to prevent crashes in the embedding model. Update Gmail tools to log execution details for better debugging. Refactor email metadata publishing to exclude body text, ensuring JSON transport integrity. Add OAuth proxy details and test harness RPC methods to documentation for clarity.
Implement skill and runtime building steps in the dev-runtime script, including error handling and logging for build success or failure. This enhancement improves the development workflow by automating the build process for skills and runtime components.
…consistency. Update email metadata publishing format for better clarity. Enhance sync logic by increasing max pages per content sync and refining progress reporting. Adjust error handling in tool execution to ensure robust logging.
… test script for the Notion API to evaluate connection pooling and timeout issues, including detailed logging and error handling for sequential requests.
…tion and hardcoding the latest API version (2026-03-11). Update related functions and tools to streamline database queries and improve compatibility. Enhance live test scripts to utilize increased page sizes for better performance during testing.
…tterns, improving performance and readability. Update function signatures to return values directly instead of promises where applicable. Enhance error handling and logging consistency across various API interactions, ensuring robust functionality and clearer debugging.
…ructions and update global types by removing the model API. Enhance Notion sync logic with improved state logging and error handling during the sync process. Streamline sync phases for better clarity and performance, including a new pipeline approach for page discovery and ingestion.
…clarity. Update logging for skill operations and enhance error handling. Streamline environment variable checks and consolidate helper functions for better readability. Adjust sync logic to focus on metadata and content ingestion processes, ensuring robust performance during testing.
…onous functions, enhancing user interaction and responsiveness. Update function signatures to return promises, ensuring proper handling of asynchronous operations throughout the script. Improve error handling and logging for better debugging during skill operations.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Query each newly synced database's rows via queryDataSource, upsert them into the database_rows table, and ingest their property text into memory for semantic search. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…unction syntax and enhancing formatting consistency in property text extraction.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughConverted many async Promise-based APIs, skill hooks, and tool executes to synchronous signatures; removed Notion API-version detection; refactored Gmail and Notion syncs to inline per-item ingestion with new ingest tracking and progress fields; added tool execution logging and new live-test scripts; updated types and docs accordingly. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Prevents ONNX/CoreML crash on short database row content that tokenizes
to zero tokens (shape {0} error in GliNER/GliREL).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Create live-test-sync.ts for Gmail (mirrors Notion sync test pattern) - Fix sleep() in gmail API to use busy-wait instead of broken setTimeout - Increase MIN_CONTENT_LENGTH to 50 to prevent ONNX shape errors Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… code and improve readability.
…stion - Use Gmail batch API to fetch up to 50 messages per HTTP request (with individual fallback when batch fails) - Increase PAGE_SIZE to 100, cap at MAX_EMAILS=1000 - Filter out spam and trash from sync queries (-in:spam -in:trash) - Skip messages with SPAM/TRASH labels during ingestion - Pipeline approach: upsert + memory.insert immediately per message - Support rawBatch mode in gmailFetch for multipart/mixed responses Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
… date for better uniqueness
All memory.insert() calls now use timestamp-prefixed document IDs:
- Gmail: {internalDate}-gmail-email-{id}
- Notion pages: {lastEditedTime}-notion-page-{id}
- Notion DB rows: {lastEditedTime}-notion-dbrow-{id}
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Gmail: - Check backend_submitted before re-ingesting emails into memory - Mark emails as submitted after successful memory.insert - Add backend_submitted to DatabaseEmail type Notion: - Check getDatabaseRowById + backend_submitted before re-ingesting DB rows - Mark rows as submitted via markRowsSubmitted after ingestion Both skills now skip documents that have already been synced and ingested, avoiding redundant memory.insert calls on subsequent syncs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (17)
src/core/notion/tools/get-page-content.ts (1)
36-49:⚠️ Potential issue | 🟠 Major
recursive=truestill returns an incomplete block tree.The tool description promises nested blocks, but this only does one
getBlockChildren()pass per top-level block. Grandchildren and paginated child blocks are silently dropped, so deep Notion pages still come back incomplete.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/notion/tools/get-page-content.ts` around lines 36 - 49, The current implementation in get-page-content uses a single notionApi.getBlockChildren call per top-level block so setting recursive=true only fetches one level and drops paginated children and deeper descendants; fix by implementing a recursive fetchAllChildren helper that (1) paginates getBlockChildren using has_more/next_cursor to collect all child blocks, (2) maps each child through formatBlockSummary, and (3) for each child that is a block and when recursive is true, call the helper recursively to attach grandchildren, then use that result when building blocks instead of the single childrenResult; update the code paths around notionApi.getBlockChildren, formatBlockSummary, and the recursive flag to use this helper.src/core/server-ping/tools/read-config.ts (1)
11-15:⚠️ Potential issue | 🟠 MajorReplace nullish coalescing with ternary operator for QuickJS compatibility.
Line 14 uses
??which QuickJS does not support. Use a null check instead:- return raw ?? JSON.stringify({ error: 'No config file found' }); + return raw !== null ? raw : JSON.stringify({ error: 'No config file found' });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/server-ping/tools/read-config.ts` around lines 11 - 15, In execute() inside read-config.ts, replace the nullish coalescing usage on the result of data.read('config.json') with an explicit null check/ternary so QuickJS can run; e.g., inspect the variable returned by data.read in execute(), and return raw !== null && raw !== undefined ? raw : JSON.stringify({ error: 'No config file found' }) instead of using ??.src/core/server-ping/tools/get-ping-stats.ts (1)
25-35:⚠️ Potential issue | 🟠 MajorRemove optional chaining (
?.) from this QuickJS-targeted tool.Line 34 uses
avgLatency?.avg_ms, which violates the ES2019 constraint for QuickJS. Additionally, the subsequent truthy check treats a legitimate0average latency as falsy, incorrectly returningnullinstead of the rounded value.♻️ Proposed fix
- avgLatencyMs: avgLatency?.avg_ms ? Math.round(avgLatency.avg_ms) : null, + avgLatencyMs: + avgLatency && avgLatency.avg_ms !== null && avgLatency.avg_ms !== undefined + ? Math.round(avgLatency.avg_ms) + : null,Per coding guidelines: "In TypeScript skill files, do NOT use optional chaining (
?.) or nullish coalescing (??) operators as QuickJS runtime does not support ES2020+ features."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/server-ping/tools/get-ping-stats.ts` around lines 25 - 35, The return object uses optional chaining on avgLatency (avgLatency?.avg_ms) which QuickJS doesn't support and also treats 0 as falsy; update the avgLatency handling in the get-ping-stats return so it does not use ?. or ?? and instead checks explicitly (e.g. if avgLatency exists and typeof avgLatency.avg_ms === 'number') then set avgLatencyMs to Math.round(avgLatency.avg_ms) else null; locate the avgLatency variable and the return block constructing avgLatencyMs and replace the optional-chaining/truthy check with this explicit numeric check.src/core/gmail/tools/get-profile.ts (1)
14-17:⚠️ Potential issue | 🟠 MajorRemove
accessTokenfrom the tool schema or implement token threading ingmailFetch.The tool documents an
accessTokeninput, but theexecutefunction cannot use it—gmailFetch()has no mechanism to accept caller-supplied tokens. It always resolves from internal credential sources (advanced auth or OAuth credential). Either removeaccessTokenfrominput_schemaand the description, or modifygmailFetch()to accept and use a per-call token parameter when provided.Also note:
get-emails.tshas the same contract violation (line 77 claims accessToken "is used directly" but does not actually pass it togmailFetch()).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/tools/get-profile.ts` around lines 14 - 17, The schema exposes an accessToken field that isn’t threaded into gmailFetch, causing a contract mismatch; either remove accessToken from the tool input_schema (and its description) in get-profile's input_schema and similarly in get-emails, or modify gmailFetch to accept an optional per-call token (e.g., add an argument like tokenOrAccessToken to the gmailFetch function signature and propagate it through its internal auth resolution), then update the execute functions in get-profile (and get-emails) to pass the caller-supplied accessToken into gmailFetch (with clear precedence: use provided token if present, otherwise fall back to existing credential resolution) and ensure logging/validation handles the new parameter.src/core/gmail/tools/get-email.ts (1)
137-137:⚠️ Potential issue | 🟡 MinorAvoid nullish coalescing operator (
??) — QuickJS does not support ES2020+ features.Line 137 uses
??which is not supported in the QuickJS runtime. Use||or a ternary instead.Proposed fix
- const showSensitive = s.config.showSensitiveMessages ?? false; + const showSensitive = s.config.showSensitiveMessages || false;As per coding guidelines: "In TypeScript skill files, do NOT use optional chaining (
?.) or nullish coalescing (??) operators as QuickJS runtime does not support ES2020+ features."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/tools/get-email.ts` at line 137, Replace the nullish coalescing usage in the get-email logic: the variable showSensitive (assigned as const showSensitive = s.config.showSensitive ?? false) uses ?? which QuickJS doesn't support; change this assignment to use a QuickJS-safe fallback (e.g., use || or a ternary) so showSensitive defaults to false when s.config.showSensitive is null/undefined/falsey, and update the assignment site in the get-email.ts function where showSensitive is defined.src/core/gmail/tools/send-email.ts (2)
111-127:⚠️ Potential issue | 🟠 MajorReject CR/LF in header fields before building
rawMessage.
fromEmail,subject,addr.name, andaddr.emailare interpolated straight into RFC 822 headers. A newline in any of them can inject extra headers or corrupt the MIME body.Also applies to: 233-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/tools/send-email.ts` around lines 111 - 127, Header injection risk: sanitize or reject CR/LF characters before interpolating into rawMessage in send-email.ts—specifically validate fromEmail, subject and any address fields used by formatEmailAddresses (addr.email and addr.name) to ensure they do not contain '\r' or '\n'; either strip/normalize those characters or throw a validation error and fail sending. Update the code that builds rawMessage (the blocks appending From, To, Cc, Bcc, Subject, In-Reply-To) and the formatEmailAddresses helper to perform this check so no newline characters can be injected into headers (also apply same fix to the other occurrence mentioned around lines 233-241).
97-105:⚠️ Potential issue | 🟠 MajorRemove the remaining
?.and??operators from this execute function.The function still uses QuickJS-incompatible ES2020+ syntax in multiple places:
- Line 98:
s.profile?.emailAddress- Line 198:
response.error?.message- Lines 214-216:
sentMessage?.id ?? null,sentMessage?.threadId ?? null,sentMessage?.labelIds ?? []Replace with ternary operators or
||: usex ? x.y : nullinstead ofx?.y, andx !== null && x !== undefined ? x : fallbackinstead ofx ?? fallback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/tools/send-email.ts` around lines 97 - 105, The execute function still uses optional chaining and nullish coalescing incompatible with QuickJS; replace all instances: for getGmailSkillState() usage change s.profile?.emailAddress to s.profile ? s.profile.emailAddress : null (or s.profile ? s.profile.emailAddress : undefined as appropriate) in the execute function; change response.error?.message to response.error ? response.error.message : null; and change sentMessage?.id ?? null, sentMessage?.threadId ?? null, sentMessage?.labelIds ?? [] to the explicit checks sentMessage ? (sentMessage.id !== undefined && sentMessage.id !== null ? sentMessage.id : null) : null, sentMessage ? (sentMessage.threadId !== undefined && sentMessage.threadId !== null ? sentMessage.threadId : null) : null, and sentMessage ? (sentMessage.labelIds !== undefined && sentMessage.labelIds !== null ? sentMessage.labelIds : []) : [] respectively (or use x !== null && x !== undefined ? x : fallback pattern); update all occurrences around getGmailSkillState, response, and sentMessage variables to follow this pattern.src/core/server-ping/tools/update-server-url.ts (1)
12-25:⚠️ Potential issue | 🟠 MajorReplace nullish coalescing, optional chaining, and tighten URL validation to meet QuickJS compatibility requirements.
Lines 13 and 25 use ES2020+ operators not supported by QuickJS. Additionally,
startsWith('http')on line 14 is too permissive and contradicts the error message.Proposed fix
execute(args: Record<string, unknown>): string { - const url = ((args.url as string) ?? '').trim(); - if (!url || !url.startsWith('http')) { + const rawUrl = args.url !== null && args.url !== undefined ? String(args.url) : ''; + const url = rawUrl.trim(); + if (!url || !(url.startsWith('http://') || url.startsWith('https://'))) { return JSON.stringify({ error: 'Invalid URL — must start with http:// or https://' }); } @@ - (globalThis as { publishState?: () => void }).publishState?.(); + const publishState = (globalThis as { publishState?: () => void }).publishState; + if (publishState) { + publishState(); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/server-ping/tools/update-server-url.ts` around lines 12 - 25, The execute function currently uses nullish coalescing and optional chaining and a too-permissive URL check; update execute to avoid '??' and '?.' by using explicit typeof/undefined checks and conditional property access (e.g., getSkillState() result checked before reading config), validate the url variable by ensuring args.url is a string, trim it, and check it startsWith either 'http://' or 'https://' (not just 'http'), keep setting s.config.serverUrl and calling state.set('config', s.config) as before, and invoke publishState only after confirming globalThis.publishState is a function (e.g., check typeof publishState === 'function' before calling) to maintain QuickJS compatibility.src/core/gmail/tools/get-emails.ts (1)
9-71:⚠️ Potential issue | 🟠 MajorReplace
?.and??operators with QuickJS-compatible alternatives throughout this tool file.This file uses ES2020+ operators (
?.and??) in multiple functions that violate the QuickJS compatibility requirement forsrc/**/*.tsfiles.Violations found:
- Lines 20:
args.max_results ?? args.maxResults ?? 20→ useargs.max_results || args.maxResults || 20- Lines 34–37, 44, 46, 52–53, 66–68: Optional chaining in
buildListParams,hasAttachments,messageToEmailRow→ replace with ternary operators or||- Lines 130, 144, 173, 184: Optional chaining and nullish coalescing in
executefunction → use ternary or||insteadExample fixes:
message.payload?.body?.attachmentId→message.payload && message.payload.body && message.payload.body.attachmentIdx ?? y→x !== null && x !== undefined ? x : yorx || ywhere appropriate🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/tools/get-emails.ts` around lines 9 - 71, The file uses optional chaining (?.) and nullish coalescing (??) which QuickJS doesn't support; update all occurrences in buildListParams, hasAttachments, messageToEmailRow, and the execute function to use QuickJS-compatible checks: replace x?.y?.z with explicit guarded accesses (e.g., x && x.y && x.y.z) and replace a ?? b with either a !== null && a !== undefined ? a : b (or a || b where the intent allows falsy values to fall through); specifically change args.max_results ?? args.maxResults ?? 20 to a null/undefined-safe expression, replace message.payload?.body?.attachmentId and other payload/parts/header optional chains with guarded access, and adjust headers/find/getHeader calls that use ?. to use conditional checks so behavior is preserved.src/core/notion/tools/append-text.ts (1)
20-29:⚠️ Potential issue | 🟡 MinorUpdate schema to reflect that
textandcontentare interchangeable.The schema declares
required: ['text'], but the execute function treatscontentas an equally valid input (lines 35-39). The schema should be updated to accurately represent this contract: either field satisfies the requirement, but at least one must be provided.Since the code already validates at runtime (lines 49-54) that at least one of them is present, the schema should reflect this by changing:
- required: ['text'], + required: [],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/notion/tools/append-text.ts` around lines 20 - 29, The JSON schema currently requires 'text' but the execute function accepts either 'text' or 'content'; update the schema so the required constraint enforces that at least one of 'text' or 'content' is present (e.g., replace required: ['text'] with an anyOf/oneOf or a custom validation requiring either property) so the schema matches the runtime check in the execute function; ensure you reference the same property names ('text' and 'content') in the updated schema and keep the existing descriptions intact.src/core/gmail/api/index.ts (1)
147-157:⚠️ Potential issue | 🟠 Major401 retries still reuse the stale bearer token.
accessTokenis captured before the retry loop. After Line 210 clears the cache and Line 211 resolves a fresh token, the next iteration still constructsAuthorizationfrom the original value, so self-hosted auth never actually recovers from a 401.🔁 Re-resolve the token per retry attempt
- const accessToken = resolveAccessToken(); const oauthCred = oauth.getCredential(); - const useProxy = !accessToken && !!oauthCred; - - if (!accessToken && !useProxy) { - console.log('[gmail] gmailFetch: no access token and no OAuth credential'); - return { - success: false, - error: { code: 401, message: 'Gmail not connected. Complete setup first.' }, - }; - } for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) { + const accessToken = resolveAccessToken(); + const useProxy = !accessToken && !!oauthCred; + + if (!accessToken && !useProxy) { + console.log('[gmail] gmailFetch: no access token and no OAuth credential'); + return { + success: false, + error: { code: 401, message: 'Gmail not connected. Complete setup first.' }, + }; + } + try { let response: { status: number; headers: Record<string, string>; body: string };Also applies to: 207-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/api/index.ts` around lines 147 - 157, The code captures accessToken once (resolveAccessToken) before the retry loop, so after a 401 and cache clear the loop still uses the stale token; to fix, re-resolve the token inside each retry attempt (move or call resolveAccessToken within the retry logic) and rebuild the Authorization header from that freshly-resolved token (and likewise re-check oauth.getCredential/useProxy per attempt); ensure the retry flow that clears the cache (the logic around lines referencing cache clear and re-resolve) uses the new token on the subsequent request so self-hosted OAuth can recover from 401s.src/core/gmail/tools/search-emails.ts (1)
83-95:⚠️ Potential issue | 🟠 MajorThis batching loop is now fully serial.
gmailFetchreturns synchronously in this PR, sobatch.map(...)executes each metadata request one after another. For larger searches the tool still does dozens of inline follow-up calls, so the timeout mitigation in the comment no longer applies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/tools/search-emails.ts` around lines 83 - 95, The batching loop is effectively serial because gqlFetch/gmailFetch calls are being invoked synchronously inside batch.map; fix by making the batch fetches execute concurrently: inside the loop map each msgRef to a promise (call gmailFetch but do not await inside the map), then await Promise.all on that array so the batch runs in parallel; use the existing CONCURRENCY constant and the same message URL construction (msgRef.id) and then merge the resolved results into emails. Reference: CONCURRENCY, refs, gmailFetch, emails, msgRef.src/core/gmail/sync.ts (1)
81-99:⚠️ Potential issue | 🟠 MajorReplace optional chaining with explicit guards for QuickJS compatibility.
Lines 93, 137, 155, and 186 use the
?.operator, which QuickJS does not support. Rewrite using ternary operators or explicit checks per the guideline:x ? x.y : nullinstead ofx?.y, andx !== null && x !== undefined ? x : fallbackinstead ofx ?? fallback.Instances found:
93: if (!response.success || !response.data?.messages) { 137: log?.(`Fetching page ${page}...`, Math.min(5 + page * 8, 80)); 155: log?.(`Page ${page}: ${newEmails} new, ${skipped} skipped`, Math.min(10 + page * 10, 90)); 186: onProgress?.(msg, pct);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/sync.ts` around lines 81 - 99, Several occurrences of the optional chaining operator (?.) are used and must be rewritten for QuickJS compatibility: in fetchMessagePage replace response.data?.messages with an explicit guard like response.data ? response.data.messages : undefined (and update the conditional to check for that), and replace all call-site optional chains log?.(...) and onProgress?.(...) with explicit checks such as if (log) log(...); and if (onProgress) onProgress(...); follow the guideline for nullish coalescing where needed (x !== null && x !== undefined ? x : fallback) and update the code around the symbols fetchMessagePage, log, and onProgress accordingly.src/core/gmail/index.ts (2)
441-441:⚠️ Potential issue | 🔴 CriticalIncorrect radix value in
parseInt— 100 should be 10.The radix
100is out of range (valid: 2-36) and will causeparseIntto returnNaN.🐛 Proposed fix
- s.config.maxEmailsPerSync = parseInt(args.value as string, 100); + s.config.maxEmailsPerSync = parseInt(args.value as string, 10);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/index.ts` at line 441, The code sets s.config.maxEmailsPerSync using parseInt(args.value as string, 100) which uses an invalid radix; replace the radix 100 with 10 (e.g., use parseInt(args.value as string, 10) or Number(args.value)) so parseInt returns the intended decimal integer for s.config.maxEmailsPerSync when parsing args.value.
410-410:⚠️ Potential issue | 🔴 CriticalNullish coalescing operator (
??) is not supported in QuickJS.🔧 Suggested fix
- value: s.config.showSensitiveMessages ?? false, + value: s.config.showSensitiveMessages !== null && s.config.showSensitiveMessages !== undefined ? s.config.showSensitiveMessages : false,As per coding guidelines: "do NOT use optional chaining (
?.) or nullish coalescing (??) operators as QuickJS runtime does not support ES2020+ features."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/index.ts` at line 410, The value assignment using the nullish coalescing operator on s.config.showSensitiveMessages must be rewritten without "??" because QuickJS doesn't support it; locate the property where value: s.config.showSensitiveMessages ?? false is set (the assignment for showSensitiveMessages) and replace the expression with an explicit null/undefined check (e.g., value: (s.config && s.config.showSensitiveMessages !== undefined && s.config.showSensitiveMessages !== null) ? s.config.showSensitiveMessages : false or an equivalent ternary/check) so that it behaves the same but uses only ES5-compatible operators.src/core/notion/helpers.ts (2)
46-46:⚠️ Potential issue | 🔴 CriticalNullish coalescing operator (
??) is not supported in QuickJS.Replace with a ternary or explicit null/undefined checks per coding guidelines.
🔧 Suggested fix
- const token = (creds.api_token ?? creds.content ?? creds.access_token) as string | undefined; + const token = (creds.api_token !== null && creds.api_token !== undefined ? creds.api_token : (creds.content !== null && creds.content !== undefined ? creds.content : creds.access_token)) as string | undefined;Or more readably:
const token = (creds.api_token || creds.content || creds.access_token) as string | undefined;As per coding guidelines: "do NOT use optional chaining (
?.) or nullish coalescing (??) operators as QuickJS runtime does not support ES2020+ features."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/notion/helpers.ts` at line 46, The expression assigning to token uses the nullish coalescing operator (creds.api_token ?? creds.content ?? creds.access_token) which QuickJS doesn't support; replace it with an ES5-compatible fallback such as chaining logical ORs or explicit null/undefined checks so the assignment to the token variable uses creds.api_token, then creds.content, then creds.access_token in that order (refer to the token variable and the creds.api_token / creds.content / creds.access_token symbols).
286-311:⚠️ Potential issue | 🔴 CriticalMultiple ES2020+ operators (
?.and??) violate QuickJS compatibility.Lines 286-311 use optional chaining (
?.) and nullish coalescing (??) extensively. These must be replaced with ternary operators or explicit checks.🔧 Suggested fix
export function formatUserSummary(user: Record<string, unknown>): Record<string, unknown> { // Default to top-level user fields let id = user.id as string; let name = user.name as string | undefined; let email: string | undefined; let avatarUrl = user.avatar_url as string | undefined; let userType = user.type as string | undefined; // For bot-type users, drill into bot.owner.user.person to get the human owner info if (userType === 'bot') { - const bot = user.bot as Record<string, unknown> | undefined; - const owner = bot?.owner as Record<string, unknown> | undefined; - const ownerUser = owner?.user as Record<string, unknown> | undefined; - const ownerPerson = ownerUser?.person as Record<string, unknown> | undefined; + const bot = user.bot as Record<string, unknown> | undefined; + const owner = bot ? bot.owner as Record<string, unknown> | undefined : undefined; + const ownerUser = owner ? owner.user as Record<string, unknown> | undefined : undefined; + const ownerPerson = ownerUser ? ownerUser.person as Record<string, unknown> | undefined : undefined; if (ownerUser) { - id = (ownerUser.id as string) || id; - name = (ownerUser.name as string) || name; - avatarUrl = (ownerUser.avatar_url as string) || avatarUrl; - userType = (ownerUser.type as string) || userType; + id = (ownerUser.id as string) || id; + name = (ownerUser.name as string) || name; + avatarUrl = (ownerUser.avatar_url as string) || avatarUrl; + userType = (ownerUser.type as string) || userType; } if (ownerPerson) { email = (ownerPerson.email as string) || email; } } else { - const person = user.person as Record<string, unknown> | undefined; - email = (person?.email as string) || (user.email as string | undefined); + const person = user.person as Record<string, unknown> | undefined; + email = (person ? person.email as string : undefined) || (user.email as string | undefined); } return { id, - name: name ?? null, - email: email ?? null, - type: userType ?? null, - avatar_url: avatarUrl ?? null, + name: name !== undefined ? name : null, + email: email !== undefined ? email : null, + type: userType !== undefined ? userType : null, + avatar_url: avatarUrl !== undefined ? avatarUrl : null, }; }As per coding guidelines: "do NOT use optional chaining (
?.) or nullish coalescing (??) operators as QuickJS runtime does not support ES2020+ features."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/notion/helpers.ts` around lines 286 - 311, The code in this helper uses optional chaining (e.g., owner?.user, person?.email) and nullish coalescing (name ?? null) which QuickJS doesn't support; update the logic in this function to use explicit checks and ternary operators instead: replace expressions like owner?.user, ownerUser?.person and person?.email with guarded accesses (e.g., owner && owner.user, ownerUser && ownerUser.person) and replace nullish coalescing in the return and assignments (e.g., name ?? null, email ?? null, userType ?? null, avatarUrl ?? null) with ternaries or explicit checks (e.g., name !== undefined && name !== null ? name : null). Ensure all uses of ?. and ?? in the snippet (variables owner, ownerUser, ownerPerson, person and the returned fields id/name/email/type/avatar_url) are rewritten to the compatible form.
🧹 Nitpick comments (2)
src/core/gmail/__tests__/test-gmail.ts (2)
69-89: Consider adding a brief comment to empty catch blocks for clarity.The empty
catch {}blocks at lines 72 and 87 are intentional for cleanup (ignoring errors when stopping a skill that may not be running), but ESLint flags them. Adding a brief comment would silence the linter and document the intent.Proposed fix
beforeAll(() => { try { stopSkill(SKILL_ID); - } catch {} + } catch { + // Ignore - skill may not be running + } startSkill(SKILL_ID); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/__tests__/test-gmail.ts` around lines 69 - 89, Add brief explanatory comments inside the empty catch blocks around the test cleanup to satisfy ESLint and document intent: in the beforeAll and afterAll blocks where stopSkill(SKILL_ID) is wrapped in try/catch, replace the empty catches with a short comment (e.g. "// ignore if skill not running") so it's clear the swallow is intentional; update the catch blocks adjacent to stopSkill, startSkill, beforeAll, and afterAll accordingly.
96-133: Same suggestion for empty catch blocks in the Tools test suite.Lines 99 and 131 have empty catch blocks that could use a brief comment for the same reasons.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/gmail/__tests__/test-gmail.ts` around lines 96 - 133, The empty catch blocks around stopSkill(SKILL_ID) in the beforeAll and afterAll hooks should include brief comments explaining why exceptions are being suppressed (e.g., skill may not be running), so update the beforeAll and afterAll handlers that call stopSkill(SKILL_ID) / startSkill(SKILL_ID) to replace the empty catch {} with a short explanatory comment inside the catch (referencing beforeAll, afterAll, stopSkill, startSkill, and SKILL_ID) to make the intent clear to future readers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Around line 486-488: The documentation incorrectly states skills can control
provider-specific headers like API version; update the OAuth Proxy section to
state the Notion API version is fixed to 2026-03-11 because the implementation
sets apiVersion = NOTION_API_VERSION inside notionFetch() (and passes that value
in request headers) with no parameter or header-forwarding path for skills to
override; reference notionFetch() and NOTION_API_VERSION when clarifying that
the proxy enforces the hardcoded Notion version rather than allowing
skill-supplied headers to change it.
- Around line 474-479: Replace all optional chaining and nullish coalescing in
the examples: change calls like onProgress?.(...) to onProgress &&
onProgress(...), change property access patterns like db.get(...)?.value to a
conditional expression such as (db.get(...) ? db.get(...).value : null) or use a
temporary variable to avoid repeated calls, and replace nullish coalescing (a ??
b) with a || b; update the specific usages referenced (onProgress calls in the
Sync Logic, and db.get(...) usages in State Publishing such as the expressions
around db.get(...).value and (db.get(...) as {...}).count) so the examples only
use &&, ternary ?:, or || as recommended.
In `@openhuman`:
- Line 1: The submodule pointer for the openhuman submodule references an
invalid commit 05004ec868bcaa82f32e2b2fc5a9d575dc7410ba; update the submodule
reference to a valid commit by checking out the openhuman repo, identifying a
reachable commit or tag, and updating the submodule pointer (e.g., via git
submodule set-branch / git submodule update --init --remote or by editing the
submodule entry to point to the valid commit SHA), then commit the updated
submodule reference so the repository no longer points to the nonexistent
commit.
In `@scripts/dev-runtime.mjs`:
- Around line 77-98: The build steps for skills and runtime (the execSync calls
that run 'yarn build' and 'cargo build') are placed after the existence checks
for CORE_BINARY and SKILLS_DIR so fresh clones exit before building; move the
entire "Building skills..." and "Building runtime (cargo build)..." try/catch
blocks so they run before the checks, or delete/convert the pre-checks into
post-build sanity checks that verify CORE_BINARY and SKILLS_DIR after the builds
complete; update references to CORE_BINARY and SKILLS_DIR accordingly and keep
the same console logging and process.exit behavior on build failure.
In `@src/core/gmail/api/index.ts`:
- Around line 13-15: The sleep function as implemented (function sleep(ms:
number): void { setTimeout(() => {}, ms); }) does not block and so the 429 retry
backoff is ineffective; replace it with a busy-wait implementation (e.g.,
compute end = Date.now()+ms and loop while Date.now() < end) inside the same
function signature to synchronously pause execution. Update any other uses of
sleep in this module (the other occurrence flagged in the file) to rely on this
blocking implementation so Retry-After backoffs are honored. Ensure the function
name sleep(ms: number): void is preserved and that the loop is tight (no
async/await) to work in the QuickJS environment.
In `@src/core/gmail/sync.ts`:
- Around line 225-237: The mapping that builds the emails array before calling
state.setPartial is still including raw message text via the snippet field;
update the code that constructs emails (the getEmails().map(...) block) to omit
snippet (and any other body_text-like properties) so only structural metadata
(id, subject, sender_email, sender_name, date, is_read, is_starred, labels) is
published, and ensure any UI that needs a preview uses an on-demand fetch
function (e.g., a new fetchEmailPreview or existing mail fetcher) instead of
exposing snippet through state.setPartial; apply the same change to the other
occurrence that builds/publishes emails (the second emails mapping around the
other state.setPartial call).
In `@src/core/gmail/tools/index.ts`:
- Around line 18-30: The execute method currently logs argument values (via
argSummary) which can leak PII; modify execute (the argKeys/argSummary logic for
toolName) to redact sensitive fields instead of emitting raw values: treat keys
like "to", "cc", "bcc", "recipients", "body", "subject", "query", "search" (and
any Gmail-specific recipient/body keys) as sensitive and replace their values
with a placeholder like "<redacted>" (for strings and nested objects), for
non-sensitive keys keep only type/length metadata (e.g., "<string 24 chars>" or
"<object>") and JSON.stringify only safe-to-log primitive types; ensure the
console.log call for `[gmail][tool:${toolName}] called with ${argSummary}` never
includes raw PII by applying this redaction logic before building argSummary.
In `@src/core/notion/helpers.ts`:
- Around line 24-30: The current sleep function (sleep) does a blocking
busy-wait which freezes the single-threaded QuickJS runtime; replace it with a
non-blocking async sleep (returning a Promise that resolves via setTimeout) and
update any callers (the retry/backoff logic that calls sleep) to await the async
sleep or be converted into an async function so backoff waits are non-blocking;
ensure the exported/used symbol name remains sleep (or update all call sites if
renamed) and preserve the same backoff durations and behavior (exponential
jitter if used).
In `@src/core/notion/index.ts`:
- Line 8: Remove the unused import by editing the import statement in
src/core/notion/index.ts: remove the identifier notionFetch from the
destructured import (currently "formatUserSummary, isNotionConnected,
notionFetch") so only used symbols remain (e.g., "formatUserSummary,
isNotionConnected"); ensure there are no other references to notionFetch in this
module before committing.
In `@src/core/notion/live-test-stress.ts`:
- Around line 130-137: The Notion API version header in the fetch call inside
src/core/notion/live-test-stress.ts is hardcoded to '2025-09-03' which
mismatches the runtime version; update the 'Notion-Version' header in the fetch
request (the block around the fetch call using ep.method and headers) to
'2026-03-11' so it matches the runtime used in src/core/notion/index.ts and
helpers.ts, ensuring both the direct proxy baseline and skill-runtime use the
same API contract.
In `@src/core/notion/live-test-sync.ts`:
- Around line 186-235: The polling currently treats completion as "!inProg &&
pages > 0" which fails for empty workspaces and can be fooled by prior runs'
lastSyncTime; before entering the loop call getState() (or the same status
source) to capture an initialLastSyncTime (e.g. const initialLastSyncTime =
s?.lastSyncTime) or generate a per-run marker (runId) and require the state to
include an advancing lastSyncTime or an explicit completion flag for this
invocation; then, inside the loop (in the while where getState() is polled),
consider sync finished only when !inProg AND (pages>0 OR (s.lastSyncTime &&
s.lastSyncTime > initialLastSyncTime) OR s.syncCompletedForRun === runId), and
on exit persist or log that run-specific completion so future runs can't reuse
old timestamps (update uses of getState, syncInProgress, lastSyncError,
lastSyncTime, and any post.lastSyncTime checks to implement this).
In `@src/core/notion/sync.ts`:
- Around line 409-425: The pages snapshot built from getLocalPages should not
include full page bodies; remove the content_text field from the pages object
construction (keep has_content and content_length only) and ensure the structure
passed into syncIntegrationMetadata(...) is the metadata-only pages array (no
content_text). Update the code that builds pages (the mapping over
getLocalPages) and any usage that forwards pages to syncIntegrationMetadata to
use this trimmed metadata object so large page bodies are not serialized or
sent.
- Around line 92-95: The code only updates the watermark when getEntityCounts()
reports pages/databases > 0; change it so s.syncStatus.lastSyncTime is advanced
to nowMs on every successful sync run (regardless of counts) — e.g., after
calling getEntityCounts() or at the end of the successful sync flow, remove the
conditional around s.syncStatus.lastSyncTime and unconditionally assign nowMs to
it so callers relying on lastSyncTime see completion.
- Around line 208-214: The fast-path that skips pages when
existing.last_edited_time === lastEdited (inside the objectType === 'page'
branch using getPageById and pageSkipped) must also check whether the page's
content/ingestion state is complete; modify the condition so you only continue
early when both the last_edited_time matches AND a persisted/contentSyncSuccess
flag (or non-empty persisted content/ingestion metadata on the existing page)
indicates the page was successfully fetched/ingested—otherwise leave the page to
be processed (or requeued) so failed or backfilled pages are retried; apply the
same guard to the later content-fetch/ingest logic paths referenced around lines
handling contentSyncEnabled and the fetch/ingest block.
In `@src/core/notion/tools/index.ts`:
- Around line 36-48: The execute method (function execute) currently logs arg
values verbatim or JSON-stringified; change it to redact content-bearing args by
logging only size/summary information: for any string argument (including short
ones) log as "<N chars>", for objects/arrays log a compact summary like "<object
with X keys>" or "<array of length N>" rather than JSON.stringify, and for other
primitives log their type/value safely; update the argSummary construction used
in the console.log(`[notion][tool:${toolName}] called with ${argSummary}`) to
implement these size-only/redacted summaries for all args.
In `@src/core/notion/tools/sync-now.ts`:
- Line 12: The execute() method is reporting last_sync_time even when
lastSyncTime remains 0 (epoch) because performSync() only updates lastSyncTime
when counts.pages>0 || counts.databases>0; either change performSync() to set
lastSyncTime unconditionally after a successful run or modify execute() to omit
last_sync_time when lastSyncTime===0 — locate performSync(), lastSyncTime, and
execute() and implement one of these fixes so execute() only reports meaningful
last_sync_time (or ensure lastSyncTime is updated on empty syncs).
In `@src/core/server-ping/tools/ping-now.ts`:
- Line 10: The optional chaining operator is used on globalThis.doPing which
QuickJS doesn't support; update the call site by explicitly checking for the
function before invoking it (e.g., test that (globalThis as { doPing?: () =>
void }).doPing is a function or not undefined) and then call it, referencing the
globalThis.doPing symbol so the conditional guard replaces the `?.` usage.
---
Outside diff comments:
In `@src/core/gmail/api/index.ts`:
- Around line 147-157: The code captures accessToken once (resolveAccessToken)
before the retry loop, so after a 401 and cache clear the loop still uses the
stale token; to fix, re-resolve the token inside each retry attempt (move or
call resolveAccessToken within the retry logic) and rebuild the Authorization
header from that freshly-resolved token (and likewise re-check
oauth.getCredential/useProxy per attempt); ensure the retry flow that clears the
cache (the logic around lines referencing cache clear and re-resolve) uses the
new token on the subsequent request so self-hosted OAuth can recover from 401s.
In `@src/core/gmail/index.ts`:
- Line 441: The code sets s.config.maxEmailsPerSync using parseInt(args.value as
string, 100) which uses an invalid radix; replace the radix 100 with 10 (e.g.,
use parseInt(args.value as string, 10) or Number(args.value)) so parseInt
returns the intended decimal integer for s.config.maxEmailsPerSync when parsing
args.value.
- Line 410: The value assignment using the nullish coalescing operator on
s.config.showSensitiveMessages must be rewritten without "??" because QuickJS
doesn't support it; locate the property where value:
s.config.showSensitiveMessages ?? false is set (the assignment for
showSensitiveMessages) and replace the expression with an explicit
null/undefined check (e.g., value: (s.config && s.config.showSensitiveMessages
!== undefined && s.config.showSensitiveMessages !== null) ?
s.config.showSensitiveMessages : false or an equivalent ternary/check) so that
it behaves the same but uses only ES5-compatible operators.
In `@src/core/gmail/sync.ts`:
- Around line 81-99: Several occurrences of the optional chaining operator (?.)
are used and must be rewritten for QuickJS compatibility: in fetchMessagePage
replace response.data?.messages with an explicit guard like response.data ?
response.data.messages : undefined (and update the conditional to check for
that), and replace all call-site optional chains log?.(...) and
onProgress?.(...) with explicit checks such as if (log) log(...); and if
(onProgress) onProgress(...); follow the guideline for nullish coalescing where
needed (x !== null && x !== undefined ? x : fallback) and update the code around
the symbols fetchMessagePage, log, and onProgress accordingly.
In `@src/core/gmail/tools/get-email.ts`:
- Line 137: Replace the nullish coalescing usage in the get-email logic: the
variable showSensitive (assigned as const showSensitive = s.config.showSensitive
?? false) uses ?? which QuickJS doesn't support; change this assignment to use a
QuickJS-safe fallback (e.g., use || or a ternary) so showSensitive defaults to
false when s.config.showSensitive is null/undefined/falsey, and update the
assignment site in the get-email.ts function where showSensitive is defined.
In `@src/core/gmail/tools/get-emails.ts`:
- Around line 9-71: The file uses optional chaining (?.) and nullish coalescing
(??) which QuickJS doesn't support; update all occurrences in buildListParams,
hasAttachments, messageToEmailRow, and the execute function to use
QuickJS-compatible checks: replace x?.y?.z with explicit guarded accesses (e.g.,
x && x.y && x.y.z) and replace a ?? b with either a !== null && a !== undefined
? a : b (or a || b where the intent allows falsy values to fall through);
specifically change args.max_results ?? args.maxResults ?? 20 to a
null/undefined-safe expression, replace message.payload?.body?.attachmentId and
other payload/parts/header optional chains with guarded access, and adjust
headers/find/getHeader calls that use ?. to use conditional checks so behavior
is preserved.
In `@src/core/gmail/tools/get-profile.ts`:
- Around line 14-17: The schema exposes an accessToken field that isn’t threaded
into gmailFetch, causing a contract mismatch; either remove accessToken from the
tool input_schema (and its description) in get-profile's input_schema and
similarly in get-emails, or modify gmailFetch to accept an optional per-call
token (e.g., add an argument like tokenOrAccessToken to the gmailFetch function
signature and propagate it through its internal auth resolution), then update
the execute functions in get-profile (and get-emails) to pass the
caller-supplied accessToken into gmailFetch (with clear precedence: use provided
token if present, otherwise fall back to existing credential resolution) and
ensure logging/validation handles the new parameter.
In `@src/core/gmail/tools/search-emails.ts`:
- Around line 83-95: The batching loop is effectively serial because
gqlFetch/gmailFetch calls are being invoked synchronously inside batch.map; fix
by making the batch fetches execute concurrently: inside the loop map each
msgRef to a promise (call gmailFetch but do not await inside the map), then
await Promise.all on that array so the batch runs in parallel; use the existing
CONCURRENCY constant and the same message URL construction (msgRef.id) and then
merge the resolved results into emails. Reference: CONCURRENCY, refs,
gmailFetch, emails, msgRef.
In `@src/core/gmail/tools/send-email.ts`:
- Around line 111-127: Header injection risk: sanitize or reject CR/LF
characters before interpolating into rawMessage in send-email.ts—specifically
validate fromEmail, subject and any address fields used by formatEmailAddresses
(addr.email and addr.name) to ensure they do not contain '\r' or '\n'; either
strip/normalize those characters or throw a validation error and fail sending.
Update the code that builds rawMessage (the blocks appending From, To, Cc, Bcc,
Subject, In-Reply-To) and the formatEmailAddresses helper to perform this check
so no newline characters can be injected into headers (also apply same fix to
the other occurrence mentioned around lines 233-241).
- Around line 97-105: The execute function still uses optional chaining and
nullish coalescing incompatible with QuickJS; replace all instances: for
getGmailSkillState() usage change s.profile?.emailAddress to s.profile ?
s.profile.emailAddress : null (or s.profile ? s.profile.emailAddress : undefined
as appropriate) in the execute function; change response.error?.message to
response.error ? response.error.message : null; and change sentMessage?.id ??
null, sentMessage?.threadId ?? null, sentMessage?.labelIds ?? [] to the explicit
checks sentMessage ? (sentMessage.id !== undefined && sentMessage.id !== null ?
sentMessage.id : null) : null, sentMessage ? (sentMessage.threadId !== undefined
&& sentMessage.threadId !== null ? sentMessage.threadId : null) : null, and
sentMessage ? (sentMessage.labelIds !== undefined && sentMessage.labelIds !==
null ? sentMessage.labelIds : []) : [] respectively (or use x !== null && x !==
undefined ? x : fallback pattern); update all occurrences around
getGmailSkillState, response, and sentMessage variables to follow this pattern.
In `@src/core/notion/helpers.ts`:
- Line 46: The expression assigning to token uses the nullish coalescing
operator (creds.api_token ?? creds.content ?? creds.access_token) which QuickJS
doesn't support; replace it with an ES5-compatible fallback such as chaining
logical ORs or explicit null/undefined checks so the assignment to the token
variable uses creds.api_token, then creds.content, then creds.access_token in
that order (refer to the token variable and the creds.api_token / creds.content
/ creds.access_token symbols).
- Around line 286-311: The code in this helper uses optional chaining (e.g.,
owner?.user, person?.email) and nullish coalescing (name ?? null) which QuickJS
doesn't support; update the logic in this function to use explicit checks and
ternary operators instead: replace expressions like owner?.user,
ownerUser?.person and person?.email with guarded accesses (e.g., owner &&
owner.user, ownerUser && ownerUser.person) and replace nullish coalescing in the
return and assignments (e.g., name ?? null, email ?? null, userType ?? null,
avatarUrl ?? null) with ternaries or explicit checks (e.g., name !== undefined
&& name !== null ? name : null). Ensure all uses of ?. and ?? in the snippet
(variables owner, ownerUser, ownerPerson, person and the returned fields
id/name/email/type/avatar_url) are rewritten to the compatible form.
In `@src/core/notion/tools/append-text.ts`:
- Around line 20-29: The JSON schema currently requires 'text' but the execute
function accepts either 'text' or 'content'; update the schema so the required
constraint enforces that at least one of 'text' or 'content' is present (e.g.,
replace required: ['text'] with an anyOf/oneOf or a custom validation requiring
either property) so the schema matches the runtime check in the execute
function; ensure you reference the same property names ('text' and 'content') in
the updated schema and keep the existing descriptions intact.
In `@src/core/notion/tools/get-page-content.ts`:
- Around line 36-49: The current implementation in get-page-content uses a
single notionApi.getBlockChildren call per top-level block so setting
recursive=true only fetches one level and drops paginated children and deeper
descendants; fix by implementing a recursive fetchAllChildren helper that (1)
paginates getBlockChildren using has_more/next_cursor to collect all child
blocks, (2) maps each child through formatBlockSummary, and (3) for each child
that is a block and when recursive is true, call the helper recursively to
attach grandchildren, then use that result when building blocks instead of the
single childrenResult; update the code paths around notionApi.getBlockChildren,
formatBlockSummary, and the recursive flag to use this helper.
In `@src/core/server-ping/tools/get-ping-stats.ts`:
- Around line 25-35: The return object uses optional chaining on avgLatency
(avgLatency?.avg_ms) which QuickJS doesn't support and also treats 0 as falsy;
update the avgLatency handling in the get-ping-stats return so it does not use
?. or ?? and instead checks explicitly (e.g. if avgLatency exists and typeof
avgLatency.avg_ms === 'number') then set avgLatencyMs to
Math.round(avgLatency.avg_ms) else null; locate the avgLatency variable and the
return block constructing avgLatencyMs and replace the optional-chaining/truthy
check with this explicit numeric check.
In `@src/core/server-ping/tools/read-config.ts`:
- Around line 11-15: In execute() inside read-config.ts, replace the nullish
coalescing usage on the result of data.read('config.json') with an explicit null
check/ternary so QuickJS can run; e.g., inspect the variable returned by
data.read in execute(), and return raw !== null && raw !== undefined ? raw :
JSON.stringify({ error: 'No config file found' }) instead of using ??.
In `@src/core/server-ping/tools/update-server-url.ts`:
- Around line 12-25: The execute function currently uses nullish coalescing and
optional chaining and a too-permissive URL check; update execute to avoid '??'
and '?.' by using explicit typeof/undefined checks and conditional property
access (e.g., getSkillState() result checked before reading config), validate
the url variable by ensuring args.url is a string, trim it, and check it
startsWith either 'http://' or 'https://' (not just 'http'), keep setting
s.config.serverUrl and calling state.set('config', s.config) as before, and
invoke publishState only after confirming globalThis.publishState is a function
(e.g., check typeof publishState === 'function' before calling) to maintain
QuickJS compatibility.
---
Nitpick comments:
In `@src/core/gmail/__tests__/test-gmail.ts`:
- Around line 69-89: Add brief explanatory comments inside the empty catch
blocks around the test cleanup to satisfy ESLint and document intent: in the
beforeAll and afterAll blocks where stopSkill(SKILL_ID) is wrapped in try/catch,
replace the empty catches with a short comment (e.g. "// ignore if skill not
running") so it's clear the swallow is intentional; update the catch blocks
adjacent to stopSkill, startSkill, beforeAll, and afterAll accordingly.
- Around line 96-133: The empty catch blocks around stopSkill(SKILL_ID) in the
beforeAll and afterAll hooks should include brief comments explaining why
exceptions are being suppressed (e.g., skill may not be running), so update the
beforeAll and afterAll handlers that call stopSkill(SKILL_ID) /
startSkill(SKILL_ID) to replace the empty catch {} with a short explanatory
comment inside the catch (referencing beforeAll, afterAll, stopSkill,
startSkill, and SKILL_ID) to make the intent clear to future readers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5cdc1ae-b522-4a15-97d1-48de3cd6056e
📒 Files selected for processing (65)
CLAUDE.mdopenhumanscripts/dev-runtime.mjssrc/core/gmail/__tests__/test-gmail.tssrc/core/gmail/api/helpers.tssrc/core/gmail/api/index.tssrc/core/gmail/index.tssrc/core/gmail/sync.tssrc/core/gmail/tools/get-email.tssrc/core/gmail/tools/get-emails.tssrc/core/gmail/tools/get-labels.tssrc/core/gmail/tools/get-profile.tssrc/core/gmail/tools/index.tssrc/core/gmail/tools/mark-email.tssrc/core/gmail/tools/search-emails.tssrc/core/gmail/tools/send-email.tssrc/core/notion/api/blocks.tssrc/core/notion/api/client.tssrc/core/notion/api/comments.tssrc/core/notion/api/databases.tssrc/core/notion/api/index.tssrc/core/notion/api/pages.tssrc/core/notion/api/search.tssrc/core/notion/api/users.tssrc/core/notion/helpers.tssrc/core/notion/index.tssrc/core/notion/live-test-stress.tssrc/core/notion/live-test-sync.tssrc/core/notion/live-test.tssrc/core/notion/state.tssrc/core/notion/sync.tssrc/core/notion/tools/append-blocks.tssrc/core/notion/tools/append-text.tssrc/core/notion/tools/create-comment.tssrc/core/notion/tools/create-database.tssrc/core/notion/tools/create-page.tssrc/core/notion/tools/delete-block.tssrc/core/notion/tools/delete-page.tssrc/core/notion/tools/get-block-children.tssrc/core/notion/tools/get-block.tssrc/core/notion/tools/get-database.tssrc/core/notion/tools/get-page-content.tssrc/core/notion/tools/get-page.tssrc/core/notion/tools/get-user.tssrc/core/notion/tools/index.tssrc/core/notion/tools/list-all-databases.tssrc/core/notion/tools/list-all-pages.tssrc/core/notion/tools/list-comments.tssrc/core/notion/tools/list-users.tssrc/core/notion/tools/query-database.tssrc/core/notion/tools/search.tssrc/core/notion/tools/summarize-pages.tssrc/core/notion/tools/sync-now.tssrc/core/notion/tools/sync-status.tssrc/core/notion/tools/update-block.tssrc/core/notion/tools/update-database.tssrc/core/notion/tools/update-page.tssrc/core/server-ping/tools/get-ping-history.tssrc/core/server-ping/tools/get-ping-stats.tssrc/core/server-ping/tools/list-peer-skills.tssrc/core/server-ping/tools/ping-now.tssrc/core/server-ping/tools/read-config.tssrc/core/server-ping/tools/update-server-url.tstypes/globals.d.tstypes/skill.d.ts
| @@ -1 +1 @@ | |||
| Subproject commit 88eb5ac46126dcb0fc265cd960fa172537f14383 | |||
| Subproject commit 05004ec868bcaa82f32e2b2fc5a9d575dc7410ba | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify the openhuman submodule commit exists and show what changed
cd openhuman || exit 1
# Verify the new commit exists
if ! git cat-file -e 05004ec868bcaa82f32e2b2fc5a9d575dc7410ba 2>/dev/null; then
echo "ERROR: Commit 05004ec868bcaa82f32e2b2fc5a9d575dc7410ba does not exist"
exit 1
fi
echo "✓ Commit exists"
echo ""
# Show commit message for the new revision
echo "=== New commit details ==="
git log -1 --format="%H%n%an <%ae>%n%ad%n%n%s%n%b" 05004ec868bcaa82f32e2b2fc5a9d575dc7410ba
echo ""
# Show changes between old and new commits
echo "=== Changes between commits ==="
git log --oneline 88eb5ac46126dcb0fc265cd960fa172537f14383..05004ec868bcaa82f32e2b2fc5a9d575dc7410ba
echo ""
# Check for any breaking changes or API modifications
echo "=== Files changed ==="
git diff --name-status 88eb5ac46126dcb0fc265cd960fa172537f14383..05004ec868bcaa82f32e2b2fc5a9d575dc7410baRepository: tinyhumansai/openhuman-skills
Length of output: 142
Fix invalid submodule commit hash — the target commit does not exist.
The new commit 05004ec868bcaa82f32e2b2fc5a9d575dc7410ba does not exist in the openhuman repository. This will cause build failures and prevent the code from running. Update the submodule pointer to a valid commit in the openhuman repository.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@openhuman` at line 1, The submodule pointer for the openhuman submodule
references an invalid commit 05004ec868bcaa82f32e2b2fc5a9d575dc7410ba; update
the submodule reference to a valid commit by checking out the openhuman repo,
identifying a reachable commit or tag, and updating the submodule pointer (e.g.,
via git submodule set-branch / git submodule update --init --remote or by
editing the submodule entry to point to the valid commit SHA), then commit the
updated submodule reference so the repository no longer points to the
nonexistent commit.
- Renamed functions and variables related to email submission to 'ingested' for clarity. - Updated database schema to replace 'backend_submitted' with 'ingested'. - Adjusted logic in sync process to check and mark emails as ingested instead of submitted. This change enhances the consistency of terminology used throughout the Gmail sync process.
No backend submission anymore — the column now tracks whether a document has been ingested into local memory via memory.insert(). Renamed: - Column: backend_submitted → ingested - Gmail: markEmailsSubmitted → markEmailsIngested, getUnsubmittedEmails → getUningestedEmails, markSensitiveAsSubmitted → markSensitiveAsIngested - Notion: markPagesSubmitted → markPagesIngested, markRowsSubmitted → markRowsIngested, getUnsubmittedPages → getUningestedPages, getUnsubmittedRows → getUningestedRows Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace all ?. and ?? operators in skill code with ES5-compatible alternatives (ternary, &&, ||) across gmail, notion, and server-ping - Fix parseInt radix 100 → 10 in gmail/index.ts - Fix Notion-Version header 2025-09-03 → 2026-03-11 in live-test-stress - Remove content_text from notion memory snapshot (too large) - Remove snippet from gmail state.setPartial (raw text breaks JSON) - Make lastSyncTime update unconditional on successful sync - Add ingested + content_text check to page skip logic - Redact PII in tool logging (gmail: sensitive keys, notion: size-only) - Move build steps before existence checks in dev-runtime.mjs - Fix sync-now.ts to handle lastSyncTime=0 - Fix notion live-test-sync polling to detect completion via lastSyncTime - Update CLAUDE.md examples to not use ?./?? - Update CLAUDE.md OAuth proxy docs for hardcoded Notion API version - Update MIN_CONTENT_LENGTH docs from 10 to 50 - Fix gmail token re-resolve in retry loop after 401 - Remove accessToken from get-profile input_schema - Sanitize CR/LF in email headers (send-email.ts) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Reformat code in CLAUDE.md and various Gmail-related files for better readability by adjusting indentation and line breaks. - Update conditional checks to use consistent formatting across multiple files, enhancing clarity. - Ensure consistent handling of optional values in function parameters and return statements. This change aims to improve code maintainability and readability without altering functionality.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Skill Submission
Skill name:
skill-name-hereType: [ ] Prompt-only | [ ] Coded (skill.ts)
Description
Brief description of what this skill does and why it's useful.
Checklist
SKILL.mdhas valid YAML frontmatter (name,description)eval(),Function(), or dynamic code executionctx.readData/ctx.writeData)skill.tshas name, description, version{ content: string }npx tsx harness/runner.ts ../skills/my-skillnpm run validatepasses indev/Testing
Describe how you tested this skill:
Category
Summary by CodeRabbit
New Features
Changes
Removals