feat(apply): add --since/--force for git-history-based deletion sync - #253
Merged
Conversation
mmanciop
added a commit
that referenced
this pull request
Aug 17, 2026
This was referenced Aug 17, 2026
mmanciop
added a commit
that referenced
this pull request
Aug 25, 2026
mmanciop
force-pushed
the
feat/sync-action
branch
from
August 25, 2026 11:19
3cad860 to
4d38819
Compare
nicolastakashi
force-pushed
the
feat/sync-action
branch
from
August 26, 2026 19:50
6d143fa to
b547cb6
Compare
nicolastakashi
pushed a commit
that referenced
this pull request
Aug 27, 2026
nicolastakashi
force-pushed
the
feat/sync-action
branch
from
August 27, 2026 12:19
46265b6 to
a6d2367
Compare
KindDisplayName had no case for the normalized kind "team", so any caller relying on it for a team asset printed the raw lowercase "team" instead of a proper display name -- inconsistent with every other kind, whose display name is a distinct, human-readable string.
…mmand RequireExperimentalFlag lets a stable command keep one flag behind --experimental/-X, for cases where the flag itself needs more time to prove out before the rest of the command's behavior is committed to.
FindNonHiddenYAMLFiles centralizes the hidden-file/YAML-extension filtering apply's directory scan already did, as a fs.WalkDirFunc callers pass to their own filepath.WalkDir call. Kept as a walk-callback rather than a self-contained walker so the WalkDir invocation stays visible at each call site.
Adds internal/git, a thin wrapper over git plumbing (rev-parse, merge-base, cat-file, ls-tree) that classifies a --since ref (empty, all-zeros sentinel, resolved ancestor, resolved non-ancestor, unresolvable) and diffs asset identifiers between that ref and the current -f contents to compute a deletion plan -- by identity (id/origin), never by file path, including PrometheusRule's CRD-shared-identifier / per-alert-name distinction. apply --since wires this into the existing create/update flow: ref-resolution errors, a confirmation prompt for a non-ancestor ref (bypassable with the new --force), per-asset deletion confirmation, and a non-zero exit when a deletion is declined. Both are gated behind --experimental/-X for now (--since via the new RequireExperimentalFlag), pending real-world testing before promotion. Includes the openspec proposal/design/specs/tasks for this change and the follow-on asset-synch GitHub Action, and a go.mod replace pointing at a local dash0-api-client-go checkout for the identifier-extraction helpers (ExtractIdentifier/ExtractPrometheusAlertNames) this depends on, pending their release.
… e2e harness Replaces the originally-planned zipped-repo fixture design with a declarative GitRepoFixture YAML format (internal/testutil/gitscenario.go) describing commit history directly -- readable and diffable in review, with no separate generation step. Validated against a new JSON Schema (git_repo_fixture.schema.json) in gitscenario_test.go. Adds the testcontainers-go-based end-to-end tier (test/e2e) that runs the real dash0 binary against a real git binary inside a container, the one gap unit and integration tests can't cover for --since, which shells out to git rather than using a Go git library. Wired into a new `make test-e2e` target and CI job. Renames internal/testutil/fixtures/apply's readme.txt placeholders to .gitkeep with explanatory comments, matching the project's convention.
Adds four checklist items for gaps a ce-doc-review pass found in the --since implementation: releasing the pending dash0-api-client-go dependency and dropping the go.mod replace directive (1.9), the check-rule name-collision risk in the alerting-rule deletion lookup (4.16), hardening the ID-only spam-filter deletion path to fail instead of warn (4.17), and documenting dash0 diff's exit-code CI-consumption pattern once that command exists (8.8). Tracking only; no code changes.
Merges --dry-run's validation preview with --since's deletion preview
into one per-file listing (previously two separately-headed sections
that repeated a file's path when it had both a surviving and a removed
document), sorted by id/origin within each file. Every line now reads
"Apply|Delete <Kind> "<name>" (<id>)", including for a deleted asset --
its name is resolved by re-reading the asset's content from git
history at --since's ref (internal/git's ReadFileAtRef), falling back
to a "<name>" placeholder only if that lookup fails. Extends the same
name lookup to the real (non-dry-run) per-asset deletion confirmation
prompt and success/decline messages.
Adds agent-mode JSON output for --dry-run: an array of
{path, changes: [{op, name, originOrId}]}, covering the plain,
--since-merged, and single-file-target cases uniformly. Factors the
row-building/grouping logic previously duplicated between apply.go and
since.go into a shared buildDryRunRows so text and JSON rendering
cannot drift from each other.
…ironment Adds three roundtrip scripts run against a real Dash0 environment, closing the one test tier --since previously only had mock-server/e2e-container coverage for: whole-file and multi-document partial deletion, apply --since idempotency (second run against the new baseline reports no changes), and the all-zeros-sentinel / non-ancestor-ref edge cases (including the --force confirmation bypass). Registers all three in run_all.sh's API_TESTS list. Deliberately does not cover PrometheusRule alerting-rule partial removal: verified against the real API that a CRD with 2+ alerts sharing one dash0.com/id never produces more than one live check rule via create/apply (each alert's PUT overwrites the previous one under the shared id) -- filed as #254, since it's a pre-existing bug in the sibling dash0-api-client-go's CRD conversion, not something --since introduced, but it does mean that scenario has no real-world-reachable coverage beyond the existing mock-server-based tests.
…bdirectory -f target
gitutil.Deletion.Path (from git ls-tree) is always repo-root-relative,
while assetDocument.filePath is always relative to the -f target itself.
When -f points at a subdirectory of the repo rather than the repo root,
these two bases diverge: a deletion candidate's path (e.g.
"dashboards/removed.yaml") no longer matches its file's surviving
documents' path ("keep.yaml"), so the merged --dry-run listing grouped
them under different, inconsistently-prefixed entries instead of one
entry per file.
Threads the --since target's scope (already computed in
computeDeletionPlan for the git-side pathspec) through to the dry-run
renderer, stripping it from each deletion path before grouping so both
sides use the same basis. Found while writing documentation examples for
apply --since and noticing the discrepancy against real output.
apply --since/--force shipped in this branch with no documentation at all beyond an incidental spam-filter note. Adds: - docs/commands.md: --since/--force in the apply flags table and usage line, a dedicated `apply --since` (experimental) subsection covering identity/deletion semantics, the merged --dry-run preview (including its agent-mode JSON shape), per-asset confirmation and --force, the non-zero exit on a declined deletion, the ref-resolution error messages (empty/all-zeros/unresolvable), the non-ancestor warning, the no-identifier hard-fail, PrometheusRule alerting-rule deletion, the git/Docker requirement, and a GitHub Actions invocation example; plus a "Common workflows" entry and a taxonomy-intro mention. - README.md: a --since example in "Applying assets", cross-referencing the full reference. - docs/installation.md + README.md: a note that the Docker image (built FROM scratch) has no git, so --since is unavailable from it. - docs/promoting-commands-to-stable.md: a new "Flag-level promotion" section (apply --since is the first flag-level experimental gate in this CLI, so the existing whole-command guide didn't cover it). - internal/skill/gen bundle regenerated (make skill-bundle) to pick up the docs/commands.md changes; SKILL.md's apply summary and workflow list hand-updated to match. Also fixes a pre-existing inaccuracy noticed while writing these examples: apply's per-file output prefixes each line with the file's path relative to -f's own target, not the invoking shell's directory, so `dash0 apply -f assets/` prints "dashboard.yaml: ...", not "assets/dashboard.yaml: ...". Marks tasks.md's Section 8 items done for the --since/--force scope; the diff-specific portions of 8.1/8.2/8.3/8.6 and all of 8.8 remain open until dash0 diff (Section 5) exists.
…leted apply --since failed outright once every asset definition under -f's target was removed: an empty (but surviving) directory hit readDirectory's "no .yaml or .yml files found" error, and a fully removed target directory failed even earlier at os.Stat, both before computeDeletionPlan ever ran. Both cases are now treated as a legitimate all-deletions run, reporting every asset found at --since's ref as a deletion instead of erroring. Also adds regression coverage (and a shared git-scenario fixture) for renaming a subdirectory within the scanned scope, which must remain a no-op since deletion detection is by identifier, never by path -- plus the counterpart case where the -f target itself is renamed and left pointed at the old path, which is correctly reported as a deletion.
…ule CRD --since decided which of the check-rule/recording-rule endpoints to call by trusting the CRD's content at --since's own ref -- a single point in time, not a history of everything the identifier has ever used. A CRD that had its record entry dropped (keeping the alert) in an earlier commit, then had the whole file deleted later, showed --since a ref where the file only ever had an alert: the recording rule created back when the file was still mixed was silently orphaned and permanently unrecoverable, since no later git state can prove it ever existed once the file is gone. Both endpoints are now always attempted, tolerating a 404 from whichever the CRD never used, the same idempotent-delete pattern every other kind already relies on. This reopens a narrow, previously- guarded-against edge case -- an unrelated check rule and recording rule that happen to share the same identifier by coincidence would both be deleted together -- accepted as a tradeoff against silently leaving real orphaned assets behind. Removes the now-unused endpoint-tracking machinery this replaces: internal/git's PrometheusRuleEndpoints type and PrometheusRuleEndpointsByIdentifier snapshot field, and internal/asset.PrometheusRuleEndpoints.
…t record is removed
A PrometheusRule CRD that keeps at least one alerting rule but drops
its last recording rule survives with the same identifier, so --since
never saw it as a whole-CRD deletion -- and applyPrometheusRule simply
stops calling ImportRecordingRule once the CRD has zero records left,
so the recording rule created earlier was never touched at all. Dash0
ended up permanently out of sync with git while --since reported "no
deletions" and exited 0: a false all-clear on a state that no longer
matched.
Recording rules get the same treatment alerting rules already had:
Diff now tracks recording-rule presence per identifier
(PrometheusRecordingRoleByIdentifier, populated via the new
asset.PrometheusRuleHasRecordingRule) and reports a true -> false
transition on a surviving CRD as a "recordingrule"-kind deletion,
dispatched through the same confirm-then-delete path as everything
else in the plan. Unlike alerting rules, this is a coarse
presence/absence signal rather than a per-item diff: Dash0 models a
CRD's recording rules as one server-side resource, not one per
`record:` entry, so there is no per-record identity to track the way
AlertsByName tracks alerts by composed name.
Inverts the existing prometheus-recording-partial-removal fixture and
its unit/integration/e2e tests, whose prior documented behavior
("plain update, not a deletion: there is no per-record identity to
diff") was the bug this fixes -- the "no identity to diff" reasoning
only ever justified skipping a per-record diff, not skipping deletion
of the recording-rule role entirely once it drops to zero.
…out --force A 404 while --since was deleting a planned asset only counted as "already deleted" (and the run kept going) when --force was passed -- otherwise it hard-failed the entire run, even though the asset being gone already IS the state --since is trying to reach. This coupled two unrelated decisions into one flag: whether to skip the confirmation prompt, and whether a concurrent deletion (e.g. someone removing the asset directly in the Dash0 UI between --since's plan and its own delete call) is tolerable. A standalone `<kind> delete --force` reasonably ties these together, since it acts on one asset the caller named by hand; --since reconciles a whole scanned scope, where one already-gone asset shouldn't fail every other deletion (and every earlier create/update) in the same run. deleteAssetByKindAndIdentifier, deletePrometheusRuleCRD, and deleteCheckRuleByName no longer take a force parameter for this purpose: a 404 (or, for deleteCheckRuleByName, not finding the check rule by name at all) is now always treated as already-deleted, independent of --force. --force keeps its existing, separate job in applyDeletions of skipping the confirmation prompt itself.
--since's non-ancestor-ref warning (printed when the ref resolves but isn't an ancestor of HEAD, e.g. after a force-push) could only be accepted by passing --force -- which also silently skips every per-asset deletion confirmation. These are two separate decisions: accepting a doubtful ref is not the same as wanting the whole run unattended. A CI job pointed at the wrong ref by --force alone had no way to ask a human before deleting anything once past that warning. --accept-non-ancestor-ref answers only the ref-acceptance question, leaving every per-asset deletion prompt in place. --force keeps its existing behavior unchanged (it still implies both, for backward compatibility) since it's the documented way to run --since fully unattended in CI/CD, where there's no terminal to answer any prompt either way.
deleteAssetByKindAndIdentifier, deletePrometheusRuleCRD, and deleteCheckRuleByName returned a plain error, so applyDeletions had no way to tell "genuinely deleted just now" apart from "already gone, tolerated" -- it printed "<kind> <name> deleted" unconditionally after every successful call, even when the call itself had only printed "<kind> <name> was already deleted" moments earlier. The two lines contradicted each other and claimed a deletion that never happened, which a CI log or audit trail would take at face value. All three functions now return (alreadyDeleted bool, err error); applyDeletions skips the "deleted" print when alreadyDeleted is true, since IsAlreadyDeleted already printed its own line for that case.
…PrometheusRule CRDs A PrometheusRule CRD's dash0.com/id label names the CRD, not any one alert -- but ParseAsPrometheusAlertRules stamps that same label onto every alerting rule's converted check rule identically. Since a non-empty id always upserts via PUT (create-or-*replace*), a CRD with two or more alerts silently overwrote its own check rules on every apply: only the last alert in document order ended up with a real check rule server-side, while the CLI reported success for all of them. --since's per-alert deletion (AlertsByName, which resolves a removed alert to a check rule by its exact composed name) could then delete the one physical resource backing whichever alert currently matched that name -- which, in the common case, was the resource the CRD's *surviving* alert also depended on. composePrometheusRuleNames now derives a distinct id per alert when a CRD has more than one -- the CRD's own label plus a slug of the alert's composed name -- so each alert gets its own upsert target. A single-alert CRD is unaffected: its one check rule keeps the label verbatim, exactly as before. The derivation is stable across repeated applies of unchanged content and across reordering the CRD's rules, preserving the upsert idempotency a single-alert CRD already had. Root cause lives in dash0-api-client-go's ParseAsPrometheusAlertRules (a separate module); this works around it entirely within dash0-cli, since deriving the id after the fact needs no changes to that library or to --since's own name-based deletion logic, which already resolves by exact composed name rather than by id. Documents the migration implication in docs/commands.md: re-applying an existing multi-alert CRD under this fix leaves an orphaned duplicate at the CRD's literal dash0.com/id (whichever alert applied last under the old behavior), since nothing targets that literal id directly anymore -- delete it by hand once the new per-alert check rules look correct.
…n JSON
Agent mode's --dry-run JSON gave an approving agent strictly less
context than the text output it mirrors: the text renderer's
`Delete View "Team Logs" (team-logs)` names the asset's kind, but the
JSON change entry carried only {op, name, originOrId} -- no kind, and
no mention of which --since ref determined the deletion. An agent
deciding whether to approve a deletion needs to know what kind of
asset is about to be removed (a view is a very different risk than a
production dashboard) at least as much as a human glancing at the
text output already does.
Adds `kind` (the asset's display kind, matching the text renderer)
to every change, and `since` (the --since ref) to delete-op changes
only, since an apply-op row comes from -f's current contents, not
from any ref.
…y refs Two --since error messages were misleading or hard to read: - Running --since against a directory that was never a git repository produced three layers of wrapped errors: "--since '<ref>' requires <dir> to be inside a git repository: failed to determine repository root for <dir>: git rev-parse --show-toplevel: exit status 128 (stderr: fatal: not a git repository (or any of the parent directories): .git)". gitutil.IsNotAGitRepository recognizes git's own stable "not a git repository" wording and collapses this into one direct sentence for that specific, common case. - A ref written as "<base>~N"/"<base>^N" that fails to resolve only because its own history has fewer than N+1 commits -- --since HEAD~1 against a fresh, single-commit repository, the first thing many people try when setting up a --since test or demo repo -- was told to check for a typo or a too-shallow clone. Neither applies: there is simply no earlier commit yet. Repo.ExplainUnresolvableRef recognizes this specific shape (best-effort; it doesn't attempt to parse the rest of git's revision syntax) and names the real reason instead. Any other unresolvable ref still gets the generic message, since git's own error text doesn't distinguish a typo from a too-shallow clone on its own.
…endering correctly
git -C <path-to-a-file> fails outright ("Not a directory"), so a single-file
--since target must be rehomed to its parent directory, not to the file
itself -- computeDeletionPlan had this inverted, breaking --since against
any single-file -f target. Also teach runApply to recover whether a target
that no longer exists on disk at all used to be a file or a directory (via
a new Repo.IsTreeAtRef helper reading git history at the --since ref),
instead of always guessing "directory" once the target is gone, which
misrendered a vanished single-file target's dry-run output as a multi-file
directory scan.
…tion and its use in --since deletion A CRD with two or more alerting rules derives a distinct check-rule id per alert (asset.DeriveAlertCheckRuleID) instead of sharing the CRD's literal dash0.com/id, to avoid every alert upserting to the same check rule. Two bugs followed from that: - Nothing validated that two alerts' composed names could still collide once folded through slugify (e.g. "High CPU" and "High_CPU" both become "high-cpu"), silently reopening the exact overwrite the derivation exists to prevent. Both composePrometheusRuleNames (the actual dispatch path) and apply's own pre-flight validation now reject such a CRD before any API call. - --since's whole-CRD deletion path only ever targeted the CRD's literal identifier, which is never where a multi-alert CRD's real check rules live once derivation kicks in -- so deleting a removed multi-alert CRD silently left every one of its check rules orphaned. git.Deletion now carries the CRD's alerting-rule list from the "before" snapshot, and deletePrometheusRuleCRD attempts each alert's own derived id individually when there is more than one.
…arest existing ancestor nearestExistingAncestor used os.Lstat, so a dangling symlink at -f's own target path counted as "existing" and computeDeletionPlan tried to resolve it as a real git-trackable path and failed. The two sibling existence checks for the same target -- runApply's os.Stat(flags.File) and BuildSnapshotFromDisk's os.Stat(scope) -- both follow symlinks and already treat a dangling symlink as gone, routing to the all-deletions-tolerant path. Switching to os.Stat here makes all three checks agree.
…since's before snapshot resolveDeletionNames re-read every deletion candidate's file via a fresh git cat-file call to resolve its display name for --dry-run and confirmation prompts, even though BuildSnapshotFromRef already read that exact content moments earlier while building the "before" snapshot. Snapshot now retains that content (RawContent, populated only on the ref side, where every deletion candidate's content comes from), so resolveDeletionNames looks it up from memory instead of shelling out to git a second time per file.
The replace directive pointed at a sibling checkout, so CI could not build. The two symbols the sibling supplied, ExtractIdentifier and PrometheusAlertName, exist only as local shims and are not in any released version of the module, so bumping alone does not resolve it. Both are CLI-specific -- ExtractIdentifier encodes this CLI's own asset identifier table, and PrometheusAlertName.CheckRuleName composes the CLI's display name -- so they now live in internal/asset. go.mod requires the released v1.21.1.
buildDryRunRows's dp.plan.AlertsByName loop had no test at any tier. Add a --dry-run analogue of TestApply_Since_PrometheusRuleAlertPartialDeletion in both renderers: the text one asserts both rows group under the surviving CRD's own file and that the delete line carries the detail suffix naming that CRD, and the agent-mode one pins the JSON change entry. Both fail when the loop is disabled.
renderDryRunText and renderDryRunJSON each carried a verbatim copy of the flatten-and-sort the single-file -f target needs. Pull it into flattenSorted and call it from both. Behavior is unchanged.
--since renders deletions in the same listing as creates and updates, so plain --dry-run changed shape too: row markers carry the operation instead of an ordinal, rows sort by identifier rather than file position, and agent mode emits the JSON array instead of text.
Every user-actionable --since error buried its guidance in a parenthetical, so agent mode emitted it as one opaque "error" string with no "hint" field. Move the guidance behind the "\nHint:" marker agentmode.PrintJSONError splits on, matching internal/login/logout.go. Covers all six: -f - (stdin), not-a-git-repository, the empty ref, the all-zeros SHA, both unresolvable-ref cases, and the no-identifier failure. Updates the docs' error examples and regenerates the skill bundle.
A whole-CRD deletion dispatched to the check-rule endpoint by identifier alone, dropping the (kind, identifier) keying the plan uses everywhere else. Two documents of different kinds sharing an identifier string are two plan entries but one DELETE URL, so removing a PrometheusRule labelled dash0.com/id: X deleted the check rule a surviving `kind: CheckRule` with `id: X` had just been created from, in the same run. computeDeletionPlan now records every check-rule id -f's current contents still declare, and dispatch skips those with a warning instead. This tolerates an orphaned check rule on an identifier collision: a leftover is one manual delete away, a deleted asset the user declared is not. The guard covers both directions, including a removed CheckRule whose id a surviving single-alert CRD's check rule lives at. Also fixes the mirror-image under-deletion: a multi-alert CRD's deletion only attempted the derived ids, so a CRD applied before per-alert derivation existed 404'd on all of them, reported "already deleted", exited 0, and stranded its real check rule at the literal id forever. That id is now attempted alongside the derived ones, which the per-id 404 tolerance already makes free.
Removing one alert from a surviving PrometheusRule CRD is the one --since deletion with no id to dispatch on: the CRD's identifier is shared by all its alerts, so the check rule is resolved by its composed name instead. That lookup listed the whole dataset and took the first exact name match, with no ambiguity guard and no ownership check -- escaping both bounds every other --since deletion has, so a same-named check rule managed by the UI, by Terraform, or by the Kubernetes operator was deletable. Build a name index once per run instead, and refuse to guess: one deletable match is deleted, two error out naming both ids, and a match owned by another system of record is skipped with a warning. The owner set is a denylist because the CLI strips dash0.com/origin before sending, so its own check rules read back as "api" or unset, and CrdSource's contract treats an unknown value as "api" too -- an allowlist would refuse to delete the CLI's own rules. Building the index once also drops the listing count from one per removed alert to one per run.
An alert's check-rule id depends on how many siblings it has: the CRD's literal dash0.com/id for one alert, a derived id per alert for two or more. An edit moving a CRD across that line therefore relocates alerts it never touched, and apply only ever writes the new address. Dropping from two alerts to one left the survivor live at both its old derived id and the literal one; adding a second alert orphaned the rule at the literal id. Either way one edit produced a duplicate and an orphan, and the resulting pair of same-named check rules would later fail the by-name lookup as ambiguous. Diff now compares where a CRD's alerts lived at the ref against where they live now, and deletes any address a survivor moved off. Comparing the two address sets rather than testing the threshold directly keeps the one-versus-many rule in a single place and covers the cases the threshold test got wrong on its own, such as a recording-rule-only CRD gaining alerts. That rule moves to asset.CheckRuleIDsOccupiedByCRD so internal/git and internal/apply share one copy instead of each restating it.
git C-quotes any path holding a non-ASCII byte unless core.quotePath is
turned off ("a/caf\303\251.yaml"). filepath.Ext on that returns `.yaml"`,
so IsYAMLFile said no and ListYAMLFilesAtRef skipped the entry: the file
never entered the --since "before" snapshot and its deletion was never
detected, silently, with exit 0. Every existing test passed because the
fixtures and testrepo helper use ASCII names only.
Pass -z and split on NUL. That drops quoting entirely rather than relying
on a config setting, and also survives a path containing a newline, which
core.quotePath=false would not. The per-line TrimSpace goes with it: NUL
output has no padding to trim, so its only remaining effect would be to
corrupt a name that legitimately starts or ends with a space.
The two snapshots come from different sources: the ref side runs `git ls-tree -r`, which enumerates the whole commit, while the disk side walks only what is materialized. A sparse checkout (or a manual skip-worktree bit) makes those disagree, so every tracked-but-absent asset became a deletion candidate and --force deleted assets git still declares, silently and with exit 0. Detect the skip-worktree bit and refuse, naming sparse checkout as the reason. Detection is per scope, not repo-wide, so a monorepo whose sparse cone lies outside -f's target is unaffected; and it keys on the bit itself rather than core.sparseCheckout, which covers a manual `git update-index --skip-worktree` too. Failing closed rather than intersecting the plan with `git ls-files`: quiet narrowing on a path that deletes things is the wrong default, and the intersect leaves creates wrong anyway, since assets outside the cone are never applied either.
BuildSnapshotFromRef spawned one `git cat-file` per file, sequentially, and process spawn (~9ms each) dominated the whole --since run: 21.8s for a 500-file tree, 88s for 2000. Read them through a 16-slot semaphore instead, which cuts those to 2.0s and 7.8s. 16 is where the returns flatten (500 files: 1 slot 18.7s, 8 slots 3.0s, 16 slots 2.0s, 32 slots 1.9s, 64 slots 1.9s) -- spawning is syscall-bound, not CPU-bound, so the useful limit sits above core count. Only the reads are concurrent. ingestDocuments still runs sequentially in ListYAMLFilesAtRef's order, so every snapshot mutation stays single-threaded and the built Snapshot is identical to before. Errors go into a position-indexed slice so the first failing path still wins deterministically.
ExtractIdentifier fell back across fields, so it could return a value no live asset is keyed on: a View carrying dash0.com/origin but no dash0.com/id reported that origin, while ImportView upserts by dash0.com/id alone and the CLI strips origin outbound. --since recorded the asset, deleted by an identifier matching nothing, took the 404, and printed "was already deleted" with exit 0. The asset was still live, and the non-empty identifier had let it slip past the NoIdentifier hard-fail written for exactly this case. Seven kinds were affected: View, SyntheticCheck, PersesDashboard and PrometheusRule fell back to origin; Dashboard and CheckRule to the dash0.com/id label rather than dash0Extensions.id and the top-level id; Dash0NotificationChannel to an id it has no user-settable field for. Each now reads only what its Import helper upserts by, so a document with the wrong field populated returns "" and reaches the hard-fail. Teams and spam filters keep their fallback: both Import helpers genuinely accept origin or id, teams via a GET preflight, which is what makes a UI-downloaded team YAML reapply cleanly. The hard-fail message named the two labels, which read as wrong for a document that does carry one of them; it now names the kind's own upsert field instead.
GOOS=linux is a cross-compile from macOS, where cgo defaults off, but native on a linux CI runner, where it defaults on. The runner therefore produced a binary dynamically linked against glibc and copied it into an alpine (musl-only) image, where it cannot exec -- while the same code built statically and passed locally, which is why the divergence went unnoticed. Setting CGO_ENABLED=0 makes both hosts emit the same static binary. From macOS the flag is a no-op: the two builds are byte-identical, same BuildID.
KindDisplayName gained a "team" case, so apply now prints `Team "x" created` where the missing case previously fell through to the raw `Dash0Team`. The declarative team roundtrip grepped for the old fallback and failed on CI. Recorded in the breaking changelog entry alongside --dry-run's output change, since it is the same kind of user-visible shift.
nicolastakashi
force-pushed
the
feat/sync-action
branch
from
August 27, 2026 12:41
f053c10 to
a81d3ea
Compare
nicolastakashi
approved these changes
Aug 27, 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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
apply --since <ref>andapply --force, both experimental: deletes assets whose definition existed at<ref>(viainternal/git, a thinos/execwrapper over git plumbing) but is no longer present in-f's current contents, matched by identifier (id/origin), never by file path — including PrometheusRule's CRD-shared-identifier vs. per-alert-name distinction.--dry-run --sincepreviews the deletion plan alongside the existing create/update preview, merged into one per-file listing sorted by identifier; a deleted asset's display name is resolved from git history rather than left as a bare id.--dry-run:[{"path": ..., "changes": [{"op": "apply"|"delete", "name": ..., "originOrId": ...}]}].test/e2etier (testcontainers-go): the realdash0binary invoking the realgitbinary across a process boundary, since--sinceshells out to git rather than using a Go git library.test/roundtrip/test_apply_since_{roundtrip,idempotency,ref_edge_cases}.sh, run and passing against a real Dash0 dev environment.--since/--forceend to end:docs/commands.md,README.md, the Docker/gitlimitation, the GitHub Actions invocation pattern, theinternal/skillagent-skill bundle, and a new "flag-level promotion" section indocs/promoting-commands-to-stable.md(this is the first flag-level experimental gate in the CLI).openspec/changes/add-diff-and-since-flag/) and the follow-onasset-synchGitHub Action proposal (not yet implemented).Both
--sinceand--forceremain gated behind--experimental/-X.dash0 diff(a separate preview command planned for this same change) is not part of this PR — see Section 5 of the tasks doc.Found while testing / writing docs
PrometheusRuleCRD with 2+ alerting rules sharing onedash0.com/idnever produces more than one live check rule via ordinarycreate/apply— each alert's PUT overwrites the previous one under the shared id. Predates this branch (root cause is in the siblingdash0-api-client-go's CRD-to-check-rule conversion) but means--since's alerting-rule-partial-removal scenario (1.6/4.7 in the tasks doc) has no real-world-reachable roundtrip coverage — only the mock-server-based unit/integration tests exercise it.--dry-run --sincebug where a-ftarget that's a subdirectory of the repo (not the repo root) grouped a deletion under its repo-root-relative path while that file's surviving documents grouped under their-f-target-relative path — inconsistent prefixing in the same listing. Fixed, with regression tests.docs/commands.md's plainapply -f assets/example: real output prefixes each line with the path relative to-f's own target, not the invoking shell's directory (dashboard.yaml: ..., notassets/dashboard.yaml: ...). Fixed alongside the--sincedocs.Known follow-ups (tracked in
openspec/changes/add-diff-and-since-flag/tasks.md)dash0-api-client-gochange and drop thego.modreplace directive it's currently consumed through — CI will not go green until this lands, since the replace directive points at a local sibling checkout.dash0 diffitself (Section 5) — not part of this PR.Test plan
make buildmake test(unit + integration)make lintmake skill-validatetest_apply_since_roundtrip.sh,test_apply_since_idempotency.sh,test_apply_since_ref_edge_cases.sh— run individually against a real Dash0 dev environment, all pass, all cleaned upmake test-e2e— not run in this session (requires Docker)make test-roundtrip(every asset type, not just--since's new scripts) — not run in this session