Skip to content

feat(integrations): shared-layer enrollment + target-collection plumbing (#505, #506) - #511

Merged
mavaali merged 4 commits into
mainfrom
claude/open-github-issues-z5qt38
Sep 6, 2026
Merged

feat(integrations): shared-layer enrollment + target-collection plumbing (#505, #506)#511
mavaali merged 4 commits into
mainfrom
claude/open-github-issues-z5qt38

Conversation

@mavaali

@mavaali mavaali commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

Stacked slices 1–2 of the accepted M365 ingestion design (#492, spec docs/superpowers/specs/2026-08-31-m365-selected-ingestion-design.md). No M365-specific code in either — the shared layer that a selected-source adapter (#508) will build on.

Closes #505 — provider-neutral enrollment in the shared connector layer

  • src/integrations/types.ts: "m365" joins ProviderName (with an optional config block so config[provider] indexing stays typed); new EnrollmentRecord (ref, kind: "file" | "folder", label, targetCollection, enrolledAt, enrolledBy); ProviderState gains optional enrollment and opaque adapter-owned adapterData. Absent fields = existing google/notion behavior.
  • src/integrations/engine.ts: optional ProviderAdapter.resolveEnrollment(candidates, state) capability; the provisional-cursor discipline now also snapshots/restores adapterData (deep-copied) — intermediate writes keep the replayable values, commit only when no source in the page failed; UnavailableSourceEvent.reason widens to no_longer_discovered | access_denied | deleted | unenrolled; ReconcileLimits gains an optional maxCycleMs soft wall-time cap.
  • src/integrations/routes.ts: the provider route matcher derives names from the registered adapters instead of the hardcoded (google|notion) regex.
  • src/integrations/state.ts / runtime.ts: validation for the new fields; DEFAULT_FACTORIES becomes Partial with a named error if a configured provider has no factory.

Closes #506 — thread targetCollection through the integration distill path

  • src/distill/propose.ts / state.ts: DistillIds/DistillUpsertInput gain an optional collection override, defaulting to DISTILL_COLLECTION; derivePath and the frontmatter.collection field use it. Every existing caller (no override passed) is unaffected.
  • src/integrations/engine.ts: DistillationInput gains targetCollection?: string; RemoteSource gains an optional enrolledRef so a folder-enrolled provider can attest which enrollment record owns a discovered descendant. The reconcile loop looks up the owning EnrollmentRecord (exact ref match, or enrolledRef for a folder descendant) and threads its targetCollection into deps.distill(). google/notion carry no enrollment, so they see no behavior change.
  • src/integrations/distill.ts forwards targetCollection into the distillUpsert collection field.
  • refuseRawDistillOutput needed no code change — it is purely structural (path + frontmatter.tier), so it already fires identically against an overridden collection; added tests proving it.
  • New requireCollectionWriteAccess(role, targetCollection) reuses canWrite — the same stage-time write-gate rationale as vault_stage_action (docs/architecture.md "Stage-time write gate"): manage_integrations alone must not let an enrollment aim proposals at a collection the serve process cannot write. feat(m365): File Picker page, enrollment routes, cost preview, and status surfaces #509's enroll route will call this before persisting an EnrollmentRecord.

Test plan

Invariants

  • Frontmatter stays the only metadata layer; the SQLite index stays derived/ephemeral (enrollment/adapterData live in the existing encrypted integrations envelope, metadata-only)
  • Curation stays advisory
  • RBAC is config-driven — the new gate reuses canWrite against the existing .daftari/config.yaml roles, no new permission model
  • Error handling stays Result<T, Error>; no classes; no throws from tool handlers

🤖 Generated with Claude Code

https://claude.ai/code/session_01BgaRyc5Nji2GpMYwXEYpmk

…or layer (#505)

Slice 1 of the accepted M365 ingestion design (#492):
- EnrollmentRecord + optional enrollment/adapterData on ProviderState;
  "m365" joins ProviderName with an optional config block
- optional ProviderAdapter.resolveEnrollment capability for selected-source
  providers
- adapterData held provisionally with the change cursor: snapshotted before
  discovery, restored for intermediate writes, committed only when no source
  in the page failed
- UnavailableSourceEvent.reason widens to no_longer_discovered |
  access_denied | deleted | unenrolled (backward-compatible JSONL)
- optional maxCycleMs soft wall-time cap in ReconcileLimits: stop starting
  new fetches, leave the remainder retryable, cursor uncommitted
- integration route matcher derives provider names from registered adapters
  instead of a hardcoded (google|notion) list
- state validation covers the new fields; google/notion adapters unchanged

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BgaRyc5Nji2GpMYwXEYpmk
…l path (#506)

Slice 2 of the accepted M365 ingestion design (#492). Depends on #505's
EnrollmentRecord/adapterData (already on this branch, PR #511).

- DistillIds/DistillUpsertInput gain an optional collection override,
  defaulting to DISTILL_COLLECTION; propose.ts's derivePath and the
  frontmatter.collection field use it instead of the hardcoded constant.
  Every existing caller (no override passed) is unaffected.
- DistillationInput gains targetCollection?: string. RemoteSource gains an
  optional enrolledRef so a folder-enrolled provider can attest which
  enrollment record owns a discovered descendant; the reconcile loop looks
  up the owning EnrollmentRecord (exact ref match, or enrolledRef for a
  folder descendant) and threads its targetCollection into deps.distill().
  google/notion carry no enrollment, so they see no behavior change.
- src/integrations/distill.ts forwards targetCollection into the
  distillUpsert collection field.
- refuseRawDistillOutput needed no change: it is purely structural
  (path + frontmatter.tier), so it already fires identically against an
  overridden collection — added tests proving it.
- New requireCollectionWriteAccess(role, targetCollection) reuses canWrite
  (the same stage-time write-gate rationale as vault_stage_action,
  docs/architecture.md): manage_integrations alone must not let an
  enrollment aim proposals at a collection the serve process cannot write.
  #509's enroll route will call this before persisting an EnrollmentRecord.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BgaRyc5Nji2GpMYwXEYpmk
@mavaali mavaali changed the title feat(integrations): provider-neutral enrollment in the shared connector layer feat(integrations): shared-layer enrollment + target-collection plumbing (#505, #506) Sep 4, 2026

mavaali commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

CI red on build (22) at 4178890, but it's not this PR's: the single failure is test/sleep/vanished-source-wake.test.ts > wakes a fresh canonical dependent with the last commit that contained its deleted source, with

Error: ENOTEMPTY: directory not empty, rmdir '/tmp/daftari-sleep-vanished-4vJJo3/.git/objects'

— a filesystem-cleanup race in that test's tmp-git-repo teardown, in src/sleep//test/sleep/, which this PR never touches (only src/distill/, src/integrations/, and their tests). Everything else passed: 403/404 test files, 4785/4786 tests. No fix exists for this yet; re-running the failed job once to confirm the flake.


Generated by Claude Code

@mavaali
mavaali marked this pull request as ready for review September 5, 2026 23:47
Comment thread src/distill/propose.ts
Comment thread src/distill/propose.ts
Comment thread src/distill/propose.ts Outdated
Comment thread src/integrations/runtime.ts
Comment thread src/integrations/types.ts
The Security Review bot on PR #511 flagged that derivePath() joins the
caller-supplied `collection` string into the staged file path without
sanitizing it, even though the comment above it claims all join
components are traversal-safe. Before #506, `collection` was always the
hardcoded DISTILL_COLLECTION constant; now it can come from
EnrollmentRecord.targetCollection, which state.ts only checked with
typeof === "string" — no charset restriction.

Add isValidCollectionName() (single path segment: [A-Za-z0-9_-]+, no
separators or traversal) and enforce it in two places: proposeAllClaims
rejects the whole batch with a clear error rather than silently
sanitizing (sanitizing could make the written path diverge from the
string requireCollectionWriteAccess checks against RBAC), and
validEnrollmentRecord rejects an unsafe targetCollection at the
persistence boundary so bad state can never be written or read back.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BgaRyc5Nji2GpMYwXEYpmk
Comment thread src/integrations/runtime.ts
@claude

claude Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Review summary

The core plumbing for M365 selected-source ingestion (#505/#506) — enrollment records, owningEnrollment, the collection-name path-safety fence (isValidCollectionName), requireCollectionWriteAccess, adapter-data snapshotting, maxCycleMs, provider-neutral routing — is solid, well-commented, and has matching test coverage in test/distill/ and test/integrations/. No violations of the CLAUDE.md hard invariants (no classes, Result<T, Error> throughout, frontmatter-only metadata, RBAC stays config-driven, no auto-fixing curation).

However, the feature is currently unreachable end-to-end because three provider-enumeration sites outside this diff's touched files were not updated to include "m365" alongside ProviderName's widening:

  1. src/utils/config.ts:549-553 (RECOGNISED_INTEGRATIONS_KEYS) and :607 (validateIntegrations's provider loop) — still only recognize google/notion. An operator adding an integrations.m365: block to .daftari/config.yaml gets a hard failure: 'integrations.m365' is not a recognised setting. This file isn't touched anywhere in the diff.
  2. src/integrations/runtime.ts:67 (configuredProviders) — still hardcoded to ["google", "notion"] as const. This is the list createConfiguredIntegrationRuntime uses to decide which providers get OAuth-credential validation, adapter construction, and periodic sync registration, so "m365" never reaches the runtime even if (1) is fixed. Left inline comment on this one.
  3. src/integrations/queue.ts:94 (validQueueItem) — same hardcoded google/notion check; a queued m365 webhook/refresh-hint item would fail shape validation. Lower severity today since m365 has no webhook path yet, but it's the same class of gap and will resurface.

None of these three files have any m365-related test, which is presumably how the gap made it through — all the new tests exercise reconcileProvider and the encrypted state layer directly, never createConfiguredIntegrationRuntime or config-file loading with an m365 block.

Recommend fixing 1 and 2 before merge (2 is unreachable without 1); 3 can be a fast-follow but should track ProviderName for consistency with isProviderName in src/integrations/state.ts.

…drift

Three more findings from the Claude Code Review pass on PR #511, all
confirmed against the actual code path:

- runtime.ts's configuredProviders() and config.ts's RECOGNISED_INTEGRATIONS_KEYS
  / per-provider parse loop still hardcoded ["google", "notion"], even though
  this PR widened ProviderName to include "m365" specifically so an operator
  could configure it ahead of #508's adapter landing. An integrations.m365
  block in config.yaml was rejected as an unknown key before ever reaching
  the "missing factory" error this PR added for exactly that case.
- queue.ts's validQueueItem still only accepted provider "google" | "notion".
  Once an m365 webhook event reached the durable queue, the next read of the
  whole pending array would fail as malformed, taking every other provider's
  queued events down with it. Mirrors the isProviderName fix already in
  state.ts.
- proposeAllClaims stamped the current run's `collection` into every claim's
  frontmatter uniformly, including update-in-place (pathOverrides) claims
  whose targetPath is pinned to wherever a PRIOR run landed them. If an
  enrollment's targetCollection changes between runs, the physical path stays
  under the old collection while frontmatter.collection would claim the new
  one — a metadata/location mismatch that matters because collection drives
  RBAC downstream. Now derived from the landed path's own leading segment for
  update claims.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BgaRyc5Nji2GpMYwXEYpmk
@mavaali
mavaali merged commit e082be9 into main Sep 6, 2026
23 checks passed
mavaali added a commit that referenced this pull request Sep 6, 2026
…provider-id m365)

main advanced past #511 (shared-layer enrollment + target-collection
plumbing) + RBAC, which overlaps this branch. main built the enrollment
socket (EnrollmentRecord, ProviderState.enrollment[], RemoteSource.enrolledRef
-> owningEnrollment -> targetCollection -> distill, requireCollectionWriteAccess,
resolveEnrollment hook) but shipped no enroll route; this branch is that plug.
Reconciliation:

- Provider id standardized to "m365" (main released it in 3.13.0). Config key,
  routes, adapter name, factory, UI provider field, tests renamed; the
  Microsoft-named files/types/MSAL vendor and "Microsoft Graph" prose kept.
- One EnrollmentRecord: main's spine (ref, kind file|folder, targetCollection,
  enrolledAt, enrolledBy) + the m365 operational/audit fields (driveId,
  remoteId, cursorKey, includeSpeakerNotes, webUrl?, audienceAckAt,
  readersAtEnrollment). Dropped the dead siteId/listId and the branch's id.
- Storage: main's enrollment[] array (identity = ref); dropped the branch's
  enrollments Record and SourceState.enrollmentId (a read-but-never-written
  placeholder).
- Dynamic ownership via enrolledRef: discover() now tags each source with its
  owning enrollment's ref (folder descendants -> folder ref; files -> own id),
  so the enrolled targetCollection actually reaches distillation — closing the
  gap main's #511 exists to fill.
- m365 config is the rich MicrosoftProviderConfig (tenant_id + collections
  allowlist required); a bare block is now correctly rejected.
- Kept main's provider-neutral providerFrom(adapters), targetCollection
  validation (isValidCollectionName confused-deputy guard), and canWrite gate.

Verified: tsc + biome clean; full suite green on Node 22 (5092 passed);
integration/distill/config/extract green on Node 20 (full Node 20 suite blocked
locally by a better-sqlite3 ABI mismatch from an fnm node switch — CI rebuilds
natives per version). Two obsolete tests removed with notes (m365-without-factory;
bare m365 config block).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants