Skip to content

spec(openconnector): add 4 connector-category capabilities (ADR-019)#767

Merged
rubenvdlinde merged 2 commits into
developmentfrom
feat/specter-fleet-rollout
May 21, 2026
Merged

spec(openconnector): add 4 connector-category capabilities (ADR-019)#767
rubenvdlinde merged 2 commits into
developmentfrom
feat/specter-fleet-rollout

Conversation

@rubenvdlinde
Copy link
Copy Markdown
Contributor

Summary

Adds 4 new connector-CATEGORY capability specs to openconnector. These define category-level contracts (manifest shape, auth modes, capabilities) — individual per-vendor adapters land as separate changes consuming these category specs, mirroring the existing convention (add-pdok-adapter, add-stuf-adapter, ibabs-notubiz-connector).

Spec Coverage REQs
data-infra-connectors DBs, NoSQL, warehouses, event streams, object storage 7 (DIC)
document-cms-connectors SharePoint/Alfresco/M-Files/Box/Drive + NL VIP-files/NLX 7 (DCC)
endpoint-workspace-connectors Citrix/Horizon/AWS WorkSpaces/AVD + Intune/Jamf/WS1 7 (EWC)
saas-productivity-connectors M365/Slack/Teams/Notion/Jira/ServiceNow + NL Exact/Twinfield/AFAS/Unit4 8 (SPC)
Total 29 REQs + 40 scenarios

Architectural posture

  • ADR-019 integration registry: every connector category specifies the registration contract; consuming apps reference by slot name
  • ADR-024 manifest-driven: data-infra-connectors REQ-DIC-002 defines the minimum 12-field manifest shape; other 3 specs inherit
  • ADR-022 consume from OR: DMS connectors strictly EXTERNAL — Conduction-side storage stays in docudesk; mutative-action gating via ADR-023 (in-flight)
  • ADR-031 schema-declarative: new adapters MUST use OR ScheduledWorkflow, NOT per-app TimedJob
  • OAuth-first for SaaS: PKCE mandatory; basicAuth deprecated requiring ADR-005 exception
  • DMS scope strictly external: REQ-DCC-001 + REQ-DCC-006 forbid openconnector growing into a Conduction-side document store

Out of scope

  • Per-vendor adapter implementations (separate changes: add-openconnector-snowflake-adapter, -m365-adapter, -citrix-adapter, etc.)
  • Implementation code
  • Legacy adapter migration (pdok/stuf/dso/ibabs — explicitly NOT migrated by this change; optional cleanup deferred per Risk 6)
  • Federated search across DCC/SPC (deferred to potential future add-openconnector-federated-search)

Risks flagged in design.md

  1. Inheritance edge between DIC + DCC/EWC/SPC creates soft dep
  2. OAuth-first opinionated; basicAuth deprecation may break legacy
  3. ADR-023 mutative-action gating still in-flight

4 new capability specs covering connector-CATEGORY contracts (NOT
individual adapter implementations — per-vendor adapters land as
separate changes consuming these category specs):

- data-infra-connectors (REQ-DIC-001..007): Postgres/MySQL/Mongo/
  Snowflake/Kafka/S3/etc. Defines the minimum manifest field set
  (12 fields) that the other 3 specs inherit.
- document-cms-connectors (REQ-DCC-001..007): SharePoint/Alfresco/
  M-Files/Box/VIP-files/NLX. Strictly EXTERNAL — Conduction-side
  document storage stays in docudesk per ADR-022.
- endpoint-workspace-connectors (REQ-EWC-001..007): Citrix/Horizon/
  AWS WorkSpaces/AVD; device management Intune/Jamf/Workspace ONE.
- saas-productivity-connectors (REQ-SPC-001..008): M365/Workspace/
  Slack/Teams/Notion/Jira/ServiceNow/Salesforce + NL Exact/Twinfield/
  AFAS/Unit4. OAuth-first with PKCE; basicAuth deprecated.

29 RFC-2119 requirements + 40 scenarios. Built on ADR-019 integration
registry pattern. Mirrors existing convention from add-pdok-adapter /
add-stuf-adapter / ibabs-notubiz-connector.

All NEW adapters MUST register via manifest per ADR-024 (no PHP service
registration), MUST consume OR ScheduledWorkflow not TimedJob per ADR-031.
Existing legacy adapters explicitly NOT migrated by this change.
@rubenvdlinde rubenvdlinde requested a review from Rem-Dam as a code owner May 18, 2026 05:11
@github-actions
Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openconnector @ dbd2d27

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 148/148
npm
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-05-18 05:12 UTC

Download the full PDF report from the workflow artifacts.

Copy link
Copy Markdown
Contributor

@WilcoLouwerse WilcoLouwerse left a comment

Choose a reason for hiding this comment

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

Verdict: REQUEST_CHANGES — Strong, well-structured spec change that cleanly defers per-adapter implementation and correctly reuses existing ADR/abstraction infrastructure. Three issues need fixing before merge: a factual error (REQ-SPC-004 says 'two fields' but defines three), two implementation-specific PHP method references in normative spec text (REQ-DIC-003, REQ-EWC-003), and missing in-spec fallback guidance for the in-flight ADR-023 dependency in REQ-EWC-005 and REQ-SPC-006. Two minor cleanup items: missing ## Purpose section across all four files and a missing ADR-005 dependency declaration in the DCC spec header.

Additional findings:

🟡 All 4 spec files skip the ## Purpose section, deviating from the established repo pattern

Every existing openconnector change-folder spec (stuf-adapter, ibabs-notubiz-connector, dso-omgevingsloket) opens with a ## Purpose section that gives a 2–5 sentence rationale before diving into requirements. All four new specs go directly from the metadata block (**Status:**, **Depends on:**, …) into ## ADDED Requirements, leaving the 'why this capability exists' absent from the spec itself. The purpose text that would belong here is spread across design.md (Context) and proposal.md (Summary / Motivation) but is not in the normative spec file a per-adapter author will read as their primary contract. Per the writing-specs.md template, ## Purpose is a required section. Recommend adding a concise Purpose section to each of the four spec files.

🔴 REQ-SPC-004 body says 'two fields' but the table defines three

The REQ-SPC-004 requirement body reads "extended with two fields for record-shaped hits" but the field table directly below it defines three fields: entityType, recordKey, and actorLabel. The design.md table (line 356 of that file) correctly states "extends REQ-DCC-004 with three fields". The word "two" in the spec body is a copy-paste error that will produce incorrect contract documentation for implementers. Fix: change "two" to "three" in the requirement body.

(inline comment couldn't be placed at openspec/changes/add-openconnector-connector-categories/specs/saas-productivity-connectors/spec.md:1703)

🟡 REQ-DIC-003 and REQ-EWC-003 name concrete PHP method signatures in normative requirement text

REQ-DIC-003 requires that "the adapter MUST resolve credentials by calling SourceService::getCredentials(string $sourceSlug) at runtime" and REQ-EWC-003 states that user mapping MUST go through MappingService::resolve(). Naming specific PHP class and method signatures inside a normative spec violates the writing-specs guidance ("Focus on behavior, not implementation — describe what happens, not which classes/methods do it"). If either service is renamed or its contract changes, the spec becomes technically wrong without any requirement having changed. The behavioral intent (credentials MUST be resolved from openconnector's Source registry at invocation time; user-mapping MUST be declarative, not per-app PHP) is clear without the method signatures. Move the method names to tasks.md implementation notes, where they belong.

(inline comment couldn't be placed at openspec/changes/add-openconnector-connector-categories/specs/data-infra-connectors/spec.md:975)

🟡 REQ-EWC-005 and REQ-SPC-006 normatively require ADR-023 group-binding but provide no fallback in the spec text for the in-flight period

Both REQ-EWC-005 (destructive-action gating) and REQ-SPC-006 (mutative-bulk gating) list ADR-023's admin-configured action-to-group mapping as a MUST requirement, and both label ADR-023 as "in-flight". The design.md Reuse Analysis states the fallback is "falls back to per-source opt-in until ADR-023 lands", but this fallback is not stated in the normative spec text. A per-adapter author implementing REQ-EWC-005 today would see a MUST requirement pointing at an unavailable ADR and no spec-level guidance on what to do until it lands. The fix is to add a note in REQ-EWC-005 and REQ-SPC-006 stating: until ADR-023 ships, per-source enabled*Actions[] opt-in alone MUST gate the action; the group-binding step is added via a delta once ADR-023 is finalised. The risk is acknowledged correctly in design.md and proposal.md but needs to be visible in the normative spec.

(inline comment couldn't be placed at openspec/changes/add-openconnector-connector-categories/specs/endpoint-workspace-connectors/spec.md:1492)

🟢 DCC spec Depends-on header omits ADR-005 despite using it in two requirements

The document-cms-connectors spec **Depends on:** line does not list hydra ADR-005 (security), but the requirement body references it explicitly in REQ-DCC-003 ("Per ADR-005 (security) and ADR-022…, ACL bridging MUST default to read-only") and REQ-DCC-005 ("Per ADR-005, the adapter manifest entry MUST declare a defaultReadOnly: boolean field"). The EWC and SPC specs correctly list ADR-005 in their Depends-on. This is a minor metadata inconsistency that could mislead a reader doing a dependency scan.

(inline comment couldn't be placed at openspec/changes/add-openconnector-connector-categories/specs/document-cms-connectors/spec.md:1113)

🟢 REQ-SPC-004 scenario omits actorLabel from the list of fields DCC hits must exclude

The REQ-SPC-004 scenario states: "SharePoint hits MUST omit entityType / recordKey" but actorLabel is also an SPC-only extension not present in the base DCC envelope (REQ-DCC-004). The scenario should also state that DCC hits MUST omit actorLabel (or, equivalently, that actorLabel is optional and DCC adapters leave it null). As written, an implementer could leave actorLabel populated for DCC hits without a spec violation, which would contradict the intent that the extended fields are SPC-specific for record-shaped hits.

(inline comment couldn't be placed at openspec/changes/add-openconnector-connector-categories/specs/saas-productivity-connectors/spec.md:1726)

@github-actions
Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openconnector @ 8f53eac

Check PHP Vue Security License Tests
lint
phpcs
phpmd
psalm
phpstan
phpmetrics
eslint
stylelint
composer ✅ 148/148
npm ✅ 674/674
PHPUnit ⏭️
Newman ⏭️
Playwright ⏭️

Quality workflow — 2026-05-19 04:02 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde rubenvdlinde merged commit ff24acf into development May 21, 2026
22 checks passed
@rubenvdlinde rubenvdlinde deleted the feat/specter-fleet-rollout branch May 21, 2026 19:33
rubenvdlinde added a commit that referenced this pull request May 22, 2026
Resolves 30-file conflict between i18n's Tier-4 refactor (OR-adoption +
PHPCS docblock harmonisation + manifest v2 schema URL flip) and the 9
commits dev accumulated independently (#823 LogIndex wrapper, #842
.php-cs-fixer cleanup, #849 root-config sync + phpmd cleanup, #727
cross-entity slug refs, #752 PDOK adapter, #762 brand cobalt, #767
specter spec, #703 .gitignore harmonise, #679 openspec sync workflows).

Resolution strategy:
- 17 DU conflicts (Db classes + ExportService) — confirmed i18n's
  deletions (Tier-4 OR-adoption: data moved off bespoke Db/ classes
  to OR-backed objects).
- l10n/en.json + l10n/nl.json — took HEAD's union (translation work
  was done on i18n).
- composer.lock — took HEAD's (i18n had it regenerated for new deps).
- src/manifest.json — took HEAD (v2 schema URL + 2-space indent + the
  typed-primitive page shapes; whitespace-only conflict otherwise).
- 8 UU conflicts on PHP controllers/services + routes.php + registry.js
  — took HEAD (i18n). The systematic pattern: i18n calls the new OR
  API (->getObject()) while dev still references the now-deleted Db
  classes (->jsonSerialize()). Dev's references would break at
  runtime against i18n's structural state; HEAD is the only
  internally-consistent resolution.

All conflict-resolved files: 0 markers remaining, PHP syntactically
valid. Manifest still validates clean against v2 schema 2.7.0.
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.

2 participants