chore: remove dead notion-sync-worker + monitor script#23
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
chittyid | 6c6ebae | Jun 10 2026, 02:00 AM |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR removes the Notion synchronization worker implementation and monitoring CLI script, eliminating all Notion sync endpoints, DLQ operations, health checks, and continuous monitoring capabilities from the codebase. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/registry-client.js (1)
205-206:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMissing mandatory ChittyID agent auth headers on direct ChittyID call.
The fetch to ChittyID health endpoint is sent without
X-ChittyOS-Agent-ChittyIDandX-ChittyOS-API-Key, and there is no visible ChittyTrust signature validation step before/for this call path.As per coding guidelines, "Every agent call to ChittyID MUST present
X-ChittyOS-Agent-ChittyIDheader ... andX-ChittyOS-API-Keyheader ... and validate token signature against ChittyTrust on every call."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/registry-client.js` around lines 205 - 206, The direct fetch to the ChittyID health endpoint is missing required auth headers and signature validation; update the call in src/services/registry-client.js (the fetch that assigns to response) to include the X-ChittyOS-Agent-ChittyID and X-ChittyOS-API-Key headers populated from the current agent identity/secret, and before making the request ensure you validate the agent token signature against ChittyTrust (e.g., call the existing ChittyTrust validation helper such as validateChittyTrustSignature or ChittyTrust.verifySignature with the agent token) and abort/log on validation failure so every agent call to the ChittyID health endpoint presents both headers and has its token signature verified.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/registry-client.js`:
- Around line 21-24: buildServiceInfo now uses env vars (CHITTYID_SERVICE_URL /
SERVICE_PUBLIC_URL) to set publicUrl but getHealthStatus still calls the legacy
hardcoded health endpoint, causing split-brain; update getHealthStatus to derive
the health probe URL from the same source used in buildServiceInfo (use
this.env?.CHITTYID_SERVICE_URL || this.env?.SERVICE_PUBLIC_URL ||
'https://id.chitty.cc') and append the health path (/api/health) so health
checks target the advertised service; ensure references to publicUrl logic are
consolidated (e.g., reuse the same variable or helper used in buildServiceInfo)
and remove the hardcoded 'https://chittyid.chitty.workers.dev/api/health' string
in getHealthStatus.
---
Outside diff comments:
In `@src/services/registry-client.js`:
- Around line 205-206: The direct fetch to the ChittyID health endpoint is
missing required auth headers and signature validation; update the call in
src/services/registry-client.js (the fetch that assigns to response) to include
the X-ChittyOS-Agent-ChittyID and X-ChittyOS-API-Key headers populated from the
current agent identity/secret, and before making the request ensure you validate
the agent token signature against ChittyTrust (e.g., call the existing
ChittyTrust validation helper such as validateChittyTrustSignature or
ChittyTrust.verifySignature with the agent token) and abort/log on validation
failure so every agent call to the ChittyID health endpoint presents both
headers and has its token signature verified.
🪄 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: 927f128c-43b5-42e9-bde8-fd8889a9c858
📒 Files selected for processing (3)
scripts/monitor-notion-sync.jssrc/services/registry-client.jssrc/workers/notion-sync-worker.js
💤 Files with no reviewable changes (2)
- src/workers/notion-sync-worker.js
- scripts/monitor-notion-sync.js
| const publicUrl = | ||
| this.env?.CHITTYID_SERVICE_URL || | ||
| this.env?.SERVICE_PUBLIC_URL || | ||
| 'https://id.chitty.cc'; |
There was a problem hiding this comment.
Endpoint source changed, but health checks still use the legacy hardcoded host.
buildServiceInfo() now advertises an env-derived endpoint (Line 21-24, Line 30), but getHealthStatus() still probes https://chittyid.chitty.workers.dev/api/health (Line 205). This creates split-brain health reporting when CHITTYID_SERVICE_URL/SERVICE_PUBLIC_URL points elsewhere.
Suggested fix
buildServiceInfo() {
const publicUrl =
this.env?.CHITTYID_SERVICE_URL ||
this.env?.SERVICE_PUBLIC_URL ||
'https://id.chitty.cc';
return {
@@
endpoint: publicUrl,
@@
};
}
@@
async getHealthStatus() {
try {
- const response = await fetch('https://chittyid.chitty.workers.dev/api/health');
+ const base = this.serviceInfo?.endpoint || 'https://id.chitty.cc';
+ const response = await fetch(`${base.replace(/\/$/, '')}/api/health`);Also applies to: 30-30
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/registry-client.js` around lines 21 - 24, buildServiceInfo now
uses env vars (CHITTYID_SERVICE_URL / SERVICE_PUBLIC_URL) to set publicUrl but
getHealthStatus still calls the legacy hardcoded health endpoint, causing
split-brain; update getHealthStatus to derive the health probe URL from the same
source used in buildServiceInfo (use this.env?.CHITTYID_SERVICE_URL ||
this.env?.SERVICE_PUBLIC_URL || 'https://id.chitty.cc') and append the health
path (/api/health) so health checks target the advertised service; ensure
references to publicUrl logic are consolidated (e.g., reuse the same variable or
helper used in buildServiceInfo) and remove the hardcoded
'https://chittyid.chitty.workers.dev/api/health' string in getHealthStatus.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec72dc9dad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| version: '2.0.0', | ||
| description: 'Identity management system with hardened security pipeline', | ||
| endpoint: 'https://chittyid.chitty.workers.dev', | ||
| endpoint: publicUrl, |
There was a problem hiding this comment.
When this registers the service at CHITTYID_SERVICE_URL/SERVICE_PUBLIC_URL/https://id.chitty.cc, subsequent /api/registry/update calls still send health: await this.getHealthStatus(), and getHealthStatus() probes the old hard-coded https://chittyid.chitty.workers.dev/api/health. In the production config I checked, wrangler.jsonc has workers_dev: false and routes only id.chitty.cc/*, so status updates for the newly advertised endpoint can report an unhealthy service even when id.chitty.cc is healthy.
Useful? React with 👍 / 👎.
These files are leftovers from an older notion-sync architecture: - src/workers/notion-sync-worker.js is not bound by any wrangler config (wrangler.jsonc, wrangler.hybrid.toml, wrangler-pages.toml all have zero Notion references) and is only referenced by itself. - scripts/monitor-notion-sync.js reads NOTION_SYNC_WORKER_URL which is not set anywhere in the repo or ecosystem and targets a worker that is not deployed. - No cross-repo references exist in CHITTYFOUNDATION/* or CHITTYOS/*. - Current Notion integration flows through src/services/notion-sync.js and src/api/notion-bridge.js, which are unaffected. Surfaced during chittyid PR #22 cleanup (orphan task 4b35b6e3-51a4-4c44-a8e4-ac43b9b17e68). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
chitcommit
force-pushed
the
chore/remove-dead-notion-sync-worker
branch
from
ec72dc9 to
6c6ebae
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 participant
Summary
src/workers/notion-sync-worker.js— not bound by any wrangler config and only self-referenced.scripts/monitor-notion-sync.js— readsNOTION_SYNC_WORKER_URLwhich is unset anywhere; targets a worker that is not deployed.Investigation
wrangler.jsonc,wrangler.hybrid.toml,wrangler-pages.toml: zero Notion references.CHITTYFOUNDATION/*andCHITTYOS/*: zero external references.src/services/notion-sync.jsandsrc/api/notion-bridge.js(untouched).Surfaced during chittyid PR #22 cleanup (orphan task
4b35b6e3-51a4-4c44-a8e4-ac43b9b17e68).Test plan
Generated with Claude Code
Summary by CodeRabbit