Skip to content

feat(intake): add mounted Notion specs - #213

Merged
khaliqgant merged 3 commits into
mainfrom
codex/notion-intake-20260805
Aug 5, 2026
Merged

feat(intake): add mounted Notion specs#213
khaliqgant merged 3 commits into
mainfrom
codex/notion-intake-20260805

Conversation

@khaliqgant

@khaliqgant khaliqgant commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

  • add first-class factory intake notion <manifest> support for read-only Relayfile mounts
  • require an explicit # Chief Spec header or an exact operator-authorized bootstrap mapping
  • normalize repository work into the existing GitHub lifecycle and exact-path work into fleet dispatch
  • bind every destination to a stable Notion page identity and content digest
  • redact mounted bodies from public issues unless a reviewed publicSummary is supplied
  • preserve existing GitHub and Linear discovery behavior

Verification

  • npm run build
  • npm test
  • npm run featuremap:check
  • npx vitest run src/intake/notion.test.ts src/cli/fleet.test.ts (90 passed)
  • git diff --cached --check
  • two-phase veto_diff_review: PASS, code 86, security 94, secrets clean, no decision drift

Operational proof

The four operator-authorized mounted pages were dry-run through this command into five destinations: Cloud, Relay, Chief, Internal Agents, and one exact local benchmark workspace. Every result was ready with a stable digest and no writes.


Summary by cubic

Adds first-class support for mounted Notion specs with a CLI and API to turn read-only pages into GitHub lifecycle issues or exact-path fleet tasks. Concurrent runs are now serialized to avoid duplicate lifecycle issues.

  • New Features

    • New CLI: factory intake notion <manifest> with --dry-run.
    • Strict spec: requires # Chief Spec with Status: ready or exact operator bootstrap; parses Repos, Project-Paths, and optional Node.
    • Stable binding: binds each destination to notion:<page-id> plus a content digest; rejects duplicates; blocks on mounted spec drift.
    • GitHub flow: publishes via gh; requires factory-ready and agent:<recipe> labels; idempotent via hidden source marker and digest; never copies mounted bodies to public repos without a reviewed publicSummary.
    • Workspace flow: dispatches exact-path tasks and stores a digest-bound receipt in .factory/notion-intake-state.json; dry-run skips fleet construction; exact-path intake returns while preserving worker infrastructure.
    • Public API: exports from @agent-relay/factory/intake (runNotionIntake, GhCliIssuePublisher, normalizeNotionPageId, parseChiefSpecHeader).
  • Bug Fixes

    • Serialize overlapping Notion intake runs with a file lock to ensure a single lifecycle issue per destination; reconcile source markers with local receipts and retain per-destination results when a later publisher fails.

Written for commit 88c1268. Summary will update on new commits.

Review in cubic

@cursor

cursor Bot commented Aug 5, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds mounted Notion specification intake with strict validation, digest-based idempotency, GitHub issue publication, workspace dispatch, dry-run support, CLI integration, public exports, tests, documentation, and feature catalog updates.

Changes

Notion intake feature

Layer / File(s) Summary
Manifest contracts and normalization
src/intake/notion.ts, src/intake/notion.test.ts
Defines intake schemas and public types. Validates page IDs and Chief Spec headers. Normalizes mounted pages and computes content digests.
Repository and workspace execution
src/intake/notion.ts, src/intake/notion.test.ts
Publishes or deduplicates GitHub issues. Dispatches workspace tasks with persistent receipts. Validates privacy, labels, bootstrap authorization, state writes, and bounded gh execution.
CLI and published feature surface
src/cli/fleet.ts, src/cli/fleet.test.ts, src/intake/index.ts, src/index.ts, package.json, README.md, .agentworkforce/features/manifest.yaml, .agentworkforce/agents/factory-feature-guardian/*
Adds factory intake notion <manifest> with dry-run support. Exports the intake module. Registers the feature and documents its manifest, destinations, labels, idempotency, and blocking rules.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Suggested reviewers: kjgbot

Poem

A rabbit reads the mounted page,
Checks every field and turns the page.
Issues bloom or fleets depart,
Digests guard each careful start.
Dry runs hop without a trace.
The intake burrow finds its place.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: adding mounted Notion specification intake support.
Description check ✅ Passed The description directly explains the new Notion intake CLI, API, validation, routing, safety controls, and verification.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/notion-intake-20260805

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 34a456b99a

ℹ️ 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".

Comment thread src/intake/notion.ts Outdated
const spec = task.bootstrap
? normalizedBootstrapSpec(task.bootstrap, pageId)
: parseChiefSpecHeader(content)
const digest = createHash('sha256').update(content).digest('hex')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bind bootstrap changes into the intake digest

For bootstrapped/headerless pages, the dispatch contract comes from the manifest's bootstrap fields, but the digest used for existing-issue and receipt idempotency is only the mounted page content. If an operator corrects any bootstrap field that does not change the source key, such as publicSummary, recipe/labels, title, summary, or workspace node, a rerun still sees the old digest and reports already-dispatched, leaving stale or unsafe lifecycle work in place instead of blocking or updating it. Include the normalized bootstrap spec in the digest for bootstrap tasks.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 88c1268. Headerless bootstrap authorization is canonicalized and included in the contract digest; the mounted content digest is retained separately for worker byte verification. The regression test changes the bootstrap summary and proves the contract digest changes.

Comment thread src/intake/notion.ts Outdated
}

function renderIssueBody(task: NormalizedNotionTask, summary: string): string {
const mountedPath = `.integrations/notion/pages/${task.pageId}/content.md`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Render the configured Notion mount path

When a manifest uses a non-default mountRoot, normalization reads the mounted spec from that configured location, but the dispatched issue body still tells workers to open .integrations/notion/...; the workspace task renderer below hard-codes the same default. In those custom-mount deployments, dispatch can succeed while the agent follows instructions to a path that does not contain the authorized spec, so the intake work is effectively broken. Render the path from the normalized task/manifest rather than this literal default.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 88c1268 with an explicit workerMountRoot manifest field. mountRoot remains the operator-side read path, while workerMountRoot names the repo-relative read-only mount available on fleet workers; exact-path tasks retain their resolved local source path. A custom worker root is covered by the issue-body test.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (4)
src/cli/fleet.test.ts (1)

556-561: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that the dry run created no state file.

.agentworkforce/features/manifest.yaml registers cli-notion-intake-plan as reporting the plan "without writes", and this is the only test on that path. The test proves that no fleet is constructed, but it does not prove that statePath stays absent. A future change to the input.dispatch guard at src/intake/notion.ts line 156 would not fail any test.

Add the negative assertion. existsSync is already imported in this file.

💚 Proposed assertion
       expect(code).toBe(0)
+      expect(existsSync(join(root, 'state.json'))).toBe(false)
       expect(JSON.parse(output.text())).toMatchObject({
🤖 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/cli/fleet.test.ts` around lines 556 - 561, Extend the dry-run test
assertion near the existing JSON result checks to verify that
existsSync(statePath) is false, confirming no state file is created. Reuse the
already imported existsSync and the test’s existing statePath symbol.
src/intake/notion.test.ts (2)

212-215: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use the exported NotionIntakeTarget type for this helper parameter.

The conditional-type chain resolves to the element type of bootstrap.targets, which is exactly NotionIntakeTarget. src/intake/notion.ts already exports that type. The direct type is easier to read and it fails compilation if the contract changes.

♻️ Proposed simplification
-function bootstrap(target: NotionIntakeManifest['tasks'][number]['bootstrap'] extends infer T
-  ? T extends { targets: infer Targets } ? Targets extends unknown[] ? Targets[number] : never : never
-  : never,
-): NonNullable<NotionIntakeManifest['tasks'][number]['bootstrap']> {
+function bootstrap(
+  target: NotionIntakeTarget,
+): NonNullable<NotionIntakeManifest['tasks'][number]['bootstrap']> {

Add type NotionIntakeTarget to the existing import block at lines 7-15.

🤖 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/intake/notion.test.ts` around lines 212 - 215, Update the bootstrap
helper parameter to use the exported NotionIntakeTarget type directly, adding it
to the existing import from notion.ts; remove the conditional-type chain while
keeping the helper’s return type unchanged.

156-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the digest-change block.

The suite proves the idempotent path in both directions but never proves the fail-closed path. dispatchWorkspaceTask line 347 and publishRepoTask line 307 both block when the mounted content changed after dispatch. The README presents that behavior as the guarantee against silently mutating already-dispatched work, and neither branch is exercised.

Rewrite content.md between the two runs in this test, then assert status: 'blocked', the retained agent, and that dispatch was still called only once.

🤖 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/intake/notion.test.ts` around lines 156 - 165, Extend the test around the
two runNotionIntake calls to modify content.md between runs, exercising the
digest-change fail-closed path. Assert the second result has status 'blocked',
retains the original agent, and workspace.dispatch remains called exactly once.
README.md (1)

191-201: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Show an example intake manifest.

This section documents the mounted page format in full but describes the manifest only as the file that "identifies pages and the local mount root". The documented commands at lines 199 and 200 both require that file, and an operator cannot author it from this text. The reader has to open manifestSchema in src/intake/notion.ts to learn the field names, the version: 1 literal, and the mountRoot and statePath defaults.

Add a short JSON block, matching the style of the config example at lines 85-94.

📝 Proposed example
 The intake manifest identifies pages and the local mount root. Headerless legacy
 pages can be admitted only with a bootstrap entry containing the exact
 `authorizedPageId`, destination, safe summary, and operator reason. That escape
 hatch is deliberately page-specific; there is no title or content heuristic.
+
+```json
+{
+  "version": 1,
+  "mountRoot": ".integrations/notion",
+  "statePath": ".factory/notion-intake-state.json",
+  "tasks": [
+    { "page": "https://app.notion.com/p/Reconcile-3b36800c1c90801db1cfc8f2e1cff7cf" }
+  ]
+}
+```
+
+`mountRoot` and `statePath` resolve relative to the manifest file and default to
+the values shown above. `page` accepts a Notion URL or a bare page id.
🤖 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 `@README.md` around lines 191 - 201, Add a short JSON intake manifest example
to the README near the documented Notion intake commands, matching the existing
config-example style. Include the version 1 literal, mountRoot, statePath, and a
tasks entry with a representative page URL, then state that mountRoot and
statePath are manifest-relative defaults and page accepts a Notion URL or bare
page ID.
🤖 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/intake/index.ts`:
- Around line 8-14: Update the public type exports in the src/intake/index.ts
barrel to include ExistingGithubIssue and NotionRecipe alongside the existing
intake contracts. Ensure consumers can name the referenced types returned by
GithubIssuePublisher.findBySource and used by NormalizedNotionTask.recipe
without changing those interfaces.

In `@src/intake/notion.ts`:
- Around line 381-389: Update renderIssueBody and renderWorkspaceTask to use
NormalizedNotionTask.sourcePath when publishing the Relayfile path instead of
constructing the hardcoded .integrations/notion root. Preserve the existing
formatting while ensuring both issue-body and workspace-task references reflect
the manifest-resolved mountRoot.
- Around line 202-208: Update normalizeNotionPageId so both the hyphenated UUID
regex and compact 32-character hex regex require a non-hex character or start of
input immediately before the match, while retaining their existing right-side
boundary checks. Preserve matching for IDs preceded by URL separators such as
the hyphen in Notion app URLs.
- Around line 139-143: In the task-processing flow around dispatchWorkspaceTask,
persist the updated state with writeIntakeState before appending the dispatch
result to results. Keep the existing dispatch and state-path conditions,
ensuring a failed receipt write reaches the catch path without first recording a
successful result, while successful writes still produce one result for the
sourceKey.
- Around line 455-494: Update runGh to pass an appropriate timeout and
killSignal in the spawn options so stalled gh processes are terminated and the
existing fail handler rejects the promise. Replace the shared size counter with
independent stdout and stderr byte counts, enforcing the 1 MiB limit per stream
while preserving the existing output and error handling.
- Around line 260-274: Remove the fixed 1000-item limits from missingLabels and
findBySource. Query the exact labels record and search GitHub issues by the
source marker in the issue body rather than scanning a capped list; keep
validating the returned issue shape and confirm its body contains
sourceMarker(sourceKey) before returning it.

---

Nitpick comments:
In `@README.md`:
- Around line 191-201: Add a short JSON intake manifest example to the README
near the documented Notion intake commands, matching the existing config-example
style. Include the version 1 literal, mountRoot, statePath, and a tasks entry
with a representative page URL, then state that mountRoot and statePath are
manifest-relative defaults and page accepts a Notion URL or bare page ID.

In `@src/cli/fleet.test.ts`:
- Around line 556-561: Extend the dry-run test assertion near the existing JSON
result checks to verify that existsSync(statePath) is false, confirming no state
file is created. Reuse the already imported existsSync and the test’s existing
statePath symbol.

In `@src/intake/notion.test.ts`:
- Around line 212-215: Update the bootstrap helper parameter to use the exported
NotionIntakeTarget type directly, adding it to the existing import from
notion.ts; remove the conditional-type chain while keeping the helper’s return
type unchanged.
- Around line 156-165: Extend the test around the two runNotionIntake calls to
modify content.md between runs, exercising the digest-change fail-closed path.
Assert the second result has status 'blocked', retains the original agent, and
workspace.dispatch remains called exactly once.
🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 06d78083-d171-4c91-9878-8a7fc1535f1d

📥 Commits

Reviewing files that changed from the base of the PR and between 565690e and 34a456b.

📒 Files selected for processing (11)
  • .agentworkforce/agents/factory-feature-guardian/agent.test.ts
  • .agentworkforce/agents/factory-feature-guardian/manifest-contract.test.ts
  • .agentworkforce/features/manifest.yaml
  • README.md
  • package.json
  • src/cli/fleet.test.ts
  • src/cli/fleet.ts
  • src/index.ts
  • src/intake/index.ts
  • src/intake/notion.test.ts
  • src/intake/notion.ts

Comment thread src/intake/index.ts
Comment thread src/intake/notion.ts Outdated
Comment thread src/intake/notion.ts
Comment thread src/intake/notion.ts
Comment thread src/intake/notion.ts Outdated
Comment thread src/intake/notion.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

5 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/cli/fleet.ts">

<violation number="1" location="src/cli/fleet.ts:187">
P2: A spawn/receipt crash gap can lead to duplicate exact-path work: an already-running agent is reported without creating the digest-bound receipt, so a later retry can spawn it again once that agent exits. Recording or reconciling the receipt for this deterministic running agent would preserve the advertised idempotency.</violation>

<violation number="2" location="src/cli/fleet.ts:187">
P3: The already-running branch added to the workspace dispatcher returns status 'already-running', but that status is never consumed: the intake path (dispatchWorkspaceTask) records a receipt and reports the task as 'dispatched' whatever the dispatcher returns. So when a matching agent is already on the roster the operator still sees 'dispatched' with a freshly written dispatchedAt, not the true 'already-running' outcome. Consider having the dispatcher return the already-running status into a result that flows through, or dropping the branch status and treating this path as a re-dispatch with stable attribution, so the reported outcome matches what actually happened.</violation>
</file>

<file name="src/intake/notion.ts">

<violation number="1" location="src/intake/notion.ts:142">
P2: A transient receipt-write failure after a successful workspace spawn is reported as an additional blocked result for the same destination. This can make the CLI exit unsuccessfully and misrepresent a completed dispatch; separating persistence failures from per-task dispatch errors and ensuring one result per destination would keep the report consistent.</violation>

<violation number="2" location="src/intake/notion.ts:382">
P1: Workers cannot read the authorized page when the manifest uses a non-default or manifest-relative mount root: normalization reads one path, while both GitHub and workspace tasks point workers at hard-coded `.integrations/notion/...`. Rendering the configured mount path, including an absolute path when the worker cwd differs, keeps dispatched work connected to the page that was validated.</violation>
</file>

<file name="src/intake/notion.test.ts">

<violation number="1" location="src/intake/notion.test.ts:135">
P2: The digest-binding blocks are the core safety mechanism of this intake feature, but no test here exercises them: nothing mutates the mounted content between runs to force either 'mounted spec changed after ...' status, and the ready/dry-run (dispatch:false) and 'missing required GitHub labels' branches are untested. Adding a case that rewrites content.md and re-runs, expecting the digest-mismatch blocked result, would lock in the safety contract the PR advertises.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/cli/fleet.ts
Comment thread src/intake/notion.ts
Comment thread src/intake/notion.ts Outdated
Comment thread src/intake/notion.ts Outdated
}

function renderIssueBody(task: NormalizedNotionTask, summary: string): string {
const mountedPath = `.integrations/notion/pages/${task.pageId}/content.md`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: Workers cannot read the authorized page when the manifest uses a non-default or manifest-relative mount root: normalization reads one path, while both GitHub and workspace tasks point workers at hard-coded .integrations/notion/.... Rendering the configured mount path, including an absolute path when the worker cwd differs, keeps dispatched work connected to the page that was validated.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/intake/notion.ts, line 382:

<comment>Workers cannot read the authorized page when the manifest uses a non-default or manifest-relative mount root: normalization reads one path, while both GitHub and workspace tasks point workers at hard-coded `.integrations/notion/...`. Rendering the configured mount path, including an absolute path when the worker cwd differs, keeps dispatched work connected to the page that was validated.</comment>

<file context>
@@ -0,0 +1,494 @@
+}
+
+function renderIssueBody(task: NormalizedNotionTask, summary: string): string {
+  const mountedPath = `.integrations/notion/pages/${task.pageId}/content.md`
+  return [
+    '## Factory intake',
</file context>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 88c1268. workerMountRoot explicitly models the fleet worker mount independently from the operator-side mountRoot, and custom roots are rendered and tested. Exact-path workers receive the resolved local source path.

Comment thread src/intake/notion.ts
Comment thread src/intake/notion.ts
const result = await dispatchWorkspaceTask(task, input, state)
results.push(result)
if (input.dispatch && result.status === 'dispatched') {
await writeIntakeState(input.manifest.statePath, state)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: A transient receipt-write failure after a successful workspace spawn is reported as an additional blocked result for the same destination. This can make the CLI exit unsuccessfully and misrepresent a completed dispatch; separating persistence failures from per-task dispatch errors and ensuring one result per destination would keep the report consistent.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/intake/notion.ts, line 142:

<comment>A transient receipt-write failure after a successful workspace spawn is reported as an additional blocked result for the same destination. This can make the CLI exit unsuccessfully and misrepresent a completed dispatch; separating persistence failures from per-task dispatch errors and ensuring one result per destination would keep the report consistent.</comment>

<file context>
@@ -0,0 +1,494 @@
+      const result = await dispatchWorkspaceTask(task, input, state)
+      results.push(result)
+      if (input.dispatch && result.status === 'dispatched') {
+        await writeIntakeState(input.manifest.statePath, state)
+      }
+    } catch (error) {
</file context>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in cee6736. State persistence occurs before appending success, preserving one result per destination.

Comment thread src/intake/notion.ts Outdated
Comment thread src/intake/notion.ts
Comment thread src/intake/notion.test.ts Outdated
Comment thread src/cli/fleet.ts
dispatch: async (task) => {
fleet ??= await buildFleet(globals, undefined, deps)
const running = (await fleet.roster()).agents.find((agent) => agent.name === task.name)
if (running) return { agent: running.name, node: running.node, status: 'already-running' }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The already-running branch added to the workspace dispatcher returns status 'already-running', but that status is never consumed: the intake path (dispatchWorkspaceTask) records a receipt and reports the task as 'dispatched' whatever the dispatcher returns. So when a matching agent is already on the roster the operator still sees 'dispatched' with a freshly written dispatchedAt, not the true 'already-running' outcome. Consider having the dispatcher return the already-running status into a result that flows through, or dropping the branch status and treating this path as a re-dispatch with stable attribution, so the reported outcome matches what actually happened.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/cli/fleet.ts, line 187:

<comment>The already-running branch added to the workspace dispatcher returns status 'already-running', but that status is never consumed: the intake path (dispatchWorkspaceTask) records a receipt and reports the task as 'dispatched' whatever the dispatcher returns. So when a matching agent is already on the roster the operator still sees 'dispatched' with a freshly written dispatchedAt, not the true 'already-running' outcome. Consider having the dispatcher return the already-running status into a result that flows through, or dropping the branch status and treating this path as a re-dispatch with stable attribution, so the reported outcome matches what actually happened.</comment>

<file context>
@@ -171,6 +178,33 @@ export async function runFleetCli(argv: string[], deps: FleetCliDeps = {}): Prom
+        dispatch: async (task) => {
+          fleet ??= await buildFleet(globals, undefined, deps)
+          const running = (await fleet.roster()).agents.find((agent) => agent.name === task.name)
+          if (running) return { agent: running.name, node: running.node, status: 'already-running' }
+          const spawned = await fleet.spawn({
+            name: task.name,
</file context>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Fixed in 88c1268. The dispatcher status now flows through: an already-running agent is reconciled into a durable receipt and reported as already-dispatched rather than newly dispatched.

@khaliqgant

Copy link
Copy Markdown
Member Author

@codex review

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

3 issues found across 6 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/intake/notion.ts">

<violation number="1" location="src/intake/notion.ts:39">
P2: Workers can be directed to read a path outside the repository even though `workerMountRoot` is documented as repo-relative. A manifest containing `../...` is accepted and the resulting `join(...)` path escapes the intended mount boundary; validating that the value is relative and contains no parent-directory escape would keep the worker contract aligned with the manifest format.</violation>

<violation number="2" location="src/intake/notion.ts:158">
P2: A concurrent dispatch can still fail with a lock error instead of waiting for the first run to finish: this retry budget expires well before a normal multi-destination GitHub dispatch can complete. Using an unbounded retry policy, as the shared `FileStateStore` does, would preserve the promised serialized/idempotent behavior.</violation>

<violation number="3" location="src/intake/notion.ts:399">
P2: A successful GitHub issue creation can leave an unrecoverable orphan when the subsequent intake-state write fails. Because reruns now reject a matching source marker without a local receipt, the next invocation cannot reconcile the already-created lifecycle issue and instead blocks it; a recovery path should atomically or durably reconcile the external issue and local receipt before treating the destination as unclaimable.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic

Comment thread src/intake/notion.ts
realpath: false,
stale: INTAKE_LOCK_STALE_MS,
update: INTAKE_LOCK_STALE_MS / 2,
retries: { retries: 50, factor: 1.2, minTimeout: 10, maxTimeout: 100, randomize: true },

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: A concurrent dispatch can still fail with a lock error instead of waiting for the first run to finish: this retry budget expires well before a normal multi-destination GitHub dispatch can complete. Using an unbounded retry policy, as the shared FileStateStore does, would preserve the promised serialized/idempotent behavior.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/intake/notion.ts, line 158:

<comment>A concurrent dispatch can still fail with a lock error instead of waiting for the first run to finish: this retry budget expires well before a normal multi-destination GitHub dispatch can complete. Using an unbounded retry policy, as the shared `FileStateStore` does, would preserve the promised serialized/idempotent behavior.</comment>

<file context>
@@ -119,28 +140,51 @@ export async function loadNotionIntakeManifest(path: string): Promise<NotionInta
+    realpath: false,
+    stale: INTAKE_LOCK_STALE_MS,
+    update: INTAKE_LOCK_STALE_MS / 2,
+    retries: { retries: 50, factor: 1.2, minTimeout: 10, maxTimeout: 100, randomize: true },
+  })
+  try {
</file context>
Suggested change
retries: { retries: 50, factor: 1.2, minTimeout: 10, maxTimeout: 100, randomize: true },
retries: { forever: true, factor: 1.2, minTimeout: 10, maxTimeout: 100, randomize: true },

Comment thread src/intake/notion.ts
const manifestSchema = z.object({
version: z.literal(1),
mountRoot: z.string().trim().min(1).default('.integrations/notion'),
workerMountRoot: z.string().trim().min(1).default('.integrations/notion'),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Workers can be directed to read a path outside the repository even though workerMountRoot is documented as repo-relative. A manifest containing ../... is accepted and the resulting join(...) path escapes the intended mount boundary; validating that the value is relative and contains no parent-directory escape would keep the worker contract aligned with the manifest format.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/intake/notion.ts, line 39:

<comment>Workers can be directed to read a path outside the repository even though `workerMountRoot` is documented as repo-relative. A manifest containing `../...` is accepted and the resulting `join(...)` path escapes the intended mount boundary; validating that the value is relative and contains no parent-directory escape would keep the worker contract aligned with the manifest format.</comment>

<file context>
@@ -33,6 +36,7 @@ const bootstrapSchema = z.object({
 const manifestSchema = z.object({
   version: z.literal(1),
   mountRoot: z.string().trim().min(1).default('.integrations/notion'),
+  workerMountRoot: z.string().trim().min(1).default('.integrations/notion'),
   statePath: z.string().trim().min(1).default('.factory/notion-intake-state.json'),
   tasks: z.array(z.object({
</file context>
Suggested change
workerMountRoot: z.string().trim().min(1).default('.integrations/notion'),
workerMountRoot: z.string().trim().min(1).refine((value) => !isAbsolute(value) && !value.split(/[\\/]/u).some((segment) => segment === '..'), 'workerMountRoot must be repo-relative').default('.integrations/notion'),

Comment thread src/intake/notion.ts
labels,
body: renderIssueBody(task, visibility === 'public' ? target.publicSummary! : task.summary),
})
state.receipts[task.sourceKey] = {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: A successful GitHub issue creation can leave an unrecoverable orphan when the subsequent intake-state write fails. Because reruns now reject a matching source marker without a local receipt, the next invocation cannot reconcile the already-created lifecycle issue and instead blocks it; a recovery path should atomically or durably reconcile the external issue and local receipt before treating the destination as unclaimable.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/intake/notion.ts, line 399:

<comment>A successful GitHub issue creation can leave an unrecoverable orphan when the subsequent intake-state write fails. Because reruns now reject a matching source marker without a local receipt, the next invocation cannot reconcile the already-created lifecycle issue and instead blocks it; a recovery path should atomically or durably reconcile the external issue and local receipt before treating the destination as unclaimable.</comment>

<file context>
@@ -325,6 +396,12 @@ async function publishRepoTask(
     labels,
     body: renderIssueBody(task, visibility === 'public' ? target.publicSummary! : task.summary),
   })
+  state.receipts[task.sourceKey] = {
+    kind: 'github',
+    digest: task.digest,
</file context>

@khaliqgant
khaliqgant merged commit 1de0138 into main Aug 5, 2026
7 checks passed
@khaliqgant
khaliqgant deleted the codex/notion-intake-20260805 branch August 5, 2026 22:18

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 88c12680b1

ℹ️ 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".

Comment thread src/intake/notion.ts
Comment on lines +220 to +221
const contentDigest = createHash('sha256').update(content).digest('hex')
const digest = contractDigest(content, authorizationDigestInput)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Bind worker mount paths into the contract digest

When an operator corrects workerMountRoot after a repository task has been dispatched, the rendered worker path changes but digest remains based only on page content/bootstrap authorization. The receipt and existing issue therefore still match this digest, so the rerun reports already-dispatched while the issue continues directing workers to the obsolete path. Fresh evidence after the earlier mount-path fix is that the configured path is now rendered at lines 232–234 but is still omitted from the digest computed here; include the normalized worker path in the per-destination digest so this drift blocks rather than silently retaining broken instructions.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant