diff --git a/crates/nuncio-cli/src/runner.rs b/crates/nuncio-cli/src/runner.rs index 33b51e1..10fd6a3 100644 --- a/crates/nuncio-cli/src/runner.rs +++ b/crates/nuncio-cli/src/runner.rs @@ -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, ) -> Result, 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, ) -> Result, 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)); diff --git a/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin b/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin index 828d294..b13188a 100644 Binary files a/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin and b/crates/nuncio-proto/proto/nuncio/v1/descriptor.bin differ diff --git a/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto b/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto index 2024e64..742c34b 100644 --- a/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto +++ b/crates/nuncio-proto/proto/nuncio/v1/nuncio.proto @@ -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; diff --git a/crates/nunciod/src/grpc.rs b/crates/nunciod/src/grpc.rs index 234b4e1..0b95c1c 100644 --- a/crates/nunciod/src/grpc.rs +++ b/crates/nunciod/src/grpc.rs @@ -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, ) -> Result, 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, ) -> Result, 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, ) -> Result, 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, diff --git a/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs b/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs index 8da6b0d..37a5b33 100644 --- a/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs +++ b/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs @@ -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"); diff --git a/crates/nunciod/tests/grpc_auth_test.rs b/crates/nunciod/tests/grpc_auth_test.rs index 22b543e..bdd5db8 100644 --- a/crates/nunciod/tests/grpc_auth_test.rs +++ b/crates/nunciod/tests/grpc_auth_test.rs @@ -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"); diff --git a/crates/nunciod/tests/spine_e2e_test.rs b/crates/nunciod/tests/spine_e2e_test.rs index d7ad21f..e99d5b3 100644 --- a/crates/nunciod/tests/spine_e2e_test.rs +++ b/crates/nunciod/tests/spine_e2e_test.rs @@ -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 {}) diff --git a/crates/nunciod/tests/typed_errors_e2e_test.rs b/crates/nunciod/tests/typed_errors_e2e_test.rs index 6e36958..ee4f4c0 100644 --- a/crates/nunciod/tests/typed_errors_e2e_test.rs +++ b/crates/nunciod/tests/typed_errors_e2e_test.rs @@ -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");