KofTwentyTwo/nuncio
2
0 Fork 0
Code Issues 73 Pull Requests 12 Actions Packages Projects Releases Wiki Activity

feat(proto): enforce id-naming law and return full config from AddAccount #353

Merged
KofTwentyTwo merged 1 commits from story/ws-a4-id-conventions into dev 2026-07-31 22:04:11 +00:00
Conversation 0 Commits 1 Files Changed 8
+113 -68
8 changed files with 113 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/nuncio-cli/src/runner.rs
+24 -17
View File
@@ -876,7 +876,7 @@ impl HeadlessRunner {
.iter()
.map(|h| {
json!({
"id": h.id,
"id": h.message_id,
"title": h.title,
"snippet": h.snippet,
})
@@ -1383,7 +1383,9 @@ impl HeadlessRunner {
};
match client
.delete_rule(nuncio_proto::v1::DeleteRuleRequest { id: id.to_string() })
.delete_rule(nuncio_proto::v1::DeleteRuleRequest {
rule_id: id.to_string(),
})
.await
{
Ok(_) => {
@@ -1518,7 +1520,7 @@ impl HeadlessRunner {
match client
.update_rule(nuncio_proto::v1::UpdateRuleRequest {
id: id.to_string(),
rule_id: id.to_string(),
name: name.map(str::to_string),
nsql: sql.map(str::to_string),
priority,
@@ -1888,7 +1890,14 @@ impl HeadlessRunner {
.await
{
Ok(response) => {
let account_id = response.into_inner().id;
// Prefer the daemon-confirmed id from the persisted config it
// echoes back; fall back to the id this call requested if
// the response is somehow missing its config.
let account_id = response
.into_inner()
.config
.map(|c| c.id)
.unwrap_or(account_id);
if is_caldav {
let collection = collection_url.unwrap_or_default();
if json_mode {
@@ -2173,7 +2182,9 @@ impl HeadlessRunner {
};
match client
.remove_account(nuncio_proto::v1::RemoveAccountRequest { id: id.to_string() })
.remove_account(nuncio_proto::v1::RemoveAccountRequest {
account_id: id.to_string(),
})
.await
{
Ok(_) => {
@@ -2208,7 +2219,7 @@ impl HeadlessRunner {
match client
.test_account_connection(nuncio_proto::v1::TestAccountConnectionRequest {
id: id.to_string(),
account_id: id.to_string(),
})
.await
{
@@ -3046,16 +3057,12 @@ mod tests {
request: tonic::Request<AddAccountRequest>,
) -> Result<tonic::Response<AddAccountResponse>, tonic::Status> {
let req = request.into_inner();
let id = req
.config
.as_ref()
.map(|c| c.id.clone())
.unwrap_or_default();
let config = req.config.clone();
*self
.last_add_request
.lock()
.unwrap_or_else(|e| e.into_inner()) = Some(req);
Ok(tonic::Response::new(AddAccountResponse { id }))
Ok(tonic::Response::new(AddAccountResponse { config }))
}
async fn update_account(
@@ -3397,7 +3404,7 @@ mod tests {
let req = request.into_inner();
Ok(tonic::Response::new(SearchMessagesResponse {
hits: vec![MessageSearchHit {
id: "msg-stub-1".to_string(),
message_id: "msg-stub-1".to_string(),
title: "Stub Subject".to_string(),
snippet: format!("...{}...", req.query),
}],
@@ -4283,13 +4290,13 @@ mod tests {
request: tonic::Request<UpdateRuleRequest>,
) -> Result<tonic::Response<UpdateRuleResponse>, tonic::Status> {
let req = request.into_inner();
if req.id != "rule-stub-1" {
if req.rule_id != "rule-stub-1" {
return Err(tonic::Status::not_found("no such rule"));
}
*self.last_update.lock().unwrap_or_else(|e| e.into_inner()) = Some(req.clone());
Ok(tonic::Response::new(UpdateRuleResponse {
rule: Some(FilterRuleProto {
id: req.id,
id: req.rule_id,
name: req.name.unwrap_or_else(|| "Stub Rule".to_string()),
target_account: "*".to_string(),
priority: req.priority.unwrap_or(5),
@@ -4467,7 +4474,7 @@ mod tests {
.unwrap_or_else(|e| e.into_inner())
.clone()
.expect("stub daemon received a delete_rule request");
assert_eq!(recorded_delete.id, "rule-stub-1");
assert_eq!(recorded_delete.rule_id, "rule-stub-1");
// `filter validate` valid + invalid.
let validate_ok = runner
@@ -4533,7 +4540,7 @@ mod tests {
.unwrap_or_else(|e| e.into_inner())
.clone()
.expect("stub daemon received an update_rule request");
assert_eq!(recorded_update.id, "rule-stub-1");
assert_eq!(recorded_update.rule_id, "rule-stub-1");
assert_eq!(recorded_update.name.as_deref(), Some("Renamed Rule"));
assert_eq!(recorded_update.nsql, None);
assert_eq!(recorded_update.priority, Some(9));
crates/nuncio-proto/proto/nuncio/v1/descriptor.bin
BIN
View File
Binary file not shown.
crates/nuncio-proto/proto/nuncio/v1/nuncio.proto
+32 -14
View File
@@ -255,7 +255,10 @@ enum TlsMode {
// a separate `password` field) and as the response payload of
// `ListAccounts`, and the password MUST NEVER appear in the latter.
message AccountConfig {
// Unique identifier for the account configuration.
// Unique identifier for the account configuration. OPAQUE and
// server-assigned: clients MUST NOT parse or construct this value, only
// pass it back verbatim (see `Message.id`'s doc comment for the general
// rule that applies to every entity id in this API).
string id = 1;
// User-friendly display name (e.g. "Work Mail").
string name = 2;
@@ -300,10 +303,13 @@ message AddAccountRequest {
string password = 2;
}
// AddAccountResponse deliberately carries only the persisted account's id,
// never the password nor the full config (callers already have both).
// AddAccountResponse carries the persisted account configuration exactly as
// `ListAccounts` would report it, mirroring how the other create RPCs
// (`CreateRule`, `CreateContact`) return the created object rather than just
// its id. The password credential is NEVER included here -- see
// `AccountConfig`'s doc comment.
message AddAccountResponse {
string id = 1;
AccountConfig config = 1;
}
// ListAccountsRequest currently carries no parameters.
@@ -333,7 +339,7 @@ message UpdateAccountResponse {}
// RemoveAccountRequest names the account configuration to delete by id.
message RemoveAccountRequest {
string id = 1;
string account_id = 1;
}
// RemoveAccountResponse currently carries no fields; a successful RPC
@@ -342,7 +348,7 @@ message RemoveAccountResponse {}
// TestAccountConnectionRequest names the persisted account to probe by id.
message TestAccountConnectionRequest {
string id = 1;
string account_id = 1;
}
// TestAccountConnectionResponse reports the REAL, independent outcome of
@@ -427,7 +433,13 @@ message Attachment {
// (rather than `Email`) to avoid colliding with common generated-code
// identifiers in hand-written Rust using this module.
message Message {
// Unique message identifier.
// Unique message identifier. OPAQUE and server-assigned: a deterministic
// surrogate derived from the message's stable protocol identity, never a
// raw upstream protocol id (e.g. an IMAP UID, which is only stable within
// a single IMAP session and is NOT a durable cross-session identifier).
// Clients MUST treat this as an opaque token -- pass it back verbatim in
// later requests, but never parse, construct, or assume any structure in
// its value.
string id = 1;
// Account identifier owning the message.
string account_id = 2;
@@ -553,7 +565,7 @@ message SearchMessagesRequest {
// Mirrors `nuncio_store::search::SearchHit`.
message MessageSearchHit {
// Matching message ID.
string id = 1;
string message_id = 1;
// Matching subject/title.
string title = 2;
// Text snippet matching the search term.
@@ -690,7 +702,9 @@ service Filters {
// NSQL-rendered form (`RuleAction::to_nsql`) of the rule's parsed action
// list, included for display purposes.
message FilterRule {
// Rule unique identifier.
// Rule unique identifier. OPAQUE and server-assigned -- see `Message.id`'s
// doc comment for the general rule that applies to every entity id in
// this API.
string id = 1;
// Rule display name.
string name = 2;
@@ -752,7 +766,7 @@ message ListRulesResponse {
// DeleteRuleRequest.
message DeleteRuleRequest {
// Rule unique identifier.
string id = 1;
string rule_id = 1;
}
// DeleteRuleResponse currently carries no parameters; a successful RPC
@@ -810,12 +824,12 @@ message PreviewRuleResponse {
repeated string condition_traces = 7;
}
// UpdateRuleRequest carries a persisted rule's `id` plus optional
// UpdateRuleRequest carries a persisted rule's `rule_id` plus optional
// per-field overrides; an absent field leaves that field unchanged on the
// existing rule.
message UpdateRuleRequest {
// Rule unique identifier.
string id = 1;
string rule_id = 1;
// New display name, if changing.
optional string name = 2;
// New full NSQL source code text, if changing.
@@ -1019,7 +1033,9 @@ service Calendar {
// `CalendarEvent` (matching the core model) rather than `Event` -- `Event` is
// already taken by `System`'s push-event wrapper.
message CalendarEvent {
// Event identifier.
// Event identifier. OPAQUE and server-assigned -- see `Message.id`'s doc
// comment for the general rule that applies to every entity id in this
// API.
string id = 1;
// Account identifier owning the event.
string account_id = 2;
@@ -1153,7 +1169,9 @@ message ContactPhone {
// for them was identified for this vertical; they can be added additively
// later if one arises.
message Contact {
// Contact unique identifier.
// Contact unique identifier. OPAQUE and server-assigned -- see
// `Message.id`'s doc comment for the general rule that applies to every
// entity id in this API.
string id = 1;
// Account identifier owning this contact's address book entry.
optional string account_id = 2;
crates/nunciod/src/grpc.rs
+36 -28
View File
@@ -592,7 +592,9 @@ impl Accounts for AccountsGrpcService {
return Err(Status::internal(message));
}
Ok(Response::new(AddAccountResponse { id: config.id }))
Ok(Response::new(AddAccountResponse {
config: Some(map_account_config_to_proto(config)),
}))
}
async fn list_accounts(
@@ -671,25 +673,25 @@ impl Accounts for AccountsGrpcService {
request: Request<RemoveAccountRequest>,
) -> Result<Response<RemoveAccountResponse>, Status> {
let req = request.into_inner();
if req.id.is_empty() {
return Err(Status::invalid_argument("id is required"));
if req.account_id.is_empty() {
return Err(Status::invalid_argument("account_id is required"));
}
let existing = self
.db
.get_account(&req.id)
.get_account(&req.account_id)
.await
.map_err(|e| Status::internal(format!("failed to look up account: {e}")))?
.ok_or_else(|| {
errors::status_with_metadata(
ErrorReason::AccountNotFound,
format!("account '{}' not found", req.id),
[("account_id".to_string(), req.id.clone())],
format!("account '{}' not found", req.account_id),
[("account_id".to_string(), req.account_id.clone())],
)
})?;
self.db
.delete_account(&req.id)
.delete_account(&req.account_id)
.await
.map_err(|e| Status::internal(format!("failed to remove account: {e}")))?;
@@ -713,20 +715,20 @@ impl Accounts for AccountsGrpcService {
request: Request<TestAccountConnectionRequest>,
) -> Result<Response<TestAccountConnectionResponse>, Status> {
let req = request.into_inner();
if req.id.is_empty() {
return Err(Status::invalid_argument("id is required"));
if req.account_id.is_empty() {
return Err(Status::invalid_argument("account_id is required"));
}
let config = self
.db
.get_account(&req.id)
.get_account(&req.account_id)
.await
.map_err(|e| Status::internal(format!("failed to look up account: {e}")))?
.ok_or_else(|| {
errors::status_with_metadata(
ErrorReason::AccountNotFound,
format!("account '{}' not found", req.id),
[("account_id".to_string(), req.id.clone())],
format!("account '{}' not found", req.account_id),
[("account_id".to_string(), req.account_id.clone())],
)
})?;
@@ -1557,7 +1559,7 @@ impl Mail for MailGrpcService {
let hits = hits
.into_iter()
.map(|hit| MessageSearchHit {
id: hit.id,
message_id: hit.id,
title: hit.title,
snippet: hit.snippet,
})
@@ -1872,12 +1874,12 @@ impl Filters for FiltersGrpcService {
request: Request<DeleteRuleRequest>,
) -> Result<Response<DeleteRuleResponse>, Status> {
let req = request.into_inner();
if req.id.is_empty() {
return Err(Status::invalid_argument("id is required"));
if req.rule_id.is_empty() {
return Err(Status::invalid_argument("rule_id is required"));
}
self.db
.delete_filter_rule(&req.id)
.delete_filter_rule(&req.rule_id)
.await
.map_err(|e| Status::internal(format!("failed to delete filter rule: {e}")))?;
@@ -1972,12 +1974,12 @@ impl Filters for FiltersGrpcService {
.await
.map_err(|e| Status::internal(format!("failed to list filter rules: {e}")))?
.into_iter()
.find(|r| r.id == req.id)
.find(|r| r.id == req.rule_id)
.ok_or_else(|| {
errors::status_with_metadata(
ErrorReason::RuleNotFound,
format!("filter rule '{}' not found", req.id),
[("rule_id".to_string(), req.id.clone())],
format!("filter rule '{}' not found", req.rule_id),
[("rule_id".to_string(), req.rule_id.clone())],
)
})?;
@@ -3466,7 +3468,13 @@ mod tests {
.await
.expect("add_account succeeds")
.into_inner();
assert_eq!(add_response.id, "acct-restart-1");
assert_eq!(
add_response
.config
.expect("response carries the created config")
.id,
"acct-restart-1"
);
db_first_run.close().await;
@@ -4216,7 +4224,7 @@ mod tests {
.into_inner()
.hits;
assert_eq!(hits.len(), 1);
assert_eq!(hits[0].id, "msg-mail-1");
assert_eq!(hits[0].message_id, "msg-mail-1");
// MarkRead persists: a second GetMessage call reflects the flip.
client
@@ -4745,7 +4753,7 @@ mod tests {
let err = client
.delete_rule(DeleteRuleRequest {
id: "rule-1".to_string(),
rule_id: "rule-1".to_string(),
})
.await
.expect_err("missing bearer token must be rejected");
@@ -4770,7 +4778,7 @@ mod tests {
let err = client
.update_rule(UpdateRuleRequest {
id: "rule-1".to_string(),
rule_id: "rule-1".to_string(),
name: None,
nsql: None,
priority: None,
@@ -4877,7 +4885,7 @@ mod tests {
// DeleteRule removes it AND reloads the engine back to empty.
client
.delete_rule(authed_bearer_request(DeleteRuleRequest {
id: rule_id.clone(),
rule_id: rule_id.clone(),
}))
.await
.expect("delete_rule succeeds");
@@ -5018,7 +5026,7 @@ mod tests {
let err = client
.delete_rule(authed_bearer_request(DeleteRuleRequest {
id: String::new(),
rule_id: String::new(),
}))
.await
.expect_err("empty id must be rejected");
@@ -5221,7 +5229,7 @@ mod tests {
let updated = client
.update_rule(authed_bearer_request(UpdateRuleRequest {
id: rule_id.clone(),
rule_id: rule_id.clone(),
name: Some("Renamed".to_string()),
nsql: None,
priority: Some(1),
@@ -5281,7 +5289,7 @@ mod tests {
let err = client
.update_rule(authed_bearer_request(UpdateRuleRequest {
id: "no-such-rule".to_string(),
rule_id: "no-such-rule".to_string(),
name: Some("Whatever".to_string()),
nsql: None,
priority: None,
@@ -5313,7 +5321,7 @@ mod tests {
let err = client
.update_rule(authed_bearer_request(UpdateRuleRequest {
id: created.id,
rule_id: created.id,
name: None,
nsql: Some("THIS IS NOT VALID NSQL AT ALL {{{".to_string()),
priority: None,
crates/nunciod/tests/accounts_lifecycle_e2e_test.rs
+10 -4
View File
@@ -164,7 +164,13 @@ async fn account_lifecycle_round_trips_over_authenticated_grpc_offline() {
.await
.expect("add_account succeeds")
.into_inner();
assert_eq!(add_response.id, ACCOUNT_ID);
assert_eq!(
add_response
.config
.expect("response carries the created config")
.id,
ACCOUNT_ID
);
// ---- (b) ListAccounts round-trips BOTH distinct TLS modes ----
let listed = accounts_client
@@ -204,7 +210,7 @@ async fn account_lifecycle_round_trips_over_authenticated_grpc_offline() {
// ---- (d) TestAccountConnection reports the GENUINE per-protocol result ----
let report = accounts_client
.test_account_connection(TestAccountConnectionRequest {
id: ACCOUNT_ID.to_string(),
account_id: ACCOUNT_ID.to_string(),
})
.await
.expect("test_account_connection succeeds")
@@ -220,7 +226,7 @@ async fn account_lifecycle_round_trips_over_authenticated_grpc_offline() {
// ---- (e) RemoveAccount genuinely deletes it ----
accounts_client
.remove_account(RemoveAccountRequest {
id: ACCOUNT_ID.to_string(),
account_id: ACCOUNT_ID.to_string(),
})
.await
.expect("remove_account succeeds");
@@ -237,7 +243,7 @@ async fn account_lifecycle_round_trips_over_authenticated_grpc_offline() {
// fabricated success.
let err = accounts_client
.test_account_connection(TestAccountConnectionRequest {
id: ACCOUNT_ID.to_string(),
account_id: ACCOUNT_ID.to_string(),
})
.await
.expect_err("testing a removed account must fail");
crates/nunciod/tests/grpc_auth_test.rs
+3 -3
View File
@@ -178,7 +178,7 @@ async fn rejects_calls_to_new_filters_rpcs_without_a_bearer_token() {
let err = client
.update_rule(nuncio_proto::v1::UpdateRuleRequest {
id: "rule-1".to_string(),
rule_id: "rule-1".to_string(),
name: None,
nsql: None,
priority: None,
@@ -221,7 +221,7 @@ async fn rejects_calls_to_new_accounts_rpcs_without_a_bearer_token() {
let remove_err = client
.remove_account(nuncio_proto::v1::RemoveAccountRequest {
id: "acct-1".to_string(),
account_id: "acct-1".to_string(),
})
.await
.expect_err("unauthenticated remove_account must be rejected");
@@ -229,7 +229,7 @@ async fn rejects_calls_to_new_accounts_rpcs_without_a_bearer_token() {
let test_err = client
.test_account_connection(nuncio_proto::v1::TestAccountConnectionRequest {
id: "acct-1".to_string(),
account_id: "acct-1".to_string(),
})
.await
.expect_err("unauthenticated test_account_connection must be rejected");
crates/nunciod/tests/spine_e2e_test.rs
+7 -1
View File
@@ -202,7 +202,13 @@ async fn full_mail_spine_round_trips_over_authenticated_grpc_with_mocked_protoco
.await
.expect("add_account succeeds")
.into_inner();
assert_eq!(add_response.id, ACCOUNT_ID);
assert_eq!(
add_response
.config
.expect("response carries the created config")
.id,
ACCOUNT_ID
);
let list_response = accounts_client
.list_accounts(ListAccountsRequest {})
crates/nunciod/tests/typed_errors_e2e_test.rs
+1 -1
View File
@@ -93,7 +93,7 @@ async fn typed_error_reasons_decode_from_status_details_over_the_wire() {
.expect("accounts client connects");
let status = accounts
.remove_account(RemoveAccountRequest {
id: "no-such-account".to_string(),
account_id: "no-such-account".to_string(),
})
.await
.expect_err("removing a nonexistent account must fail");