Skip to content

Pass bootroot's registration id to service add and info #140

Closed
#141
Closed
Pass bootroot's registration id to service add and info#140
#141
Assignees
sehkone

Description

@sehkone
sehkone
opened on Sep 29, 2026

Context

bootroot main splits a registration's namespace key from its SAN label (aicers/bootroot#756, commit 314c5f3). The split is not in bootroot's 0.3.0 tag (a14641f), which it follows; it is in bootroot main at 8b92d250, whose Cargo.toml version is 0.3.0 and which bootler is about to pin. --registration-id is now the key that names every namespace a registration owns: its state.json entry, its AppRole and policy, its bootroot/services/<key> KV subtree, its managed agent.toml block and its fast-poll state file. --service-name is only the SAN's service label, and several registrations may share one. The lookup commands take the key alone. At bootroot 8b92d250 (src/cli/args.rs):

  • service add takes --registration-id and --service-name, both optional at parse time. service add prompts for a missing registration id. deploy-core runs bootroot with standard input closed, and bootroot's prompt fails on end of input (src/cli/prompt.rs).
  • service info takes --registration-id, required. --service-name is not an argument of service info any more, so clap rejects it and exits non-zero (the test test_cli_lookup_commands_reject_service_name pins this for service info, service update, service remove, verify, rotate approle-secret-id and rotate force-reissue).

deploy-core's registration primitives (src/registration.rs) still speak the 0.2.0 contract:

  • service_add_args (src/registration.rs:186) emits --service-name <name> and no --registration-id. Against 8b92d250 every service add fails on the prompt, so every install fails at service registration.
  • service_registered (src/registration.rs:257) runs service info --service-name <name>. Against 8b92d250 that exits non-zero on the unknown argument, which the function reads as "not registered". A re-install then runs service add for every registration. bootroot accepts that for an unchanged remote-bootstrap registration as an idempotent re-run, but refuses it for a local-file one as a duplicate (bootroot src/commands/service.rs:282–289), so a re-install fails.

bootler is the one consumer that calls these functions. It pins deploy-core at 3e49e1964d75, whose src/registration.rs is identical to main's. It moves its pin, and passes a registration id, in aicers/bootler#498, which depends on this one. roxyd pins an older deploy-core and does not call these functions in code.

Decision

  1. ServiceAddSpec carries the registration id as its own field. Add pub registration_id: &'a str to ServiceAddSpec, placed before service_name. Its doc says it is bootroot's namespace key (the state.json entry, the AppRole and policy, the bootroot/services/<key> KV subtree, the managed agent.toml block, the fast-poll state file). service_name's doc changes to say it is the SAN's service label only. deploy-core does not derive one from the other and does not validate either: the caller chooses both, and bootroot validates them.
  2. service_add_args emits both flags. The vector starts service add --registration-id <registration_id> --service-name <service_name>, followed by every flag it emits today in today's order. No other flag changes.
  3. service_registered queries by registration id. Its second parameter is renamed registration_id, and it runs service info --registration-id <registration_id> as Identity::Root. Exit status is still the whole answer: success means registered, any non-zero exit means not registered. Its doc says so, and names the argument as the registration id.
  4. run_service_add, CoreError::ServiceRegistration and ServiceOutcome do not change. Their service/service_name is the name the caller reports to its operator, which is the caller's choice.
  5. The --secret-id-path comment stops stating a bootroot rule that no longer holds. The comment in service_add_args (from src/registration.rs:222) says bootroot rejects --secret-id-path for remote-bootstrap delivery. bootroot 8b92d250 accepts it for both modes (Accept a target secret_id path for remote-bootstrap service add bootroot#1019). The argv behaviour is unchanged: the flag is emitted exactly when secret_id_path is Some. The comment and the secret_id_path field doc say that, and that the caller decides when it applies.
  6. The module doc (src/registration.rs:1) says that service info checks a registration by its registration id.

Scope

  • src/registration.rs: Decisions 1–6, and unit tests (Test plan).
  • Nothing else. There is no changelog entry: deploy-core has not been released.

Out of scope

  • module_spec::RegistrationTemplate and its service_name. A module's registration template stays as it is; how a module registration gets its registration id is decided where that registration is built.
  • Any other bootroot command. deploy-core builds no service remove, rotate, verify or registrar argv; src/bootroot_cmd.rs only runs what its caller passes.
  • bootler's and roxyd's pins.

Acceptance criteria

  • ServiceAddSpec has a registration_id field, and service_add_args emits --registration-id <registration_id> and --service-name <service_name> exactly once each, as the first flags after service add.
  • Every other flag service_add_args emits is unchanged, in both delivery modes, with and without secret_id_path, cert_group_gid and endpoints.
  • service_registered runs exactly service info --registration-id <id> as Identity::Root, and no argv it builds contains --service-name.
  • service_registered returns Ok(true) on a zero exit and Ok(false) on a non-zero exit.
  • Rustdoc of ServiceAddSpec, service_add_args, service_registered and the module describes the two names as above, and no comment says bootroot rejects --secret-id-path for remote-bootstrap delivery.
  • cargo fmt -- --check --config group_imports=StdExternalCrate, cargo clippy --all-targets -- -D warnings, cargo clippy --all-targets --features test-support -- -D warnings, cargo doc --no-deps --document-private-items --features test-support with RUSTDOCFLAGS=-D warnings, cargo test and cargo test --features test-support pass.

Test plan

src/registration.rs has no tests today. Add a #[cfg(test)] module:

  • service_add_args with a local-file spec whose registration_id and service_name differ (for example roxyd-mgmt and roxyd) asserts the exact full vector, so a swap of the two values or a dropped flag fails.
  • The same for a remote-bootstrap spec with secret_id_path, cert_group_gid and endpoints all set, asserting --secret-id-wrap-ttl 60m and the endpoint flags keep their positions after the new pair.
  • A local-file spec with secret_id_path, cert_group_gid and endpoints all None, and a remote-bootstrap spec with secret_id_path and cert_group_gid None and endpoints set: each exact vector, so the absent flags are proven absent.
  • A spec whose registration_id equals its service_name emits both flags with the same value.
  • service_registered against executor::test_support::RecordingExecutor (a #[cfg(test)] module can use it), scripted with a zero exit and then a non-zero exit, returns true and then false. BootrootRunner::run goes through Executor::run_in, which records a RecordedCall::Run of sh with the working-directory wrapper (src/executor.rs:1452). Assert that the call's identity is Identity::Root, that its arguments contain the runner's state directory, and that they end with the runner's command path followed by service, info, --registration-id, <id>. Assert that no argument is --service-name.

Pointers

  • src/registration.rs:128 — ServiceAddSpec.
  • src/registration.rs:186 — service_add_args.
  • src/registration.rs:222 — the --secret-id-path comment.
  • src/registration.rs:257 — service_registered.
  • src/executor/test_support.rs — RecordingExecutor.
  • bootroot 8b92d250 src/cli/args.rs — ServiceAddArgs (registration_id, service_name) and ServiceInfoArgs (registration_id, required).

Activity

  1. sehkone
    self-assigned this
    on Sep 29, 2026
  2. sehkone
    closed this as completedin #141on Sep 29, 2026

  3. sehkone commented on Sep 29, 2026

    @sehkone
    sehkone
    on Sep 29, 2026
    ContributorAuthor

    Verification summary

    • Delivery: COMPLETE — the merged code delivers what the issue asked for. (the auditor's own verdict)
    • Extra changes: NO_EXTRA_CHANGES — the merged code changed nothing beyond what the issue asked for. (the auditor's own verdict)
    • Defects: NO_DEFECT — the merged pull requests left no defect of their own behind. (the auditor's own verdict)

    Delivery decides whether this issue is closed or reopened. Defects widens what the follow-up filing considers. Extra changes gates nothing — it is an observation for a human; the report names what went beyond the issue.

    Implementing pull requests, at the merge commits this audit read:

    The full report — the assessment included verbatim — stays local, at:
    ~/.agentcoop/runs/aicers/deploy-core/verify/MTQw/report.md

    Findings this run did not file:

    Finding not filed — This finding did not meet the certainty bar for filing.
    Finding (the auditor's accepted statement):
    ServiceAddSpec carries a separate registration id with the requested namespace and SAN label documentation
    Explanation (the auditor's whole response):
    `ServiceAddSpec` has a `registration_id` field before `service_name`. Its docs identify the registration id as bootroot’s namespace key and the service name as the SAN label, with both values chosen by the caller. [src/registration.rs:129](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:129) This fulfills issue #140’s first decision, so it presents no shortfall to file.
    Interpretation (the auditor's accepted statement):
    As a delivery finding, this records that the required separate field and documentation are present.
    Interpretation (the auditor's accepted statement):
    As a proposed shortfall, it would require those elements to be missing or wrong; the cited code rules that interpretation out, so there is no genuine competing reading of the issue.

    Finding not filed — This finding did not meet the certainty bar for filing.
    Finding (the auditor's accepted statement):
    service_add_args emits both name flags first and preserves the remaining flags in both delivery modes
    Explanation (the auditor's whole response):
    `service_add_args` emits `--registration-id` and `--service-name` as the first flags after `service add`, then preserves the existing flags and their conditions in both delivery modes. [src/registration.rs:201](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:201), [src/registration.rs:241](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:241), [src/registration.rs:249](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:249) This satisfies the issue’s argv requirement; there is no missing behavior to file.
    Interpretation (the auditor's accepted statement):
    As a delivery finding, the merged argv builder satisfies the required flag order and preserves the other flags.
    Interpretation (the auditor's accepted statement):
    As a proposed shortfall, it would require a missing, misplaced, or changed flag; the cited builder does not support that reading, so there is no genuine competing interpretation of the issue.

    Finding not filed — This finding did not meet the certainty bar for filing.
    Finding (the auditor's accepted statement):
    service_registered queries by registration id as root and maps exit status to a boolean
    Explanation (the auditor's whole response):
    `service_registered` runs `service info --registration-id <id>` as `Identity::Root` and returns the command’s success value, so a zero exit yields `Ok(true)` and a nonzero exit yields `Ok(false)`. [src/registration.rs:274](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:274), [src/executor.rs:639](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/executor.rs:639) This fulfills the issue’s lookup requirement; it is not a shortfall to file.
    Interpretation (the auditor's accepted statement):
    As a delivery finding, the merged function satisfies the required lookup command and exit-status behavior.
    Interpretation (the auditor's accepted statement):
    As a proposed shortfall, it would require an incorrect command, identity, or result; the cited code rules those out, so no genuine competing interpretation remains.

    Finding not filed — This finding did not meet the certainty bar for filing.
    Finding (the auditor's accepted statement):
    run_service_add and ServiceOutcome preserve the caller's operator-facing service name
    Explanation (the auditor's whole response):
    `run_service_add` still uses the caller’s `service` argument when reporting `CoreError::ServiceRegistration`, and `ServiceOutcome` still carries `service_name`. [src/registration.rs:293](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:293), [src/registration.rs:304](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:304), [src/registration.rs:98](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:98) The issue explicitly required this reporting behavior to remain unchanged, so this finding records compliance rather than work to file.
    Interpretation (the auditor's accepted statement):
    As a delivery finding, the reporting types and function retain the caller-chosen service name, as the issue requires.
    Interpretation (the auditor's accepted statement):
    As a proposed shortfall, it would require the merged code to replace that name with the registration id; the cited code does not, so no genuine competing interpretation remains.

    Finding not filed — This finding did not meet the certainty bar for filing.
    Finding (the auditor's accepted statement):
    The module and function docs describe registration-id lookup, and the obsolete secret-id-path restriction was removed
    Explanation (the auditor's whole response):
    The module, builder, and lookup docs describe registration-id lookup and distinguish the namespace key from the SAN service label. The `secret_id_path` field doc and builder comment say the caller decides when relocation applies and the flag is emitted when supplied; the obsolete “local-file only” claim is gone. [src/registration.rs:4](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:4), [src/registration.rs:160](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:160), [src/registration.rs:177](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:177), [src/registration.rs:238](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:238), [src/registration.rs:262](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:262) These are the documentation corrections issue #140 requested, so there is no documentation shortfall to file.
    Interpretation (the auditor's accepted statement):
    As a delivery finding, the merged documentation makes the distinctions and corrections the issue requested.
    Interpretation (the auditor's accepted statement):
    As a proposed shortfall, it would require a missing distinction or the obsolete restriction to remain; the cited text rules that out, so no genuine competing interpretation remains.

    Finding not filed — This finding did not meet the certainty bar for filing.
    Finding (the auditor's accepted statement):
    The five requested service_add_args vector test cases are present
    Explanation (the auditor's whole response):
    The five requested `service_add_args` test cases are present in `src/registration.rs`: differing names in both delivery modes with optional flags set, both specified combinations of absent optional flags, and equal names with both flags emitted. [src/registration.rs:399](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:399), [src/registration.rs:421](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:421), [src/registration.rs:450](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:450), [src/registration.rs:461](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:461), [src/registration.rs:484](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:484) This fulfills that part of the test plan, so there is no missing test case to file.
    Interpretation (the auditor's accepted statement):
    As a delivery finding, the five requested argv test cases are present in the merged code.
    Interpretation (the auditor's accepted statement):
    As a proposed shortfall, it would require at least one requested case to be absent; the cited tests account for all five, so no genuine competing interpretation remains.

    Finding not filed — This finding did not meet the certainty bar for filing.
    Finding (the auditor's accepted statement):
    The requested RecordingExecutor test covers lookup arguments, identity, and exit status
    Explanation (the auditor's whole response):
    `service_registered_queries_by_registration_id_and_reads_the_exit_status` uses `RecordingExecutor` to script zero and nonzero exits, then checks the root identity, state directory, `service info --registration-id <id>` command suffix, and absence of `--service-name`. [src/registration.rs:511](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:511), [src/registration.rs:531](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:531), [src/registration.rs:544](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:544) It covers the lookup test plan, so there is no missing test to file.
    Interpretation (the auditor's accepted statement):
    As a delivery finding, the merged RecordingExecutor test covers the lookup behavior requested in the issue.
    Interpretation (the auditor's accepted statement):
    As a proposed shortfall, it would require a requested assertion or exit case to be missing; the cited test contains them, so no genuine competing interpretation remains.

    Finding not filed — This finding did not meet the certainty bar for filing.
    Finding (the auditor's accepted statement):
    The sole changed path is src/registration.rs, with no changelog or beyond-scope change
    Explanation (the auditor's whole response):
    PR #141’s complete changed-path list contains only `src/registration.rs`. The changes there implement the requested fields, command arguments, documentation, and tests; for example, the new registration-id argument is at [src/registration.rs:204](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:204) and the tests begin at [src/registration.rs:312](/Users/sehkone/.agentcoop/worktrees/aicers/deploy-core/verify-issue-140/src/registration.rs:312). The diff contains no changelog change, as the issue required. I found no change outside its scope to file.
    Interpretation (the auditor's accepted statement):
    As a scope finding, the sole changed path is src/registration.rs and the diff contains no changelog change, matching the issue.
    Interpretation (the auditor's accepted statement):
    As a proposed extra-change finding, it would require a changed path or change beyond the requested work; the complete changed-path list and reviewed diff show none.

    Finding not filed — This finding did not meet the certainty bar for filing.
    Finding (the auditor's accepted statement):
    The merge-commit CI checks completed successfully
    Explanation (the auditor's whole response):
    The later merge-commit CI reread reports every check for PR #141 as completed with a successful conclusion: CI, Markdown, both platform checks, `check`, and `test`. That replaces the pending-check qualification in my initial assessment. A successful check is evidence of verification, not a fault requiring a follow-up issue.
    Interpretation (the auditor's accepted statement):
    As a verification finding, the later CI reread reports all merge-commit checks completed successfully.
    Interpretation (the auditor's accepted statement):
    As a proposed shortfall, it would require a failed or still-pending check; the adjusted check facts report neither, so no genuine competing interpretation remains.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions