feat: complete runtime profile management#2246
Merged
Merged
Conversation
Add managed runtime profile storage, resolution, and CLI commands for profile variables and secrets. Propagate selected profiles through local runs, queued runs, retries, and distributed task metadata, while masking managed secret values in status output.
Retry now always inherits the source run profile from stored status and no longer accepts profile overrides through CLI, API, queue, scheduler, or subprocess retry paths.
Expose selected runtime profiles consistently in DAG-run summary and detail responses, omit empty profile values, and show profiles in DAG-run list views.
…-complete # Conflicts: # internal/runtime/agent/agent.go # internal/runtime/executor/dag_runner.go
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds runtime profiles as a managed feature, exposes profile CRUD APIs and UI, allows selected profiles on DAG start and enqueue flows, propagates profile metadata through execution and retries, masks resolved secret values in status output, and displays the chosen profile on DAG runs. ChangesRuntime profiles feature
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Web UI / CLI
participant API as Frontend API
participant Worker as Worker or Local Runner
participant Agent as Runtime Agent
participant Stores as ProfileStore + SecretStore
User->>UI: create/select profile
UI->>API: POST /profiles or start/enqueue with profileName
API->>Stores: validate/load profile
API->>Worker: launch run with profileName
Worker->>Agent: build agent options
Agent->>Stores: resolve profile variables and secrets
Agent-->>Worker: status with profile metadata
Worker-->>API: DAG run status
API-->>UI: DAGRunSummary/ProfileName
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
1 issue found across 88 files
Partial review: This PR has more than 50 files, so cubic reviewed the highest-priority files first. During the trial, paid plans get a higher file limit.
You can try an ultrareview to bypass the file limit, comment @cubic-dev-ai ultrareview. Learn more.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/src/features/dags/components/dag-details/DAGDetailsSidePanel.tsx (1)
229-244:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing profileName parameter forwarding in handleEnqueue.
The
handleEnqueuecallback is typed asEnqueueHandlerwhich requires 4 parameters (params, dagRunId, immediate, profileName), but the implementation only accepts and forwards 3 parameters.Profile selection will be lost when DAG start/enqueue flows pass through this wrapper.
🔧 Fix: Accept and forward profileName
const handleEnqueue = React.useCallback<EnqueueHandler>( - async (params, dagRunId, immediate) => { + async (params, dagRunId, immediate, profileName) => { if (!onEnqueue) { return; } - const result = await onEnqueue(params, dagRunId, immediate); + const result = await onEnqueue(params, dagRunId, immediate, profileName); setActiveTab('status'); if (typeof result === 'string' && result) { setTrackedDagRunId(result); } await mutate(); return result; }, [mutate, onEnqueue] );🤖 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 `@ui/src/features/dags/components/dag-details/DAGDetailsSidePanel.tsx` around lines 229 - 244, The handleEnqueue callback currently declared as EnqueueHandler ignores the fourth parameter profileName, causing profile selection to be dropped; update the handleEnqueue signature to accept (params, dagRunId, immediate, profileName) and forward profileName when calling onEnqueue (i.e., await onEnqueue(params, dagRunId, immediate, profileName)), leaving the rest of the logic (setActiveTab, setTrackedDagRunId, await mutate, return result) unchanged so trackedDagRunId and mutate continue to work as before.ui/src/features/dags/contexts/DAGContext.ts (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd GPL v3 license headers to these files
These files are missing
SPDX-License-Identifier: GPL-3.0-or-later:
ui/src/features/dags/contexts/DAGContext.tsui/src/features/dags/components/dag-execution/StartDAGModal.tsxui/src/features/dags/components/dag-details/DAGStatusOverview.tsxui/src/features/dag-runs/components/dag-run-list/DAGRunCard.tsxRun
make addlicenseto apply the headers.🤖 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 `@ui/src/features/dags/contexts/DAGContext.ts` at line 1, Add the missing GPL v3 SPDX header to the top of each affected module (DAGContext, StartDAGModal, DAGStatusOverview, DAGRunCard) by running the repository license tooling: run make addlicense to automatically prepend "SPDX-License-Identifier: GPL-3.0-or-later" to ui/src/features/dags/contexts/DAGContext.ts, ui/src/features/dags/components/dag-execution/StartDAGModal.tsx, ui/src/features/dags/components/dag-details/DAGStatusOverview.tsx and ui/src/features/dag-runs/components/dag-run-list/DAGRunCard.tsx, then commit the updated files; verify the header appears above the existing imports (e.g., above the import React statement in DAGContext.ts).
🧹 Nitpick comments (6)
ui/src/features/dags/components/dag-execution/StartDAGModal.tsx (1)
251-801: ⚡ Quick winMissing test coverage for profile submission flow.
The profile selection logic is well-implemented, but there's no test verifying that when a profile is selected and the form is submitted,
onSubmitis called with theprofileNameargument. Consider adding a test case that selects a profile and verifies the fourth parameter is passed correctly.🤖 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 `@ui/src/features/dags/components/dag-execution/StartDAGModal.tsx` around lines 251 - 801, Add a unit test for StartDAGModal that mounts the component with a non-empty activeProfiles array and a mock onSubmit, then simulate selecting a profile from the runtime profile <Select> (use the profile.name value, not NO_PROFILE_VALUE), fill any minimal required inputs (e.g., leave default params or set dagRunId), click the submit button, and assert the mock onSubmit was called with the expected fourth argument equal to the selected profile name; reference StartDAGModal, the onSubmit prop, the runtime profile Select/SelectItem, and the submit button click to locate where to interact and assert.internal/profile/store.go (1)
8-14: ⚡ Quick winAdd godoc comments to document the Store interface.
The
Storeinterface lacks documentation explaining its purpose, error semantics, and implementation requirements. Consider documenting:
- What the interface represents
- Error behavior for
GetByNamewhen a profile is not found (specific error type vs. nil)- Whether implementations must be thread-safe
- Validation responsibilities for
CreateandUpdate📝 Proposed documentation enhancement
+// Store provides persistence operations for runtime profiles. +// Implementations must be safe for concurrent use. +// GetByName returns an error if the profile is not found. type Store interface { + // Create persists a new profile. Returns an error if a profile + // with the same name already exists. Create(ctx context.Context, profile *Profile) error + + // GetByName retrieves a profile by name. Returns an error if not found. GetByName(ctx context.Context, name string) (*Profile, error) + + // List returns all profiles. List(ctx context.Context) ([]*Profile, error) + + // Update modifies an existing profile. Returns an error if not found. Update(ctx context.Context, profile *Profile) error + + // Delete removes a profile by name. Returns an error if not found. Delete(ctx context.Context, name string) error }🤖 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 `@internal/profile/store.go` around lines 8 - 14, Add GoDoc for the Store interface describing its purpose (persistent CRUD for Profile entities) and document semantics for each method: state that Create(ctx, *Profile) and Update(ctx, *Profile) expect callers to validate business rules (or specify which validations the implementation must perform), that GetByName(ctx, string) returns (nil, ErrNotFound) or another clearly named error when a profile does not exist (declare the expected error symbol), and describe List and Delete behaviors and error conditions. Also state whether implementations must be safe for concurrent use (thread-safe) and note any other implementation requirements (transactions, uniqueness guarantees) so implementers know expectations; reference the Store type and its methods Create, GetByName, List, Update, Delete and the Profile type in the docblock.internal/profile/secret_ref.go (1)
11-13: 💤 Low valueAdd a doc comment to the exported
SecretRef.The exported function lacks a doc comment; revive (enabled in your golangci-lint v2 config) will flag this. A brief comment also clarifies the reference format contract that callers/storage depend on.
As per coding guidelines: "Use golangci-lint v2 with checks including errcheck, govet, staticcheck, gosec, and revive for Go linting".📝 Proposed doc comment
+// SecretRef builds the deterministic secret store reference for a runtime +// profile entry, in the form "runtime-profiles/<profileName>/key-<hexKey>". func SecretRef(profileName, key string) string { return fmt.Sprintf("runtime-profiles/%s/key-%s", profileName, hex.EncodeToString([]byte(key))) }🤖 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 `@internal/profile/secret_ref.go` around lines 11 - 13, Add a proper Go doc comment above the exported SecretRef function describing its purpose and the exact reference format it returns (e.g., "runtime-profiles/<profileName>/key-<hexEncodedKey>") so revive/lint won't flag it; place the comment immediately above the SecretRef declaration and keep it concise, starting with "SecretRef ..." to follow Go doc conventions and clarify the contract for callers/storage that depend on the reference format.internal/profile/profile.go (1)
234-239: ⚡ Quick winEnforce UTC normalization for all timestamps.
The
normalizeTimehelper converts zero times totime.Now().UTC()but preserves the timezone of non-zero times. This could lead to inconsistent timezone handling if callers pass times in other zones. For consistent storage and comparison, all timestamps should be normalized to UTC.⏰ Proposed fix to enforce UTC
func normalizeTime(t time.Time) time.Time { if t.IsZero() { return time.Now().UTC() } - return t + return t.UTC() }🤖 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 `@internal/profile/profile.go` around lines 234 - 239, The normalizeTime helper currently returns time.Now().UTC() for zero times but leaves non-zero times in their original timezone; update normalizeTime to always return a UTC time by returning time.Now().UTC() for zero values and calling t.UTC() for non-zero values so all timestamps are consistently normalized to UTC (refer to the normalizeTime function).internal/cmd/profile.go (1)
255-283: 💤 Low valueConsider more robust whitespace trimming for stdin secret values.
Line 265 uses
strings.TrimRight(string(data), "\r\n")which handles trailing carriage returns and newlines but not other trailing whitespace like tabs or spaces. For secret values read from stdin, consider usingstrings.TrimSpacefor more consistent behavior, or document why only\r\nare trimmed.💡 Alternative approach
- return strings.TrimRight(string(data), "\r\n"), nil + return strings.TrimSpace(string(data)), nil🤖 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 `@internal/cmd/profile.go` around lines 255 - 283, The stdin branch in runtimeProfileSecretValue currently uses strings.TrimRight(string(data), "\r\n") which only removes CR/LF; replace that with strings.TrimSpace(string(data)) to strip all leading/trailing whitespace (tabs, spaces, CR/LF) for secret values read when the value-stdin flag is true (refer to useStdin and the "value-stdin" flag handling). Ensure you only change the trimming of the data variable returned from io.ReadAll and do not alter the term.ReadPassword path so behavior remains consistent for interactive input.internal/core/exec/context.go (1)
46-50: ⚡ Quick winAdd godoc comment for RuntimeProfileEntry.
The exported type
RuntimeProfileEntryshould have a package-level comment explaining what it represents and when it's used.📝 Suggested documentation
+// RuntimeProfileEntry represents non-secret metadata about a single profile key +// injected into a run. It records the key name and kind (variable or secret) +// without exposing resolved values. type RuntimeProfileEntry struct { Key string `json:"key"` Kind string `json:"kind"` }🤖 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 `@internal/core/exec/context.go` around lines 46 - 50, Add a proper godoc comment for the exported type RuntimeProfileEntry: insert a comment immediately above the type declaration that begins with "RuntimeProfileEntry" and concisely explains that it represents non‑secret metadata about a profile key that is injected into a run (when/why it is used), and briefly describe the meaning of the Key and Kind fields so future readers understand its purpose and usage in the runtime execution context.
🤖 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 `@api/v1/api.yaml`:
- Around line 15287-15292: The RuntimeProfileKey schema currently allows keys
beginning with the reserved DAGU_ prefix; update the RuntimeProfileKey entry to
enforce that keys must still start with a letter or underscore and contain only
alphanumeric/underscore characters but must not begin with the literal "DAGU_";
implement this by adding a negative lookahead check to the existing pattern for
RuntimeProfileKey and update the description to state that DAGU_* keys are
rejected by validation rather than merely reserved.
- Around line 8413-8809: The operations for runtime profiles
(listRuntimeProfiles, createRuntimeProfile, getRuntimeProfile,
updateRuntimeProfile, deleteRuntimeProfile, setRuntimeProfileVariable,
setRuntimeProfileSecret, deleteRuntimeProfileEntry) currently inherit a global
permissive security requirement that allows unauthenticated access; add an
operation-level security override for each of these operations to require
authentication (for example add a security object listing the required schemes
such as apiToken and/or basicAuth) so each operation explicitly requires the API
token/basic auth instead of the empty global requirement.
In `@internal/runtime/agent/status_masking.go`:
- Around line 92-98: The unchecked type assertions on step.ExecutorConfig.Config
and step.ExecutorConfig.Metadata can panic if maskAnyStringValues returns a non
map[string]any; update the code to perform safe type checks (use a type
assertion with the "ok" pattern or a type switch) on the result of
maskAnyStringValues(masker, ...) before assigning back to
step.ExecutorConfig.Config or Metadata; if the result is a map[string]any assign
it, otherwise leave the original value or convert/handle it safely so no panic
occurs (refer to symbols: step.ExecutorConfig.Config,
step.ExecutorConfig.Metadata, and maskAnyStringValues(masker, ...)).
In `@internal/service/frontend/api/v1/dagruns_edit_retry.go`:
- Around line 85-89: EditRetryDAGRun fails to populate plan.profileName during
the real launch path, so the coordinator and executor never receive the
inherited profile; call inheritedRunProfileName in the execution path inside
EditRetryDAGRun (same way as in preview) to set plan.profileName when
plan.sourceStatus.ProfileName is present, then ensure the coordinator path uses
plan.profileName so executor.WithProfileName(...) is invoked; update references
in EditRetryDAGRun and any coordinator launch code to use the resolved
plan.profileName instead of a blank string.
In `@internal/service/frontend/api/v1/profiles.go`:
- Around line 64-83: ListRuntimeProfiles (and similarly GetRuntimeProfile)
currently only checks requireManagerOrAbove, allowing managers to read protected
profiles; enforce the same protected-profile restriction used by
requireRuntimeProfileManage/requireRuntimeProfileUpdate by checking whether each
profile is protected and requiring admin rights before exposing its values or
secret IDs. Update ListRuntimeProfiles to filter or redact protected entries (or
call a helper that performs requireRuntimeProfileManage-like checks per profile)
when serializing via toRuntimeProfileResponse, and apply the same change to
GetRuntimeProfile (and the other read handlers noted) so protected profiles are
only readable by admins.
- Around line 413-455: The ensureRunnableRuntimeProfile function currently
enforces requireManagerOrAbove unconditionally which blocks ordinary
profile-backed executions; remove the call to a.requireManagerOrAbove(ctx) and
instead only enforce role checks for protected profiles by keeping the existing
a.requireAdmin(ctx) inside the item.Protected branch (leave validation,
profileStore checks, and the disabled/not-found handling intact). Ensure the
function returns item.Name for non-protected profiles without requiring manager
privileges.
In `@internal/service/worker/remote_handler.go`:
- Around line 190-193: In handleRetry in
internal/service/worker/remote_handler.go, don’t let task.ProfileName override
the stored retry profile: replace the current precedence logic that starts with
profileName := task.ProfileName and falls back to status.ProfileName with logic
that unconditionally uses status.ProfileName (e.g., profileName :=
status.ProfileName), or assert that task.ProfileName is empty and fail/log if it
isn't; update all uses of profileName in handleRetry accordingly so retry
runtime profiles remain immutable and always come from status.ProfileName
(reference symbols: handleRetry, task.ProfileName, status.ProfileName,
task.PreviousStatus).
In `@rfcs/020_runtime_profiles.html`:
- Around line 930-936: In the "Retry Defaults" section, remove the CLI example
that shows overriding the profile (the `dagu retry --profile staging
20260601T143120Z` snippet) and update the paragraph to state that retries
inherit the original run's profile from stored status and that profile selection
is immutable across all retry paths (CLI/API/queue/scheduler/subprocess); ensure
the example shows only the immutable retry invocation (e.g., `dagu retry
20260601T143120Z`) and add a short clarifying sentence that profile overrides
are no longer accepted.
---
Outside diff comments:
In `@ui/src/features/dags/components/dag-details/DAGDetailsSidePanel.tsx`:
- Around line 229-244: The handleEnqueue callback currently declared as
EnqueueHandler ignores the fourth parameter profileName, causing profile
selection to be dropped; update the handleEnqueue signature to accept (params,
dagRunId, immediate, profileName) and forward profileName when calling onEnqueue
(i.e., await onEnqueue(params, dagRunId, immediate, profileName)), leaving the
rest of the logic (setActiveTab, setTrackedDagRunId, await mutate, return
result) unchanged so trackedDagRunId and mutate continue to work as before.
In `@ui/src/features/dags/contexts/DAGContext.ts`:
- Line 1: Add the missing GPL v3 SPDX header to the top of each affected module
(DAGContext, StartDAGModal, DAGStatusOverview, DAGRunCard) by running the
repository license tooling: run make addlicense to automatically prepend
"SPDX-License-Identifier: GPL-3.0-or-later" to
ui/src/features/dags/contexts/DAGContext.ts,
ui/src/features/dags/components/dag-execution/StartDAGModal.tsx,
ui/src/features/dags/components/dag-details/DAGStatusOverview.tsx and
ui/src/features/dag-runs/components/dag-run-list/DAGRunCard.tsx, then commit the
updated files; verify the header appears above the existing imports (e.g., above
the import React statement in DAGContext.ts).
---
Nitpick comments:
In `@internal/cmd/profile.go`:
- Around line 255-283: The stdin branch in runtimeProfileSecretValue currently
uses strings.TrimRight(string(data), "\r\n") which only removes CR/LF; replace
that with strings.TrimSpace(string(data)) to strip all leading/trailing
whitespace (tabs, spaces, CR/LF) for secret values read when the value-stdin
flag is true (refer to useStdin and the "value-stdin" flag handling). Ensure you
only change the trimming of the data variable returned from io.ReadAll and do
not alter the term.ReadPassword path so behavior remains consistent for
interactive input.
In `@internal/core/exec/context.go`:
- Around line 46-50: Add a proper godoc comment for the exported type
RuntimeProfileEntry: insert a comment immediately above the type declaration
that begins with "RuntimeProfileEntry" and concisely explains that it represents
non‑secret metadata about a profile key that is injected into a run (when/why it
is used), and briefly describe the meaning of the Key and Kind fields so future
readers understand its purpose and usage in the runtime execution context.
In `@internal/profile/profile.go`:
- Around line 234-239: The normalizeTime helper currently returns
time.Now().UTC() for zero times but leaves non-zero times in their original
timezone; update normalizeTime to always return a UTC time by returning
time.Now().UTC() for zero values and calling t.UTC() for non-zero values so all
timestamps are consistently normalized to UTC (refer to the normalizeTime
function).
In `@internal/profile/secret_ref.go`:
- Around line 11-13: Add a proper Go doc comment above the exported SecretRef
function describing its purpose and the exact reference format it returns (e.g.,
"runtime-profiles/<profileName>/key-<hexEncodedKey>") so revive/lint won't flag
it; place the comment immediately above the SecretRef declaration and keep it
concise, starting with "SecretRef ..." to follow Go doc conventions and clarify
the contract for callers/storage that depend on the reference format.
In `@internal/profile/store.go`:
- Around line 8-14: Add GoDoc for the Store interface describing its purpose
(persistent CRUD for Profile entities) and document semantics for each method:
state that Create(ctx, *Profile) and Update(ctx, *Profile) expect callers to
validate business rules (or specify which validations the implementation must
perform), that GetByName(ctx, string) returns (nil, ErrNotFound) or another
clearly named error when a profile does not exist (declare the expected error
symbol), and describe List and Delete behaviors and error conditions. Also state
whether implementations must be safe for concurrent use (thread-safe) and note
any other implementation requirements (transactions, uniqueness guarantees) so
implementers know expectations; reference the Store type and its methods Create,
GetByName, List, Update, Delete and the Profile type in the docblock.
In `@ui/src/features/dags/components/dag-execution/StartDAGModal.tsx`:
- Around line 251-801: Add a unit test for StartDAGModal that mounts the
component with a non-empty activeProfiles array and a mock onSubmit, then
simulate selecting a profile from the runtime profile <Select> (use the
profile.name value, not NO_PROFILE_VALUE), fill any minimal required inputs
(e.g., leave default params or set dagRunId), click the submit button, and
assert the mock onSubmit was called with the expected fourth argument equal to
the selected profile name; reference StartDAGModal, the onSubmit prop, the
runtime profile Select/SelectItem, and the submit button click to locate where
to interact and assert.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e513909-7098-4c54-afd0-9448429d3a1c
⛔ Files ignored due to path filters (2)
proto/coordinator/v1/coordinator.pb.gois excluded by!**/*.pb.goproto/coordinator/v1/coordinator_protoopaque.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (86)
api/v1/api.gen.goapi/v1/api.yamlcmd/main.gocmd/main_test.gointernal/agent/runtime_stores.gointernal/cmd/dry.gointernal/cmd/enqueue.gointernal/cmd/enqueue_internal_test.gointernal/cmd/exec.gointernal/cmd/flags.gointernal/cmd/local_execution.gointernal/cmd/profile.gointernal/cmd/profile_flag.gointernal/cmd/remote_commands.gointernal/cmd/restart.gointernal/cmd/retry.gointernal/cmd/retry_internal_test.gointernal/cmd/retry_test.gointernal/cmd/start.gointernal/core/exec/context.gointernal/core/exec/dispatch.gointernal/core/exec/enqueue_retry_test.gointernal/core/exec/runstatus.gointernal/dagrun/intake/local.gointernal/dagrun/intake/queue.gointernal/dagrun/intake/queue_test.gointernal/engine/run.gointernal/intg/one_off_schedule_test.gointernal/launcher/launcher.gointernal/launcher/launcher_test.gointernal/persis/file/agent_stores.gointernal/persis/store/profile.gointernal/persis/store/profile_test.gointernal/profile/profile.gointernal/profile/profile_test.gointernal/profile/resolver.gointernal/profile/resolver_test.gointernal/profile/secret_ref.gointernal/profile/store.gointernal/proto/convert/dispatch.gointernal/proto/convert/dispatch_test.gointernal/runtime/agent/agent.gointernal/runtime/agent/agent_test.gointernal/runtime/agent/status_masking.gointernal/runtime/context.gointernal/runtime/executor/dag_runner.gointernal/runtime/executor/subworkflow.gointernal/runtime/executor/task.gointernal/runtime/transform/status.gointernal/service/frontend/api/v1/api.gointernal/service/frontend/api/v1/dagruns.gointernal/service/frontend/api/v1/dagruns_edit_retry.gointernal/service/frontend/api/v1/dagruns_test.gointernal/service/frontend/api/v1/dags.gointernal/service/frontend/api/v1/profiles.gointernal/service/frontend/api/v1/profiles_test.gointernal/service/frontend/api/v1/transformer.gointernal/service/frontend/api/v1/transformer_test.gointernal/service/frontend/api/v1/webhooks.gointernal/service/frontend/server.gointernal/service/scheduler/dag_executor.gointernal/service/worker/remote_handler.gointernal/service/worker/remote_handler_test.gointernal/subflow/runner.goproto/coordinator/v1/coordinator.protorfcs/020_runtime_profiles.htmlui/src/App.tsxui/src/__tests__/menu.test.tsxui/src/api/v1/schema.tsui/src/contexts/AuthContext.tsxui/src/features/cockpit/components/DAGPreviewModal.tsxui/src/features/dag-runs/components/dag-run-details/DAGRunHeader.tsxui/src/features/dag-runs/components/dag-run-list/DAGRunCard.tsxui/src/features/dag-runs/components/dag-run-list/DAGRunTable.tsxui/src/features/dag-runs/components/dag-run-list/__tests__/DAGRunTable.test.tsxui/src/features/dags/components/common/DAGActions.tsxui/src/features/dags/components/common/__tests__/DAGActions.test.tsxui/src/features/dags/components/dag-details/DAGDetailsContent.tsxui/src/features/dags/components/dag-details/DAGDetailsSidePanel.tsxui/src/features/dags/components/dag-details/DAGStatusOverview.tsxui/src/features/dags/components/dag-execution/StartDAGModal.tsxui/src/features/dags/components/dag-execution/__tests__/StartDAGModal.test.tsxui/src/features/dags/contexts/DAGContext.tsui/src/menu.tsxui/src/pages/profiles/__tests__/index.test.tsxui/src/pages/profiles/index.tsx
…-complete-pr # Conflicts: # internal/runtime/executor/dag_runner.go
…-complete-pr # Conflicts: # internal/service/worker/remote_handler.go # internal/subflow/local_cli.go
Collaborator
Author
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
Add persisted DAG settings for default runtime profiles and resolve them for manual and scheduled starts. Keep retry behavior tied to the original run profile, support explicit no-profile overrides, and expose the setting in the UI.
Collaborator
Author
|
@codex full review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13021c366c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| func (a *API) inheritedRunProfileName(ctx context.Context, inherited string) (string, error) { | ||
| return a.ensureRunnableRuntimeProfile(ctx, strings.TrimSpace(inherited)) |
There was a problem hiding this comment.
Allow inherited protected profiles without re-authorizing
When retrying or rescheduling a run that inherited a protected DAG default profile, this helper reuses ensureRunnableRuntimeProfile, which enforces admin access for protected profiles. Operators can start the same DAG with the protected default because the authorization happened when the default was configured, but retryDAGRun and rescheduleDAGRun call inheritedRunProfileName, so those non-admin retries/reschedules now fail with 403 despite not choosing a new explicit profile.
Useful? React with 👍 / 👎.
| // Enqueue directly from the API layer so accepted webhook payloads do not | ||
| // need to fit in a subprocess command line. | ||
| if err := a.enqueuePreparedDAGRun(ctx, dag, params, dagRunID, core.TriggerTypeWebhook); err != nil { | ||
| if err := a.enqueuePreparedDAGRun(ctx, dag, params, dagRunID, core.TriggerTypeWebhook, ""); err != nil { |
There was a problem hiding this comment.
Apply DAG default profiles to webhook runs
For webhook-triggered DAGs, this hard-codes an empty profileName into enqueuePreparedDAGRun, so the queued status is created without the DAG's saved default profile and the agent never injects that profile's variables or secrets. Manual start/enqueue paths call runProfileForDAG and scheduled runs resolve the default, so a DAG that relies on its default runtime profile behaves differently when triggered through its webhook.
Useful? React with 👍 / 👎.
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
profileas the API/UI request fieldNotes
profileas "use DAG default," emptyprofileas "run without a profile," and non-emptyprofileas an explicit override.Testing
make apipnpm gen:apimake fmtmake lintmake api-validatepnpm typecheckpnpm buildpnpm test -- src/features/dags/components/dag-execution/__tests__/StartDAGModal.test.tsxgo test ./internal/dagsettings ./internal/persis/store ./internal/service/frontend/api/v1 ./internal/service/scheduler ./internal/cmd/process -count=1make test