You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit b2549a8
Browse filesBrowse the repository at this point in the historyBrowse files
feat(spec): ConnectorProviderContext.resolvePackagePath, a host-provided package anchor for provider factories (#22434) (#22465)
Fixes#22434
Clause-②: yes
## What this changes
`ConnectorProviderContext`
(`packages/spec/src/integration/connector-provider.ts`) gains one
optional, host-provided member, `resolvePackagePath(relativePath)`,
beside `loadPackageFile`. It takes the card's **preferred, confined
shape**:
- it resolves a relative path against the root of the stack/package that
declared the connector entry and resolves to the absolute path;
- `'.'` gives the root itself;
- it refuses (rejects) an empty path, an absolute path (posix or Windows
drive-letter) and a path that escapes the root after resolution (`../x`,
`a/../../x`);
- it reads nothing and does not check that the path exists.
Its signature is exactly its sibling's: `(relativePath: string)`,
resolving to a string. It is async for the same reason the loader is:
`service-automation` imports `node:path` lazily, so constructing either
capability never touches a Node builtin on a host without a filesystem.
The consumer is an async provider factory, which awaits it.
`@objectstack/service-automation`'s materializer hands it to every
provider factory, right beside `loadPackageFile` (the `providerCtx`
literal in `plugin.ts`), anchored at the same `packageRoot` option. The
confinement check that lived inline in `createPackageFileLoader` is
extracted into one private helper, `resolveInsidePackageRoot`, which
both members call, so they cannot disagree about what is inside the
root. `loadPackageFile`'s behaviour and its three error strings (pinned
verbatim by the
`docs/qa/platform-checklist/areas/integration-system.json` item) are
byte-identical; only the escape refusal's tail word is per member
(`reads are confined to` for the loader, `paths are confined to` for the
resolver).
`createPackagePathResolver` is exported from `plugin.ts` for its tests,
and deliberately NOT from the package index: no caller outside the
materializer needs to build one today, so the published surface of
`service-automation` does not grow.
**Not changed:** `packages/connectors/connector-mcp` (using the member
as the stdio transport's working directory is the next stage; #22423
remains open and carries those pins), the `declarativeStdio` policy
(default deny, exact allowlist), and ADR-0097's text.
## Measurements (the dispatch's four hypotheses, at `da159f74e6`)
- **H1, confirmed, with one sharpening.** `createPackageFileLoader`
(`plugin.ts:206`) held the whole rule inline: a non-empty check, an
absolute check (`path.isAbsolute` plus a drive-letter regex),
`path.resolve(packageRoot ?? process.cwd())`, then `path.relative`
refused when it starts with `..` or is absolute. **Symlink escapes are
not handled today**: the check is lexical, with no `realpath`, so a
symlink inside the root is judged by where it sits. The new member
matches it exactly because it calls the same helper rather than a copy;
a parity test pins "refused for confinement by both members or by
neither" over 11 refs.
- **H2, confirmed.** Every construction site of the context type in
`packages/` (tests included): one production host, the materializer in
`service-automation/src/plugin.ts`, which now hands the member.
Test-only sites: the `ctx()` helpers in
`connector-mcp/src/mcp-provider.test.ts`,
`connector-openapi/src/openapi-provider.test.ts` and
`connector-rest/src/rest-provider.test.ts`. Each builds a context
without the member, which still type-checks because it is optional, and
none of those factories reads it, so none needs it.
`connector-materialization.test.ts` and
`connector-reload-reingest.test.ts` only receive contexts from the
materializer.
- **H3, the note stays true and is not edited.**
`packages/spec/liveness/connector.json`'s `_note` (and its README row)
say `loadPackageFile` is host-injected, and that an authored entry
reaches exactly the author-supplied context fields plus `provider` and
`enabled`. A second host-injected member makes neither sentence false:
it is not author-supplied and it reads no entry key. `check:liveness` is
green on this head.
- **H4, consistent.** The loader falls back to `process.cwd()` at call
time when `packageRoot` is unset; the resolver does the same, through
the same helper (pinned: an unanchored resolver resolves `'.'` to
`path.resolve(process.cwd())`). `'.'` returns the root. For the loader,
`'.'` passes the same confinement check and then fails to read a
directory (`could not be read`), so the two members differ only where
one reads and the other does not.
## Pins
-
`packages/services/service-automation/src/connector-materialization.test.ts`,
eight new cases:
- the resolver: a relative path resolves under the root and `'.'` is the
root (including an inside-landing `./specs/../specs` and a path that
does not exist yet); `..`, `../outside` and `specs/../../outside` are
refused, naming the ref and the root; an absolute path (posix, `/etc`,
`C:\evil`) and an empty ref are refused; parity with the loader; the
`process.cwd()` default;
- the materializer: a factory resolving `'.'` and
`./scripts/fixture.mjs` gets locations under `packageRoot`; a factory
resolving `../outside` fails boot loudly with the confinement refusal; a
factory that never reads the member materializes exactly as before.
- Each refusal case also asserts the rejection is not a `TypeError`, so
a missing member cannot pass for a refusal.
- `packages/spec/src/integration/connector-provider.test.ts`: type-level
pins that the member is optional (`{} extends Pick` alias, which
`toEqualTypeOf` cannot tell apart from a required member typed with
`undefined`) and has exactly `loadPackageFile`'s type, plus a context
literal without the member that must keep compiling.
`check:test-typecheck` compiles this file.
## One-off proofs (run against committed head `159042aed0`, mutated
through `scripts/ablation-replace.mjs`, every leg restored and proven:
blob equals HEAD, `git diff HEAD` empty)
| leg | mutation | expected | observed |
|:---|:---|:---|:---|
| RV1 (reverse verification) | the materializer hands
`this.options.packageRoot` (a string) as the member |
`service-automation` tsc reads the rebuilt spec `.d.ts` and refuses it |
red: `src/plugin.ts(2022,17): error TS2322: Type 'string or undefined'
is not assignable to type '((relativePath: string) =) Promise(string))
or undefined'` (angle brackets and pipes respelled here) |
| A1 | the spec member made required | the optionality pins go red |
red: `check:test-typecheck` names
`src/integration/connector-provider.test.ts` with 4 type errors |
| A2 | the resolver bypasses the helper (bare `path.resolve`) | the
escape, absolute/empty, parity and escape-at-boot cases go red | red: 4
failed, 46 passed; exactly those four cases |
| A3 | the materializer anchors the resolver at `process.cwd()` | only
the anchored-locations case goes red | red: 1 failed, 49 passed; the one
red is that case |
## Verification
All of the following ran on head `159042aed0` (branch point
`da159f74e6`).
- **Build.** `pnpm turbo run build
--filter='@objectstack/service-automation...' --concurrency=2`, under
the verify lock: 30 of 30 tasks successful, `@objectstack/spec` and
`@objectstack/service-automation` rebuilt (cache misses). The rebuilt
`packages/spec/dist` carries `resolvePackagePath`.
- **Tests.** `@objectstack/service-automation` (`vitest run
--maxWorkers=2`): 178 files, 2187 tests passed. The touched file run
verbose: 50 passed, the eight new cases among them. `@objectstack/spec`
(`vitest run --project local --maxWorkers=2`): 627 files, 18758 passed,
1 todo. `connector-provider.test.ts` run verbose: 15 passed.
- **Typecheck.** `pnpm --filter @objectstack/service-automation
typecheck` and `pnpm --filter @objectstack/spec typecheck` (tsc, the
scripts program, `check:test-typecheck`): exit 0, both.
- **Gates.** `node scripts/pm/dispatch-gates.mjs --commands`, re-derived
on the actual diff: 87 families. That is the dispatch's list plus six
families the test-file edits add: `check:engine-double-contract`,
`check:objectql-double-limit`, `check:query-options-erasure`,
`check:type-check-coverage`, `check:type-check-debt`,
`check:where-matcher`. 86 exited 0. `check:dual-build-cjs-loads` exited
3, PREREQUISITE NOT MET: it reads every package's built output, and only
this diff's 30-package closure was built. It is NOT MEASURED here and
left to CI. `dispatch-gates --ran` answers: "87 derived famil(ies)
accounted for — 86 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit
3)".
- **Changeset level axis**, driven offline with a payload carrying this
body's first two lines: "this PR declares clause-② `yes`, and no package
whose `packages/**/src/**` it moves is graded `patch`".
- **Lint, narrowed (a measurement, not a skip).** `eslint
--no-inline-config --format json` over the four changed TypeScript
files: 4 files, 0 errors, 0 warnings. All four are in the linted
population (`--print-config` returns a rule set for each).
`eslint.config.mjs` enables no type-aware linting (no
`parserOptions.project`), so this diff cannot move any untouched file's
verdict. The repo-wide `pnpm lint` is CI's.
- **Branch freshness.** `origin/main` has moved four commits past the
branch point (to `3ca71b6e05`). None of them touches the five paths in
this diff, and a local merge-tree merges clean. CI runs on the merge
ref.
## Acceptance notes
- **Lexical confinement (both members).** Neither member follows
symlinks when judging confinement, which is the family's rule as it
stood. The authoring doors (`defineStack`, `PUT /meta/connector/:name`)
can set a path but cannot create a symlink, so the declared threat (an
entry reaching outside its package) is covered; a `realpath` rule would
change `loadPackageFile`'s behaviour and is not this card. Noted, not
filed. Carrier: none.
- **`..`-prefixed names are refused although they stay inside the
root.** `path.relative` of a file named `..foo` under the root is
`..foo`, which the `startsWith('..')` test refuses. It is loud (a
refusal, never a silent read), the loader has always done it, and it is
kept byte-identical here so the two members stay one rule. Noted, not
filed. Carrier: none.
- **ADR-0097** describes `loadPackageFile` only
(`docs/adr/0097-declarative-connector-instances.md`, the
`providerConfig.spec` bullet). A sentence for the sibling would be a
governed (Tier H) edit, so it is left out of this PR. Carrier: none.
- **Platform checklist.** The `integration-system` item pins the
loader's three strings, which are unchanged. No checklist item covers
the resolver yet; the user-visible behaviour arrives with the
connector-mcp stage. Carrier: #22423.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01VZqqwTj2wsihZEbfT6yyYN)_
Co-authored-by: Claude <noreply@anthropic.com>
feat(spec): `ConnectorProviderContext.resolvePackagePath` — a connector provider factory can resolve a path against the declaring app's root
7
+
8
+
Clause-②: yes (widening)
9
+
10
+
-**What is new.**`ConnectorProviderContext` gains one optional, host-provided member, `resolvePackagePath(relativePath): Promise<string>`, beside `loadPackageFile`. It resolves a relative path against the root of the stack or package that declared the connector entry and returns the absolute path. `'.'` returns the root itself. It refuses (throws on) an empty path, an absolute path, and a path that escapes the root after resolution, such as `../x` or `a/../../x`. That is the same rule `loadPackageFile` follows. It reads nothing and does not check that the path exists.
11
+
-**Who hands it.** The connector materializer in `@objectstack/service-automation` hands it to every provider factory. It is anchored at the plugin's `packageRoot` option, the same root `loadPackageFile` reads under, and falls back to `process.cwd()` the same way. The two members now share one confinement check, so they cannot disagree about what is inside the root. `loadPackageFile`'s behaviour and error messages are unchanged.
12
+
-**Who reads it.** Nothing yet. `@objectstack/connector-mcp` is next: a declarative stdio transport will use it as the launched process's working directory, so a relative command or argument resolves against the app's root instead of the directory the server was started from.
13
+
-**What does not change.** Which commands a declarative stdio transport may launch is untouched. A factory that does not read the member behaves as before.
14
+
-**Nothing to migrate.** A host that builds its own `ConnectorProviderContext` may leave the member out. A factory must then keep its existing behaviour, or fail with a clear message if it needs the root.
0 commit comments