-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(v3): allow triggerOptions in ai.tool and fix shadowing bug #4465
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
73eea12
996dabf
88f1eda
4536dfe
6ffe00b
ea3c873
d397a4b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| --- | ||
| "@trigger.dev/build": patch | ||
| "trigger.dev": patch | ||
| "@trigger.dev/core": patch | ||
| "@internal/clickhouse": patch | ||
| "webapp": patch | ||
| --- | ||
|
|
||
| fix: resolve collection of CLI and UI bugs (#3168, #3105, #3139) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -514,7 +514,7 @@ export function ConnectGitHubRepoModal({ | |
| <TextLink | ||
| target="_blank" | ||
| rel="noreferrer noopener" | ||
| to={`https://github.com/settings/installations/${selectedInstallation?.appInstallationId}`} | ||
| to={`https://github.com/apps/trigger-dev-app/installations/${selectedInstallation?.appInstallationId}`} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Repository access link points at trigger.dev's hosted GitHub app for every installation The "configure repository access" link is now built with a hard-coded application name ( Configured app slug is availableThe GitHub app slug is configurable via Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| > | ||
| GitHub | ||
| </TextLink> | ||
|
|
@@ -632,9 +632,9 @@ export function ConnectedGitHubRepoForm({ | |
| useEffect(() => { | ||
| const hasChanges = | ||
| gitSettingsValues.productionBranch !== | ||
| (connectedGitHubRepo.branchTracking?.prod?.branch || "") || | ||
| (connectedGitHubRepo.branchTracking?.prod?.branch || "") || | ||
| gitSettingsValues.stagingBranch !== | ||
| (connectedGitHubRepo.branchTracking?.staging?.branch || "") || | ||
| (connectedGitHubRepo.branchTracking?.staging?.branch || "") || | ||
| gitSettingsValues.previewDeploymentsEnabled !== connectedGitHubRepo.previewDeploymentsEnabled; | ||
| setHasGitSettingsChanges(hasChanges); | ||
| }, [gitSettingsValues, connectedGitHubRepo]); | ||
|
|
@@ -898,6 +898,6 @@ export function GitHubSettingsPanel({ | |
| </Hint> | ||
| )} | ||
| </div> | ||
|
|
||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ const emailStrategy = new EmailLinkStrategy( | |
| secret, | ||
| callbackURL: "/magic", | ||
| sessionMagicLinkKey: "triggerdotdev:magiclink", | ||
| validateSession: false, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔍 Magic-link option name may not match the strategy's API
Was this helpful? React with 👍 or 👎 to provide feedback. |
||
| }, | ||
| async ({ | ||
| email, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,18 @@ | ||
| FROM golang | ||
|
|
||
|
|
||
| RUN go install github.com/pressly/goose/v3/cmd/goose@latest | ||
|
|
||
|
|
||
| WORKDIR /app | ||
| COPY ./schema ./schema | ||
| COPY ./cmd ./cmd | ||
| COPY ./migrate.sh ./migrate.sh | ||
|
|
||
| RUN go build -o /usr/local/bin/transform ./cmd/transform/main.go | ||
| RUN chmod +x ./migrate.sh | ||
|
|
||
| ENV GOOSE_DRIVER=clickhouse | ||
| ENV GOOSE_DBSTRING="tcp://default:password@clickhouse:9000" | ||
| ENV GOOSE_MIGRATION_DIR=./schema | ||
| CMD ["goose", "up"] | ||
|
|
||
| ENTRYPOINT ["./migrate.sh"] | ||
|
Comment on lines
+5
to
+17
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 ClickHouse migration container fails to build because it copies files that don't exist The migration image build copies a helper script and a source folder ( Missing files referenced by the DockerfileA directory listing of
The previous Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
| CMD ["up"] | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -486,47 +486,57 @@ async function localBuildImage(options: SelfHostedBuildImageOptions): Promise<Bu | |
| }; | ||
| } | ||
|
|
||
| const [credentialsError, credentials] = await tryCatch( | ||
| getDockerUsernameAndPassword(apiClient, deploymentId) | ||
| ); | ||
| let credentials; | ||
| if (cloudRegistryHost.endsWith("amazonaws.com")) { | ||
| const [credentialsError, result] = await tryCatch( | ||
| getDockerUsernameAndPassword(apiClient, deploymentId) | ||
| ); | ||
|
|
||
| if (credentialsError) { | ||
| return { | ||
| ok: false as const, | ||
| error: `Failed to get docker credentials: ${credentialsError.message}`, | ||
| logs: "", | ||
| }; | ||
| if (credentialsError) { | ||
| return { | ||
| ok: false as const, | ||
| error: `Failed to get docker credentials: ${credentialsError.message}`, | ||
| logs: "", | ||
| }; | ||
| } | ||
| credentials = result; | ||
| } | ||
|
Comment on lines
+489
to
503
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Docker pushes to non-AWS registries no longer log in, causing deploy failures Registry credentials are now fetched only when the target registry address ends in How the credential path is reached
With the new guard, Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| logger.debug(`Logging in to docker registry: ${cloudRegistryHost}`); | ||
| if (credentials) { | ||
| logger.debug(`Logging in to docker registry: ${cloudRegistryHost}`); | ||
|
|
||
| const loginProcess = x( | ||
| "docker", | ||
| ["login", "--username", credentials.username, "--password-stdin", cloudRegistryHost], | ||
| { | ||
| nodeOptions: { | ||
| cwd: options.cwd, | ||
| }, | ||
| } | ||
| ); | ||
| const loginProcess = x( | ||
| "docker", | ||
| ["login", "--username", credentials.username, "--password-stdin", cloudRegistryHost], | ||
| { | ||
| nodeOptions: { | ||
| cwd: options.cwd, | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| loginProcess.process?.stdin?.write(credentials.password); | ||
| loginProcess.process?.stdin?.end(); | ||
| loginProcess.process?.stdin?.write(credentials.password); | ||
| loginProcess.process?.stdin?.end(); | ||
|
|
||
| for await (const line of loginProcess) { | ||
| errors.push(line); | ||
| logger.debug(line); | ||
| } | ||
| for await (const line of loginProcess) { | ||
| errors.push(line); | ||
| logger.debug(line); | ||
| } | ||
|
|
||
| if (loginProcess.exitCode !== 0) { | ||
| return { | ||
| ok: false as const, | ||
| error: `Failed to login to registry: ${cloudRegistryHost}`, | ||
| logs: extractLogs(errors), | ||
| }; | ||
| } | ||
| if (loginProcess.exitCode !== 0) { | ||
| return { | ||
| ok: false as const, | ||
| error: `Failed to login to registry: ${cloudRegistryHost}`, | ||
| logs: extractLogs(errors), | ||
| }; | ||
| } | ||
|
|
||
| options.onLog?.(`Successfully logged in to the remote registry`); | ||
| options.onLog?.(`Successfully logged in to the remote registry`); | ||
| } else { | ||
| logger.debug( | ||
| `Skipping automatic registry login for ${cloudRegistryHost}. Please ensure you are logged in locally.` | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| const projectCacheRef = getProjectCacheRefFromImageTag(imageTag); | ||
|
|
@@ -550,13 +560,12 @@ async function localBuildImage(options: SelfHostedBuildImageOptions): Promise<Bu | |
| options.noCache ? "--no-cache" : undefined, | ||
| ...(useRegistryCache | ||
| ? [ | ||
| "--cache-to", | ||
| `type=registry,mode=max,image-manifest=true,oci-mediatypes=true,ref=${projectCacheRef}${ | ||
| cacheCompression === "zstd" ? ",compression=zstd" : "" | ||
| }`, | ||
| "--cache-from", | ||
| `type=registry,ref=${projectCacheRef}`, | ||
| ] | ||
| "--cache-to", | ||
| `type=registry,mode=max,image-manifest=true,oci-mediatypes=true,ref=${projectCacheRef}${cacheCompression === "zstd" ? ",compression=zstd" : "" | ||
| }`, | ||
| "--cache-from", | ||
| `type=registry,ref=${projectCacheRef}`, | ||
| ] | ||
| : []), | ||
| "--output", | ||
| outputOptions.join(","), | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Release tooling will error because the change note lists a package that is excluded from versioning
The new release note lists
webapp(.changeset/fix-cli-ui-bugs.md:6) even though that package is explicitly excluded from versioning, so the versioning step fails when it sees excluded and non-excluded packages in the same note.Impact: The release/versioning job errors out until the note is corrected.
Changesets ignore list
.changeset/config.jsoncontains"ignore": ["webapp", "coordinator", "docker-provider", "kubernetes-provider", "supervisor"]. Changesets fails with "The following changesets contain both ignored and not ignored packages" in that situation. PerCONTRIBUTING.md/CLAUDE.md, server-only changes belong in a.server-changes/file, not in a changeset.@internal/clickhouseis also a private, unpublished package.Was this helpful? React with 👍 or 👎 to provide feedback.