Lewis/improvements - #8
Merged
Merged
Conversation
- Updated .env.example to clarify the purpose of AGENT_BRIDGE_SECRET and its role in agent communication. - Added new hooks and utilities for better task management and logging within the agent. - Refactored existing code to streamline task dispatching and improve error handling. - Introduced new tools for writing workspace profiles and enriching company data. - Enhanced the overall structure and readability of the agent's codebase.
…sign-in configuration - Enhanced .env.example with detailed comments on GOOGLE_CLIENT_ID and GOOGLE_CLIENT_SECRET, explaining their roles in sign-in and Gmail/Calendar sync. - Updated environment.md to reflect the optional nature of Google sign-in variables and their interdependence, improving documentation clarity for new installations.
… messaging - Introduced `signsInWithGoogle` function to determine if a user is solely signed in with Google. - Updated `GoogleConnectionService` to include a `required` status indicating if Google sign-in is necessary. - Enhanced `ConnectGoogle` component to display specific error messages based on connection issues. - Modified `ConnectionsSettingsPage` to pass connection error messages to the `GoogleConnection` component. - Refactored Google token handling to improve account linking logic and streamline database queries.
- Moved onboarding state management from `onboardedAt` column to `metadata` in the `Organization` model. - Introduced `isOnboarded` and `markOnboarded` functions to handle onboarding status within metadata. - Updated `proxy.ts` to implement new onboarding gate logic, improving session handling and redirection. - Refactored onboarding-related components to remove unnecessary redirects and streamline user experience. - Enhanced documentation to clarify the new onboarding flow and its implications for workspace management.
- Enhanced the API proxy handler to catch fetch errors and return a 502 response with a descriptive error message when the target API is unreachable. - Introduced a new test suite for onboarding logic, covering various scenarios including API responses and session handling. - Implemented tests to ensure proper redirection based on onboarding status and to handle cases where the API cannot be accessed.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
24 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/app/app/(app)/settings/sso/add-sso-provider-sheet.tsx">
<violation number="1" location="apps/app/app/(app)/settings/sso/add-sso-provider-sheet.tsx:81">
P2: Mixed-case or padded provider names show and copy a redirect URI different from the URI actually registered. Normalize `providerId` before constructing the URI so administrators configure the endpoint the saved provider uses.</violation>
</file>
<file name="packages/auth/src/auth.ts">
<violation number="1" location="packages/auth/src/auth.ts:79">
P1: Organization client APIs and session types will drift because the browser client does not register the new organization plugin. Add the matching organization client plugin to `packages/auth/src/client.ts`.</violation>
</file>
<file name="apps/app/app/(app)/settings/workspace-form.tsx">
<violation number="1" location="apps/app/app/(app)/settings/workspace-form.tsx:49">
P2: Edits made while a save is pending are lost: inputs remain editable, then this callback clears the newer `draft` after the earlier request succeeds. Disable both inputs while `save.isPending` (or retain a draft that differs from submitted variables).</violation>
</file>
<file name="apps/agent/agent/lib/preamble.ts">
<violation number="1" location="apps/agent/agent/lib/preamble.ts:329">
P2: A workspace value such as `httpx://example.com` produces an invalid fetch target, so the workspace-profile task cannot read its own site. Validate/canonicalize workspace websites to an HTTP(S) URL/domain before generating or queueing this task; `startsWith("http")` accepts unsupported schemes.</violation>
</file>
<file name="apps/app/app/(auth)/onboarding/onboarding-form.tsx">
<violation number="1" location="apps/app/app/(auth)/onboarding/onboarding-form.tsx:61">
P2: The displayed company name cannot be accepted as-is: `placeholder` does not populate a required input, so users must retype the default/current name before continuing. Initialize the field with that workspace name so it is submitted unchanged when appropriate.</violation>
</file>
<file name="apps/agent/agent/lib/workspace.ts">
<violation number="1" location="apps/agent/agent/lib/workspace.ts:35">
P1: Website-derived profile text becomes executable session instructions for every later agent run. Treat the profile and section values as untrusted data in a clearly delimited block, with an instruction that their contents must never override agent rules or request tool actions.</violation>
<violation number="2" location="apps/agent/agent/lib/workspace.ts:52">
P3: `ourWebsite()` is newly added but has no callers anywhere in the repository, so it is dead code and an unused API surface. Removing it (or adding the intended call site) keeps the workspace helper focused on the functions actually used.</violation>
</file>
<file name="apps/api/src/backfill/backfill.service.ts">
<violation number="1" location="apps/api/src/backfill/backfill.service.ts:99">
P2: A workspace whose profile cannot be produced will enqueue and run another `workspace-profile` task after every automatic sweep, repeatedly spending agent work. Persist or query a retry timestamp/outcome and apply a cooldown before calling `workspaceChanged`.</violation>
<violation number="2" location="apps/api/src/backfill/backfill.service.ts:132">
P3: Automatic-backfill reporting omits artwork-only companies from `remaining`; e.g. 1,000 completed companies without artwork return zero remaining after this queues 500. Include the artwork candidate count (accounting for overlap) in the result.</violation>
<violation number="3" location="apps/api/src/backfill/backfill.service.ts:145">
P2: One company sweep can now enqueue up to 1,000 high-priority brand tasks, doubling the declared run cap and potentially crowding the direct-task lane. Limit the deduplicated union to `MAX_PER_RUN` before passing it to `backfill`.</violation>
</file>
<file name="apps/api/src/agent/agent-trigger.service.ts">
<violation number="1" location="apps/api/src/agent/agent-trigger.service.ts:200">
P3: A malformed `AGENT_URL` makes `new URL` throw before `fetch` returns a promise. `backfill()` then reports failure after persisting its tasks, rather than logging the failed poke and allowing the cron fallback; defer URL construction into the promise chain.</violation>
</file>
<file name="apps/api/src/workspace/workspace.service.ts">
<violation number="1" location="apps/api/src/workspace/workspace.service.ts:83">
P2: Invalid website text is accepted, marks onboarding complete, and queues an agent workspace-profile task despite the BadRequest message. Reuse `normalizeDomain` (or apply equivalent hostname validation) before persisting the value.</violation>
<violation number="2" location="apps/api/src/workspace/workspace.service.ts:234">
P1: Concurrent role changes can remove every workspace owner because the owner count and mutation are not atomic. Serialize this invariant in a transaction/conditional database operation before applying the demotion.</violation>
</file>
<file name="docs/agent.md">
<violation number="1" location="docs/agent.md:749">
P2: Queued tasks still sit in the dev queue when `AGENT_BRIDGE_SECRET` is unset because `AgentTriggerService.poke()` skips the request entirely; this makes the unqualified claim that the poke makes dev behave like production misleading for the documented optional-secret setup. Qualifying this behavior and pointing developers to the manual dispatch fallback would prevent a dead queue that looks like a slow agent.</violation>
<violation number="2" location="docs/agent.md:765">
P2: The manual dispatch description says every claimed task starts an Eve session, but `brand` and `portrait` tasks are deliberately handled directly without a session. Documenting the two lane limits and that only research tasks create sessions would keep the troubleshooting output and credit expectations accurate.</violation>
</file>
<file name="apps/app/app/(app)/settings/sso/copy-value.tsx">
<violation number="1" location="apps/app/app/(app)/settings/sso/copy-value.tsx:15">
P2: `navigator.clipboard` is undefined in non-secure contexts (HTTP), causing a synchronous TypeError that the `.catch()` on the promise cannot handle. Guard with an optional chain or check before calling.</violation>
</file>
<file name="packages/auth/test/organization.integration.spec.ts">
<violation number="1" location="packages/auth/test/organization.integration.spec.ts:17">
P3: Module-scoped `laterId` is set and read only inside the third `it` block. Declaring it at module level suggests cross-test state but no other test references it, making the suite harder to read and risking accidental interference if a later test mutates it.</violation>
<violation number="2" location="packages/auth/test/organization.integration.spec.ts:93">
P2: Test ordering depends on shared mutable state set by earlier tests. Module-level `firstId`, `secondId`, `snapshot` and database state created in test 1 are assumed by tests 2–4. Running a single test in isolation (`.only`, `--grep`) would fail.</violation>
</file>
<file name="apps/api/src/config/env.validation.ts">
<violation number="1" location="apps/api/src/config/env.validation.ts:53">
P3: API setup docs will still tell SSO-only deployments that Google credentials are mandatory, although this validation now permits their absence. Update the required-variable list and explain the Google/SSO condition alongside this change.</violation>
<violation number="2" location="apps/api/src/config/env.validation.ts:93">
P2: Invalid `AGENT_URL` values now pass startup validation, then `new URL("/internal/crm/dispatch", base)` throws when a task is queued. Validate this as a URL so a bad deployment fails at boot rather than breaking dispatch pokes.</violation>
</file>
<file name="packages/auth/test/sso.spec.ts">
<violation number="1" location="packages/auth/test/sso.spec.ts:20">
P2: This assertion is tautological: `ssoCallbackURL` is implemented as `${ssoCallbackBase()}/${providerId}`, so both sides of `toBe` call `ssoCallbackBase()`. The test can never fail independently — any change to `ssoCallbackBase()` affects both sides equally. Use a concrete URL string instead, e.g. `toBe("http://localhost:3001/api/auth/sso/callback/okta")`.
The second test in the same block already independently verifies the URL path structure.</violation>
</file>
<file name="packages/db/prisma/migrations/20260803162518_workspace_profile/migration.sql">
<violation number="1" location="packages/db/prisma/migrations/20260803162518_workspace_profile/migration.sql:6">
P2: An existing organization with an empty/whitespace website is incorrectly marked onboarded by this backfill and can bypass the required website setup. Restrict the migration to nonblank values so only an actual website counts as prior onboarding.</violation>
</file>
<file name="packages/auth/src/sso.ts">
<violation number="1" location="packages/auth/src/sso.ts:5">
P3: SSO configuration permissions can silently drift from workspace settings when role policy changes, since this repeats the rename/change-role predicate. Delegate to `canRenameWorkspace` or extract one shared permission helper; the accompanying test explicitly requires matching rename access.</violation>
</file>
<file name="packages/auth/src/organization.ts">
<violation number="1" location="packages/auth/src/organization.ts:83">
P2: A failed membership transaction still creates an authenticated session with no active workspace; when workspace creation failed, `WorkspaceService.get` also retries indefinitely. Propagating the error lets sign-in fail/retry instead of persisting this inconsistent session.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| hooks: {}, | ||
|
|
||
| plugins: [ | ||
| organization({ |
Contributor
There was a problem hiding this comment.
P1: Organization client APIs and session types will drift because the browser client does not register the new organization plugin. Add the matching organization client plugin to packages/auth/src/client.ts.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/auth/src/auth.ts, line 79:
<comment>Organization client APIs and session types will drift because the browser client does not register the new organization plugin. Add the matching organization client plugin to `packages/auth/src/client.ts`.</comment>
<file context>
@@ -72,6 +75,33 @@ export const auth = betterAuth({
hooks: {},
+ plugins: [
+ organization({
+ allowUserToCreateOrganization: false,
+ disableOrganizationDeletion: true,
</file context>
| return lines.join("\n"); | ||
| } | ||
|
|
||
| lines.push(us.profile.narrative, ""); |
Contributor
There was a problem hiding this comment.
P1: Website-derived profile text becomes executable session instructions for every later agent run. Treat the profile and section values as untrusted data in a clearly delimited block, with an instruction that their contents must never override agent rules or request tool actions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/lib/workspace.ts, line 35:
<comment>Website-derived profile text becomes executable session instructions for every later agent run. Treat the profile and section values as untrusted data in a clearly delimited block, with an instruction that their contents must never override agent rules or request tool actions.</comment>
<file context>
@@ -0,0 +1,54 @@
+ return lines.join("\n");
+ }
+
+ lines.push(us.profile.narrative, "");
+
+ const { sells, sellsTo, edge } = us.profile.sections;
</file context>
| ...(dealsOnly ? { deals: { some: {} } } : {}), | ||
| }; | ||
|
|
||
| const artworkRows = await this.db.company.findMany({ |
Contributor
There was a problem hiding this comment.
P3: Automatic-backfill reporting omits artwork-only companies from remaining; e.g. 1,000 completed companies without artwork return zero remaining after this queues 500. Include the artwork candidate count (accounting for overlap) in the result.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/backfill/backfill.service.ts, line 132:
<comment>Automatic-backfill reporting omits artwork-only companies from `remaining`; e.g. 1,000 completed companies without artwork return zero remaining after this queues 500. Include the artwork candidate count (accounting for overlap) in the result.</comment>
<file context>
@@ -107,12 +124,42 @@ export class BackfillService implements OnModuleInit {
+ ...(dealsOnly ? { deals: { some: {} } } : {}),
+ };
+
+ const artworkRows = await this.db.company.findMany({
+ where: artwork,
+ orderBy: { createdAt: "asc" },
</file context>
Comment on lines
+200
to
+209
| void fetch(new URL("/internal/crm/dispatch", base), { | ||
| method: "POST", | ||
| headers: { authorization: `Bearer ${secret}` }, | ||
| signal: AbortSignal.timeout(POKE_TIMEOUT_MS), | ||
| }).catch((error) => { | ||
| this.logger.debug({ | ||
| message: "Agent poke did not land; the cron will pick this up", | ||
| reason: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
P3: A malformed AGENT_URL makes new URL throw before fetch returns a promise. backfill() then reports failure after persisting its tasks, rather than logging the failed poke and allowing the cron fallback; defer URL construction into the promise chain.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/agent/agent-trigger.service.ts, line 200:
<comment>A malformed `AGENT_URL` makes `new URL` throw before `fetch` returns a promise. `backfill()` then reports failure after persisting its tasks, rather than logging the failed poke and allowing the cron fallback; defer URL construction into the promise chain.</comment>
<file context>
@@ -150,11 +181,31 @@ export class AgentTriggerService {
+
+ const base = process.env.AGENT_URL?.trim() || "http://127.0.0.1:2000";
+
+ void fetch(new URL("/internal/crm/dispatch", base), {
+ method: "POST",
+ headers: { authorization: `Bearer ${secret}` },
</file context>
Suggested change
| void fetch(new URL("/internal/crm/dispatch", base), { | |
| method: "POST", | |
| headers: { authorization: `Bearer ${secret}` }, | |
| signal: AbortSignal.timeout(POKE_TIMEOUT_MS), | |
| }).catch((error) => { | |
| this.logger.debug({ | |
| message: "Agent poke did not land; the cron will pick this up", | |
| reason: error instanceof Error ? error.message : String(error), | |
| }); | |
| }); | |
| void Promise.resolve() | |
| .then(() => | |
| fetch(new URL("/internal/crm/dispatch", base), { | |
| method: "POST", | |
| headers: { authorization: `Bearer ${secret}` }, | |
| signal: AbortSignal.timeout(POKE_TIMEOUT_MS), | |
| }), | |
| ) | |
| .catch((error) => { | |
| this.logger.debug({ | |
| message: "Agent poke did not land; the cron will pick this up", | |
| reason: error instanceof Error ? error.message : String(error), | |
| }); | |
| }); |
| let snapshot: Snapshot; | ||
| let firstId: string; | ||
| let secondId: string; | ||
| let laterId: string; |
Contributor
There was a problem hiding this comment.
P3: Module-scoped laterId is set and read only inside the third it block. Declaring it at module level suggests cross-test state but no other test references it, making the suite harder to read and risking accidental interference if a later test mutates it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/auth/test/organization.integration.spec.ts, line 17:
<comment>Module-scoped `laterId` is set and read only inside the third `it` block. Declaring it at module level suggests cross-test state but no other test references it, making the suite harder to read and risking accidental interference if a later test mutates it.</comment>
<file context>
@@ -0,0 +1,140 @@
+let snapshot: Snapshot;
+let firstId: string;
+let secondId: string;
+let laterId: string;
+
+const seedUser = async (label: string, createdAt: Date): Promise<string> => {
</file context>
| "GOOGLE_CLIENT_ID is required — Google is the only sign-in method. Create an OAuth client ID (web) in the Google Cloud console.", | ||
| }) | ||
| GOOGLE_CLIENT_ID!: string; | ||
| GOOGLE_CLIENT_ID?: string; |
Contributor
There was a problem hiding this comment.
P3: API setup docs will still tell SSO-only deployments that Google credentials are mandatory, although this validation now permits their absence. Update the required-variable list and explain the Google/SSO condition alongside this change.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/config/env.validation.ts, line 53:
<comment>API setup docs will still tell SSO-only deployments that Google credentials are mandatory, although this validation now permits their absence. Update the required-variable list and explain the Google/SSO condition alongside this change.</comment>
<file context>
@@ -48,16 +48,13 @@ export class EnvironmentVariables {
- "GOOGLE_CLIENT_ID is required — Google is the only sign-in method. Create an OAuth client ID (web) in the Google Cloud console.",
- })
- GOOGLE_CLIENT_ID!: string;
+ GOOGLE_CLIENT_ID?: string;
+ @IsOptional()
</file context>
| return lines.join("\n"); | ||
| } | ||
|
|
||
| export async function ourWebsite(): Promise<string | null> { |
Contributor
There was a problem hiding this comment.
P3: ourWebsite() is newly added but has no callers anywhere in the repository, so it is dead code and an unused API surface. Removing it (or adding the intended call site) keeps the workspace helper focused on the functions actually used.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/agent/agent/lib/workspace.ts, line 52:
<comment>`ourWebsite()` is newly added but has no callers anywhere in the repository, so it is dead code and an unused API surface. Removing it (or adding the intended call site) keeps the workspace helper focused on the functions actually used.</comment>
<file context>
@@ -0,0 +1,54 @@
+ return lines.join("\n");
+}
+
+export async function ourWebsite(): Promise<string | null> {
+ return (await identity())?.website ?? null;
+}
</file context>
- Added a new `release-please` configuration to manage versioning and changelog generation. - Introduced CI workflow to automate checks for types, linting, and testing on pull requests and main branch pushes. - Updated `package.json` to include versioning and specify the package manager as Bun. - Created a release manifest to track versioning for the project. - Enhanced the contributing guidelines to clarify the release process and commit message conventions.
- Removed the "package-name" field from the release-please configuration in `release-please-config.json`, streamlining the setup for versioning and changelog generation.
…date logic - Added a new `fillable` function to determine which fields can be updated based on the current company snapshot. - Updated the `brandToUpdate` function to utilize `fillable`, improving the logic for updating brand fields. - Introduced `stillFillable` function to filter out fields that should not be updated after a placeholder has been answered. - Enhanced tests for `brandToUpdate` and added tests for `stillFillable` to ensure correct functionality.
The API's sign-in-options e2e asserts google: true, and @crm/auth snapshots GOOGLE_CLIENT_ID at module-evaluation time. A runner has no .env, so the snapshot was taken empty and the assertion failed on CI while passing on any machine with a stale apps/api/.env for bun to auto-load.
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
1 participant
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 by cubic
Adds single‑workspace roles and SSO, gated onboarding with workspace settings, and rebuilds agent dispatch into lanes with priorities. Brand enrichment avoids clobbering rep edits. The API proxy returns a clear 502 when unreachable, release automation uses
release-please, and CI now sets a Google client pair so sign‑in tests pass.New Features
@better-auth/sso; add/delete providers in Settings → SSO; sign‑in lists Google only if configured and shows SSO options.stillFillableavoids clobbering rep input; newwrite_workspace_profiletool; preamble ends with “Who we are”; agent can be poked via API.configured,linked, andrequired; task triggers get kind‑specific priorities; backfill sweeps workspace and brand; public route exposes sign‑in options for the sign‑in page..envclarify Google pairing and the agent bridge; tests added (lanes, preamble, workspace, onboarding gate, brand fill rules).Migration
@better-auth/sso) and run DB migrations.ALLOWED_SIGN_IN. If using Google, set bothGOOGLE_CLIENT_IDandGOOGLE_CLIENT_SECRET; otherwise configure SSO in Settings → SSO.AGENT_URLandAGENT_BRIDGE_SECRETso the API can notify the agent between cron runs.Written for commit 42b4460. Summary will update on new commits.