Merged
Conversation
|
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 pull request introduces a comprehensive migration from "tags" to "labels" as the primary metadata mechanism for DAGs and DAG runs across all layers of the system. The changes maintain backward compatibility by treating Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (12)
ui/src/lib/utils.ts (1)
1-2:⚠️ Potential issue | 🟡 MinorAdd the GPL header to this touched source file.
Please run
make addlicenseor add the standard header before the imports.Proposed header addition
+// Copyright (C) 2026 Yota Hamada +// SPDX-License-Identifier: GPL-3.0-or-later + import { clsx, type ClassValue } from 'clsx'; import { twMerge } from 'tailwind-merge';As per coding guidelines, “
**/*.{go,ts,tsx,js}: Apply GPL v3 license headers on source files, managed viamake addlicense”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/lib/utils.ts` around lines 1 - 2, Add the GPL v3 license header to the top of the touched source file (before any code or imports) — either run the repository helper (make addlicense) or insert the standard GPL header block immediately above the existing import lines (the ones importing "clsx" and "twMerge"). Ensure the header follows the project's standard format for .ts files and is placed before the lines "import { clsx, type ClassValue } from 'clsx';" and "import { twMerge } from 'tailwind-merge';".internal/persis/filedagrun/query_cursor.go (1)
82-119:⚠️ Potential issue | 🟡 MinorFilter-hash JSON key change invalidates any cursors issued before this deploy.
Renaming the normalized JSON field from
tagstolabelswhile keepingqueryCursorVersion = 1means cursors minted by the previous binary will now failFilterHashcomparison and returnErrInvalidQueryCursorto clients mid-pagination after a rolling upgrade. In practice callers also migrate their filter input fromtagstolabels, so the hash would mismatch regardless — but consider bumpingqueryCursorVersionto 2 so the failure surfaces as a clearer "unsupported cursor version" rather than "cursor does not match the current filters", and so future schema changes have a clean break point.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/persis/filedagrun/query_cursor.go` around lines 82 - 119, The JSON key rename from `tags` to `labels` changes the output of queryFilterHash (function queryFilterHash) and will invalidate existing cursors; update the cursor version constant (queryCursorVersion) to 2 so older cursors fail with an unsupported-version error instead of a filter-hash mismatch. Locate the declaration of queryCursorVersion and increment its value to 2, and add a short comment noting the bump corresponds to the tags->labels JSON key change for future reference.ui/src/hooks/useDAGsListSSE.ts (2)
1-2:⚠️ Potential issue | 🟡 MinorAdd the GPL license header.
This source file is missing the repository-managed GPL header. As per coding guidelines,
**/*.{go,ts,tsx,js}: Apply GPL v3 license headers on source files, managed viamake addlicense.Proposed header
+// Copyright (C) 2026 Yota Hamada +// SPDX-License-Identifier: GPL-3.0-or-later + import { components } from '../api/v1/schema';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/useDAGsListSSE.ts` around lines 1 - 2, Add the repository-managed GPL v3 license header to this TypeScript source (useDAGsListSSE.ts) so it conforms to the project's rule for **/*.{go,ts,tsx,js}; either run the repository tool (make addlicense) to automatically apply the standard header or insert the canonical GPL v3 header text at the top of the file before the first import (the import lines include symbols like components, buildSSEEndpoint, SSEState, useSSE) to ensure the file is properly licensed.
22-27:⚠️ Potential issue | 🟠 MajorThread
remoteNodethrough the DAG list SSE hook.This SSE API request cannot be routed to the selected node, so multi-node deployments may subscribe to local DAG events while the rest of the UI targets a remote node. As per coding guidelines,
ui/**/*.{ts,tsx}: All API calls MUST include theremoteNodeparameter to route requests to the correct node in multi-node Dagu deployments.Proposed fix
export function useDAGsListSSE( params: DAGsListParams = {}, - enabled: boolean = true + enabled: boolean = true, + remoteNode?: string ): SSEState<DAGsListSSEResponse> { const endpoint = buildSSEEndpoint('/events/dags', params); - return useSSE<DAGsListSSEResponse>(endpoint, enabled); + return useSSE<DAGsListSSEResponse>(endpoint, enabled, remoteNode); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/useDAGsListSSE.ts` around lines 22 - 27, The hook must thread the remoteNode through the SSE request so multi-node deployments route correctly; update useDAGsListSSE to accept or include a remoteNode and ensure buildSSEEndpoint gets it: modify the signature of useDAGsListSSE (or ensure DAGsListParams contains remoteNode) and pass that remoteNode into buildSSEEndpoint('/events/dags', paramsWithRemoteNode) before calling useSSE<DAGsListSSEResponse>(endpoint, enabled) so the SSE is correctly routed to the selected node.ui/src/pages/design/index.tsx (1)
665-671:⚠️ Potential issue | 🟡 MinorFallback to
tagswhenlabelsis empty.The spread operator with
item.dag.labels ?? item.dag.tags ?? []will use an empty array iflabelsis explicitly set to[], bypassing thetagsfallback. During schema migration or if the API returnslabels: [], legacy tag values would be excluded from search results.Consider using a length check to ensure fallback only happens when labels is both empty and missing:
Proposed fix
return dagFiles.filter((item) => { + const metadataLabels = + item.dag.labels?.length ? item.dag.labels : item.dag.tags ?? []; const searchableText = [ item.fileName, item.dag.name, item.dag.group, - ...(item.dag.labels ?? item.dag.tags ?? []), + ...metadataLabels, ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/design/index.tsx` around lines 665 - 671, The search array construction in the dagFiles.filter callback currently uses the nullish-coalescing chain item.dag.labels ?? item.dag.tags ?? [], which treats an explicit empty labels array as a valid value and prevents falling back to tags; update the logic in the searchableText building inside the dagFiles.filter callback so it checks labels' length (e.g., use item.dag.labels && item.dag.labels.length ? item.dag.labels : item.dag.tags ?? []) to only use labels when non-empty and otherwise fall back to item.dag.tags or an empty array; adjust the spread expression in the searchableText declaration accordingly (referencing dagFiles.filter, searchableText, item.dag.labels, and item.dag.tags).ui/src/features/cockpit/hooks/useDateKanbanData.ts (1)
1-1:⚠️ Potential issue | 🟡 MinorAdd the GPL license header.
This TypeScript source file is missing the project license header.
Proposed fix
+// Copyright (C) 2026 Yota Hamada +// SPDX-License-Identifier: GPL-3.0-or-later + import { useContext, useMemo } from 'react';As per coding guidelines,
**/*.{go,ts,tsx,js}: Apply GPL v3 license headers on source files, managed viamake addlicense.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/features/cockpit/hooks/useDateKanbanData.ts` at line 1, This file (the useDateKanbanData hook) is missing the project GPL v3 license header; add the standard GPL v3 header block at the very top of the source file (before any imports) matching the project's header format, or run the repository tooling (make addlicense) to apply it automatically, then commit the updated file so useDateKanbanData.ts contains the same GPL header used across other .ts/.tsx/.js files.ui/src/features/cockpit/components/TemplateSelector.tsx (1)
1-1:⚠️ Potential issue | 🟡 MinorAdd the GPL header to this source file.
This changed TSX source file is missing the repository license header; running
make addlicenseshould fix it.As per coding guidelines, Apply GPL v3 license headers on source files, managed via
make addlicense.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/features/cockpit/components/TemplateSelector.tsx` at line 1, The file TemplateSelector.tsx is missing the repository GPL v3 license header; run the repository tooling (make addlicense) or manually prepend the standard GPL v3 header to the top of the file so the header appears before the import statement (the existing import line "import React, { useState, useEffect, useRef, useCallback, useContext, useMemo } from 'react';" should remain unchanged); ensure the header matches the project's canonical GPL v3 text used by other source files.ui/src/pages/dags/index.tsx (2)
198-224:⚠️ Potential issue | 🟠 MajorInclude
remoteNodein the shared query params.
queryParamsis passed touseDAGsListSSE(queryParams)beforeremoteNodeis added for the/dagsREST request. In multi-node deployments, the REST fetch targets the selected node while the SSE stream can still default elsewhere.Suggested fix
const queryParams = React.useMemo( () => ({ + remoteNode, page, perPage: preferences.pageLimit || 200, name: apiSearchText || undefined, labels: apiSearchLabels.length > 0 ? apiSearchLabels.join(',') : undefined, sort: sortField, order: sortOrder, }), [ + remoteNode, page, preferences.pageLimit, apiSearchText, apiSearchLabels, sortField, sortOrder, @@ query: { ...queryParams, - remoteNode, sort: sortField as PathsDagsGetParametersQuerySort, order: sortOrder as PathsDagsGetParametersQueryOrder, },As per coding guidelines, All API calls MUST include the
remoteNodeparameter to route requests to the correct node in multi-node Dagu deployments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/dags/index.tsx` around lines 198 - 224, queryParams used for the SSE subscription does not include remoteNode, so useDAGsListSSE(queryParams) may subscribe to the wrong node; update the memoized queryParams object (and its dependency array) to include remoteNode (e.g., add remoteNode: remoteNode) so both useDAGsListSSE(queryParams) and the useQuery('/dags', { params: { query: { ...queryParams, remoteNode } } }) use the same routing parameter.
1-1:⚠️ Potential issue | 🟡 MinorAdd the GPL header to this source file.
This changed TSX source file is missing the repository license header; running
make addlicenseshould fix it.As per coding guidelines, Apply GPL v3 license headers on source files, managed via
make addlicense.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/dags/index.tsx` at line 1, Add the GPL v3 license header comment at the top of this TSX source file (above the first token, e.g., above the existing import { debounce } from 'lodash'; line) to match the repository's license headers; you can run make addlicense to apply it automatically or paste the standard GPL v3 header block manually so the file passes license checks.ui/src/pages/dag-runs/index.tsx (1)
1-1:⚠️ Potential issue | 🟡 MinorAdd the required GPL source header.
This changed TSX source file is missing the repository license header.
🪪 Proposed header
+// Copyright (C) 2026 Yota Hamada +// SPDX-License-Identifier: GPL-3.0-or-later + import dayjs from 'dayjs';As per coding guidelines,
**/*.{go,ts,tsx,js}: “Apply GPL v3 license headers on source files, managed viamake addlicense”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/dag-runs/index.tsx` at line 1, This file is missing the repository GPL v3 source header; add the standard GPL v3 license header at the very top of the TSX file (above the existing import statement like "import dayjs from 'dayjs';") so the header precedes all code, or run the repository tool to apply it (e.g., execute the project's make addlicense or equivalent) to ensure the file ui/src/pages/dag-runs/index.tsx includes the required header block.ui/src/features/dags/components/dag-list/DAGTable.tsx (2)
1-1:⚠️ Potential issue | 🟡 MinorAdd the GPL header to this TSX source file.
This changed source file is missing the repository license header; run
make addlicenseor add it explicitly.Proposed header
+// Copyright (C) 2026 Yota Hamada +// SPDX-License-Identifier: GPL-3.0-or-later + import {As per coding guidelines, "
**/*.{go,ts,tsx,js}: Apply GPL v3 license headers on source files, managed viamake addlicense".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/features/dags/components/dag-list/DAGTable.tsx` at line 1, Add the repository GPL v3 license header to this TypeScript React source file (DAGTable.tsx). Insert the standard GPL header used by the repo at the top of the file (above all imports and before the DAGTable component or any other symbols) — you can run `make addlicense` to apply it automatically or paste the exact GPL v3 header used elsewhere in the repo so that the header appears before the first import and covers this file (DAGTable.tsx / DAGTable component).
467-510:⚠️ Potential issue | 🟠 MajorKeep selected labels as structured filter data.
Line 932 serializes selected labels with
join(','), but Line 481 only enables exact AND label matching whenfilterValueis an array. With multiple labels selected, the filter searches for a single label containing"a,b"and hides valid DAGs.Proposed fix
filterFn: (row, _, filterValue) => { // Use row instead of props const data = row.original!; if (data.kind === ItemKind.Group) { return true; // Always show group rows during filtering } if (data.kind === ItemKind.DAG) { const name = data.dag.dag.name.toLowerCase(); const fileName = data.dag.fileName.toLowerCase(); const description = (data.dag.dag.description || '').toLowerCase(); - const searchValue = Array.isArray(filterValue) - ? '' - : String(filterValue).toLowerCase(); + const filterObject = + typeof filterValue === 'object' && + filterValue !== null && + !Array.isArray(filterValue) + ? (filterValue as { searchText?: string; labels?: string[] }) + : undefined; + const searchValue = filterObject + ? (filterObject.searchText ?? '').toLowerCase() + : Array.isArray(filterValue) + ? '' + : String(filterValue ?? '').toLowerCase(); - const labelFilters = Array.isArray(filterValue) - ? filterValue.map((t) => t.toLowerCase()) - : []; + const labelFilters = ( + filterObject?.labels ?? (Array.isArray(filterValue) ? filterValue : []) + ).map((label) => label.toLowerCase()); - // Search in name and description - if ( - !labelFilters.length && // Only search text if no label filters - (fileName.includes(searchValue) || - name.includes(searchValue) || - description.includes(searchValue)) - ) { - return true; - } - - // Also search in labels if needed const labels = data.dag.dag.labels ?? data.dag.dag.tags ?? []; + const rowLabels = labels.map((label) => label.toLowerCase()); - if (labelFilters.length > 0) { - const rowLabels = labels.map((label) => label.toLowerCase()); - // AND logic: all selected labels must be present - if (labelFilters.every((label) => rowLabels.includes(label))) { - return true; - } - } else if ( - labels.some((label) => label.toLowerCase().includes(searchValue)) - ) { - return true; - } + const matchesText = + searchValue === '' || + fileName.includes(searchValue) || + name.includes(searchValue) || + description.includes(searchValue) || + rowLabels.some((label) => label.includes(searchValue)); + const matchesLabels = + labelFilters.length === 0 || + labelFilters.every((label) => rowLabels.includes(label)); + + return matchesText && matchesLabels; } return false; },useEffect(() => { const nameFilter = columnFilters.find((f) => f.id === 'Name'); // Combine searchText and searchLabels for the Name filter const combinedFilter = - searchLabels.length > 0 ? searchLabels.join(',') : searchText || ''; - const currentValue = nameFilter?.value || ''; + searchText || searchLabels.length > 0 + ? { searchText, labels: searchLabels } + : ''; + const currentValue = nameFilter?.value || ''; + const currentFilterKey = JSON.stringify(currentValue); + const combinedFilterKey = JSON.stringify(combinedFilter); let updated = false; const newFilters = [...columnFilters]; - if (combinedFilter !== currentValue) { + if (combinedFilterKey !== currentFilterKey) { const idx = newFilters.findIndex((f) => f.id === 'Name'); if (combinedFilter) { if (idx > -1) newFilters[idx] = { id: 'Name', value: combinedFilter }; else newFilters.push({ id: 'Name', value: combinedFilter }); } else if (idx > -1) {Also applies to: 927-952
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/features/dags/components/dag-list/DAGTable.tsx` around lines 467 - 510, filterFn currently expects filterValue to be an array for multi-label AND matching, but elsewhere selected labels are serialized with join(',') so multi-select becomes a single string like "a,b" and fails matching; update the code that constructs the table filter value (the place that uses selectedLabels.join(',')) to pass the labels as an array (e.g., selectedLabels) instead of a comma-joined string, and ensure filterFn remains unchanged so it receives an array and performs the labelFilters.every(...) AND logic correctly (refer to filterFn and the selectedLabels serialization site).
🧹 Nitpick comments (11)
internal/core/label_test.go (1)
94-109: Finish moving tests off deprecated tag names.
TestTag_IsZeroand the filter tests still exerciseTag*aliases. Since this suite is validating the canonical label API, preferLabel*names here and leave alias-specific coverage to a small compatibility test.Representative cleanup
-func TestTag_IsZero(t *testing.T) { +func TestLabel_IsZero(t *testing.T) { ... - wantType TagFilterType + wantType LabelFilterType ... - wantType: TagFilterTypeKeyOnly, + wantType: LabelFilterTypeKeyOnly, ... - filters := make([]TagFilter, len(tt.filters)) + filters := make([]LabelFilter, len(tt.filters))Also applies to: 268-289, 428-432, 443-490
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/core/label_test.go` around lines 94 - 109, Rename tests and their usages to exercise the canonical Label API instead of deprecated Tag aliases: change the test function name TestTag_IsZero to TestLabel_IsZero and update any occurrences of Tag* types/aliases used in this suite to their Label* counterparts (e.g., replace Tag struct/alias usages with Label, and any Tag-prefixed helper names with Label-prefixed names) so the suite validates Label.IsZero and related behavior; leave a minimal separate compatibility test that covers Tag* aliases elsewhere. Ensure you update references inside the test body (table entries, t.Run names, and calls like tt.label.IsZero()) and apply the same replacement for the other affected test blocks mentioned (lines covering the other Tag* usages).internal/core/label.go (1)
355-399: Add deprecation comments to tag compatibility aliases.These compatibility exports follow Go's standard deprecation convention but lack the required doc comments. While the project's linter doesn't currently enforce this, follow the pattern below to document deprecation and surface them in IDEs and pkg.go.dev:
Deprecation comment pattern
// Deprecated compatibility aliases. Prefer the Label/Labels names for new code. +// Deprecated: use Label instead. type Tag = Label +// Deprecated: use Labels instead. type Tags = Labels +// Deprecated: use LabelFilterType instead. type TagFilterType = LabelFilterType +// Deprecated: use LabelFilter instead. type TagFilter = LabelFilter const ( + // Deprecated: use MaxLabelKeyLength instead. MaxTagKeyLength = MaxLabelKeyLength + // Deprecated: use MaxLabelValueLength instead. MaxTagValueLength = MaxLabelValueLength // ... apply same pattern to remaining constants var ( + // Deprecated: use ValidLabelKeyPattern instead. ValidTagKeyPattern = ValidLabelKeyPattern + // Deprecated: use ValidLabelValuePattern instead. ValidTagValuePattern = ValidLabelValuePattern ) +// Deprecated: use ValidateLabel instead. func ValidateTag(label Label) error { return ValidateLabel(label) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/core/label.go` around lines 355 - 399, Add proper Go deprecation doc comments to each compatibility alias and wrapper in this block so IDEs and pkg.go.dev surface them; for example prepend comments like "Deprecated: use Label/Labels/LabelFilterType/etc. instead." to the type aliases (Tag, Tags, TagFilterType, TagFilter), constants (MaxTagKeyLength, TagKeyPatternStr, TagFilterTypeKeyOnly, etc.), vars (ValidTagKeyPattern, ValidTagValuePattern) and the thin wrapper functions (ValidateTag, ValidateTags, ParseTag, NewTags, ParseTagFilter, and the method MatchesTags on LabelFilter) referencing the preferred names (Label, Labels, LabelFilterType, LabelFilter) in each comment.internal/core/spec/types/stringarray_test.go (1)
217-225: Nit: stale sub-test names still reference "Tag".The function was renamed to
TestLabelsValue, but the inner cases (SingleTag,MultipleTags) still use the old terminology. Consider renaming for consistency with the label vocabulary used throughout the PR.Proposed rename
- name: "SingleTag", + name: "SingleLabel", input: "production", wantValues: []string{"production"}, }, { - name: "MultipleTags", + name: "MultipleLabels", input: `["production", "critical", "monitored"]`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/core/spec/types/stringarray_test.go` around lines 217 - 225, Rename the stale sub-test names inside the TestLabelsValue test to use "Label" terminology for consistency: update the case names currently "SingleTag" and "MultipleTags" to something like "SingleLabel" and "MultipleLabels" (or "SingleLabelValue"/"MultipleLabelValues") in the test cases where input and wantValues are defined so the sub-test identifiers in TestLabelsValue reflect the label vocabulary used across the PR.internal/persis/filedag/dagindex/dagindex_test.go (1)
155-182: Add index coverage for legacytags:YAML.This test now only proves
labels:is indexed. Please keep a legacytags:fixture here as well and assert it lands inentry.Labels, since old DAG files must remain searchable after the canonical field rename.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/persis/filedag/dagindex/dagindex_test.go` around lines 155 - 182, Update TestBuild_BasicDAGs to include a legacy tags fixture and assert it is indexed: add a second YAML input (or extend dagContent) containing the legacy "tags:" field (e.g., tags: ["env=prod"] or tags:\n - env=prod) and include its metadata in the files slice (YAMLFileMeta). After calling Build(context.Background(), dir, files, nil) assert that the resulting entry.Labels contains the tag value from the legacy "tags" field (use the same assertions style as the existing labels check), keeping references to TestBuild_BasicDAGs, dagContent (or newLegacyDagContent), YAMLFileMeta and entry.Labels so the test verifies backward compatibility.internal/service/frontend/api/v1/dagruns_test.go (1)
254-397: Also assert runtime labels are persisted.These tests prove whether labels patch the stored spec, but they would still pass if request labels were dropped from DAG-run metadata. Add a details/status assertion for
Labelsafter each labeled start/enqueue path, especially the file enqueue case where the spec intentionally remains unchanged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/service/frontend/api/v1/dagruns_test.go` around lines 254 - 397, Add an assertion in each labeled-start/enqueue test to verify the runtime labels are persisted in the DAG-run metadata: after obtaining the dagRunId (and after the existing require.Eventually wait for run status), call the GET /api/v1/dag-runs/{name}/{dagRunId} response (parsed into api.GetDAGRunDetails200JSONResponse) and assert that DagRunDetails.Labels contains the expected label strings; update TestGetDAGRunSpecInlineStartWithLabelsDoesNotPatchSpec, TestGetDAGRunSpecInlineEnqueueWithLabelsPatchesSpec, and TestGetDAGRunSpecFileEnqueueWithLabelsDoesNotPatchSpec to include this Labels check (use the same status-wait or a new GET similar to the one in TestGetDAGRunSpecInlineEnqueueWithLabelsPatchesSpec).internal/runtime/executor/task_test.go (1)
348-366: Keep the deprecatedWithTagswrapper covered.Since this PR promises API compatibility for legacy
tags, add a small subtest provingexecutor.WithTags(...)still populatestask.Labels. Otherwise the wrapper can regress while all newWithLabelstests still pass.🧪 Proposed test addition
t.Run("WithLabelsEmpty", func(t *testing.T) { t.Parallel() task := &coordinatorv1.Task{} executor.WithLabels("")(task) assert.Empty(t, task.Labels) }) + + t.Run("WithTagsDeprecatedAlias", func(t *testing.T) { + t.Parallel() + + task := &coordinatorv1.Task{} + + executor.WithTags("env=prod,team=backend")(task) + + assert.Equal(t, "env=prod,team=backend", task.Labels) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/runtime/executor/task_test.go` around lines 348 - 366, Add a subtest in the same Test block that verifies the deprecated wrapper still works: create a coordinatorv1.Task{}, call executor.WithTags("env=prod,team=backend")(task) and assert task.Labels equals "env=prod,team=backend" (and similarly a case with empty string asserting Empty). This ensures executor.WithTags(...) remains API-compatible with executor.WithLabels and prevents regressions of the wrapper.internal/service/frontend/api/v1/dags_test.go (1)
526-598: Assert the labels are applied, not just accepted.Both valid label cases only check for a successful response. Please read the created DAG-run status/details and assert the requested labels are present so the tests catch handlers that silently ignore
Labels.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/service/frontend/api/v1/dags_test.go` around lines 526 - 598, The tests ExecuteDAGWithLabels and EnqueueDAGWithLabels only assert success but not that labels were actually applied; after getting execResp.DagRunId and enqResp.DagRunId call the DAG-run read endpoint for the created run (e.g. GET the run resource using the returned DagRunId via server.Client().Get(...).ExpectStatus(http.StatusOK).Send(t)), unmarshal the response into the DAG-run DTO and assert its Labels field contains the expected labels (compare to the labels slice passed to ExecuteDAGJSONRequestBody / EnqueueDAGDAGRunJSONRequestBody) so the test fails if handlers accept but ignore Labels.internal/intg/history_test.go (1)
288-326: Assert that label filtering actually filters results.This test currently passes even if
--labels=prodis accepted but ignored. Consider extending the command test helper to capture stdout, or add a lower-level assertion against the history query results solabeled-dag-1is included andlabeled-dag-2is excluded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/intg/history_test.go` around lines 288 - 326, The test currently only ensures the history CLI runs without error but doesn't assert filtering works; update TestHistoryCommand_Labels to verify results by either extending the test helper to capture stdout from th.RunCommand(cmd.History(), test.CmdTest{...}) and assert the output contains "labeled-dag-1" and does not contain "labeled-dag-2", or bypass the CLI and call the underlying history query API (e.g., the history manager used by the CLI) directly after starting the DAGs and assert that the returned history for labels="prod" includes dag1.DAG (or its name) and excludes dag2.DAG; reference TestHistoryCommand_Labels, th.RunCommand, cmd.History(), test.CmdTest, th.DAG and th.DAGRunMgr/GetLatestStatus to locate where to add the assertions.internal/persis/filedagrun/store_test.go (1)
904-916: UseLabelFilterin label-filter tests for consistency.These tests call
core.ParseLabelFilterand pass filters toresolveStatus, which accepts[]core.LabelFilter. WhileTagFilteris an alias forLabelFilterand the code compiles, switching to the canonicalLabelFiltertype name andlabelFiltersvariable names improves consistency with the label-focused API.♻️ Proposed cleanup
-func TestResolveStatus_FastPath_TagFilterReject(t *testing.T) { +func TestResolveStatus_FastPath_LabelFilterReject(t *testing.T) { store := &Store{} ctx := context.Background() @@ - tagFilters := []core.TagFilter{core.ParseLabelFilter("env=prod")} - status := store.resolveStatus(ctx, dagRun, tagFilters, nil, false) + labelFilters := []core.LabelFilter{core.ParseLabelFilter("env=prod")} + status := store.resolveStatus(ctx, dagRun, labelFilters, nil, false) assert.Nil(t, status) } @@ // Standard path (no summary) with matching label filter. - tagFilters := []core.TagFilter{core.ParseLabelFilter("env=prod")} - status := store.resolveStatus(ctx, dagRuns[0], tagFilters, nil, false) + labelFilters := []core.LabelFilter{core.ParseLabelFilter("env=prod")} + status := store.resolveStatus(ctx, dagRuns[0], labelFilters, nil, false) require.NotNil(t, status, "should resolve status via standard path with matching label") // Standard path with non-matching label filter. - tagFilters = []core.TagFilter{core.ParseLabelFilter("env=staging")} - status = store.resolveStatus(ctx, dagRuns[0], tagFilters, nil, false) + labelFilters = []core.LabelFilter{core.ParseLabelFilter("env=staging")} + status = store.resolveStatus(ctx, dagRuns[0], labelFilters, nil, false) assert.Nil(t, status, "should reject via standard path when label doesn't match")Also applies to: 937–944
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/persis/filedagrun/store_test.go` around lines 904 - 916, Tests use the alias type and variable name "TagFilter"; change them to the canonical label-focused names and types: in TestResolveStatus_FastPath_TagFilterReject (and the similar test around lines 937–944) replace the variable declaration tagFilters := []core.TagFilter{core.ParseLabelFilter("env=prod")} with labelFilters := []core.LabelFilter{core.ParseLabelFilter("env=prod")} and pass labelFilters to resolveStatus(ctx, dagRun, labelFilters, nil, false); keep references to resolveStatus, DAGRun, DAGRunSummary and core.ParseLabelFilter the same so the only edits are the type and variable name for consistency.internal/persis/filedag/store_test.go (1)
1410-1423: Assert the exact indexed label set, not just its length.This test would still pass if the indexed path returned the wrong labels but happened to return the same count. Prefer
assert.ElementsMatchfor both calls so key-only labels likeenv/teamare also protected.🧪 Proposed test tightening
labels1, errList1, err := store.LabelList(ctx) require.NoError(t, err) require.Empty(t, errList1) - assert.GreaterOrEqual(t, len(labels1), 3) // env=prod, env=staging, team=backend, env, team + assert.ElementsMatch(t, []string{ + "env=prod", + "env=staging", + "team=backend", + "env", + "team", + }, labels1) @@ labels2, errList2, err := store.LabelList(ctx) require.NoError(t, err) require.Empty(t, errList2) - assert.Equal(t, len(labels1), len(labels2)) + assert.ElementsMatch(t, labels1, labels2)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/persis/filedag/store_test.go` around lines 1410 - 1423, Replace the weak length assertions with exact set comparisons: after the first call to store.LabelList(ctx) compare labels1 against the expected label slice using assert.ElementsMatch (to ensure keys like "env"/"team" are present), and for the second call use assert.ElementsMatch to compare labels2 to the same expected set and also assert.ElementsMatch(labels1, labels2) to ensure the index returned the identical label set; keep the existing checks on err and errList variables (errList1/errList2) and the indexPath/fileExists verification.internal/persis/filedag/store.go (1)
958-962: Return labels in deterministic order.
labelSetmap iteration makesLabelListresponse order unstable, which can cause label dropdowns or snapshot tests to reorder between calls.🔀 Proposed deterministic ordering
labelList := make([]string, 0, len(labelSet)) for t := range labelSet { labelList = append(labelList, t) } + sort.Strings(labelList) return labelList, errList, nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/persis/filedag/store.go` around lines 958 - 962, The labelList is built from iterating labelSet which yields non-deterministic ordering; after populating labelList (the slice created from labelSet), sort it (e.g., call sort.Strings(labelList)) before returning so the function (the block that creates labelList from labelSet) always returns labels in a deterministic order; ensure the sort package is imported and then return labelList, errList, nil as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@api/v1/api.yaml`:
- Around line 781-793: Update the OpenAPI parameter and request-body
descriptions to document that the `labels` parameter and the deprecated alias
`tags` are mutually exclusive: for each occurrence of the pair (query params in
GET /dags and GET /dag-runs; request bodies for POST /dags/{dagName}/start, POST
/dags/{dagName}/start-sync, POST /dags/{dagName}/enqueue, POST /dag-runs/spec,
POST /dag-runs/spec/enqueue) change the `labels` and `tags` descriptions to
explicitly state they cannot be used together (e.g. "Mutually exclusive with
`tags`—server returns HTTP 400 if both are set") and keep `tags` marked
deprecated while referencing `labels` in its description (e.g. "Deprecated alias
for `labels`; mutually exclusive with `labels`"). Ensure the textual change is
applied to the `labels` and `tags` parameter descriptions and to any
request-body fields named `labels`/`tags` so generated clients and validators
reflect the server-side rejection.
In `@internal/agent/system_prompt.txt`:
- Line 196: The wording in the system prompt currently calls "[Doc: id | title]"
entries "labels", which may be confused with DAG labels; update the sentence to
call them "markers" or "references" instead (e.g., change "labels" to "markers"
or "document references") so the line reads something like "When a user message
begins with [Doc: id | title] markers, these reference markdown" and ensure the
"[Doc: id | title]" token is used exactly to locate the text to edit.
In `@internal/core/dag.go`:
- Around line 307-323: The UnmarshalJSON currently treats an explicit
"labels":[] the same as a missing field because it checks len(d.Labels)==0;
change the aux struct to capture whether the labels field was present (e.g. add
Labels *Labels `json:"labels,omitempty"` to the aux used in DAG.UnmarshalJSON)
and after json.Unmarshal use a presence check: if aux.Labels != nil set d.Labels
= *aux.Labels, else if aux.DeprecatedTags not empty set d.Labels =
aux.DeprecatedTags; keep the existing alias embedding and return behavior but
use aux.Labels presence instead of len(d.Labels).
In `@internal/core/exec/runstatus.go`:
- Around line 228-244: The UnmarshalJSON for DAGRunStatus treats an explicit
empty labels slice the same as missing by checking len(st.Labels); change aux to
capture presence by using Labels *[]string `json:"labels,omitempty"` in the aux
struct, then after unmarshalling and assigning *st = DAGRunStatus(aux.alias) use
the pointer check (if aux.Labels == nil && len(aux.DeprecatedTags) > 0) to copy
DeprecatedTags into st.Labels so an explicit labels: [] is preserved while
legacy tags fill only when labels is absent.
In `@internal/core/label.go`:
- Around line 221-225: ParseLabelFilter can produce a key-only empty filter when
given "" and pagination code appends raw opts.Labels entries, causing silent
non-matching filters; fix by trimming and skipping empty label strings before
calling ParseLabelFilter (e.g., iterate opts.Labels, do trimmed :=
strings.TrimSpace(label) and only append core.ParseLabelFilter(trimmed) when
trimmed != ""), and add Go deprecation comments ("// Deprecated: ...") to the
legacy functions ValidateTag, ValidateTags, ParseTag, NewTags, and
ParseTagFilter to guide users to the Label/Labels API.
In `@internal/core/spec/dag.go`:
- Around line 735-743: The code appends labels without trimming the key first,
so entries with whitespace-only keys slip through; in the loop over
labelsValue.Entries() (building core.Labels and core.Label), normalize the
entry.Key() and entry.Value() by trimming and lowercasing into local variables
first, then skip if the trimmed key is empty, and only then append
core.Label{Key: normalizedKey, Value: normalizedValue}; update the block that
constructs labels to use these normalized variables.
In `@internal/core/spec/types/labels.go`:
- Around line 65-72: In LabelsValue.UnmarshalYAML, clear the existing parsed
state before decoding so prior labels don't leak into subsequent decodes: reset
t.entries (and any other derived/parsed fields such as t.raw if appropriate) and
set t.isSet accordingly at the start of UnmarshalYAML, then proceed with
yaml.Unmarshal and repopulation; update the method that populates t.entries to
rebuild from scratch rather than append to existing slices/maps.
In `@internal/service/coordinator/handler.go`:
- Line 482: The initial distributed status is being written with DAG-level
labels (dag.Labels.Strings()) instead of the runtime request labels carried by
the Task; update h.writeInitialStatus calls (the one at the shown call and the
other occurrences around the 568 and 588–598 areas) to pass
task.Labels.Strings() (or task.Labels merged with dag labels if you want
fallback) instead of dag.Labels.Strings(), ensuring the arguments to
h.writeInitialStatus (e.g., attempt, dag.Name, task.DagRunId, task.AttemptKey,
task.ScheduleTime, exec.DAGRunRef{}) use the task-provided labels so file-based
starts/enqueues persist the correct initial labels.
In `@internal/service/frontend/api/v1/dags.go`:
- Around line 1634-1647: The current check uses the parsed slice `labels` (via
`parseCommaSeparatedLabels`) so an input like `labels=,&tags=prod` slips
through; change the validation to reject when the raw `labelsParam` string is
non-empty together with `tagsParam` (i.e., check `labelsParam != "" && tagsParam
!= ""`) before calling `parseCommaSeparatedLabels`, and ensure the same shared
resolver used by SSE/HTTP is invoked so both paths behave identically; update
the error return (the existing `&Error{HTTPStatus: http.StatusBadRequest, Code:
api.ErrorCodeBadRequest, Message: "labels and deprecated tags cannot both be
set"}`) to remain used in this new early validation.
In `@skills/dagu/references/cli.md`:
- Around line 59-60: The CLI docs use inconsistent label grammar: `start`
documents labels as "key=value or key-only" but `enqueue` and `history` just say
"comma-separated"; update the `--labels` (and deprecated `--tags`) option
descriptions for the `enqueue` and `history` command entries to mirror the
`start` wording (e.g., "Additional labels (comma-separated, accepted as
key=value or key-only)") so users know key-only labels are supported; ensure you
update all occurrences (including the mention at lines 110-111) and keep the
`--tags` alias note consistent with `--labels`.
In `@skills/dagu/references/schema.md`:
- Line 10: Update the `labels` (and deprecated `tags`) schema description to
explicitly state that the array form accepts both bare label strings (e.g.,
"prod", "critical") and key=value pairs, and clarify the constraints for each
(keys max 63 chars, values max 255 chars); modify the line containing `labels`
to read something like "map or array — Key-value labels (keys max 63 chars,
values max 255 chars) or an array of bare label strings (e.g. 'prod',
'critical') and/or key=value entries; `tags` is accepted as a deprecated alias."
to make the accepted array formats unambiguous.
In `@ui/src/components/ui/label-combobox.tsx`:
- Around line 1-4: Add the GPL v3 license header to the top of this touched TSX
file (ui/src/components/ui/label-combobox.tsx) before any imports; either run
the repository helper (make addlicense) or paste the standard GPL header comment
at the very top of the file so it appears above the existing import block
(import { Badge } ..., import { cn, parseLabelParts } ..., etc.).
- Around line 67-76: The duplicate check fails to canonicalize spaces around '='
(so "env = prod" ≠ "env=prod"); modify addLabel to canonicalize labels by
trimming, collapsing any whitespace around '=' (e.g. replace occurrences of
/\s*=\s*/ with '=') and then lowercasing to produce normalized; use the same
canonicalization when checking selectedLabels (apply it to each t in
selectedLabels during the some(...) duplicate check) and when passing the new
label into onLabelsChange so all stored labels use the same canonical form
(references: addLabel, selectedLabels, onLabelsChange).
---
Outside diff comments:
In `@internal/persis/filedagrun/query_cursor.go`:
- Around line 82-119: The JSON key rename from `tags` to `labels` changes the
output of queryFilterHash (function queryFilterHash) and will invalidate
existing cursors; update the cursor version constant (queryCursorVersion) to 2
so older cursors fail with an unsupported-version error instead of a filter-hash
mismatch. Locate the declaration of queryCursorVersion and increment its value
to 2, and add a short comment noting the bump corresponds to the tags->labels
JSON key change for future reference.
In `@ui/src/features/cockpit/components/TemplateSelector.tsx`:
- Line 1: The file TemplateSelector.tsx is missing the repository GPL v3 license
header; run the repository tooling (make addlicense) or manually prepend the
standard GPL v3 header to the top of the file so the header appears before the
import statement (the existing import line "import React, { useState, useEffect,
useRef, useCallback, useContext, useMemo } from 'react';" should remain
unchanged); ensure the header matches the project's canonical GPL v3 text used
by other source files.
In `@ui/src/features/cockpit/hooks/useDateKanbanData.ts`:
- Line 1: This file (the useDateKanbanData hook) is missing the project GPL v3
license header; add the standard GPL v3 header block at the very top of the
source file (before any imports) matching the project's header format, or run
the repository tooling (make addlicense) to apply it automatically, then commit
the updated file so useDateKanbanData.ts contains the same GPL header used
across other .ts/.tsx/.js files.
In `@ui/src/features/dags/components/dag-list/DAGTable.tsx`:
- Line 1: Add the repository GPL v3 license header to this TypeScript React
source file (DAGTable.tsx). Insert the standard GPL header used by the repo at
the top of the file (above all imports and before the DAGTable component or any
other symbols) — you can run `make addlicense` to apply it automatically or
paste the exact GPL v3 header used elsewhere in the repo so that the header
appears before the first import and covers this file (DAGTable.tsx / DAGTable
component).
- Around line 467-510: filterFn currently expects filterValue to be an array for
multi-label AND matching, but elsewhere selected labels are serialized with
join(',') so multi-select becomes a single string like "a,b" and fails matching;
update the code that constructs the table filter value (the place that uses
selectedLabels.join(',')) to pass the labels as an array (e.g., selectedLabels)
instead of a comma-joined string, and ensure filterFn remains unchanged so it
receives an array and performs the labelFilters.every(...) AND logic correctly
(refer to filterFn and the selectedLabels serialization site).
In `@ui/src/hooks/useDAGsListSSE.ts`:
- Around line 1-2: Add the repository-managed GPL v3 license header to this
TypeScript source (useDAGsListSSE.ts) so it conforms to the project's rule for
**/*.{go,ts,tsx,js}; either run the repository tool (make addlicense) to
automatically apply the standard header or insert the canonical GPL v3 header
text at the top of the file before the first import (the import lines include
symbols like components, buildSSEEndpoint, SSEState, useSSE) to ensure the file
is properly licensed.
- Around line 22-27: The hook must thread the remoteNode through the SSE request
so multi-node deployments route correctly; update useDAGsListSSE to accept or
include a remoteNode and ensure buildSSEEndpoint gets it: modify the signature
of useDAGsListSSE (or ensure DAGsListParams contains remoteNode) and pass that
remoteNode into buildSSEEndpoint('/events/dags', paramsWithRemoteNode) before
calling useSSE<DAGsListSSEResponse>(endpoint, enabled) so the SSE is correctly
routed to the selected node.
In `@ui/src/lib/utils.ts`:
- Around line 1-2: Add the GPL v3 license header to the top of the touched
source file (before any code or imports) — either run the repository helper
(make addlicense) or insert the standard GPL header block immediately above the
existing import lines (the ones importing "clsx" and "twMerge"). Ensure the
header follows the project's standard format for .ts files and is placed before
the lines "import { clsx, type ClassValue } from 'clsx';" and "import { twMerge
} from 'tailwind-merge';".
In `@ui/src/pages/dag-runs/index.tsx`:
- Line 1: This file is missing the repository GPL v3 source header; add the
standard GPL v3 license header at the very top of the TSX file (above the
existing import statement like "import dayjs from 'dayjs';") so the header
precedes all code, or run the repository tool to apply it (e.g., execute the
project's make addlicense or equivalent) to ensure the file
ui/src/pages/dag-runs/index.tsx includes the required header block.
In `@ui/src/pages/dags/index.tsx`:
- Around line 198-224: queryParams used for the SSE subscription does not
include remoteNode, so useDAGsListSSE(queryParams) may subscribe to the wrong
node; update the memoized queryParams object (and its dependency array) to
include remoteNode (e.g., add remoteNode: remoteNode) so both
useDAGsListSSE(queryParams) and the useQuery('/dags', { params: { query: {
...queryParams, remoteNode } } }) use the same routing parameter.
- Line 1: Add the GPL v3 license header comment at the top of this TSX source
file (above the first token, e.g., above the existing import { debounce } from
'lodash'; line) to match the repository's license headers; you can run make
addlicense to apply it automatically or paste the standard GPL v3 header block
manually so the file passes license checks.
In `@ui/src/pages/design/index.tsx`:
- Around line 665-671: The search array construction in the dagFiles.filter
callback currently uses the nullish-coalescing chain item.dag.labels ??
item.dag.tags ?? [], which treats an explicit empty labels array as a valid
value and prevents falling back to tags; update the logic in the searchableText
building inside the dagFiles.filter callback so it checks labels' length (e.g.,
use item.dag.labels && item.dag.labels.length ? item.dag.labels : item.dag.tags
?? []) to only use labels when non-empty and otherwise fall back to
item.dag.tags or an empty array; adjust the spread expression in the
searchableText declaration accordingly (referencing dagFiles.filter,
searchableText, item.dag.labels, and item.dag.tags).
---
Nitpick comments:
In `@internal/core/label_test.go`:
- Around line 94-109: Rename tests and their usages to exercise the canonical
Label API instead of deprecated Tag aliases: change the test function name
TestTag_IsZero to TestLabel_IsZero and update any occurrences of Tag*
types/aliases used in this suite to their Label* counterparts (e.g., replace Tag
struct/alias usages with Label, and any Tag-prefixed helper names with
Label-prefixed names) so the suite validates Label.IsZero and related behavior;
leave a minimal separate compatibility test that covers Tag* aliases elsewhere.
Ensure you update references inside the test body (table entries, t.Run names,
and calls like tt.label.IsZero()) and apply the same replacement for the other
affected test blocks mentioned (lines covering the other Tag* usages).
In `@internal/core/label.go`:
- Around line 355-399: Add proper Go deprecation doc comments to each
compatibility alias and wrapper in this block so IDEs and pkg.go.dev surface
them; for example prepend comments like "Deprecated: use
Label/Labels/LabelFilterType/etc. instead." to the type aliases (Tag, Tags,
TagFilterType, TagFilter), constants (MaxTagKeyLength, TagKeyPatternStr,
TagFilterTypeKeyOnly, etc.), vars (ValidTagKeyPattern, ValidTagValuePattern) and
the thin wrapper functions (ValidateTag, ValidateTags, ParseTag, NewTags,
ParseTagFilter, and the method MatchesTags on LabelFilter) referencing the
preferred names (Label, Labels, LabelFilterType, LabelFilter) in each comment.
In `@internal/core/spec/types/stringarray_test.go`:
- Around line 217-225: Rename the stale sub-test names inside the
TestLabelsValue test to use "Label" terminology for consistency: update the case
names currently "SingleTag" and "MultipleTags" to something like "SingleLabel"
and "MultipleLabels" (or "SingleLabelValue"/"MultipleLabelValues") in the test
cases where input and wantValues are defined so the sub-test identifiers in
TestLabelsValue reflect the label vocabulary used across the PR.
In `@internal/intg/history_test.go`:
- Around line 288-326: The test currently only ensures the history CLI runs
without error but doesn't assert filtering works; update
TestHistoryCommand_Labels to verify results by either extending the test helper
to capture stdout from th.RunCommand(cmd.History(), test.CmdTest{...}) and
assert the output contains "labeled-dag-1" and does not contain "labeled-dag-2",
or bypass the CLI and call the underlying history query API (e.g., the history
manager used by the CLI) directly after starting the DAGs and assert that the
returned history for labels="prod" includes dag1.DAG (or its name) and excludes
dag2.DAG; reference TestHistoryCommand_Labels, th.RunCommand, cmd.History(),
test.CmdTest, th.DAG and th.DAGRunMgr/GetLatestStatus to locate where to add the
assertions.
In `@internal/persis/filedag/dagindex/dagindex_test.go`:
- Around line 155-182: Update TestBuild_BasicDAGs to include a legacy tags
fixture and assert it is indexed: add a second YAML input (or extend dagContent)
containing the legacy "tags:" field (e.g., tags: ["env=prod"] or tags:\n -
env=prod) and include its metadata in the files slice (YAMLFileMeta). After
calling Build(context.Background(), dir, files, nil) assert that the resulting
entry.Labels contains the tag value from the legacy "tags" field (use the same
assertions style as the existing labels check), keeping references to
TestBuild_BasicDAGs, dagContent (or newLegacyDagContent), YAMLFileMeta and
entry.Labels so the test verifies backward compatibility.
In `@internal/persis/filedag/store_test.go`:
- Around line 1410-1423: Replace the weak length assertions with exact set
comparisons: after the first call to store.LabelList(ctx) compare labels1
against the expected label slice using assert.ElementsMatch (to ensure keys like
"env"/"team" are present), and for the second call use assert.ElementsMatch to
compare labels2 to the same expected set and also assert.ElementsMatch(labels1,
labels2) to ensure the index returned the identical label set; keep the existing
checks on err and errList variables (errList1/errList2) and the
indexPath/fileExists verification.
In `@internal/persis/filedag/store.go`:
- Around line 958-962: The labelList is built from iterating labelSet which
yields non-deterministic ordering; after populating labelList (the slice created
from labelSet), sort it (e.g., call sort.Strings(labelList)) before returning so
the function (the block that creates labelList from labelSet) always returns
labels in a deterministic order; ensure the sort package is imported and then
return labelList, errList, nil as before.
In `@internal/persis/filedagrun/store_test.go`:
- Around line 904-916: Tests use the alias type and variable name "TagFilter";
change them to the canonical label-focused names and types: in
TestResolveStatus_FastPath_TagFilterReject (and the similar test around lines
937–944) replace the variable declaration tagFilters :=
[]core.TagFilter{core.ParseLabelFilter("env=prod")} with labelFilters :=
[]core.LabelFilter{core.ParseLabelFilter("env=prod")} and pass labelFilters to
resolveStatus(ctx, dagRun, labelFilters, nil, false); keep references to
resolveStatus, DAGRun, DAGRunSummary and core.ParseLabelFilter the same so the
only edits are the type and variable name for consistency.
In `@internal/runtime/executor/task_test.go`:
- Around line 348-366: Add a subtest in the same Test block that verifies the
deprecated wrapper still works: create a coordinatorv1.Task{}, call
executor.WithTags("env=prod,team=backend")(task) and assert task.Labels equals
"env=prod,team=backend" (and similarly a case with empty string asserting
Empty). This ensures executor.WithTags(...) remains API-compatible with
executor.WithLabels and prevents regressions of the wrapper.
In `@internal/service/frontend/api/v1/dagruns_test.go`:
- Around line 254-397: Add an assertion in each labeled-start/enqueue test to
verify the runtime labels are persisted in the DAG-run metadata: after obtaining
the dagRunId (and after the existing require.Eventually wait for run status),
call the GET /api/v1/dag-runs/{name}/{dagRunId} response (parsed into
api.GetDAGRunDetails200JSONResponse) and assert that DagRunDetails.Labels
contains the expected label strings; update
TestGetDAGRunSpecInlineStartWithLabelsDoesNotPatchSpec,
TestGetDAGRunSpecInlineEnqueueWithLabelsPatchesSpec, and
TestGetDAGRunSpecFileEnqueueWithLabelsDoesNotPatchSpec to include this Labels
check (use the same status-wait or a new GET similar to the one in
TestGetDAGRunSpecInlineEnqueueWithLabelsPatchesSpec).
In `@internal/service/frontend/api/v1/dags_test.go`:
- Around line 526-598: The tests ExecuteDAGWithLabels and EnqueueDAGWithLabels
only assert success but not that labels were actually applied; after getting
execResp.DagRunId and enqResp.DagRunId call the DAG-run read endpoint for the
created run (e.g. GET the run resource using the returned DagRunId via
server.Client().Get(...).ExpectStatus(http.StatusOK).Send(t)), unmarshal the
response into the DAG-run DTO and assert its Labels field contains the expected
labels (compare to the labels slice passed to ExecuteDAGJSONRequestBody /
EnqueueDAGDAGRunJSONRequestBody) so the test fails if handlers accept but ignore
Labels.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+781
to
+793
| - name: "labels" | ||
| in: "query" | ||
| required: false | ||
| schema: | ||
| type: "string" | ||
| description: "Filter DAGs by labels (comma-separated). Returns DAGs that have ALL specified labels." | ||
| - name: "tags" | ||
| in: "query" | ||
| required: false | ||
| schema: | ||
| type: "string" | ||
| description: "Filter DAGs by tags (comma-separated). Returns DAGs that have ALL specified tags." | ||
| deprecated: true | ||
| description: "Deprecated alias for labels. Filter DAGs by labels (comma-separated). Returns DAGs that have ALL specified labels." |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify whether handlers reject requests that include both labels and tags,
# then compare that behavior against the OpenAPI spec locations that currently allow both.
set -euo pipefail
echo "Handler references to labels/tags conflict handling:"
rg -n -C4 'labels|tags|both|mutual|deprecated alias' \
internal/service/frontend/api/v1/dags.go \
internal/service/frontend/api/v1/dagruns.go
echo
echo "OpenAPI alias pairs to update/document:"
rg -n -C3 'name: "labels"|name: "tags"|labels:|tags:' api/v1/api.yamlRepository: dagucloud/dagu
Length of output: 50370
Document mutual exclusion of labels and tags in the OpenAPI spec.
The handlers explicitly reject requests with both labels and tags parameters set (returning HTTP 400 with message "labels and deprecated tags cannot both be set"), but the OpenAPI spec permits both to be included in a single request. Update the parameter/schema descriptions at each pair of alias locations to clarify this constraint so generated clients and validators match server behavior.
Affected locations: query parameters in GET /dags and GET /dag-runs (lines 781–793, 1794–1806); request body in POST /dags/{dagName}/start, POST /dags/{dagName}/start-sync, POST /dags/{dagName}/enqueue, POST /dag-runs/spec, and POST /dag-runs/spec/enqueue (lines 1058–1061, 1131–1134, 1211–1214, 1863–1866, 1940–1943).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@api/v1/api.yaml` around lines 781 - 793, Update the OpenAPI parameter and
request-body descriptions to document that the `labels` parameter and the
deprecated alias `tags` are mutually exclusive: for each occurrence of the pair
(query params in GET /dags and GET /dag-runs; request bodies for POST
/dags/{dagName}/start, POST /dags/{dagName}/start-sync, POST
/dags/{dagName}/enqueue, POST /dag-runs/spec, POST /dag-runs/spec/enqueue)
change the `labels` and `tags` descriptions to explicitly state they cannot be
used together (e.g. "Mutually exclusive with `tags`—server returns HTTP 400 if
both are set") and keep `tags` marked deprecated while referencing `labels` in
its description (e.g. "Deprecated alias for `labels`; mutually exclusive with
`labels`"). Ensure the textual change is applied to the `labels` and `tags`
parameter descriptions and to any request-body fields named `labels`/`tags` so
generated clients and validators reflect the server-side rejection.
|
|
||
| <docs> | ||
| When a user message begins with [Doc: id | title] tags, these reference markdown | ||
| When a user message begins with [Doc: id | title] labels, these reference markdown |
There was a problem hiding this comment.
Avoid calling document references “labels”.
This line refers to [Doc: id | title] markers, not DAG labels, so the wording can confuse the assistant’s instructions.
Proposed wording
-When a user message begins with [Doc: id | title] labels, these reference markdown
+When a user message begins with [Doc: id | title] document markers, these reference markdown📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| When a user message begins with [Doc: id | title] labels, these reference markdown | |
| When a user message begins with [Doc: id | title] document markers, these reference markdown |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/agent/system_prompt.txt` at line 196, The wording in the system
prompt currently calls "[Doc: id | title]" entries "labels", which may be
confused with DAG labels; update the sentence to call them "markers" or
"references" instead (e.g., change "labels" to "markers" or "document
references") so the line reads something like "When a user message begins with
[Doc: id | title] markers, these reference markdown" and ensure the "[Doc: id |
title]" token is used exactly to locate the text to edit.
Comment on lines
+307
to
+323
| // UnmarshalJSON deserializes DAGs written by both the canonical labels field | ||
| // and the deprecated tags field used by older persisted dag.json files. | ||
| func (d *DAG) UnmarshalJSON(data []byte) error { | ||
| type alias DAG | ||
| aux := struct { | ||
| *alias | ||
| DeprecatedTags Labels `json:"tags,omitempty"` | ||
| }{ | ||
| alias: (*alias)(d), | ||
| } | ||
| if err := json.Unmarshal(data, &aux); err != nil { | ||
| return err | ||
| } | ||
| if len(d.Labels) == 0 && len(aux.DeprecatedTags) > 0 { | ||
| d.Labels = aux.DeprecatedTags | ||
| } | ||
| return nil |
There was a problem hiding this comment.
Only fall back to tags when labels is absent.
len(d.Labels) == 0 treats an explicit "labels":[] the same as a missing field, so JSON containing both "labels":[] and legacy "tags":[...] will resurrect deprecated tags instead of honoring canonical labels.
🛠️ Proposed precedence fix
func (d *DAG) UnmarshalJSON(data []byte) error {
+ var raw map[string]json.RawMessage
+ if err := json.Unmarshal(data, &raw); err != nil {
+ return err
+ }
+
type alias DAG
aux := struct {
*alias
DeprecatedTags Labels `json:"tags,omitempty"`
}{
alias: (*alias)(d),
}
if err := json.Unmarshal(data, &aux); err != nil {
return err
}
- if len(d.Labels) == 0 && len(aux.DeprecatedTags) > 0 {
+ if _, hasLabels := raw["labels"]; !hasLabels && len(aux.DeprecatedTags) > 0 {
d.Labels = aux.DeprecatedTags
}
return nil
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/core/dag.go` around lines 307 - 323, The UnmarshalJSON currently
treats an explicit "labels":[] the same as a missing field because it checks
len(d.Labels)==0; change the aux struct to capture whether the labels field was
present (e.g. add Labels *Labels `json:"labels,omitempty"` to the aux used in
DAG.UnmarshalJSON) and after json.Unmarshal use a presence check: if aux.Labels
!= nil set d.Labels = *aux.Labels, else if aux.DeprecatedTags not empty set
d.Labels = aux.DeprecatedTags; keep the existing alias embedding and return
behavior but use aux.Labels presence instead of len(d.Labels).
Comment on lines
+228
to
+244
| // UnmarshalJSON keeps legacy onCancel and tags status files readable while | ||
| // normalizing canonical handler/metadata names in memory. | ||
| func (st *DAGRunStatus) UnmarshalJSON(data []byte) error { | ||
| type alias DAGRunStatus | ||
| aux := struct { | ||
| alias | ||
| OnCancel *Node `json:"onCancel,omitempty"` | ||
| OnCancel *Node `json:"onCancel,omitempty"` | ||
| DeprecatedTags []string `json:"tags,omitempty"` | ||
| }{} | ||
| if err := json.Unmarshal(data, &aux); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| *st = DAGRunStatus(aux.alias) | ||
| if len(st.Labels) == 0 && len(aux.DeprecatedTags) > 0 { | ||
| st.Labels = aux.DeprecatedTags | ||
| } |
There was a problem hiding this comment.
Preserve explicit empty canonical labels over legacy tags.
Line 242 treats labels: [] the same as a missing labels field, so JSON containing both labels: [] and legacy tags is normalized back to the deprecated values. Detect field presence instead of slice length so canonical labels always wins.
🐛 Proposed fix
func (st *DAGRunStatus) UnmarshalJSON(data []byte) error {
+ var raw map[string]json.RawMessage
+ if err := json.Unmarshal(data, &raw); err != nil {
+ return err
+ }
+ _, hasLabels := raw["labels"]
+
type alias DAGRunStatus
aux := struct {
alias
OnCancel *Node `json:"onCancel,omitempty"`
DeprecatedTags []string `json:"tags,omitempty"`
}{}
if err := json.Unmarshal(data, &aux); err != nil {
return err
}
*st = DAGRunStatus(aux.alias)
- if len(st.Labels) == 0 && len(aux.DeprecatedTags) > 0 {
+ if !hasLabels && len(aux.DeprecatedTags) > 0 {
st.Labels = aux.DeprecatedTags
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // UnmarshalJSON keeps legacy onCancel and tags status files readable while | |
| // normalizing canonical handler/metadata names in memory. | |
| func (st *DAGRunStatus) UnmarshalJSON(data []byte) error { | |
| type alias DAGRunStatus | |
| aux := struct { | |
| alias | |
| OnCancel *Node `json:"onCancel,omitempty"` | |
| OnCancel *Node `json:"onCancel,omitempty"` | |
| DeprecatedTags []string `json:"tags,omitempty"` | |
| }{} | |
| if err := json.Unmarshal(data, &aux); err != nil { | |
| return err | |
| } | |
| *st = DAGRunStatus(aux.alias) | |
| if len(st.Labels) == 0 && len(aux.DeprecatedTags) > 0 { | |
| st.Labels = aux.DeprecatedTags | |
| } | |
| // UnmarshalJSON keeps legacy onCancel and tags status files readable while | |
| // normalizing canonical handler/metadata names in memory. | |
| func (st *DAGRunStatus) UnmarshalJSON(data []byte) error { | |
| var raw map[string]json.RawMessage | |
| if err := json.Unmarshal(data, &raw); err != nil { | |
| return err | |
| } | |
| _, hasLabels := raw["labels"] | |
| type alias DAGRunStatus | |
| aux := struct { | |
| alias | |
| OnCancel *Node `json:"onCancel,omitempty"` | |
| DeprecatedTags []string `json:"tags,omitempty"` | |
| }{} | |
| if err := json.Unmarshal(data, &aux); err != nil { | |
| return err | |
| } | |
| *st = DAGRunStatus(aux.alias) | |
| if !hasLabels && len(aux.DeprecatedTags) > 0 { | |
| st.Labels = aux.DeprecatedTags | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/core/exec/runstatus.go` around lines 228 - 244, The UnmarshalJSON
for DAGRunStatus treats an explicit empty labels slice the same as missing by
checking len(st.Labels); change aux to capture presence by using Labels
*[]string `json:"labels,omitempty"` in the aux struct, then after unmarshalling
and assigning *st = DAGRunStatus(aux.alias) use the pointer check (if aux.Labels
== nil && len(aux.DeprecatedTags) > 0) to copy DeprecatedTags into st.Labels so
an explicit labels: [] is preserved while legacy tags fill only when labels is
absent.
Comment on lines
+221
to
+225
| func ParseLabelFilter(s string) LabelFilter { | ||
| s = strings.TrimSpace(s) | ||
| if s == "" { | ||
| return LabelFilter{} | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect ParseLabelFilter call sites and their surrounding blank-token handling.
rg -n -C 5 '\bParseLabelFilter\s*\('Repository: dagucloud/dagu
Length of output: 8711
🏁 Script executed:
#!/bin/bash
# 1. Check what opts.Labels is and where it comes from in pagination.go
sed -n '180,210p' internal/persis/filedagrun/pagination.go | cat -n
# 2. Find the FilterOptions definition
rg -n "type.*FilterOptions" internal/persis/filedagrun/
# 3. Examine MatchesFilters implementation and its AND logic
rg -n -A 10 "func.*MatchesFilters" internal/core/label.goRepository: dagucloud/dagu
Length of output: 1488
🏁 Script executed:
#!/bin/bash
# 4. Check if there's a test for empty filter behavior
rg -n "ParseLabelFilter.*\"\"" internal/core/label_test.go
# 5. Look for the deprecated tag aliases at lines 345-352
sed -n '340,360p' internal/core/label.go | cat -nRepository: dagucloud/dagu
Length of output: 665
🏁 Script executed:
#!/bin/bash
# 6. Check where opts.Labels originates - trace FilterOptions
rg -n -B 5 -A 5 "\.Labels\s*=" internal/persis/filedagrun/pagination.go | head -30Repository: dagucloud/dagu
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check the MatchesLabels implementation for empty key-only filter
sed -n '250,290p' internal/core/label.go | cat -n
# Check what LabelFilterType is for an empty string
sed -n '235,250p' internal/core/label.go | cat -nRepository: dagucloud/dagu
Length of output: 1740
🏁 Script executed:
#!/bin/bash
# Look for ParseTagFilter and check if it has deprecation doc
rg -n -B 3 -A 3 "func ParseTagFilter" internal/core/label.go
# Check all functions starting with "Parse" or related to deprecated tags
rg -n "^func.*Tag" internal/core/label.go | head -20Repository: dagucloud/dagu
Length of output: 458
🏁 Script executed:
#!/bin/bash
# Verify opts.Labels type definition in FilterOptions
rg -n "type.*FilterOptions\|Labels\s*\[\]" internal/persis/filedagrun/pagination.go
# Look for where opts comes from and if it's validated
rg -n "exec.ListDAGRunStatusesOptions" internal/persis/filedagrun/Repository: dagucloud/dagu
Length of output: 1557
🏁 Script executed:
#!/bin/bash
# Verify empty filter behavior with tests and what HasKey does
sed -n '160,180p' internal/core/label.go | cat -n
# Check if there's a HasKey method
rg -n "func.*HasKey" internal/core/label.goRepository: dagucloud/dagu
Length of output: 696
🏁 Script executed:
#!/bin/bash
# Look for test coverage of empty filter
rg -n "empty\|\"\"" internal/core/label_test.go | grep -i filter
# Check if opts.Labels can come from user input without validation
rg -n -B 10 "ListDAGRunStatusesOptions" internal/persis/filedagrun/store.go | grep -A 10 "prepareListOptions"Repository: dagucloud/dagu
Length of output: 295
🏁 Script executed:
#!/bin/bash
# Look at lines 345-352 exact content to see if there are deprecated markers
sed -n '345,365p' internal/core/label.go | cat -nRepository: dagucloud/dagu
Length of output: 784
🏁 Script executed:
#!/bin/bash
# Check where opts.Labels comes from in ListDAGRunStatusesOptions
rg -n "type ListDAGRunStatusesOptions\|Labels\s*\[\]string" --type go | grep -A 5 -B 5 ListDAGRunStatusesOptions
# Look at the exec package to see if Labels is user-provided
find . -path "*/exec/*.go" -type f | xargs grep -l "ListDAGRunStatusesOptions" | head -3Repository: dagucloud/dagu
Length of output: 124
🏁 Script executed:
#!/bin/bash
# Check what the pagination code does with opts.Labels - is it filtered?
sed -n '185,210p' internal/persis/filedagrun/pagination.go | cat -n
# Check the go doc for ParseTagFilter and tag-related functions
sed -n '375,400p' internal/core/label.go | cat -nRepository: dagucloud/dagu
Length of output: 1714
🏁 Script executed:
#!/bin/bash
# Check ListDAGRunStatusesOptions definition in exec package
grep -n "type ListDAGRunStatusesOptions" internal/core/exec/dagrun.go -A 20Repository: dagucloud/dagu
Length of output: 767
🏁 Script executed:
#!/bin/bash
# Check if there's documentation for the deprecated functions and test for empty filter
rg -n -B 2 "func (f LabelFilter) MatchesTags\|func ParseTagFilter\|func ValidateTag" internal/core/label.go
# Look for any test of empty filter in label_test.go
rg -n "empty\|blank\|spaces" internal/core/label_test.goRepository: dagucloud/dagu
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Check if Labels field in ListDAGRunStatusesOptions is validated anywhere
rg -n "o.Labels\|opts.Labels" internal/persis/filedagrun/ -A 2 -B 2
# Check if there's a test that uses empty string in labels
rg -n "\[\]string.*{.*\"\"" internal/core/label_test.go
# Verify the exact behavior - check if empty filter tests exist
rg -n "TestParse.*Filter\|empty" internal/core/label_test.go | head -20Repository: dagucloud/dagu
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Look at all deprecated tag functions to see if any have doc comments
sed -n '354,410p' internal/core/label.go | cat -nRepository: dagucloud/dagu
Length of output: 1472
🏁 Script executed:
#!/bin/bash
# Confirm: does pagination.go validate Labels before passing to newDAGRunStatusIterator?
rg -n -B 15 "newDAGRunStatusIterator" internal/persis/filedagrun/store.go | grep -A 15 "ListStatuses"
# Check if there are tests showing empty label filter behavior
sed -n '300,350p' internal/core/label_test.go | cat -nRepository: dagucloud/dagu
Length of output: 1502
🏁 Script executed:
#!/bin/bash
# Check the full Store.ListStatuses method to see if there's filtering before newDAGRunStatusIterator
rg -n "func.*ListStatuses" internal/persis/filedagrun/store.go -A 30 | head -50
# Verify: does pagination code do any filtering of opts.Labels before use?
rg -n "opts.Labels" internal/persis/filedagrun/ -B 3 -A 3Repository: dagucloud/dagu
Length of output: 2333
🏁 Script executed:
#!/bin/bash
# Check if there's a test covering empty filter in MatchesFilters
sed -n '380,450p' internal/core/label_test.go | cat -nRepository: dagucloud/dagu
Length of output: 1969
Fix blank label filtering in pagination and add deprecation markers.
ParseLabelFilter("") creates a key-only filter with an empty key that will always fail to match (since labels cannot have empty keys). If opts.Labels in ListDAGRunStatusesOptions contains blank entries, the filters will silently fail. At line 197–198 in pagination.go, empty strings are appended directly without checking; compare to the correct pattern at filedag/store.go:1001–1002 which validates trimmed != "" before parsing.
Filter empty labels before appending to prevent silent match failures:
pagination.go lines 196–199
labelFilters := make([]core.LabelFilter, 0, len(opts.Labels))
for _, label := range opts.Labels {
if trimmed := strings.TrimSpace(label); trimmed != "" {
labelFilters = append(labelFilters, core.ParseLabelFilter(trimmed))
}
}Additionally, the deprecated compatibility functions (lines 354–410) lack Go // Deprecated: documentation comments. Add markers to ValidateTag, ValidateTags, ParseTag, NewTags, and ParseTagFilter to guide users toward the Label/Labels API.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/core/label.go` around lines 221 - 225, ParseLabelFilter can produce
a key-only empty filter when given "" and pagination code appends raw
opts.Labels entries, causing silent non-matching filters; fix by trimming and
skipping empty label strings before calling ParseLabelFilter (e.g., iterate
opts.Labels, do trimmed := strings.TrimSpace(label) and only append
core.ParseLabelFilter(trimmed) when trimmed != ""), and add Go deprecation
comments ("// Deprecated: ...") to the legacy functions ValidateTag,
ValidateTags, ParseTag, NewTags, and ParseTagFilter to guide users to the
Label/Labels API.
Comment on lines
1634
to
1647
| var labels []string | ||
| if labelsParam := params.Get("labels"); labelsParam != "" { | ||
| labels = parseCommaSeparatedLabels(&labelsParam) | ||
| } | ||
| if tagsParam := params.Get("tags"); tagsParam != "" { | ||
| tags = parseCommaSeparatedTags(&tagsParam) | ||
| if len(labels) > 0 { | ||
| return nil, &Error{ | ||
| HTTPStatus: http.StatusBadRequest, | ||
| Code: api.ErrorCodeBadRequest, | ||
| Message: "labels and deprecated tags cannot both be set", | ||
| } | ||
| } | ||
| labels = parseCommaSeparatedLabels(&tagsParam) | ||
| } |
There was a problem hiding this comment.
Reject deprecated tags based on the raw labels parameter, not parsed labels.
Here labels=,&tags=prod is accepted because parseCommaSeparatedLabels returns an empty slice, while the HTTP path rejects any non-empty labels and tags pair. Use the shared resolver so SSE and HTTP behave the same.
Proposed fix
- var labels []string
- if labelsParam := params.Get("labels"); labelsParam != "" {
- labels = parseCommaSeparatedLabels(&labelsParam)
- }
- if tagsParam := params.Get("tags"); tagsParam != "" {
- if len(labels) > 0 {
- return nil, &Error{
- HTTPStatus: http.StatusBadRequest,
- Code: api.ErrorCodeBadRequest,
- Message: "labels and deprecated tags cannot both be set",
- }
- }
- labels = parseCommaSeparatedLabels(&tagsParam)
- }
+ var labelsParam, deprecatedTagsParam *string
+ if rawLabels := params.Get("labels"); rawLabels != "" {
+ labelsParam = &rawLabels
+ }
+ if rawTags := params.Get("tags"); rawTags != "" {
+ deprecatedTagsParam = &rawTags
+ }
+ labelQueryParam, labelErr := queryLabelsParam(labelsParam, deprecatedTagsParam)
+ if labelErr != nil {
+ return nil, labelErr
+ }
+ labels := parseCommaSeparatedLabels(labelQueryParam)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/service/frontend/api/v1/dags.go` around lines 1634 - 1647, The
current check uses the parsed slice `labels` (via `parseCommaSeparatedLabels`)
so an input like `labels=,&tags=prod` slips through; change the validation to
reject when the raw `labelsParam` string is non-empty together with `tagsParam`
(i.e., check `labelsParam != "" && tagsParam != ""`) before calling
`parseCommaSeparatedLabels`, and ensure the same shared resolver used by
SSE/HTTP is invoked so both paths behave identically; update the error return
(the existing `&Error{HTTPStatus: http.StatusBadRequest, Code:
api.ErrorCodeBadRequest, Message: "labels and deprecated tags cannot both be
set"}`) to remain used in this new early validation.
Comment on lines
+59
to
+60
| - `--labels` — Additional labels (comma-separated) | ||
| - `--tags` — Deprecated alias for `--labels` |
There was a problem hiding this comment.
Keep the label grammar consistent across CLI docs.
start documents key=value or key-only, but enqueue and history only say “comma-separated”. Mirror the same grammar so users know key-only labels are accepted there too.
Suggested wording
-- `--labels` — Additional labels (comma-separated)
+- `--labels` — Additional labels (comma-separated key=value or key-only)
- `--tags` — Deprecated alias for `--labels`
...
-- `--labels` — Filter by labels (comma-separated, AND logic)
+- `--labels` — Filter by labels (comma-separated key=value or key-only, AND logic)
- `--tags` — Deprecated alias for `--labels`Also applies to: 110-111
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/dagu/references/cli.md` around lines 59 - 60, The CLI docs use
inconsistent label grammar: `start` documents labels as "key=value or key-only"
but `enqueue` and `history` just say "comma-separated"; update the `--labels`
(and deprecated `--tags`) option descriptions for the `enqueue` and `history`
command entries to mirror the `start` wording (e.g., "Additional labels
(comma-separated, accepted as key=value or key-only)") so users know key-only
labels are supported; ensure you update all occurrences (including the mention
at lines 110-111) and keep the `--tags` alias note consistent with `--labels`.
| | `group` | string | — | Group for UI organization | | ||
| | `description` | string | — | Description | | ||
| | `tags` | map or array | — | Key-value tags (keys max 63 chars, values max 255 chars) | | ||
| | `labels` | map or array | — | Key-value labels (keys max 63 chars, values max 255 chars). `tags` is accepted as a deprecated alias. | |
There was a problem hiding this comment.
Document bare label strings in the array form.
The current wording says “Key-value labels”, but the schema/tests also use bare array labels like prod and critical. Please make the accepted array forms explicit so users don’t assume only key=value entries are valid.
📝 Proposed wording
-| `labels` | map or array | — | Key-value labels (keys max 63 chars, values max 255 chars). `tags` is accepted as a deprecated alias. |
+| `labels` | map or array | — | Labels as a map or array. Array entries may be bare labels or `key=value` labels; map keys max 63 chars and values max 255 chars. `tags` is accepted as a deprecated alias. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `labels` | map or array | — | Key-value labels (keys max 63 chars, values max 255 chars). `tags` is accepted as a deprecated alias. | | |
| | `labels` | map or array | — | Labels as a map or array. Array entries may be bare labels or `key=value` labels; map keys max 63 chars and values max 255 chars. `tags` is accepted as a deprecated alias. | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/dagu/references/schema.md` at line 10, Update the `labels` (and
deprecated `tags`) schema description to explicitly state that the array form
accepts both bare label strings (e.g., "prod", "critical") and key=value pairs,
and clarify the constraints for each (keys max 63 chars, values max 255 chars);
modify the line containing `labels` to read something like "map or array —
Key-value labels (keys max 63 chars, values max 255 chars) or an array of bare
label strings (e.g. 'prod', 'critical') and/or key=value entries; `tags` is
accepted as a deprecated alias." to make the accepted array formats unambiguous.
Comment on lines
1
to
4
| import { Badge } from '@/components/ui/badge'; | ||
| import { cn, parseTagParts } from '@/lib/utils'; | ||
| import { cn, parseLabelParts } from '@/lib/utils'; | ||
| import { ChevronDown, X } from 'lucide-react'; | ||
| import * as React from 'react'; |
There was a problem hiding this comment.
Add the GPL header to this touched TSX source.
Please run make addlicense or add the standard header before the imports.
Proposed header addition
+// Copyright (C) 2026 Yota Hamada
+// SPDX-License-Identifier: GPL-3.0-or-later
+
import { Badge } from '@/components/ui/badge';
import { cn, parseLabelParts } from '@/lib/utils';As per coding guidelines, “**/*.{go,ts,tsx,js}: Apply GPL v3 license headers on source files, managed via make addlicense”.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { Badge } from '@/components/ui/badge'; | |
| import { cn, parseTagParts } from '@/lib/utils'; | |
| import { cn, parseLabelParts } from '@/lib/utils'; | |
| import { ChevronDown, X } from 'lucide-react'; | |
| import * as React from 'react'; | |
| // Copyright (C) 2026 Yota Hamada | |
| // SPDX-License-Identifier: GPL-3.0-or-later | |
| import { Badge } from '@/components/ui/badge'; | |
| import { cn, parseLabelParts } from '@/lib/utils'; | |
| import { ChevronDown, X } from 'lucide-react'; | |
| import * as React from 'react'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/ui/label-combobox.tsx` around lines 1 - 4, Add the GPL v3
license header to the top of this touched TSX file
(ui/src/components/ui/label-combobox.tsx) before any imports; either run the
repository helper (make addlicense) or paste the standard GPL header comment at
the very top of the file so it appears above the existing import block (import {
Badge } ..., import { cn, parseLabelParts } ..., etc.).
Comment on lines
+67
to
+76
| const addLabel = (label: string) => { | ||
| const normalized = label.toLowerCase().trim(); | ||
| if (!normalized) return; | ||
|
|
||
| // Check for duplicates (case-insensitive) | ||
| if (selectedTags.some((t) => t.toLowerCase() === normalized)) { | ||
| if (selectedLabels.some((t) => t.toLowerCase() === normalized)) { | ||
| return; | ||
| } | ||
|
|
||
| onTagsChange([...selectedTags, normalized]); | ||
| onLabelsChange([...selectedLabels, normalized]); |
There was a problem hiding this comment.
Canonicalize around = before dedupe.
label.toLowerCase().trim() keeps inputs like env = prod distinct from env=prod, so duplicate checks and suggestion filtering can show semantically duplicate labels.
Proposed canonicalization
const addLabel = (label: string) => {
- const normalized = label.toLowerCase().trim();
+ const { key, value } = parseLabelParts(label);
+ const normalizedKey = key.trim().toLowerCase();
+ const normalizedValue = value?.trim().toLowerCase();
+ const normalized =
+ normalizedValue === undefined || normalizedValue === null
+ ? normalizedKey
+ : `${normalizedKey}=${normalizedValue}`;
if (!normalized) return;
// Check for duplicates (case-insensitive)
- if (selectedLabels.some((t) => t.toLowerCase() === normalized)) {
+ if (selectedLabels.some((t) => t.toLowerCase().trim() === normalized)) {
return;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/ui/label-combobox.tsx` around lines 67 - 76, The duplicate
check fails to canonicalize spaces around '=' (so "env = prod" ≠ "env=prod");
modify addLabel to canonicalize labels by trimming, collapsing any whitespace
around '=' (e.g. replace occurrences of /\s*=\s*/ with '=') and then lowercasing
to produce normalized; use the same canonicalization when checking
selectedLabels (apply it to each t in selectedLabels during the some(...)
duplicate check) and when passing the new label into onLabelsChange so all
stored labels use the same canonical form (references: addLabel, selectedLabels,
onLabelsChange).
yottahmd
force-pushed
the
rename-dag-tags-to-labels
branch
from
57e9d62 to
10774b2
Compare
This was referenced Apr 19, 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
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
labelsthe canonical metadata field while keeping deprecatedtagsparsing and API compatibilityTesting
Summary by CodeRabbit
Release Notes
New Features
/dags/labelsendpoint to retrieve available labels--labelsCLI flag and query parameters for all DAG and DAG-run operationsDeprecations
--tagsCLI flag andtagsquery/request parameters remain supported as backward-compatible aliasestagscontinue to work; migration tolabelsis recommended