Fetching a large repository looped with
Failed to consume the pack sent by the remote
even though the pack appears valid (Git indexes it in full).
The cause appears to be a bug in zlib-rs 0.6.3 according to
<https://github.com/trifectatechfoundation/zlib-rs/pull/504>.
When a delegate creates a new `Revision`, their signature is implicitly added to the revision.
Prevent the delegate from an accept verdict on two active, sibling revisions by:
1. Checking that the `state` is `Active`, and then checking `has_active_sibling_accept`
2. If there is an existing verdict, then it the verdict from the new revision is stripped.
The creation of the `Revision` must still happen, as opposed to throwing an
error, in case of existing histories.
On `Action::RevisionAccept` use `has_active_sibling_accept` to check for a sibling accept.
If one is found, the `RevisionAccept` is logged and skipped.
This preserves the invariant of a single accept per delegate on active, sibling
revisions without throwing an error on existing histories.
Verifies that when Alice redacts a revision and Bob concurrently
accepts it, both nodes converge after sync. The CRDT evaluation
order (timestamp-based) determines which operation takes effect.
The test previously had B2 as a sibling of B1 (both children of A1),
because update() always uses self.current as parent. This meant
the test was testing multi-sibling rejection, not cascading.
Now B2 is a child of B1 via a manual transaction, so when B1 is
rejected (sibling E1 accepted), B2 is rejected via RejectedBy::Parent
(cascade), which is what the test name promises.
Introduce a property testing harness for asserting a set of properties expected
of the repository identity.
The harness uses the `Network` fixture for providing four nodes that can
interact with the repository identity. The state of the harness is advanced by
providing an actor and operation on the identity document. Invariants are then
asserted about the identity.
The current invariants are:
- The `current` revision is the one and only `Accepted` revision.
- The chain of revisions is valid. That is, `Active` revisions only have a
parent that is `Accepted`, and not `Rejected` or `Redacted`.
- `Active` revisions do not contain a majority approval.
- `Accepted` revisions contain a majority approval.
- `Rejected` revisions do not contain a majority approval.
- For each revision, at most one child is `Accepted`.
- For each revision, if any of its children is `Accepted`, all other
children are `Rejected`.
- A revision which is `Rejected(RejectedBy::Parent)` has a parent that
is `Rejected`.
- A revision which is `Redacted(RedactedBy::Parent)` has a parent that
is `Redacted`.
- For each revision that is `Rejected` or `Redacted`, none of its
children is `Active`.
- A sibling or ancestor revision's rejected state applies to its sibling or
descendant.
- The repository identity documents converge when all nodes have applied all
operations.
A `BoundedVec` that implements `Arbitrary` is useful for all property testing.
Move it from the `sigrefs` module so that it can be reused by other components.
When a revision reaches qourum and is adopted, the state all competing sibling
revisions as `Rejected`.
However, the test suite lacked coverage for deeper proposal branches,
specifically testing that this rejection correctly cascades to the
children of those rejected siblings.
Introduces a test to simulate the scenario where a delegate eagerly
proposes a chain of revisions (a child branching off a sibling) while
others concurrently accept a competing branch.
The identity evaluation rewrite changed how revision redactions are
handled. Instead of removing the revision entirely, it explicitly
transitions the revision to `State::Redacted`.
Add a test to cover two redaction scenarios:
- Attempting to redact a revision that has already been `Accepted`.
- Attempting to redact a revision that has already been `Rejected`.
The new identity evaluation logic was updated to ignore `Accept` or `Reject`
actions if the revision has already reached a terminal state (`Accepted` or
`Rejected`), however this handling was not explicitly covered by the test suite.
This test ensures that the state machine correctly short-circuits the
late-arriving vote, proving that the new logic succesfully ensures
terminal states remain immutable under concurrent network conditions.
The evaluation of the reposioty identity is rewritten to better handle cases
where there are child and sibling revisions that are active at the same time.
In particular, `fn Identity::action` is now free of any references to
`self.current`.
This is achieved through two main improvements. The first is that the `State`
of revisions changes to improve clarity:
1. `Active` and `Accepted` remain, and their meanings also remain the same.
2. `Stale` is removed entirely.
3. `Rejected` is improved to also contain `RejectedBy`, to keep track of the
reason for rejection.
4. `Redacted` is promoted to a state. Similarly, the reason for
redaction is tracked by `RedactedBy`.
The transition of a revision from `Active` to one of the other states now
influences siblings and children.
If a revision transitions to `Accepted`, then the sibling revisions can no
longer transition to `Accepted`. They are considered `Rejected` where the reason
is `Sibling`. This cascades: Children of siblings are `Rejected`
recursively, tracking `RejectedBy::Parent`.
Any children of the `Accepted` revision are also evaluated to see if they can be
similarly transitioned to the `Accepted` state; since they may have already been
voted on.
If a revision was rejected by a majority, then the it transitions to `Rejected`
with `RejectedBy::Vote`. Dually to acceptance, the children of this revision
are also rejected with the reason of `Ancestor`.
Finally, when the author of a revision redacts a revision, it transitions
to `Redacted`, tracking `RedactedBy::Author`, and its children are redacted
tracking `RedactedBy::Parent`.
The test is adjusted to `remove_delegate_concurrent` reflect that concurrently
proposed revisions are retained in the timeline and explicitly marked as rejected,
rather than being dropped entirely (as before).
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.
Rūdolfs Ošiņš
Fintan Halpenny
Lorenz Leutgeb
Adrian Duke
Lorenz Leutgeb
Richard Levitte
Fred Gobry
Daniel Norman
stefan
Daniel Silverstone