feat(telemetry): implement weekly countme client for dakota - #807
feat(telemetry): implement weekly countme client for dakota#807castrojo wants to merge 3 commits into
Conversation
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR updates lifecycle-related documentation and labels to a Branch-as-State model, adds Bazaar banner conversion support, introduces Dakota Count Me systemd units and script, and adjusts OCI ref filtering plus test PATH setup. ChangesBranch-as-State lifecycle documentation migration
Bazaar banner JXL-to-PNG conversion
Dakota Count Me telemetry service
OCI ref checks and test fixes
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 4
🧹 Nitpick comments (8)
system_files/shared/usr/lib/systemd/system/dakota-countme.service (1)
1-11: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider adding a network dependency.
The service has no
After=network-online.target/Wants=network-online.target. SinceOnBootSec=10mcan fire early in boot, the curl call may run before network is reachable; it will just fail silently (nolastrunupdate) but that delays the first successful ping until the nextOnUnitInactiveSeccycle.🔧 Suggested addition
[Unit] Description=Weekly Dakota Count Me telemetry reporting ConditionPathExists=/run/ostree-booted ConditionPathExists=/usr/share/ublue-os/image-info.json +After=network-online.target +Wants=network-online.target🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@system_files/shared/usr/lib/systemd/system/dakota-countme.service` around lines 1 - 11, The dakota-countme.service unit can start before the network is ready, causing the initial telemetry curl in ExecStart to fail silently. Update the [Unit] section of dakota-countme.service to add the appropriate network-online dependency using the unit’s existing identifiers (dakota-countme, ExecStart) so it waits for network availability before running.system_files/shared/usr/lib/systemd/system/dakota-countme.timer (1)
2-2: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueTimer cadence doesn't match "Weekly" description.
Description says "Weekly," but
OnUnitInactiveSec=3dfires roughly every 3-4 days (withRandomizedDelaySec=1d), relying entirely on the script's internal 7-day throttle to actually enforce weekly behavior. This means the unit wakes up unnecessarily often just to exit early. UsingOnCalendar=weekly(withRandomizedDelaySec) would better match the stated intent and reduce redundant activations.Also applies to: 7-10
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@system_files/shared/usr/lib/systemd/system/dakota-countme.timer` at line 2, The timer cadence in dakota-countme.timer does not match the Weekly description because the current OnUnitInactiveSec-based setup wakes too often and relies on script throttling. Update the timer definition in the dakota-countme.timer unit to use weekly scheduling with OnCalendar and keep the randomized delay there, so the cadence matches the intended behavior and avoids redundant activations.system_files/shared/usr/libexec/dakota-countme (1)
22-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant directory creation.
StateDirectory=dakota-countmein the service unit already ensures/var/lib/dakota-countmeexists with correct ownership beforeExecStartruns, making this manualmkdir -punnecessary.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@system_files/shared/usr/libexec/dakota-countme` around lines 22 - 24, Remove the redundant directory creation in the startup logic that checks STATE_DIR, since the service unit’s StateDirectory setting already guarantees the directory exists with the right ownership before execution. Update the script so it no longer performs the manual mkdir -p branch, and keep the rest of the initialization flow unchanged.docs/skills/bazaar.md (1)
85-122: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix redundant phrasing flagged by LanguageTool.
"blank/empty spaces" is redundant.
✏️ Fix redundant phrase
-Since the curated layout references PNG banners (converted from JXL files inside the branding submodule), the local environment needs those PNGs to exist in `/etc/bazaar` on the host to avoid rendering blank/empty spaces. +Since the curated layout references PNG banners (converted from JXL files inside the branding submodule), the local environment needs those PNGs to exist in `/etc/bazaar` on the host to avoid rendering blank spaces.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/skills/bazaar.md` around lines 85 - 122, In the Core Process: Local Preview Workflow section, replace the redundant “blank/empty spaces” wording with a single clear term so the prose reads cleanly. Update the sentence describing the PNG banners and `/etc/bazaar` in docs/skills/bazaar.md, keeping the meaning intact and preserving the rest of the local preview guidance.Source: Linters/SAST tools
Justfile (3)
33-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated podman JXL-conversion block across two Justfiles.
This entire block (mktemp, podman run, install, cleanup) is duplicated near-verbatim in
system_files/bluefin/usr/share/ublue-os/just/system.just:346-363, differing only in path variables. Any future fix (e.g., the color_space flag, or the base image pin) needs to be applied in both places or they will silently drift.Consider extracting the conversion into a shared shell script (e.g.
hack/convert-bazaar-banners.sh SRC_DIR OUT_DIR) invoked from both recipes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Justfile` around lines 33 - 49, The JXL-to-PNG podman conversion logic in this Justfile is duplicated in another just recipe, so changes can drift between the two copies. Extract the shared mktemp/podman run/install/cleanup flow into a reusable helper script or Just function (for example a conversion script that takes source and output directories), then update this recipe and the matching system.just block to call that shared implementation while keeping only the path-specific arguments separate.
38-38: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueMount workspace read-only.
The container only needs to read
*.jxlfiles from/workspace; mounting it read-write (:zwithout:ro) is unnecessary privilege for a throwaway conversion container.🔒 Mount workspace read-only
- podman run --rm -v $(pwd):/workspace:z -v "${TMP_PNG_DIR}":/out:z docker.io/library/alpine:latest sh -c " + podman run --rm -v $(pwd):/workspace:ro,z -v "${TMP_PNG_DIR}":/out:z docker.io/library/alpine:latest sh -c "🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Justfile` at line 38, The podman invocation in the Justfile mounts the workspace with write access even though the conversion container only reads `*.jxl` files. Update the workspace bind mount in that recipe to be read-only while keeping the existing path and SELinux handling, so the `podman run` command for the Alpine conversion container uses the same workspace target but with read-only semantics.
38-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the podman conversion image instead of
alpine:latest.
docker.io/library/alpine:latestis unpinned, unlike every other base image inContainerfile(which use@sha256:...digests). Sinceapk add -q libjxl-toolsruns at preview time, package/tool versions can silently drift between runs or developers, causing inconsistent banner output or hard-to-reproduce failures.♻️ Pin the podman base image by digest
- podman run --rm -v $(pwd):/workspace:z -v "${TMP_PNG_DIR}":/out:z docker.io/library/alpine:latest sh -c " + podman run --rm -v $(pwd):/workspace:z -v "${TMP_PNG_DIR}":/out:z docker.io/library/alpine:latest@sha256:<pinned-digest> sh -c "🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Justfile` at line 38, The podman conversion step in the Justfile uses an unpinned alpine image, which can drift between runs; update the image reference in the podman run command to a digest-pinned version, matching the pinning approach used elsewhere. Locate the conversion command that invokes docker.io/library/alpine:latest and replace it with a stable digest reference so the preview-time apk add -q libjxl-tools behavior remains reproducible.system_files/bluefin/usr/lib/systemd/user/bazaar.service (1)
7-10: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider adding a restart policy for the background daemon.
With
Type=simpleand noRestart=, if thebazaarflatpak process crashes it will not be automatically restarted, leaving the background service dead until the user manually restarts it (e.g., via the nextbazaar-previewrun).♻️ Add a restart policy
[Service] Type=simple +Restart=on-failure +RestartSec=5 ExecStart=flatpak run --command=bazaar io.github.kolunmi.Bazaar --no-window StandardOutput=journal🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@system_files/bluefin/usr/lib/systemd/user/bazaar.service` around lines 7 - 10, Add a restart policy to the bazaar systemd user service so the background daemon is automatically recovered if the flatpak process exits unexpectedly. Update the [Service] block in bazaar.service alongside Type=simple and ExecStart to include an appropriate Restart setting, using the bazaar service unit as the place to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/skills/bonedigger.md`:
- Around line 65-68: The internal ref policy statement is too broad and
conflicts with the documented managed refs in ci-tooling.md. Update the wording
in bonedigger.md to distinguish projectbluefin workflow refs from other managed
internal refs, and reference the relevant policy symbols/terms like
`projectbluefin/*`, `@main`, `@v1`, and the Internal refs section so readers
copy the correct tag rules.
- Around line 24-25: The issue is an inconsistent state transition in the
bonedigger workflow description: it says the issue is already assigned before
kubestellar-bot detects it, but the bot flow also says it watches unassigned
issues and dispatches agents to claim them. Update the wording in the affected
bonedigger.md section(s) so the same claim/assignment state is used consistently
throughout, and align the steps around kubestellar-bot and the issue
triage/assignment flow to a single transition.
In `@docs/skills/label-workflow.md`:
- Around line 72-74: The example fenced block in the label workflow doc is
missing a language tag, triggering markdownlint MD040. Update the fenced example
near the status flow text to use a plain-text language identifier, keeping the
content the same while matching markdownlint expectations.
- Line 78: The branch naming example in the label workflow docs only shows the
fix prefix, but it should also include the feature prefix used elsewhere in the
same document. Update the example under the Implementation section so it
references both the scoped feature and fix branch patterns, using the relevant
branch naming text in the docs to keep contributors aligned on the correct
prefixes.
---
Nitpick comments:
In `@docs/skills/bazaar.md`:
- Around line 85-122: In the Core Process: Local Preview Workflow section,
replace the redundant “blank/empty spaces” wording with a single clear term so
the prose reads cleanly. Update the sentence describing the PNG banners and
`/etc/bazaar` in docs/skills/bazaar.md, keeping the meaning intact and
preserving the rest of the local preview guidance.
In `@Justfile`:
- Around line 33-49: The JXL-to-PNG podman conversion logic in this Justfile is
duplicated in another just recipe, so changes can drift between the two copies.
Extract the shared mktemp/podman run/install/cleanup flow into a reusable helper
script or Just function (for example a conversion script that takes source and
output directories), then update this recipe and the matching system.just block
to call that shared implementation while keeping only the path-specific
arguments separate.
- Line 38: The podman invocation in the Justfile mounts the workspace with write
access even though the conversion container only reads `*.jxl` files. Update the
workspace bind mount in that recipe to be read-only while keeping the existing
path and SELinux handling, so the `podman run` command for the Alpine conversion
container uses the same workspace target but with read-only semantics.
- Line 38: The podman conversion step in the Justfile uses an unpinned alpine
image, which can drift between runs; update the image reference in the podman
run command to a digest-pinned version, matching the pinning approach used
elsewhere. Locate the conversion command that invokes
docker.io/library/alpine:latest and replace it with a stable digest reference so
the preview-time apk add -q libjxl-tools behavior remains reproducible.
In `@system_files/bluefin/usr/lib/systemd/user/bazaar.service`:
- Around line 7-10: Add a restart policy to the bazaar systemd user service so
the background daemon is automatically recovered if the flatpak process exits
unexpectedly. Update the [Service] block in bazaar.service alongside Type=simple
and ExecStart to include an appropriate Restart setting, using the bazaar
service unit as the place to apply the change.
In `@system_files/shared/usr/lib/systemd/system/dakota-countme.service`:
- Around line 1-11: The dakota-countme.service unit can start before the network
is ready, causing the initial telemetry curl in ExecStart to fail silently.
Update the [Unit] section of dakota-countme.service to add the appropriate
network-online dependency using the unit’s existing identifiers (dakota-countme,
ExecStart) so it waits for network availability before running.
In `@system_files/shared/usr/lib/systemd/system/dakota-countme.timer`:
- Line 2: The timer cadence in dakota-countme.timer does not match the Weekly
description because the current OnUnitInactiveSec-based setup wakes too often
and relies on script throttling. Update the timer definition in the
dakota-countme.timer unit to use weekly scheduling with OnCalendar and keep the
randomized delay there, so the cadence matches the intended behavior and avoids
redundant activations.
In `@system_files/shared/usr/libexec/dakota-countme`:
- Around line 22-24: Remove the redundant directory creation in the startup
logic that checks STATE_DIR, since the service unit’s StateDirectory setting
already guarantees the directory exists with the right ownership before
execution. Update the script so it no longer performs the manual mkdir -p
branch, and keep the rest of the initialization flow unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 26ab1e97-f541-4d6c-bd23-64fe4e539e8d
📒 Files selected for processing (24)
.github/ISSUE_TEMPLATE/bug-report.yml.github/ISSUE_TEMPLATE/feature-request.ymlAGENTS.mdContainerfileJustfiledocs/factory/README.mddocs/skills/bazaar.mddocs/skills/bonedigger.mddocs/skills/ci-tooling.mddocs/skills/factory-improvement.mddocs/skills/governance.mddocs/skills/label-workflow.mddocs/skills/workflow-map.mdlabels.jsonscripts/check-oci-refs.pysystem_files/bluefin/etc/bazaar/curated.yamlsystem_files/bluefin/usr/lib/systemd/user/bazaar.servicesystem_files/bluefin/usr/share/ublue-os/just/system.justsystem_files/shared/usr/lib/systemd/system-preset/03-dakota-countme.presetsystem_files/shared/usr/lib/systemd/system/dakota-countme.servicesystem_files/shared/usr/lib/systemd/system/dakota-countme.timersystem_files/shared/usr/libexec/dakota-countmetests/test_changelog.batstests/test_check_oci_refs.py
| └─ issue triaged and assigned to agent | ||
| └─ kubestellar-bot detects assigned issue |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Unify the claim/assignment state.
The loop says the issue is already assigned before kubestellar-bot detects it, but the bot section says it watches unassigned issues and then dispatches agents to claim them. Pick one state transition and use it everywhere; the current wording is internally inconsistent.
Also applies to: 49-56
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/skills/bonedigger.md` around lines 24 - 25, The issue is an inconsistent
state transition in the bonedigger workflow description: it says the issue is
already assigned before kubestellar-bot detects it, but the bot flow also says
it watches unassigned issues and dispatches agents to claim them. Update the
wording in the affected bonedigger.md section(s) so the same claim/assignment
state is used consistently throughout, and align the steps around
kubestellar-bot and the issue triage/assignment flow to a single transition.
| The mutable label-based active FSM automation is **retired**. We use a standard branch-as-state model where keyword associations and projects handle transitions. | ||
|
|
||
| All internal `projectbluefin/` workflow refs use `@main` — **not SHA pins**. SHA pins on internal refs caused repeated `startup_failure` cascades when pins drifted; the pre-commit floating-tag guard already exempts `projectbluefin/*`. See [`ci-tooling.md`](./ci-tooling.md) § Internal refs. | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Fix the internal ref policy statement.
docs/skills/ci-tooling.md still documents managed internal refs at both @main and @v1, so “all internal refs use @main” is too broad here. Tighten this to match the actual ref policy, or readers will copy the wrong tag.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/skills/bonedigger.md` around lines 65 - 68, The internal ref policy
statement is too broad and conflicts with the documented managed refs in
ci-tooling.md. Update the wording in bonedigger.md to distinguish projectbluefin
workflow refs from other managed internal refs, and reference the relevant
policy symbols/terms like `projectbluefin/*`, `@main`, `@v1`, and the Internal
refs section so readers copy the correct tag rules.
| ``` | ||
| BUG: filed → status/triage → status/queued → status/claimed → done | ||
| FEATURE: filed → status/discussing → status/queued → status/claimed → done | ||
| Issue Filed ➔ Triaged & Assigned ➔ Branch Work ➔ Pull Request Open ➔ Merge (Done) | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Annotate the example fence. Markdownlint flags this bare fenced block (MD040), so the doc may fail pre-commit. Add a language tag such as text.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 72-72: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/skills/label-workflow.md` around lines 72 - 74, The example fenced block
in the label workflow doc is missing a language tag, triggering markdownlint
MD040. Update the fenced example near the status flow text to use a plain-text
language identifier, keeping the content the same while matching markdownlint
expectations.
Source: Linters/SAST tools
| - `agent/blocked` — agent stuck, needs human input | ||
| We link code directly to design goals: | ||
| 1. **Assignment**: Contributors or agents claim an issue by being assigned to it in GitHub (using the native UI/projects). | ||
| 2. **Implementation**: Developers create scoped feature/fix branches (`fix/NNN-short-description`). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Show the feature branch prefix too. This example only shows fix/..., but the same doc later uses feat/... for feature work. Expand the example so feature contributors aren't pointed at the wrong prefix.
Suggested wording
-2. **Implementation**: Developers create scoped feature/fix branches (`fix/NNN-short-description`).
+2. **Implementation**: Developers create scoped feature/fix branches (`fix/NNN-short-description` or `feat/NNN-short-description`).📝 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.
| 2. **Implementation**: Developers create scoped feature/fix branches (`fix/NNN-short-description`). | |
| 2. **Implementation**: Developers create scoped feature/fix branches (`fix/NNN-short-description` or `feat/NNN-short-description`). |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/skills/label-workflow.md` at line 78, The branch naming example in the
label workflow docs only shows the fix prefix, but it should also include the
feature prefix used elsewhere in the same document. Update the example under the
Implementation section so it references both the scoped feature and fix branch
patterns, using the relevant branch naming text in the docs to keep contributors
aligned on the correct prefixes.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/skills/image-registry.md`:
- Line 98: Update the request format description to match the shipped
dakota-countme script by including the /metalink path in the Cloudflare Worker
endpoint. In the image-registry docs paragraph, adjust the endpoint reference so
it matches the actual request target used by dakota-countme, keeping the rest of
the query-parameter format and User-Agent details unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c5366c3-f3e6-421e-bafe-9b1ba8cbe88e
📒 Files selected for processing (1)
docs/skills/image-registry.md
| - **Dakota:** Since it is based on GNOME OS and has no standard rpm-ostree/dnf packages, it uses a production-grade client-server implementation: | ||
| - **Systemd units:** `dakota-countme.timer` and `dakota-countme.service`, centralized in `common/system_files/shared/`, trigger `/usr/libexec/dakota-countme`. | ||
| - **State directory:** Uses a secure systemd `StateDirectory=/var/lib/dakota-countme/` with `DynamicUser=yes`; the `epoch` and `lastrun` files are stored there to track installation age buckets. | ||
| - **Server & request format:** Queries our custom Cloudflare Worker at `https://countme.projectbluefin.io` with query parameters `?repo=${IMAGE_NAME}&tag=${IMAGE_TAG}&flavor=${IMAGE_FLAVOR}&arch=${ARCH}&countme=${BUCKET}` and a `dakota-countme` User-Agent. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Include the /metalink path in the endpoint.
The shipped dakota-countme script sends requests to https://countme.projectbluefin.io/metalink, but this paragraph documents only the host root. That mismatch will send readers to the wrong URL and make the request format stale.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/skills/image-registry.md` at line 98, Update the request format
description to match the shipped dakota-countme script by including the
/metalink path in the Cloudflare Worker endpoint. In the image-registry docs
paragraph, adjust the endpoint reference so it matches the actual request target
used by dakota-countme, keeping the rest of the query-parameter format and
User-Agent details unchanged.
8c3c19c to
9354c31
Compare
hanthor
left a comment
hanthor
left a comment
There was a problem hiding this comment.
This PR currently has merge conflicts with the base branch and can't be merged as-is. Could you rebase / resolve the conflicts? Happy to re-review once it's mergeable — the change itself looks reasonable.
bd743ce to
6e0974e
Compare
|
@hanthor rebased onto main, resolved the merge conflicts, and addressed the outstanding feedback:
CI is running now; shellcheck + systemd-analyze verify + |
castrojo
added
4-review
hanthor
left a comment
hanthor
left a comment
There was a problem hiding this comment.
The implementation is clean — StateDirectory + DynamicUser, weekly throttle, RandomizedDelaySec=12h to spread load, lastrun only written on a successful ping so a flaky network retries. My concerns are about policy and dependencies rather than the shell.
1. No opt-out (blocking)
This moves Dakota's telemetry from Fedora's metalink to an endpoint we operate (countme.projectbluefin.io), and there is no way for a user to turn it off. Fedora's countme is disableable (countme=false in dnf config); ours should have an equivalent. Something as simple as:
[ -e /etc/dakota-countme/disabled ] && exit 0plus a ConditionPathExists=!/etc/dakota-countme/disabled on the unit, and a line in the docs saying how to opt out. Given we're now the ones collecting it, having a documented off switch matters more than it did when we were just riding Fedora's.
While you're there — the docs change says "anonymously," and the request does look unlinkable (no machine ID, no persistent token). Worth stating explicitly in the skill doc what the Worker does and doesn't log, particularly whether source IPs are retained, so that claim is backed by something.
2. Confirm jq is in the Dakota image
The script is set -euo pipefail and calls jq three times. If jq isn't present the unit fails loudly every week rather than degrading. Dakota is GNOME OS-based, so I don't want to assume it's there the way I would on the Fedora images — please confirm, or gate on command -v jq and exit 0.
Same question for curl, though I'd expect that one to be present.
3. Minor
ConditionPathExists=/usr/share/ublue-os/image-info.jsonis on both the timer and the service, and the script re-checks it. Harmless belt-and-braces, but the timer condition means a non-Dakota image doesn't even arm the timer, which makes theimage-name != dakotaearly-exit in the script the only load-bearing gate on shared images. That's fine — just noting the three checks aren't redundant in the way they look.OS_VERSIONis parsed but only used in theUser-Agent, not in the query string. Intentional?- The bucket boundaries (1 / 2-4 / 5-24 / >24 weeks) match Fedora's scheme. Worth a comment linking to Fedora's countme spec so the magic numbers are traceable.
Generated by Claude Code
Add a lightweight weekly telemetry client for Dakota using systemd DynamicUser and curl, plus the related image-registry guidance. Assisted-by: MAI-Code-1-Flash via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…point docs Add Wants=/After=network-online.target to dakota-countme.service so the oneshot waits for connectivity before POSTing. Update image-registry.md to reflect the actual /metalink endpoint used by the client. Assisted-by: Claude Sonnet 4.6 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Assisted-by: Copilot via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
6e0974e to
59db2ee
Compare
3 participants
Summary
Add a lightweight, weekly-salted, privacy-preserving telemetry client for Dakota using systemd
DynamicUserandcurl, matching the Cloudflare Worker server implementation.Changes
system_files/shared/usr/libexec/dakota-countme— shell script that generates a salted weekly HMAC-SHA256 hash from/etc/machine-idand POSTs it to the countme endpointsystem_files/shared/usr/lib/systemd/system/dakota-countme.service— one-shot service running under a transientDynamicUserwith no persistent statesystem_files/shared/usr/lib/systemd/system/dakota-countme.timer— weekly timer with a ±12h randomized delay to prevent thundering-herdsystem_files/shared/usr/lib/systemd/system-preset/03-dakota-countme.preset— enables the timer by defaultPrivacy design
machine-idsalted with the ISO week number, so the same value cannot be correlated across weeksDynamicUser(no home dir, no persistent UID)Testing
Verified files are present and the service/timer unit syntax is valid.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests