[beardthelion]

Summary

Carries #458 (fix(gl): report quickstart registration failure when the node returns no UCAN, for #355) onto an upstream branch so the upstream-only Code Quality check can run.

Motivation & context

gl quickstart printed "Registered successfully" whenever the node returned 2xx, even when the body carried no UCAN. The registration quietly did nothing and could leave a stale ucan.json from another node in place.

Closes #355.

Kind of change

• Bug fix
• Feature
• Security fix
• Docs
• Tests / CI
• Refactor (no behavior change)
• Breaking or protocol change (issue required first)

What changed

• gl: a 2xx response without a usable UCAN now prints a failure line instead of a false success. Success output and the UCAN write happen only inside the non-empty-UCAN branch; the nonfatal continuation to repo creation is unchanged.
• Merge detail: contributor commit preserved author-identical from a418c284; one merge commit brings main at c805e35f (no conflicts). The merge commit also resolves the outstanding review finding: the recovery hint on the new no-UCAN branch is removed rather than retained, because a gl register instruction that drops this invocation's --dir would load a different identity or a missing one entirely. The pre-existing hints on the non-2xx and transport-failure branches are unchanged.

How a reviewer can verify

cargo check -p gl --locked

Run gl quickstart against a node stub returning a 2xx body without a ucan field: the output ends at the failure line; no retry command is printed and no ucan.json is written.

Before you request review

• Scope is one logical change; no unrelated churn
• cargo test --workspace passes locally
• New behavior is covered by tests (required for fixes)
• cargo fmt --all and cargo clippy --workspace --all-targets -- -D warnings are clean
• Commit titles use Conventional Commits (feat(...), fix(...), docs(...))
• Docs / .env.example updated if behavior or config changed (or N/A)
• Checked existing PRs so this isn't a duplicate

Protocol & signing impact

• Touches DID / did:key, Ed25519 / RFC 9421 signatures, UCAN, ref certs, or P2P wire formats
• Discussed in an issue before implementation
• Backward-compatible with existing nodes and previously signed history

CLI output change only; no wire or identity handling changes.

Notes for reviewers

• Original fork PR: fix(gl): report quickstart registration failure when the node returns no UCAN (#355) #458 (still open; will be pointed here and closed after this lands or on request). Maintainer takeover lane: no fork-head review posted; the merge commit folds in the outstanding review finding (retry hint removed rather than carrying the wrong identity directory).
• The cargo-audit failure on the fork head was a base-wide advisory also present on sibling PRs from the same base, not introduced by this patch.

[beardthelion]

@coderabbitai full review

[coderabbitai]

Warning

Review limit reached

This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry.

Next included review available in 13 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 2ca1fe72-974d-4cd3-bd07-baae114d8ce3

Commits

Reviewing files that changed from the base of the PR and between c805e35 and ebdecb8.

Files selected for processing: 1

• crates/gl/src/quickstart.rs

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

[coderabbitai]

Action not completed

Review rate limited.

---

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 13 minutes.

[jatmn]

Merge readiness

Rebase onto current main (a2262d92, reconciliation sweep #551) before merge. Merge-base is c805e35f; the branch is one commit behind the live target with no conflicts predicted (mergeable: MERGEABLE).

The PR template box "New behavior is covered by tests (required for fixes)" is checked, but the diff has no test changes and quickstart.rs has no unit tests. Verification is documented as a manual stub run only. That is acceptable for a stdout-only change if maintainers agree, but the checkbox overstates what landed in the PR—either add a mocked HTTP regression (same style as register.rs tests) or uncheck and cite manual verification explicitly.

CI: test (stable), fmt + clippy, build --release, and the rest required suite passed on head ebdecb8. test (beta) failed on the PR run; stable passed on the same workflow. Treat beta as merge-gating only if your branch protection requires it—confirm whether the failure reproduces on main or is toolchain flake.

Upstream duplicate: fork PR #458 (closed) carried the same fix; this branch correctly drops the no-UCAN retry hint so gl register is not suggested without this run's --dir.

---

🟢 Thanks for the contribution. I do not see any actionable issues from my review.

The change matches #355: on POST /api/register 2xx, success and “UCAN saved” lines run only inside the existing !ucan.is_empty() write guard; an empty or missing ucan prints a failure row instead of false success. Trust score output moves with that guard. Follow-ups called out in the issue (read_json instead of unwrap_or_default(), register.rs messaging, stale ucan.json lifecycle, wizard non-fatal Step 3) are pre-existing or explicitly scoped out of this single-file port—not regressions introduced by the diff.