feat(client): add TypeScript SDK#1015
Merged
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an ChangesClient package & integration
CLI mapping & loader
SDK and connector authoring
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![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
🧹 Nitpick comments (2)
packages/client/src/client.ts (1)
7-9: ⚡ Quick winMove
sessionsobject shape to a named interface.The inline object type for
sessionsshould be declared as aninterfaceto match the repo TypeScript shape convention.As per coding guidelines, "
**/*.{ts,tsx}: Useinterfacefor defining object shapes in TypeScript files".🤖 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 `@packages/client/src/client.ts` around lines 7 - 9, The inline type for the readonly sessions property should be extracted to a named interface: declare an interface (e.g., SessionsAPI) that defines create: (input: CreateSessionRequest) => Promise<AgentSession>, then update the client class/type to use readonly sessions: SessionsAPI; keep the existing names CreateSessionRequest and AgentSession unchanged so callers and types remain compatible.packages/client/src/types.ts (1)
24-43: ⚡ Quick winExtract nested object shapes into named interfaces.
CreateSessionRequeststill embeds object literal shapes (networkConfig,mcpServersvalue,nix). Please define these asinterfaces and reference them here to align with repo TypeScript conventions and keep contracts reusable.As per coding guidelines, "
**/*.{ts,tsx}: Useinterfacefor defining object shapes in TypeScript files".🤖 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 `@packages/client/src/types.ts` around lines 24 - 43, Create named interfaces for the embedded object shapes and reference them from CreateSessionRequest: extract networkConfig into an interface (e.g., NetworkConfig) with allowedDomains and deniedDomains, extract the mcpServers value shape into an interface (e.g., McpServerConfig) and use Record<string, McpServerConfig> for mcpServers, and extract nix into an interface (e.g., NixConfig) with flakeUrl and packages; then update the CreateSessionRequest type to use these interfaces instead of inline object literals (update any imports/exports as needed to follow the repository's interface usage convention).
🤖 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 `@packages/client/package.json`:
- Line 3: Remove the manual "version" field change you added (the "version":
"9.1.1" edit) in package.json — revert that line to its previous state or remove
the manual bump so the workspace manifest remains unchanged; do not add a
chore(release) or any manual version commit, as release-please will manage
package versions for the package's package.json "version" key.
---
Nitpick comments:
In `@packages/client/src/client.ts`:
- Around line 7-9: The inline type for the readonly sessions property should be
extracted to a named interface: declare an interface (e.g., SessionsAPI) that
defines create: (input: CreateSessionRequest) => Promise<AgentSession>, then
update the client class/type to use readonly sessions: SessionsAPI; keep the
existing names CreateSessionRequest and AgentSession unchanged so callers and
types remain compatible.
In `@packages/client/src/types.ts`:
- Around line 24-43: Create named interfaces for the embedded object shapes and
reference them from CreateSessionRequest: extract networkConfig into an
interface (e.g., NetworkConfig) with allowedDomains and deniedDomains, extract
the mcpServers value shape into an interface (e.g., McpServerConfig) and use
Record<string, McpServerConfig> for mcpServers, and extract nix into an
interface (e.g., NixConfig) with flakeUrl and packages; then update the
CreateSessionRequest type to use these interfaces instead of inline object
literals (update any imports/exports as needed to follow the repository's
interface usage convention).
🪄 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 Plus
Run ID: 3a59a16c-b926-4e00-bcac-b19cf9a807c6
⛔ Files ignored due to path filters (17)
bun.lockis excluded by!**/*.lockpackages/client/src/generated/client.gen.tsis excluded by!**/generated/**packages/client/src/generated/client/client.gen.tsis excluded by!**/generated/**packages/client/src/generated/client/index.tsis excluded by!**/generated/**packages/client/src/generated/client/types.gen.tsis excluded by!**/generated/**packages/client/src/generated/client/utils.gen.tsis excluded by!**/generated/**packages/client/src/generated/core/auth.gen.tsis excluded by!**/generated/**packages/client/src/generated/core/bodySerializer.gen.tsis excluded by!**/generated/**packages/client/src/generated/core/params.gen.tsis excluded by!**/generated/**packages/client/src/generated/core/pathSerializer.gen.tsis excluded by!**/generated/**packages/client/src/generated/core/queryKeySerializer.gen.tsis excluded by!**/generated/**packages/client/src/generated/core/serverSentEvents.gen.tsis excluded by!**/generated/**packages/client/src/generated/core/types.gen.tsis excluded by!**/generated/**packages/client/src/generated/core/utils.gen.tsis excluded by!**/generated/**packages/client/src/generated/index.tsis excluded by!**/generated/**packages/client/src/generated/sdk.gen.tsis excluded by!**/generated/**packages/client/src/generated/types.gen.tsis excluded by!**/generated/**
📒 Files selected for processing (16)
config/biome.config.jsonpackage.jsonpackages/client/openapi-ts.config.tspackages/client/package.jsonpackages/client/src/__tests__/client.test.tspackages/client/src/client.tspackages/client/src/errors.tspackages/client/src/index.tspackages/client/src/rest.tspackages/client/src/session.tspackages/client/src/types.tspackages/client/tsconfig.jsonrelease-please-config.jsonscripts/bump-version.mjsscripts/publish-packages.mjstsconfig.json
Functional sugar over ConnectorRuntime: declare a connector as a spec with per-feed sync and per-action execute handlers (feed/action keys derived from the record keys). Lowers to a ConnectorRuntime subclass so connector-worker's child-runner runs it unchanged; handler closures are stripped from the serializable definition. Unit test mirrors child-runner's findRuntimeClass detection to prove a bundled default export is picked up unchanged.
…ector Adds an optional authenticate(ctx) hook to the functional spec, lowered to ConnectorRuntime.authenticate. When omitted, the connector inherits the base behavior (throws). Closes the gap where interactive-auth connectors required the class form, making defineConnector feature-complete vs the class.
Scaffolds @lobu/sdk (Apache-2.0): defineConfig/defineAgent/defineEntityType/ defineRelationshipType/defineWatcher/defineConnection/defineAuthProfile + secret(), and re-exports defineConnector + TypeBox Type from connector-sdk so a project imports its whole authoring surface from one package. Producers are pure branded data with typed handles (EntityType -> relationship rules, Agent -> watcher); the CLI loader (next slice) maps them to DesiredState, which stays CLI-private per the apply-IR boundary. Excluded from root tsconfig like the other workspace packages (own tsconfig); wired into build:packages + root test.
Adds mapProjectToDesiredState — the single place that translates the public @lobu/sdk authoring objects (defineConfig default export) into the apply-private DesiredState IR. Maps agents (providers -> installedProviders/modelSelection, network, resolved provider keys), entity/relationship types (typed handles -> slugs), watchers (agent handle -> id, sources record -> array, notification), and connections/auth profiles (connector class -> key, secret() -> $VAR + required-secrets). The esbuild entrypoint loader + apply wiring follow next.
Adds loadDesiredStateFromConfig: esbuild-bundles lobu.config.ts (relative imports inlined; node_modules externalized so @lobu/sdk + @lobu/connector-sdk resolve from the project), imports the bundle to read the defineConfig() default export, and maps it via mapProjectToDesiredState. Dynamic imports are allow-listed in desired-state.ts (esbuild loaded lazily; bundle imported by URL). E2E test bundles a real fixture config end-to-end. Not yet wired into the apply command (next).
lobu apply now loads DesiredState from the TypeScript entrypoint when a lobu.config.ts is present, falling back to lobu.toml otherwise. Downstream apply logic (required-secrets gate, org resolution, diff, mutations) is source-agnostic — it operates on DesiredState.
Member
Author
|
bug_free 62, simplicity 68, slop 8, bugs 1, 0 blockers Typecheck/unit passed. [env] Integration failed before cases because DATABASE_URL pointed at database "postgres"; harness refused destructive setup. Explored CLI: help and examples/atlas validate exited 0. Probed mapper and found --only still validates/collects skipped families. Suggested fixes
Full verdict JSON{
"bug_free_confidence": 62,
"bugs": 1,
"slop": 8,
"simplicity": 68,
"blockers": [],
"change_type": "feat",
"behavior_change_risk": "high",
"tests_adequate": false,
"suggested_fixes": [
{
"file": "packages/cli/src/commands/_lib/apply/map-config.ts",
"line": 675,
"change": "Respect --only before mapping skipped families: set agents to [] for only === \"memory\" and entityTypes/relationshipTypes/watchers to [] for only === \"agents\", so skipped-family secrets and validation cannot block the apply."
},
{
"file": "packages/cli/src/commands/_lib/apply/__tests__/map-config.test.ts",
"line": 375,
"change": "Add tests that --only memory does not collect agent provider/platform secrets, and --only agents does not validate watcher schedules or memory-only definitions."
}
],
"notes": "Typecheck/unit passed. [env] Integration failed before cases because DATABASE_URL pointed at database \"postgres\"; harness refused destructive setup. Explored CLI: help and examples/atlas validate exited 0. Probed mapper and found --only still validates/collects skipped families.",
"categories": {
"src": 10803,
"tests": 6279,
"docs": 145,
"config": 8467,
"deps": 221,
"migrations": 20,
"ci": 56,
"generated": 4445
}
}Local review gate — branch protection can require the |
- BLOCKER (#976): @lobu/sdk re-exported defineConnector/Type through connector-sdk's barrel, which flakily fails under bun's ESM linker and broke the cli unit suite. Source Type/Static from @sinclair/typebox directly and deep-import defineConnector via a new connector-sdk '/define-connector' subpath export (bypasses the barrel). Full cli suite now 320 pass, 0 fail. - Thread --only into the TS loader/mapper so 'apply --only agents' doesn't demand connector secrets (matches the TOML loader). - Port the TOML structural validations into the mapper: connection/auth-profile slug patterns, cron schedules, duplicate feed keys, and forbidding credentials on interactive (oauth_account/browser_session) auth kinds — fail loud in the CLI before any remote mutation. - Register @lobu/sdk in release-please-config, bump-version, and publish-packages so it versions and publishes. Deferred (task #7): local defineConnector definitions authored in lobu.config.ts are not yet uploaded (connectors.definitions stays []); needs connector file-discovery wired into the TS loader.
…arity) cronError now rejects schedules firing more than once a minute, matching the TOML loader + server validation, so a sub-minute cron fails loud in the CLI instead of at server mutation. Closes the last codex parity gap (slice 3 now 93%).
…ig.ts
The TypeScript apply path (loadDesiredStateFromConfig) always set
connectors.definitions to []. A connector authored in ./connectors and
referenced by a connection resolved to a key but its source was never
uploaded, so apply failed "not installed".
Discover ./connectors/*.connector.ts (non-recursive, sorted, files only)
and ship each as a DesiredConnectorDefinition{ key: null, sourcePath,
sourceCode } — the same key-null contract the YAML loader uses for
auto-discovered connector files. apply-cmd then compiles each sourcePath on
the CLI and uploads it via install_connector; the server resolves the real
key. Skipped under --only agents|memory, matching the mapper.
Key resolution is intentionally deferred to the server (no eager
compile/instantiate at load time, which would force esbuild + installed
deps + module side effects on every load, including --dry-run).
…onfig The TS authoring path (defineAgent + mapProjectToDesiredState) only covered providers + network allowed/denied + the memory schema, while the TOML loader's buildAgentSettings lifts much more. Applying a migrated example would silently drop config (office-bot's egress judges, org metadata, etc.), blocking the TOML-deletion slice. Close that gap. defineAgent gains: network.judged + judges, egress, tools (preApproved/allowed/denied/strict), guardrails, nixPackages, mcpServers (typed type/authScope unions), plus preview and dir (consumed by a later `lobu run`/loader slice — not mapped into cloud settings; a test guards the preview non-leak). defineConfig gains orgName/orgDescription/organizationId. mapProjectToDesiredState now produces the same AgentSettings shape as buildAgentSettings: judgedDomains deduped by domain (last-wins), egressConfig, preApprovedTools + toolsConfig, guardrails, nixConfig, mcpServers (with the same loose cast for authScope/oauth), and collects $VAR/secret() refs from mcp headers/env + oauth clientId/clientSecret into the apply secrets gate. Org metadata maps into DesiredState.memory. Deferred to follow-ups: agent-dir SOUL/IDENTITY/USER markdown + skills/ loading (file IO + skill merge), platforms, dev.ts preview/dir wiring, example migration, TOML deletion.
buildAgentSettings lifts SOUL.md/IDENTITY.md/USER.md prompt markdown and local skills (project ./skills + per-agent <dir>/skills, with their network/nix/mcp declarations merged into agent settings); the TS config path did neither, so a migrated agent would lose its prompt files and skills. loadDesiredStateFromConfig now reads each agent's dir (defaulting to ./agents/<id>, overridable via defineAgent.dir) using the existing readMarkdown/loadSkillFiles/buildLocalSkills helpers, and merges the result via a new pure mergeAgentDirArtifacts(settings, markdown, skills). The mapper stays file-IO-free (unit-testable); the merge mirrors buildAgentSettings exactly: agent-level network/nix/mcp first, skills on top — allowed/denied/nix unioned+deduped (skill "*" dropped), judged-domains + judges agent-wins on conflict, skill MCP servers add only ids the agent didn't define.
These two DesiredWatcher fields are used by example watchers (8 and 2 uses respectively) but had no defineWatcher equivalent, so a migrated watcher would drop them. Add the SDK fields + mapper passthrough. The executable `reaction` (reaction_script) field still lands in the reactions slice.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tsconfig.json (1)
31-45:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing TypeScript path mappings for
@lobu/sdk.The
packages/sdkdirectory is added to build, test, publish, and version-bump scripts across this PR, and Line 56 excludes it from the root tsconfig (correct, since it has its own config). However, thepathssection is missing mappings for@lobu/sdkand@lobu/sdk/*.Without these mappings, imports like
import { defineAgent } from '@lobu/sdk'in other packages will fail TypeScript resolution.🔧 Proposed fix
Add the following entries to the
pathsobject:"`@lobu/client`": ["packages/client/src/index.ts"], "`@lobu/client/`*": ["packages/client/src/*"], + "`@lobu/sdk`": ["packages/sdk/src/index.ts"], + "`@lobu/sdk/`*": ["packages/sdk/src/*"], "`@lobu/openclaw-plugin`": ["packages/openclaw-plugin/src/index.ts"],🤖 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 `@tsconfig.json` around lines 31 - 45, The root tsconfig's "paths" is missing mappings for the new package identifiers `@lobu/sdk` and `@lobu/sdk/`*, so TypeScript in other packages cannot resolve imports like import { defineAgent } from '`@lobu/sdk`'; add two entries to the existing "paths" object that map "`@lobu/sdk`" to the SDK package entry (packages/sdk/src/index.ts) and "`@lobu/sdk/`*" to the SDK source files (packages/sdk/src/*) so imports across the monorepo resolve correctly.package.json (1)
26-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd SDK typecheck to
typecheck:packagesscript.The
build:packagesscript (Line 22) includes SDK, but this typecheck script omitscd ../sdk && bun run typecheck. This inconsistency means SDK type errors won't be caught by the packages typecheck workflow.🔍 Proposed fix
- "typecheck:packages": "cd packages/core && bun run typecheck && cd ../client && bun run typecheck && cd ../agent-worker && bun run typecheck", + "typecheck:packages": "cd packages/core && bun run typecheck && cd ../client && bun run typecheck && cd ../sdk && bun run typecheck && cd ../agent-worker && bun run typecheck",🤖 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 `@package.json` at line 26, The typecheck workflow defined by the "typecheck:packages" script is missing the SDK package; update the "typecheck:packages" script to include the SDK step (i.e., run "cd ../sdk && bun run typecheck") in the same sequential fashion as "build:packages" so SDK type errors are covered; locate the "typecheck:packages" script in package.json and insert the SDK command into the sequence.
🧹 Nitpick comments (1)
packages/connector-sdk/src/define-connector.ts (1)
75-75: ⚡ Quick winUse
interfacefor the constructor shape export.Line 75 should use an
interfaceinstead of atypealias to match the TS object-shape rule.As per coding guidelines, "`**/*.ts`: Use `interface` for defining object shapes; avoid `type` aliases where interfaces would be clearer".Suggested change
-export type ConnectorClass = new () => ConnectorRuntime; +export interface ConnectorClass { + new (): ConnectorRuntime; +}🤖 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 `@packages/connector-sdk/src/define-connector.ts` at line 75, Replace the exported constructor type alias with an interface: change the declaration of ConnectorClass from a type alias to an exported interface that describes the constructor signature (i.e., export interface ConnectorClass { new(): ConnectorRuntime }), keeping the same constructor shape and retaining the ConnectorRuntime reference.
🤖 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 `@package.json`:
- Line 19: The test:coverage npm script is missing packages/sdk/src while the
test script includes it; update the "test:coverage" script entry so it matches
the "test" script by adding packages/sdk/src to its argument list (modify the
"test:coverage" script in package.json to include packages/sdk/src alongside the
other package paths).
In `@packages/connector-sdk/src/__tests__/define-connector.test.ts`:
- Around line 83-93: The test cases that assert promise rejections are not
awaiting the Jest/Bun promise matcher, so failures can be missed; update the
test functions (e.g., the "sync throws for an unknown feed" test that calls new
Github().sync({...}) and the other similar rejection test around lines 155-159)
to be async and change the assertions to await
expect(<promise>).rejects.toThrow(...) so Bun's bun:test promise matchers run
reliably.
In `@packages/sdk/src/define.ts`:
- Around line 36-38: The factory currently spreads caller-provided config after
the discriminant (return { kind: "entityType", ...config }), allowing callers to
override kind at runtime; change the object shape to spread config first and set
kind last (return { ...config, kind: "entityType" }) in defineEntityType and
apply the same pattern to all other factory functions that set a discriminant
(the other factory returns referenced in the comment) so the discriminant cannot
be overridden by caller-provided properties.
---
Outside diff comments:
In `@package.json`:
- Line 26: The typecheck workflow defined by the "typecheck:packages" script is
missing the SDK package; update the "typecheck:packages" script to include the
SDK step (i.e., run "cd ../sdk && bun run typecheck") in the same sequential
fashion as "build:packages" so SDK type errors are covered; locate the
"typecheck:packages" script in package.json and insert the SDK command into the
sequence.
In `@tsconfig.json`:
- Around line 31-45: The root tsconfig's "paths" is missing mappings for the new
package identifiers `@lobu/sdk` and `@lobu/sdk/`*, so TypeScript in other packages
cannot resolve imports like import { defineAgent } from '`@lobu/sdk`'; add two
entries to the existing "paths" object that map "`@lobu/sdk`" to the SDK package
entry (packages/sdk/src/index.ts) and "`@lobu/sdk/`*" to the SDK source files
(packages/sdk/src/*) so imports across the monorepo resolve correctly.
---
Nitpick comments:
In `@packages/connector-sdk/src/define-connector.ts`:
- Line 75: Replace the exported constructor type alias with an interface: change
the declaration of ConnectorClass from a type alias to an exported interface
that describes the constructor signature (i.e., export interface ConnectorClass
{ new(): ConnectorRuntime }), keeping the same constructor shape and retaining
the ConnectorRuntime reference.
🪄 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 Plus
Run ID: ae25d823-59fe-4509-a232-20f2256e31bb
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
package.jsonpackages/cli/package.jsonpackages/cli/src/commands/_lib/apply/__tests__/load-config.test.tspackages/cli/src/commands/_lib/apply/__tests__/map-config.test.tspackages/cli/src/commands/_lib/apply/apply-cmd.tspackages/cli/src/commands/_lib/apply/desired-state.tspackages/cli/src/commands/_lib/apply/map-config.tspackages/connector-sdk/package.jsonpackages/connector-sdk/src/__tests__/define-connector.test.tspackages/connector-sdk/src/define-connector.tspackages/connector-sdk/src/index.tspackages/sdk/package.jsonpackages/sdk/src/__tests__/define.test.tspackages/sdk/src/define.tspackages/sdk/src/index.tspackages/sdk/src/secret.tspackages/sdk/tsconfig.jsonrelease-please-config.jsonscripts/bump-version.mjsscripts/publish-packages.mjstsconfig.json
✅ Files skipped from review due to trivial changes (2)
- release-please-config.json
- packages/cli/package.json
Comment on lines
+36
to
+38
| export function defineEntityType(config: Omit<EntityType, "kind">): EntityType { | ||
| return { kind: "entityType", ...config }; | ||
| } |
There was a problem hiding this comment.
Prevent runtime override of discriminant kind.
Line 37 pattern ({ kind: "...", ...config }) lets callers override kind at runtime, which can corrupt downstream mapping. Set kind last in each factory.
Suggested fix
export function defineEntityType(config: Omit<EntityType, "kind">): EntityType {
- return { kind: "entityType", ...config };
+ return { ...config, kind: "entityType" };
}
@@
export function defineRelationshipType(
config: Omit<RelationshipType, "kind">
): RelationshipType {
- return { kind: "relationshipType", ...config };
+ return { ...config, kind: "relationshipType" };
}
@@
export function defineAuthProfile(
config: Omit<AuthProfile, "kind">
): AuthProfile {
- return { kind: "authProfile", ...config };
+ return { ...config, kind: "authProfile" };
}
@@
export function defineConnection(config: Omit<Connection, "kind">): Connection {
- return { kind: "connection", ...config };
+ return { ...config, kind: "connection" };
}
@@
export function defineWatcher(config: Omit<Watcher, "kind">): Watcher {
- return { kind: "watcher", ...config };
+ return { ...config, kind: "watcher" };
}
@@
export function defineAgent(config: Omit<Agent, "kind">): Agent {
- return { kind: "agent", ...config };
+ return { ...config, kind: "agent" };
}
@@
export function defineConfig(config: Omit<Project, "kind">): Project {
- return { kind: "project", ...config };
+ return { ...config, kind: "project" };
}Also applies to: 50-54, 81-85, 111-113, 148-150, 266-268, 292-294
🤖 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 `@packages/sdk/src/define.ts` around lines 36 - 38, The factory currently
spreads caller-provided config after the discriminant (return { kind:
"entityType", ...config }), allowing callers to override kind at runtime; change
the object shape to spread config first and set kind last (return { ...config,
kind: "entityType" }) in defineEntityType and apply the same pattern to all
other factory functions that set a discriminant (the other factory returns
referenced in the comment) so the discriminant cannot be overridden by
caller-provided properties.
defineAgent() accepted `connections` and `schema` but mapProjectToDesiredState ignored them — a silent config drop. Connections and the memory schema are declared at the project level (defineConfig), matching the apply model (there is no agent-scoped association in DesiredState). Remove the dead fields rather than leave them silently ignored; project-level remains the wired path.
The generated client vendors its fetch runtime into src/generated/client/ — no runtime code imports the @hey-api/client-fetch npm package; it is used only by @hey-api/openapi-ts at generation time (openapi-ts.config.ts plugin). Move it to devDependencies so consumers of the published @lobu/client don't install it.
defineWatcher gains `reaction?: string` — a relative POSIX path to a sibling .ts reaction script. loadDesiredStateFromConfig validates + reads it (raw source) and attaches it to DesiredWatcher.reactionScript; apply ships it via the existing setReactionScript and the server compiles it. Mirrors the TOML loader's parseWatcher exactly: relative-POSIX/.ts/no-`..`/under-config-dir/ 256KB checks, present-but-empty rejected (gate on absence, not truthiness), raw-source contract. mapWatcher stays pure; the loader zips project.watchers[i].reaction -> state.watchers[i].reactionScript. Unblocks the 6 example watchers that use reaction_script. (The runAction / operations typing on ReactionClient is a separate enhancement — no example reaction calls connector actions; they only use client.knowledge.)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/cli/src/commands/_lib/apply/desired-state.ts`:
- Around line 2113-2126: The containment check for reaction scripts is platform-
and symlink-unsafe: replace the brittle startsWith(`${baseDir}/`) test by
resolving real paths and using path.relative to verify containment.
Specifically, call fs.realpathSync on baseDir and on the candidate path (abs)
before reading, then compute path.relative(realBaseDir, realAbs) and throw the
ValidationError if that relative path starts with '..' or is absolute (or equals
'' handling as needed). Keep the existing error messages (using watcherSlug and
trimmed) but use the resolved realAbs in messages and proceed to
readFileSync(realAbs) so symlink escapes are prevented; update references to
baseDir/abs in the function (variables baseDir, abs, trimmed, watcherSlug,
readFileSync) accordingly.
🪄 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 Plus
Run ID: 67ba1f42-5f64-4499-bfc5-8dd3bdeac3ca
📒 Files selected for processing (3)
packages/cli/src/commands/_lib/apply/__tests__/load-config.test.tspackages/cli/src/commands/_lib/apply/desired-state.tspackages/sdk/src/define.ts
mapAuthProfile shipped the literal `$VAR` placeholder for env/oauth_app credentials, whereas the TOML loader (loadConnectors) resolves them to the real env value before pushing to the DB — so a TS-config auth profile would write "$GITHUB_CLIENT_SECRET" instead of the secret. Add resolveCredentialValue (mirrors loadConnectors): secret()/$VAR creds resolve against env, the ref is still collected for the apply secrets gate. mcp oauth client_secret keeps the literal pass-through (matching buildAgentSettings). Found migrating lobu-crm.
Migrate every example from lobu.toml + models/*.yaml + connectors/*.yaml to a single TypeScript lobu.config.ts using the @lobu/sdk authoring API (define*). Each was verified to produce a DesiredState byte-identical to the legacy TOML loader (modulo object key order, installedAt timestamps, and the connector sourceFile error-label). Agent dirs, *.connector.ts, and *.reaction.ts files are unchanged (still file-based, referenced from the config). The old toml/yaml files remain for now; they are removed when the TOML loader is deleted in the next commit.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@examples/agent-community/lobu.config.ts`:
- Around line 174-176: In the exported defineConfig block, the org property is
incorrectly set to "market" which can route state to the wrong org; update the
org value in the defineConfig export (the org property alongside orgName) to
match this example’s identity (e.g., "agent-community") so it aligns with
orgName "Agent Community" and avoids collisions with the market example.
🪄 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 Plus
Run ID: e823162f-cb3c-4c39-a8f5-6367e2d57786
📒 Files selected for processing (14)
examples/agent-community/lobu.config.tsexamples/atlas/lobu.config.tsexamples/delivery/lobu.config.tsexamples/ecommerce/lobu.config.tsexamples/finance/lobu.config.tsexamples/leadership/lobu.config.tsexamples/legal/lobu.config.tsexamples/lobu-crm/lobu.config.tsexamples/market/lobu.config.tsexamples/office-bot/lobu.config.tsexamples/personal-finance/lobu.config.tsexamples/sales/lobu.config.tspackages/cli/src/commands/_lib/apply/__tests__/map-config.test.tspackages/cli/src/commands/_lib/apply/map-config.ts
…ig.ts Extract loadProjectConfig (bundle+import lobu.config.ts → SDK Project) and route all consumers through it / the TS loader: apply drops the lobu.toml fallback (always loadDesiredStateFromConfig); doctor, chat, validate, and dev's preview-bot registration read the SDK Project; dev's auto-apply gate keys on lobu.config.ts. Drop writeMemoryOrganizationId — TS projects carry org/ organizationId in defineConfig + the .lobu/project.json link, so apply never rewrites the config file. Update the dryrun + dev tests to lobu.config.ts fixtures (under the worktree so the externalized @lobu/sdk import resolves). The TOML loader (loadDesiredState/loadConnectors/parse*, config/loader, lobu-toml-schema) is now dead and removed in the next commit.
… YAML `lobu memory seed` read [memory] from lobu.toml and entity/relationship/watcher schema from models/*.yaml. Migrate it onto loadDesiredStateFromConfig: org + entity/relationship types now come from lobu.config.ts (same source apply uses, seeding stays idempotent). Drop watcher seeding — watchers are agent-scoped and provisioned by `lobu apply`, not the old entity-scoped models path. The ./data instance seeding (entities + relationships) is unchanged. This removes seed's last dependency on the TOML/YAML loader, unblocking its deletion.
With every consumer (apply, dev, doctor, chat, validate, seed) on
lobu.config.ts, the TOML/YAML loader is dead code. Remove it:
- packages/cli: config/loader.ts + config/schema.ts; the TOML functions in
desired-state.ts (loadDesiredState/loadConfig use, collectEnvRefs,
buildAgentSettings, buildPlatforms, parse{Entity,Relationship,Watcher}Type,
parse*Doc, loadMemoryModels, loadConnectors, rejectUnsupportedAgentShapes,
+ their TOML-only consts). Kept the TS-path + shared helpers (readMarkdown,
loadSkillFiles, buildLocalSkills, isRecord/asString, the connector-config
validators, loadProjectConfig/loadDesiredStateFromConfig). Drop smol-toml.
- packages/core: lobu-toml-schema.ts + its index.ts export block (no server
importers).
- Delete the 2 TOML-loader test files (coverage replaced by map-config.test.ts
+ load-config.test.ts) and the 2 core schema tests.
- Delete the examples' lobu.toml + models/*.yaml + connectors/*.yaml
(lobu.config.ts + *.connector.ts + *.reaction.ts + agent dirs remain).
8339 lines removed. cli/core/server typecheck clean; cli suite green.
Found via full lifecycle E2E (init → connector → watcher → apply → run → init-from-org), re-applying a stable config repeatedly: - I1 relationship-type rules churned a perpetual '~ rules' update because the rel-type `list` action omits rules — the diff compared desired rules against an always-empty remote. Hydrate remote rules (new client.listRelationshipTypeRules via the existing list_rules action) for the types the config declares with rules, so a matching set is a true noop. - I2 an omitted optional display name (feeds, connections) churned a perpetual '~ name' update: stringChanged(undefined, 'Ticks') is always true. Add optionalNameChanged — an omitted name means 'no opinion', so the server keeps its derived/stored name and it doesn't diff. - B2 follow-through: the earlier 'upsert every platform every apply' restored rotation but made the server restart the platform on EVERY apply (it can't tell the resolved secret matches the stored secret://), dropping the bot connection each deploy. Revert to upserting only diff-flagged platforms (idempotent — no restart churn); KEEP the diff removal-detection (a removed key is still applied). In-place opaque-secret rotation needs a secret-aware compare on the server upsert (owletto) — tracked as a follow-up. Verified: 7 consecutive applies of a stable config converge to '0 update, 9 noop' (connector re-push is by-design idempotent). CLI 316 green.
…arative rules) Fresh full-diff pi review (bug_free 58) — 2 blockers + 2 bugs, all fixed: - P1 write-mode requireRelationshipType did an unscoped slug lookup and threw 'Access denied' for a slug owned only by another (private) org — an existence oracle. Scope the lookup to ctx.organizationId; a missing own row is now 'not found'. A tenant can only update/delete its OWN types anyway (public foreign types stay referenceable-but-read-only). - P2 relationship-type rules are now fully declarative: upsertRelationshipType reconciles (add_rule for missing, remove_rule by id for extras) against the current remote set, and the snapshot hydrates rules for EVERY config-declared rel-type (incl. those declared with no rules) so dropping all rules is detected. Was add-only → removing a rule never took effect and churned a perpetual 'rules changed' update. - P3 migration archives pre-fix rel-type tombstones (deleted_at set but status='active') so they leave the WHERE status='active' partial unique index and re-create can't collide. - P4 init-from-org platform config emission uses emitKey() so a non-identifier config key (e.g. hyphenated) generates valid lobu.config.ts. Tests: prune integration 10/10 (embedded PG, +foreign-private 'not found'); CLI 316. Live E2E proved the rule reconcile: stable=noop, removal removes + converges, change add+removes + converges.
…UL delimiter Final pi review (bug_free 82, 0 blockers) — 1 medium bug + 1 hygiene: - envVarFor only normalized the slug, not the suffix, so a non-identifier platform config key (e.g. `bot-token`) produced an invalid POSIX env-var name (`BOT_TELEGRAM_BOT-TOKEN`) — the .env key would be rejected and apply's required-secret check would never pass. Normalize both parts. Added a regression test (quoted key + sanitized env var + round-trip). - The rules reconcile rule-key delimiter was an actual NUL byte, which made grep/rg treat client.ts as binary. Replaced with a tab (slugs never contain whitespace, so it's still an unambiguous composite key). CLI 317 green; no NUL bytes remain in cli/src.
Found via live prune E2E (the gap I flagged). With prune:true, apply marked the per-org system entity type $member 'will be deleted' (it's absent from config and can't be declared — the server reserves $ slugs). The delete is then refused because member rows exist, which HALTS the entire apply on first failure — so prune:true apply failed on every run for any org with members (i.e. every real org). If an org somehow had no member rows, it would instead DELETE the system type and corrupt the org. computeDiff now exempts $-prefixed entity/relationship/watcher definitions from prune: they stay ignorable drift in both modes, never delete. Regression test added (prune.test covers the server refusal; diff.test covers the verb). E2E: prune now creates all defs (no halt), $member stays drift, removing lead/knows/w-drop deletes exactly those, kept defs + $member survive, re-apply is a clean noop.
Closes the coverage gap: unit/integration prove config MAPS correctly, but nothing proved the whole SDK path RUNS. scripts/sdk-e2e.sh boots `lobu run` (embedded Postgres — the linux binary ships as an @embedded-postgres optional dep, the engine prod uses), auto-applies a prune:true fixture, and drives a REAL agent turn through a spawned worker against a deterministic mock OpenAI-compatible provider (scripts/sdk-e2e/, no provider key → reproducible). Asserts (non-zero exit → red CI): auto-apply completes (not halted — guards the $member prune-halt class), every definition is created, $member is never pruned, the agent turn returns the mock reply via the worker→secret-proxy→ upstream path, and a stable re-apply is idempotent (0 deletes). Wired as the CI `sdk-e2e` job (Node 22 for isolated-vm; failure fails the PR) and `make test-e2e-sdk`. Verified locally: 5/5 assertions pass.
The sdk-e2e gate boots `lobu run`'s embedded Postgres, whose PG18 binary is dynamically linked against ICU 60 (libicuuc.so.60). ubuntu-latest ships a newer ICU, so initdb failed to load the shared lib. Install the 18.04-era libicu60 from the Ubuntu archive (with a security-mirror fallback) before the gate, and ldd the initdb binary to surface any further missing libs. Prod is unaffected (external Postgres; embedded-postgres is pruned from the app image).
…pawns Embedded PG now boots on CI (libicu60), but the agent turn failed: the orchestrator wraps Linux workers in `systemd-run --user --scope`, which needs a user systemd/dbus session the CI runner doesn't have → worker exited 1. Set LOBU_DISABLE_SYSTEMD_RUN=1 for the gate (it only talks to the loopback mock; not testing the prod network sandbox) and install bubblewrap + enable unprivileged userns for the worker's exec-sandbox. No-op on macOS.
Clears the recurring biome useOptionalChain warning (p.id && p.id.startsWith → p.id?.startsWith) on this PR's platform round-trip code.
…her-reaction gates
Embedded Postgres portability (Task 1): the @embedded-postgres PG18 binaries
are NEEDED-linked against ICU 60 with an rpath of $ORIGIN/../lib, and that lib
dir already ships libicu{uc,i18n,data}.so.60.2 — it was only missing the .so.60
SONAME symlinks the loader resolves. scripts/sdk-e2e/fix-embedded-pg-icu.mjs
creates them (idempotent, Linux-only no-op on macOS), so initdb loads its
bundled ICU with zero system deps. Drops the fragile archive .deb download from
CI; now works identically on a local Linux dev box.
Expanded gate (Task 2):
- Connector sync: a local zero-dep ./connectors/pulse.connector.ts whose sync()
emits one event; a defineConnection wires its feed. The gate triggers an
immediate sync via manage_feeds(trigger_feed), polls the run to completed, and
asserts items_collected>=1 and feed event_count>=1 — proving the compiled
connector actually RUNS and persists, not just that apply mapped it.
- Watcher reaction: a ./reactions/digest.reaction.ts that saves an assertable
SDKE2E_REACTION_OK knowledge event. The gate triggers the watcher (asserts the
run row is enqueued) then deterministically drives read_knowledge ->
complete_window (the fixed-reply mock never produces the complete_window
tool-call) so the reaction fires on the connector-emitted window, and asserts
the side effect via query_sql.
API assertions use an mcp:admin PAT minted with lobu token create against the
local-install org. Gate stays deterministic with generous polling + clear
failures; teardown unchanged.
- client/src/rest.ts: drop the trailing-slash regex (`/\/+$/`) that tripped
CodeQL's polynomial-ReDoS check; strip with a plain slice instead.
- cli apply desired-state.ts: reaction-path containment check used a hard-coded
`${baseDir}/` prefix (POSIX-only) that rejects every path on Windows; use
path.relative + isAbsolute (cross-platform).
- sdk define.ts: spread `config` before `kind` in all define* factories so a
caller can't override the discriminant at runtime.
- examples/agent-community: org slug was "market" (wrong org); set to
"agent-community".
- root package.json: add packages/sdk/src to test:coverage (was tracked by
test only).
- connector-sdk define-connector test: await the .rejects assertion (was never
asserted — non-awaited promise matcher).
Skipped (false positive): client/package.json "version" — it's a NEW package
tracked in release-please-config.json, so an initial version field is expected;
release-please adopts it on first release.
Member
Author
Review comments addressed (commit
|
| # | Finding | Resolution |
|---|---|---|
| CodeQL | Polynomial ReDoS in client/src/rest.ts (/\/+$/) |
Replaced the trailing-slash regex with a plain slice — no backtracking. |
| CodeRabbit | sdk/define.ts lets callers override discriminant kind |
Spread config before kind in all define* factories. |
| CodeRabbit | desired-state.ts reaction containment uses POSIX ${baseDir}/ (breaks on Windows) |
Switched to path.relative + isAbsolute (cross-platform). |
| CodeRabbit | examples/agent-community org slug = "market" |
Fixed to "agent-community". |
| CodeRabbit | test:coverage omits packages/sdk/src |
Added it. |
| CodeRabbit | connector-sdk define-connector test: non-awaited .rejects |
Made the test async + await the assertion. |
| CodeRabbit | client/package.json manual version |
Skipped (false positive): @lobu/client is a new package tracked in release-please-config.json; an initial version field is required and release-please adopts it on first release. |
Also added a CI gate (sdk-e2e job) that boots lobu run against embedded Postgres (self-contained ICU via SONAME symlinks, no system deps), auto-applies a prune:true fixture, runs a real agent turn through a spawned worker against a mock provider, triggers a connector sync (asserts events emitted), and a watcher reaction (asserts the side effect) — failing the PR if the end-to-end path breaks.
…works under lobu run getLobuServiceToken inserts an oauth_token with client_id='lobu-internal' (FK → oauth_clients.id), but that system client is created by no migration or signup flow. On a fresh DB — notably the embedded `lobu run` one — the FK insert fails, the function returns null, and every watcher dispatch fails the run with 'Failed to generate an embedded Lobu service token' (automation.ts), i.e. watchers never fire locally. Notifications hit the same path. Ensure the credential-less system client exists (idempotent ON CONFLICT) before minting; it's only the FK anchor for these short-lived internal tokens, never used in a real OAuth grant. Verified: a triggered watcher now dispatches to a worker and the session completes (was: immediate token failure).
…-internal fix) Now that getLobuServiceToken ensures the lobu-internal oauth_client, the watcher trigger actually dispatches under lobu run. Strengthen the gate: assert the trigger returns a run_id, that dispatch did NOT fail on the service token, and that a watcher worker session started — directly guarding the dispatch fix. The deterministic complete_window reaction-side-effect assertion stays.
…p was lossy) emitRelationshipType already emits a rules: [...] block when the RemoteRelationshipType carries rules, but the rel-type list action omits them, so init --from-org always dropped every rule binding. Hydrate each type's rules via list_rules (best-effort per type) before emitting, mirroring the apply-side hydration. Added a test that lists WITHOUT rules + returns them from list_rules and asserts the emitted config + reloaded DesiredState carry the rule.
…orm channels) Full-diff pi review found the TS loader regressed two validations the old TOML/YAML loader did, so bad config now fails MID-apply (after mutations) instead of in the plan: - duplicate identifiers: reject dup agent ids / entity-type + relationship-type keys / watcher + connection + auth-profile slugs up front (assertUniqueBy). - platform channels: reject channels on non-slack platforms and malformed channel strings (must be "<teamId>/<channelId>") in the plan. Plus a @lobu/client doc note: send/events route via baseUrl+agentId (the server advertises same-origin URLs); the sseUrl/messagesUrl fields are surfaced for callers needing a divergent origin (follow-up). Tests for all four rejections; 12 examples still validate; CLI 322 green. Not changed: platform in-place secret rotation stays a documented limitation — opaque remote secrets can't be diffed and always-upsert restart-churns; the fix is a secret-aware compare in the server (owletto) upsert.
Branch commit a5dc5ff accidentally reset packages/owletto from 887e862 (merge-base + origin/main) back to ancestor 06b1543, which would regress main's pointer on merge and roll back the merged mac-app work. Advance to current owletto/main (01a242d); clears the Submodule Drift check. No package.json changed between the two SHAs, so the lockfile is unaffected (verified with bun install --frozen-lockfile).
examples/atlas migrated to lobu.config.ts (TS authoring SDK). The published @lobu/cli lags this repo and predates the TS-config loader, so the sync job failed with "No lobu.toml found". Build packages and run the in-repo CLI bin so the dry-run (PR) and apply --yes (main) dogfood the code in this commit.
Member
Author
Review summaryReviewed the full diff + ran Fixed in this push
Follow-ups (non-blocking, not addressed here)
|
This was referenced May 23, 2026
This was referenced May 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two things ship together here: the consumption SDK (
@lobu/client) and the authoring SDK (@lobu/sdk), and the latter fully replaces the oldlobu.toml/ YAML config path.@lobu/client(consumption):Lobu/AgentSessionwrapper (create, send, async-iterable SSE events).@lobu/sdk(authoring) —lobu.config.tsreplaces TOML/YAML:defineConfig/defineAgent/defineEntityType/defineRelationshipType/defineWatcher/defineConnection/defineAuthProfile/secret, plusdefineConnectorre-exported from@lobu/connector-sdk.lobu apply/run/init/validate/seedall read the TS entrypoint (jiti loader). The entire TOML/YAML parsing path is deleted (dead code removed).lobu.config.tsand parity-proven against the old loader.lobu init --from-org <slug>bootstraps a clean re-appliable project from an existing org (the inverse of apply).defineConfig({ prune: true })deletes org-owned definitions absent from config (data/connections/agents exempt; blast-radius confirm).Hardening (review blockers + idempotency)
A fresh full-diff review surfaced 6 blockers — all fixed and tested:
SecretCollectornow auto-registers thesecretimport (the MCP-oauthclientSecretpath emittedsecret(...)without it → uncompilable config).init --from-orgreads connectorauth_schema.methods(env_keysfields[].key, oauthclientIdKey/clientSecretKey), not a JSON-schema.propertiesshape.status='archived'to vacate the(org,slug) WHERE status='active'partial unique index, so prune → re-add of the same slug no longer hits a unique violation.init --from-orground-trips per-skillnetwork.judge/judgesintoSKILL.md.Two idempotency bugs found via E2E and fixed: rel-type rules churned a perpetual update (the
listaction omits rules → hydrate them into the diff snapshot); an omitted feed/connection display name churned (optionalNameChanged).Validation
bun test packages/client/src; full CLI suite (316) and server unit (201) green;bun run typecheck+ per-packagetscclean.lobu runauto-applies): init-from-scratch → connector + watcher + entities → apply (connector compiled + installed, all resources created) → worker booted; Slack preview claim minted;lobu init --from-orground-trip validates; 7 consecutive applies converge to noop; all 12 examples validate.Follow-ups (non-blocking)
init --from-org: relationship-rule round-trip and skill-levelmcpServersemission.Summary by CodeRabbit
New Features
@lobu/clientpackage with typed client, session API, REST/SSE messaging, error type, and public exports.@lobu/sdkwith declarative define-* helpers and secret refs; exported defineConnector from connector SDK.Tests
Chores