Skip to content

Forward the guest curator election #106

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
feruzm merged 3 commits into from
Sep 22, 2026
Merged

Forward the guest curator election #106

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4968774
Forward the guest curator election
feruzm Sep 22, 2026
f7b9fed
Let a reviewer abstain
feruzm Sep 22, 2026
7485fcf
Refuse a term on a seat that cannot expire
feruzm Sep 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 125 additions & 4 deletions dotnet/EcencyApi.Tests/CurationDeskPayloadTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ public class CurationDeskPayloadTests
CurationDeskWrites.RosterList, CurationDeskWrites.RosterSet, CurationDeskWrites.RosterRetire,
CurationDeskWrites.ApplicationApply, CurationDeskWrites.ApplicationMine,
CurationDeskWrites.ApplicationWithdraw, CurationDeskWrites.ApplicationList,
CurationDeskWrites.ApplicationDecide, CurationDeskWrites.ApplicationWindow,
CurationDeskWrites.ApplicationDecide, CurationDeskWrites.ApplicationVote,
CurationDeskWrites.ApplicationWindow,
};

private const string IngestBody =
Expand Down Expand Up @@ -66,6 +67,8 @@ private static string ValidBodyFor(CurationDeskWrites.Route route)
return "{" + forged + "\"motivation\":\"why\",\"availability\":\"evenings\",\"pick\":\"a post\"}";
if (ReferenceEquals(route, CurationDeskWrites.ApplicationDecide))
return "{" + forged + "\"applicant\":\"bob\",\"state\":\"declined\"}";
if (ReferenceEquals(route, CurationDeskWrites.ApplicationVote))
return "{" + forged + "\"applicant\":\"bob\",\"vote\":\"endorse\"}";
if (ReferenceEquals(route, CurationDeskWrites.ApplicationWindow))
return "{" + forged + "\"open\":true}";
return "{" + forged + "\"limit\":5}";
Expand Down Expand Up @@ -714,7 +717,7 @@ public void TheApplicantsOwnRoutesCarryNothingTheCallerSent()
public void ADecisionNamesTheApplicantInItsOwnField()
{
var payload = Ok(CurationDeskWrites.ApplicationDecide,
"{\"applicant\":\"bob\",\"state\":\"accepted\",\"role\":\"trial\",\"note\":\"guest curator\"}");
"{\"applicant\":\"bob\",\"state\":\"accepted\",\"role\":\"curator\",\"note\":\"guest curator\"}");
Assert.Equal(new[] { "username", "applicant", "state", "role", "note" },
payload.Select(kv => kv.Key).ToArray());
Assert.Equal("alice", payload["username"]!.GetValue<string>());
Expand All @@ -738,9 +741,127 @@ public void AMalformedDecisionIsRefusedRatherThanTrimmed(string json, string exp
[Fact]
public void ANeverGrantedRoleCannotArriveThroughAnAcceptance()
{
// The seat an acceptance grants is a trial by default and a curator or mod at most.
// Admin runs the desk, so it is not something a form hands out.
// The seat an acceptance grants is a curator, or a mod when an admin says so.
// Admin runs the desk, so it is not something a form hands out. `trial` is gone
// for a different reason: a guest seat is TRAILED and bounded by its term, and an
// untrailed month would be a month of work nothing follows.
Assert.DoesNotContain("admin", CurationDeskWrites.ApplicationRoles);
Assert.DoesNotContain("trial", CurationDeskWrites.ApplicationRoles);
}

// ---- electing a guest curator ------------------------------------------

[Fact]
public void AVoteNamesTheApplicantInItsOwnField()
{
var payload = Ok(CurationDeskWrites.ApplicationVote,
"{\"applicant\":\"bob\",\"vote\":\"object\",\"note\":\"farms comments\"}");
Assert.Equal(new[] { "username", "applicant", "vote", "note" },
payload.Select(kv => kv.Key).ToArray());
Assert.Equal("alice", payload["username"]!.GetValue<string>());
Assert.Equal("bob", payload["applicant"]!.GetValue<string>());
}

[Theory]
[InlineData("{\"vote\":\"endorse\"}", "applicant required")]
[InlineData("{\"applicant\":\"Bob\",\"vote\":\"endorse\"}", "applicant required")]
[InlineData("{\"applicant\":\"bob\"}", "invalid vote")]
[InlineData("{\"applicant\":\"bob\",\"vote\":\"yes\"}", "invalid vote")]
[InlineData("{\"applicant\":\"bob\",\"vote\":\"withdraw\"}", "invalid vote")]
[InlineData("{\"applicant\":\"bob\",\"vote\":\"ENDORSE\"}", "invalid vote")]
[InlineData("{\"applicant\":\"bob\",\"vote\":null}", "invalid vote")]
[InlineData("{\"applicant\":\"bob\",\"vote\":true}", "invalid vote")]
public void AMalformedVoteIsRefusedRatherThanTrimmed(string json, string expected)
{
Assert.Equal(expected, Rejected(CurationDeskWrites.ApplicationVote, json));
}

[Fact]
public void AStopCanBeLiftedWithoutBecomingAnEndorsement()
{
// Without a third value the only way out of an objection is to endorse, which
// makes somebody who merely stopped objecting add a vote toward the quorum.
Assert.Contains("abstain", CurationDeskWrites.ApplicationVotes);
Assert.Equal(new[] { "username", "applicant", "vote" },
Ok(CurationDeskWrites.ApplicationVote, "{\"applicant\":\"bob\",\"vote\":\"abstain\"}")
.Select(kv => kv.Key).ToArray());
}

[Fact]
public void AVoteNoteIsCappedInRunesNotUtf16Units()
{
// The column counts characters and so does Python's len(). An emoji is two UTF-16
// units and one rune, so counting the wrong one refuses 250 valid characters.
var emoji = string.Concat(Enumerable.Repeat("\U0001F600", 500));
Assert.True(Ok(CurationDeskWrites.ApplicationVote,
"{\"applicant\":\"bob\",\"vote\":\"endorse\",\"note\":\"" + emoji + "\"}")
.ContainsKey("note"));
Assert.Equal("invalid note", Rejected(CurationDeskWrites.ApplicationVote,
"{\"applicant\":\"bob\",\"vote\":\"endorse\",\"note\":\"" + emoji + "\U0001F600\"}"));
}

[Theory]
[InlineData("{\"open\":true,\"quorum\":0}", "quorum must be a whole number from 1 to 50")]
[InlineData("{\"open\":true,\"quorum\":51}", "quorum must be a whole number from 1 to 50")]
[InlineData("{\"open\":true,\"quorum\":true}", "quorum must be a whole number from 1 to 50")]
[InlineData("{\"open\":true,\"quorum\":1.5}", "quorum must be a whole number from 1 to 50")]
[InlineData("{\"open\":true,\"quorum\":\"3\"}", "quorum must be a whole number from 1 to 50")]
// A present null would be copied through the allowlist and read as absent upstream,
// quietly doing nothing while this fence claimed it had checked the number.
[InlineData("{\"open\":true,\"quorum\":null}", "quorum must be a whole number from 1 to 50")]
[InlineData("{\"open\":true,\"term_days\":0}", "term_days must be a whole number from 1 to 365")]
[InlineData("{\"open\":true,\"term_days\":366}", "term_days must be a whole number from 1 to 365")]
public void AMalformedElectionKnobIsRefused(string json, string expected)
{
Assert.Equal(expected, Rejected(CurationDeskWrites.ApplicationWindow, json));
}

[Fact]
public void TheKnobsAreOptionalAndTravelWhenTheyAreSent()
{
Assert.Equal(new[] { "username", "open" },
Ok(CurationDeskWrites.ApplicationWindow, "{\"open\":true}").Select(kv => kv.Key).ToArray());
Assert.Equal(new[] { "username", "open", "quorum", "term_days" },
Ok(CurationDeskWrites.ApplicationWindow, "{\"open\":true,\"quorum\":4,\"term_days\":14}")
.Select(kv => kv.Key).ToArray());
}

[Theory]
[InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"term_days\":-1}")]
[InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"term_days\":366}")]
[InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"term_days\":true}")]
[InlineData("{\"curator\":\"bob\",\"role\":\"curator\",\"term_days\":null}")]
public void AMalformedSeatTermIsRefused(string json)
{
Assert.Equal("term_days must be a whole number from 0 to 365",
Rejected(CurationDeskWrites.RosterSet, json));
}

[Fact]
public void ATermCannotBeAskedForOnASeatThatDoesNotExpire()
{
// The backend refuses this too. The fence exists so the round trip never leaves.
Assert.Equal("a mod seat does not expire", Rejected(CurationDeskWrites.RosterSet,
"{\"curator\":\"bob\",\"role\":\"mod\",\"term_days\":30}"));
Assert.Equal("a admin seat does not expire", Rejected(CurationDeskWrites.RosterSet,
"{\"curator\":\"bob\",\"role\":\"admin\",\"term_days\":1}"));
// 0 says "no term", which is what a permanent role already is, so it stays legal.
Assert.True(Ok(CurationDeskWrites.RosterSet,
"{\"curator\":\"bob\",\"role\":\"mod\",\"term_days\":0}").ContainsKey("term_days"));
Assert.True(Ok(CurationDeskWrites.RosterSet,
"{\"curator\":\"bob\",\"role\":\"trial\",\"term_days\":30}").ContainsKey("term_days"));
}

[Fact]
public void AZeroTermIsHowASeatIsMadePermanent()
{
// 0 is not "no term given": absent means keep the term the seat has, and 0 is the
// one way to say the seat should stop expiring. So it has to reach the backend.
var payload = Ok(CurationDeskWrites.RosterSet,
"{\"curator\":\"bob\",\"role\":\"curator\",\"term_days\":0}");
Assert.Equal(new[] { "username", "curator", "role", "term_days" },
payload.Select(kv => kv.Key).ToArray());
Assert.Equal(0, payload["term_days"]!.GetValue<int>());
}

[Theory]
Expand Down
4 changes: 3 additions & 1 deletion dotnet/EcencyApi.Tests/CurationDeskTestSupport.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,9 @@ public static TestClock UseTestClock()
yield return ("application-withdraw", PrivateApi.CurationDeskApplicationWithdraw, "{" + code + "}");
yield return ("application-list", PrivateApi.CurationDeskApplicationList, "{" + code + ",\"state\":\"open\"}");
yield return ("application-decide", PrivateApi.CurationDeskApplicationDecide,
"{" + code + ",\"applicant\":\"bob\",\"state\":\"accepted\",\"role\":\"trial\"}");
"{" + code + ",\"applicant\":\"bob\",\"state\":\"accepted\",\"role\":\"curator\"}");
yield return ("application-vote", PrivateApi.CurationDeskApplicationVote,
"{" + code + ",\"applicant\":\"bob\",\"vote\":\"endorse\"}");
yield return ("application-window", PrivateApi.CurationDeskApplicationWindow,
"{" + code + ",\"open\":false,\"message\":\"back soon\"}");
}
Expand Down
96 changes: 91 additions & 5 deletions dotnet/EcencyApi/Handlers/PrivateApi.CurationDesk.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,10 @@ public static Task CurationDeskApplicationList(HttpContext ctx) =>
public static Task CurationDeskApplicationDecide(HttpContext ctx) =>
ServeDeskWrite(ctx, CurationDeskWrites.ApplicationDecide);

// POST /private-api/curation-desk/application-vote
public static Task CurationDeskApplicationVote(HttpContext ctx) =>
ServeDeskWrite(ctx, CurationDeskWrites.ApplicationVote);
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.

// POST /private-api/curation-desk/application-window
public static Task CurationDeskApplicationWindow(HttpContext ctx) =>
ServeDeskWrite(ctx, CurationDeskWrites.ApplicationWindow);
Expand Down Expand Up @@ -815,12 +819,26 @@ public static readonly (string Key, int Max)[] ApplicationAnswers =
public static readonly IReadOnlySet<string> ApplicationDecisions =
new HashSet<string> { "shortlisted", "accepted", "declined" };
/// <summary>Roles an acceptance may grant. `admin` is deliberately absent: desk keys are
/// not handed out by a form, and the backend refuses it too.</summary>
/// not handed out by a form, and the backend refuses it too. `trial` is absent as well:
/// a guest seat is trailed and bounded by its TERM, not by an untrailed role, and the
/// backend's APPLICATION_ROLES no longer carries it either.</summary>
public static readonly IReadOnlySet<string> ApplicationRoles =
new HashSet<string> { "trial", "curator", "mod" };
new HashSet<string> { "curator", "mod" };
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
/// <summary>How a reviewer may answer an application. An objection is a stop, not a
/// veto: upstream it keeps the automatic grant shut and leaves the case to an admin.
/// `abstain` is how a stop is lifted, so that stopping objecting does not force
/// somebody to endorse instead.</summary>
public static readonly IReadOnlySet<string> ApplicationVotes =
new HashSet<string> { "endorse", "object", "abstain" };
public const int MaxApplicationNoteLength = 500;
public const int MaxApplicationMessageLength = 200;
public const int MaxApplicationListLimit = 200;
public const int MaxApplicationQuorum = 50;
public const int MaxApplicationTermDays = 365;
/// <summary>Roles a seat term may expire. A term on an admin or a mod means nothing,
/// and the backend's expiry reads the role as well as the clock for the same reason.</summary>
public static readonly IReadOnlySet<string> TermRoles =
new HashSet<string> { "curator", "trial" };

/// <summary>
/// Views the roster feed takes: the public ones plus `excluded`, which is
Expand Down Expand Up @@ -877,8 +895,13 @@ public static readonly (string Key, int Max)[] ApplicationAnswers =
/// </summary>
public static readonly Route RosterList = new("curation/desk/roster/list", Array.Empty<string>());

/// <summary>
/// `term_days` is tri-state upstream: absent keeps the seat's term, 0 makes it
/// permanent, n restarts the clock. Absent is not "make permanent", so a note being
/// corrected here cannot quietly turn a guest seat into a standing one.
/// </summary>
public static readonly Route RosterSet = new("curation/desk/roster/set",
new[] { "curator", "role", "rules", "note" });
new[] { "curator", "role", "rules", "note", "term_days" });

public static readonly Route RosterRetire = new("curation/desk/roster/retire", new[] { "curator" });

Expand Down Expand Up @@ -922,8 +945,20 @@ public static readonly (string Key, int Max)[] ApplicationAnswers =
public static readonly Route ApplicationDecide = new("curation/desk/applications/decide",
new[] { "applicant", "state", "role", "note" });

/// <summary>
/// One reviewer's line on one application. `applicant` names the account being voted
/// on for the same reason `decide` does: `username` carries the validated caller and
/// Build() refuses a client's version of it.
/// </summary>
public static readonly Route ApplicationVote = new("curation/desk/applications/vote",
new[] { "applicant", "vote", "note" });

/// <summary>
/// `quorum` and `term_days` are optional here and absent means "leave as they are"
/// upstream, not "reset": this route is called every time the message is reworded.
/// </summary>
public static readonly Route ApplicationWindow = new("curation/desk/applications/window",
new[] { "open", "message" });
new[] { "open", "message", "quorum", "term_days" });

/// <summary>
/// The upstream body: the validated username plus the route's whitelisted
Expand Down Expand Up @@ -1230,6 +1265,22 @@ private static void Truncate(JsonObject payload, string key, int max)
}
return null;
}
if (ReferenceEquals(route, ApplicationVote))
{
if (body.Str("applicant") is not { } voted || !HiveNames.IsAccountName(voted))
{
return "applicant required";
}
var ballot = RequireOneOf(body, "vote", ApplicationVotes);
if (ballot != null) return ballot;
if (body.TryGetPropertyValue("note", out var voteNote) && voteNote is not null
&& (body.Str("note") is not { } voteText
|| voteText.EnumerateRunes().Count() > MaxApplicationNoteLength))
{
return "invalid note";
}
return null;
}
if (ReferenceEquals(route, ApplicationWindow))
{
if (body.Field("open")?.GetValueKind() is not (JsonValueKind.True or JsonValueKind.False))
Expand All @@ -1242,7 +1293,12 @@ private static void Truncate(JsonObject payload, string key, int max)
{
return "invalid message";
}
return null;
// Both knobs are checked when PRESENT, null included: CopyIfPresent forwards a
// null through the allowlist, and a `"quorum": null` reaching the desk would
// read as absent there, silently doing nothing while this fence claimed every
// quorum it passes is a number in range.
return RequireWholeNumber(body, "quorum", 1, MaxApplicationQuorum)
?? RequireWholeNumber(body, "term_days", 1, MaxApplicationTermDays);
}
if (ReferenceEquals(route, RosterSet))
{
Expand All @@ -1261,6 +1317,19 @@ private static void Truncate(JsonObject payload, string key, int max)
{
return "invalid note";
}
// 0 is allowed and MEANS something here: it is how a guest seat is made
// permanent. The backend reads absent as "keep the term it has".
var termError = RequireWholeNumber(body, "term_days", 0, MaxApplicationTermDays);
if (termError != null) return termError;
// A term only expires a seat that may carry one, so asking for a term on a
// permanent role is a value the backend would only reject. 0 stays legal on
// any role: it says "no term", which is what a permanent role already is.
if (body.Field("term_days")?.GetValueKind() is JsonValueKind.Number
&& body.Field("term_days")!.GetValue<int>() > 0
&& body.Str("role") is { } seat && !TermRoles.Contains(seat))
{
return $"a {seat} seat does not expire";
}
// A PRESENT `rules` must be an object, null included. `CopyIfPresent` forwards
// a null through the allowlist, and the fence should say what the contract says.
if (body.TryGetPropertyValue("rules", out var rules))
Expand Down Expand Up @@ -1347,6 +1416,23 @@ private static void CopyIfPresent(JsonObject target, JsonObject body, string key

private static string? RequireOneOf(JsonObject body, string key, IReadOnlySet<string> allowed) =>
body.Str(key) is { } value && allowed.Contains(value) ? null : $"invalid {key}";

/// <summary>
/// A PRESENT whole number in range, null included. Absent is fine and means the caller
/// is not setting it. A fractional number fails here because TryGetValue&lt;int&gt;
/// refuses one, and `true` fails because it is not JsonValueKind.Number, which matters:
/// every one of these knobs decides how a roster seat is granted or how long it lasts.
/// </summary>
private static string? RequireWholeNumber(JsonObject body, string key, int min, int max)
{
if (!body.TryGetPropertyValue(key, out var node)) return null;
if (node is not JsonValue value || value.GetValueKind() is not JsonValueKind.Number
|| !value.TryGetValue<int>(out var number) || number < min || number > max)
Comment thread
qodo-free-for-open-source-projects[bot] marked this conversation as resolved.
{
return $"{key} must be a whole number from {min} to {max}";
}
return null;
}
}

/// <summary>
Expand Down
1 change: 1 addition & 0 deletions dotnet/EcencyApi/Handlers/Routes.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ public static void Map(WebApplication app)
app.MapPost("/private-api/curation-desk/application-withdraw", PrivateApi.CurationDeskApplicationWithdraw);
app.MapPost("/private-api/curation-desk/application-list", PrivateApi.CurationDeskApplicationList);
app.MapPost("/private-api/curation-desk/application-decide", PrivateApi.CurationDeskApplicationDecide);
app.MapPost("/private-api/curation-desk/application-vote", PrivateApi.CurationDeskApplicationVote);
app.MapPost("/private-api/curation-desk/application-window", PrivateApi.CurationDeskApplicationWindow);

// ---- SSR RPC cache (internal, header-gated; see SsrRpc.cs) ----
Expand Down
4 changes: 3 additions & 1 deletion dotnet/parity/driver.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,9 @@ def norm_body(text):
"recommend-meta", "recommendation-dismiss", "ingest",
# Guest curator applications, added with the desk's apply page.
"application-apply", "application-mine", "application-withdraw",
"application-list", "application-decide", "application-window")
"application-list", "application-decide", "application-window",
# Electing them: the roster votes, a quorum grants the seat.
"application-vote")
for case in ("min", "pop", "badcode")
]

Expand Down
Loading