Evaluation of this COB is implemented under the assumption that
delegates accept at most one revision.
This can lead to issues, if some delegates create new revisions eagerly,
without waiting for the others to vote on earlier revisions. As the
eager delegates start accepting newer and newer revisions, this shadows
their acceptance of earlier revisions, which leads to failure to
recognize that these earlier revisions were actually accepted by a
majority.
To avoid such situations, remove `heads`, and always count the number of
"accept" verdicts explicitly.
The NO_COLOR convention (https://no-color.org) only calls for
suppressing ANSI *color* codes. It does not demand the removal of
other attributes such as bold, italic, underline or reverse.
Previously, Paint::is_enabled() bundled anstyle_query::no_color()
into the single boolean that decides whether to emit any formatting.
Because Paint relies on that boolean alone, NO_COLOR ended up
stripping bold, italic, reverse, etc. alongside the color codes.
That conflates two concerns: terminal styling support and color
suppression. It is also contrary to the intent of the standard.
Instead, treat the general style decision separately from the
color decision. Remove the no_color() check from is_enabled()
and add Paint::is_styling_enabled() for the coarse terminal gate.
Style::fmt_prefix and fmt_suffix now query NO_COLOR directly and
conditionally drop foreground and background codes while preserving
non-colour properties. This way bold, italic, underline and the
like continue to work when NO_COLOR is set, while colors alone are
suppressed.
Closes: rad:z3gqcJUoA1n9HaHKufZs5FCSGazv5/cob/xyz.radicle.issue/f922d90
Assisted-by: Pi:deepinfra/moonshotai/Kimi-K2.6
Signed-off-by: Richard Levitte <richard@levitte.org>
- Assert that with the peer still `Connected`, `worker_result` clears
`active[rid]` as expected
- Confirm the disconnect test is exercising the discard path rather than
passing vacuously
- Drive `worker_result` through the real wire layer with the peer in
`Disconnecting` state, mirroring a fetch that ends mid-disconnect
- Assert `active[rid]` survives, proving the result is discarded without
calling `service.fetched`, which is what orphans the entry
- Add helpers to wrap a service with an active fetch in a `Wire`
- Drive the real cancel-skip path: a connection conflict makes
`disconnected` early-return on a link mismatch, leaving the fetch
uncancelled
- Combined with an undelivered fetch result, the `active[rid]` entry is
orphaned and the repo can no longer be fetched from any node
- Confirm delivering the missing result clears the entry and unblocks
the queued fetch, showing what the fix must guarantee
The extension of the test shows that Node B's fetch cannot be dequeued because
the RID is in the active fetch set.
It can then be dequeued once Node A has been fetched from.
- Clear active[rid] only when the result's node matches the node that
started the fetch; mismatched results now report NotFound
- Prevent a late completion from a disconnected peer clearing a newer
fetch for the same repo started by a different node
- Add test covering the stale-from mismatch path
A fetch result was discarded whenever the peer was no longer `Connected`
by the time the worker reported, and queued fetches were dropped silently
when the peer disconnected before the `Io::Fetch` was processed. In both
cases the fetcher's `active` entry for the repo was never cleared. Since
that entry is keyed by repo, it blocked the repository from being fetched
from any node until the process restarted.
- Report the result in `worker_result` even when the peer is no longer
connected, instead of returning early
- Report a failed fetch when an `Io::Fetch` is dropped for a disconnected
peer, so the active entry is cleared
- Flip the wire regression test to assert the entry is now cleared on
disconnect
To improve the ability to work out what went wrong, should an external
cob helper not be found, report the command name if the spawn fails.
Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
Co-authored-by: Lorenz Leutgeb <lorenz.leutgeb@radicle.dev>
`inquire::PasswordDisplayMode::Masked` causes inquire to
render characters of the password text input marked as
different characters (asterisks).
Users reported that they would prefer password text input to not be
rendered at all, i.e., have no indication of input. This is inquire's
default, so remove the lines that set the display mode.
Introduces support for Gerrit-style magic push references via
`refs/for/<branch>`. Pushing to this ref automatically extracts the
target branch and opens a patch against it, bypassing the need for the
push option `patch.target`. Example:
```
$ git push rad HEAD:refs/for/accepted
```
Will open a patch with its `patch.target` set to `refs/heads/accepted`.
The remote helper now supports the push option `patch.target`. This
allows users to explicitly specify a target canonical reference when
opening or updating a patch. For example, to open a patch that targets
the branch "backport", use:
```
git push -o patch.target=refs/heads/backport
```
Furthermore, strict merge and revert isolation is now enforced:
patches are only marked as merged or reverted if the commits are
pushed to the target branch of the patch explicitly.
Extend `enum MergeTarget` to include a new variant, `Branch`.
The intended use of this new variant is to allow a `Patch` to have a
target branch other than the default branch.
The `Branch` variant holds a `TargetBranch` which, in turn, is ensured
to be a `Qualified` reference that begins with `refs/heads`, i.e. a
Git branch.
The `rad_cob_update` test accidentally use the `rad-cob-log` file
instead of `rad-cob-log`.
This change fixes that, and updates the `rad-cob-update` example file
to the latest patch show format.
When a connection drops and the node attempts to reconnect within the
same second, the `last_attempt` timestamp written to the database is
identitcal to the `last_success` timestamp due to the Unix time (seconds)
precision loss.
The `maintain_connections` logic filters eligible peers using the
condition `last_success >= last_attempt`. Because the truncated
timestamps are equal, this evaluates to true, causing the node to
bypass the `CONNECTION_RETRY_DELTA` backoff.
If the OS instantly rejects the dial attempt (e.g. `EHOSTUNREACH`),
the node synchronously queues another connection attempt. This traps
the reactor in an infinite `while` loop.
Changing the condition to `last_success > last_attempt` ensures that a
failed attempt in the same second correctly evaluates to false, forcing
the node to respect the backoff timer and breaking the loop.
The normalization of paths was converting valid `\n` characters to `/n`.
This had not been noticed until codespell suggest the change of
`/ndefined` to be `/undefined`.
Use `Assert::normalize_paths(false)` to disable this behaviour.
The documentation for `RateLimit::fill_rate` and `RateLimit::capacity`
were missing.
Add the documentation to help users decided on values based on how the
rate limiting works.
In case of a signature verification failure or failure to apply an
operation, the error is swallowed, but not logged.
During debugging, it can be crucial to understand whether these errors
occur, so log them.
If trace logging is enabled, also log the tips that are used. This
gives a better overview where operations originate from.
Add custom check for ellipses "...", asking for replacement of "…".
Git ranges and the CLI wildcard matches are ignored.
This change includes fixes to all sites that did not pass the check.
When invoked without any of `--patch`, `--delete`, `--accept`,
or `--reject`, the command panicked.
It will now correctly return an error to use `--accept`, `--reject`,
or to supply a message.
Note that `--patch` and `--delete` are removed as these commands are
now obsolete.
The new doc test also documents two previously-untested behaviors
of `rad patch review`: neutral reviews (verdict=None with summary)
and the COB layer's silent no-op when a duplicate review is submitted
(see `test_patch_review_duplicate` in radicle-cli/tests/commands/patch.rs).
Fixes 8ceff5d4b8fa94bcbe243f9f8ef52138f65e495d
Signed-off-by: Arthur Mariano <arthvm@proton.me>
Co-authored-by: Fintan Halpenny <fintan.halpenny@gmail.com>
Exercise the edge case where Bob, as a delegate, has never pushed to the default branch.
He then merges Alice's patch, and it is correctly marked as merged.
An edge case was found where a delegate attempted to merge a patch,
but had not pushed to their default branch yet.
This resulted in the `old` not being defined and the check to revert
and/or merge not happening – so the patch continued to appear as open.
Since `old` only matters for the revert and merge check for the
default branch, resolving the `old` side does its best to find a
commit for the checks.
It does this by falling back to getting the canonical head before the
push, and using that as the old.