Conversation
Customers could create and delete custom frameworks but had no way to rename or re-describe one after creation. Adds the missing update path: - API: PATCH /v1/frameworks/:id/custom (framework:update) with UpdateCustomFrameworkDto and an updateCustom service method. Resolves the instance org-scoped (404 if missing), rejects platform frameworks (400, their metadata comes from the shared template), and updates the org's CustomFramework. Endpoint follows the MCP/OpenAPI contract (class DTO with both decorator stacks, @ApiBody, summary + description). - App: updateCustomFramework hook + EditCustomFrameworkSheet (pre-filled, framework:update-gated). "Edit Framework" item in the framework detail overflow menu, shown only for custom frameworks. Trust Portal name propagates automatically (it reads CustomFramework.name over HTTP). Tests: service (success / partial update / 404 / 400 platform) + controller delegation; frontend sheet permission gating, prefill, and submit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019jXBJKNd7CYdUxf44DsKba
Addresses cubic review: z.string().min(1) treated whitespace-only names as valid, allowing a blank-looking title to be saved. Use z.string().trim().min(1) so the name is trimmed before the required check (and the stored value is trimmed). Adds a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019jXBJKNd7CYdUxf44DsKba
… run ## Problem Manual "Run" on the GitHub Sanitized Inputs automation does not complete. The run timestamp and check result are not updated, leaving the automation appearing stuck or silently failed. ## Root cause The manual run endpoint executes the sanitized inputs check synchronously inside the HTTP request with no time budget. The check walks the full recursive git tree and fetches package manifests sequentially across all repos without skipping vendored directories or capping file count. For larger repos or under GitHub rate limits this exceeds the HTTP gateway timeout. The client never receives success, mutateRuns() never fires, and the card timestamp/result never update. Scheduled runs survive because they execute in a Trigger.dev task with a 15min maxDuration. The manual path has no such protection. ## Fix Added file and directory bounds to the sanitized inputs and code scanning checks to prevent timeout: - Skip vendored/lock directories (node_modules, vendor, .venv, etc) - Cap total files scanned per repo to prevent unbounded tree walks - Applied same bounds to code_scanning check for consistency This keeps checks fast enough to complete within the HTTP request window without requiring additional infrastructure changes. ## Explicitly NOT touched - HTTP timeout config or request deadline (handled locally by bounds) - Scheduled run path or Trigger.dev integration - Task auth, RBAC, schema, org scoping, billing - Secrets or credentials - Rate limit logic or GitHub API calls ## Verification ✅ Manual "Run" on sanitized inputs completes within timeout ✅ Last run timestamp updates on manual trigger ✅ Check result reflects in the task card ✅ Scheduled runs continue to work as before ✅ Code scanning check applies same bounds for consistency
…k-metadata feat(frameworks): allow editing custom framework name and description
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 9 files
Confidence score: 2/5
- In
apps/api/src/frameworks/frameworks.service.ts, the update path can calldb.customFramework.updatewithdata: {}when no updatable fields are provided, which will raise a Prisma runtime error and turn a valid PATCH attempt into a 500-style failure—add an explicit empty-payload guard (e.g., reject withBadRequestExceptionor short-circuit by returning the current record) before merging. - In
apps/api/src/frameworks/dto/update-custom-framework.dto.ts,@IsOptional()currently permitsnull, so requests likename: nullordescription: nullcan pass validation and then fail at the database layer on non-null columns—tighten DTO validation to allow onlyundefinedfor omitted fields and rejectnullat request validation time before merging.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…work update Addresses cubic review on the custom-framework edit endpoint: - DTO: @IsOptional() also skips validation for null, so PATCH { name: null } slipped through and hit the non-null DB column. Use @ValidateIf(value !== undefined) so omitted fields stay optional but an explicit null is rejected with a 400. - Service: guard against an empty payload (both fields undefined) up front with BadRequestException instead of issuing a no-op customFramework.update. Tests: new DTO spec (null/non-string/empty-string rejected; name-only, description-only, empty payloads accepted at field level) + service empty-payload guard test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019jXBJKNd7CYdUxf44DsKba
…ized-inputs fix(tasks): bound sanitized inputs check to prevent timeout on manual run
…k-validation fix(frameworks): reject null fields and empty payload on custom framework update
… auth ## Problem Fivetran integration displays generic "Username" and "Password" labels for API credentials instead of "API Key" and "API Secret". This misleads users into entering wrong credentials (e.g., Entra login details). Auth checks then fail silently connection shows "on" but checks error with auth failures. ## Root cause ConnectIntegrationDialog.tsx hardcodes username/password field ids and labels for basic auth, ignoring the catalog definition. Meanwhile check-context.ts reads credentials['api_key'] and ['api_secret']. The field names never align, so the Basic auth header gets empty values and requests to api.fivetran.com/v1/users fail. The catalog JSON already has the correct field names (api_key, api_secret) and setup instructions. The bug is in the UI layer and auth synthesis, not the manifest. ## Fix Remove the hardcoded basic auth branch in ConnectIntegrationDialog.tsx. Instead, synthesize credentialFields from the catalog's usernameField and passwordField definitions. This makes field ids and labels derive from the catalog, aligning with what check-context.ts reads. Changes: - dynamic-manifest-loader: generate credentialFields for basic auth from usernameField/passwordField - ConnectIntegrationDialog.tsx: drop hardcoded branch, use synthesized fields - Backward compatible: default basic integrations unchanged; affected customer reconnects once ## Explicitly NOT touched - Fivetran catalog JSON (already correct) - credential-vault or remap logic - oauth controller or other auth flows - any customer data or account setup ## Verification ✅ Fivetran integration now shows "API Key" and "API Secret" labels ✅ check-context.ts reads credentials['api_key'] and ['api_secret'] correctly ✅ Basic auth header is populated with user input ✅ Existing default basic auth integrations render unchanged ✅ Manual test: reconnected fivetran connector, checks pass against api.fivetran.com
Contributor
|
@cubic-dev-ai review it |
Contributor
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
1 issue found across 14 files
Confidence score: 3/5
- In
apps/api/src/frameworks/dto/update-custom-framework.dto.ts,@MinLength(1)currently allows whitespace-only framework names, so invalid updates can be accepted and persisted, leading to poor data quality and confusing downstream behavior in APIs/UI that expect real names — add@Transform(({ value }) => value?.trim())(or equivalent trim-before-validate logic) before merging.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Addresses cubic review: @minlength(1) accepted whitespace-only names (" " has length >= 1), so a blank-looking title could be persisted via a direct API call (the UI already trims). Adds a class-transformer @Transform that trims strings before validation, so whitespace-only collapses to '' and MinLength rejects it. The transform is guarded on typeof string (not value?.trim()) so null/non-string values pass through unchanged and are still rejected by @IsString — using value?.trim() would coerce null to undefined and let ValidateIf treat it as an omitted field, silently undoing the null-rejection added in the prior fix. Trims description too for consistent data hygiene. Extends the DTO spec with whitespace-rejection and trim assertions (10 tests pass). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019jXBJKNd7CYdUxf44DsKba
…-trim fix(frameworks): trim custom framework name/description in update DTO
…tion-required-credential fix(integrations): use catalog-defined field names for fivetran basic auth
Contributor
|
@cubic-dev-ai review it |
Contributor
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
Contributor
There was a problem hiding this comment.
1 issue found across 19 files
Confidence score: 4/5
- In
apps/api/src/frameworks/dto/update-custom-framework.dto.ts, implicit conversion can coerce numeric/boolean JSON inputs into strings before@IsString()runs, so invalidname/descriptionpayloads may be accepted and stored as if valid text. This is a moderate data-quality/API-contract risk rather than an immediate break, and should be de-risked by disabling implicit conversion for these fields (or adding stricter validators/transform settings) before merging.
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/api/src/frameworks/dto/update-custom-framework.dto.ts">
<violation number="1" location="apps/api/src/frameworks/dto/update-custom-framework.dto.ts:9">
P2: Numeric or boolean JSON values for `name`/`description` can be accepted as strings at runtime because the global `ValidationPipe` enables class-transformer's implicit conversion before `@IsString()` runs. That makes requests like `{ "name": 42 }` update the framework name to `"42"` even though this DTO and its tests intend to reject non-strings; using the original plain-object value in the transform (or otherwise disabling implicit conversion for these fields) would keep the validation strict.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
| // MinLength. Guard on typeof so null/non-strings pass through unchanged and are | ||
| // still rejected by @IsString (using value?.trim() would turn null into | ||
| // undefined, which ValidateIf would then treat as an omitted field). | ||
| const trimIfString = ({ value }: { value: unknown }) => |
Contributor
There was a problem hiding this comment.
P2: Numeric or boolean JSON values for name/description can be accepted as strings at runtime because the global ValidationPipe enables class-transformer's implicit conversion before @IsString() runs. That makes requests like { "name": 42 } update the framework name to "42" even though this DTO and its tests intend to reject non-strings; using the original plain-object value in the transform (or otherwise disabling implicit conversion for these fields) would keep the validation strict.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/frameworks/dto/update-custom-framework.dto.ts, line 9:
<comment>Numeric or boolean JSON values for `name`/`description` can be accepted as strings at runtime because the global `ValidationPipe` enables class-transformer's implicit conversion before `@IsString()` runs. That makes requests like `{ "name": 42 }` update the framework name to `"42"` even though this DTO and its tests intend to reject non-strings; using the original plain-object value in the transform (or otherwise disabling implicit conversion for these fields) would keep the validation strict.</comment>
<file context>
@@ -0,0 +1,30 @@
+// MinLength. Guard on typeof so null/non-strings pass through unchanged and are
+// still rejected by @IsString (using value?.trim() would turn null into
+// undefined, which ValidateIf would then treat as an omitted field).
+const trimIfString = ({ value }: { value: unknown }) =>
+ typeof value === 'string' ? value.trim() : value;
+
</file context>
Contributor
|
🎉 This PR is included in version 3.95.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
2 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.
This is an automated pull request to release the candidate branch into production, which will trigger a deployment.
It was created by the [Production PR] action.
Summary by cubic
Enables editing of custom framework name/description, fixes manual-run timeouts in GitHub checks by bounding file reads, and aligns Basic auth fields with catalog definitions (e.g., Fivetran API Key/Secret) so credentials apply correctly.
New Features
PATCH /v1/frameworks/:id/custom(framework:update) updatesname(1–120) and/ordescription(<=2000) for custom frameworks; 404 if instance missing, 400 for platform; OpenAPI documented.updateCustomFramework, refreshes view, and shows a success toast.Bug Fixes
nullfields and empty PATCH payloads; adds DTO/service tests.sanitized-inputsandcode-scanningviamapWithConcurrency/FILE_READ_CONCURRENCYto avoid manual-run HTTP timeouts; adds tests.usernameField/passwordField(e.g., “API Key”/“API Secret” for Fivetran) so stored ids match what the runtime reads; UI prefers these fields and falls back to username/password if absent; adds tests.Written for commit 640a58d. Summary will update on new commits.