From 844fc0679b0dae8cd1fb80be2aa2157cbeda2dd2 Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Sun, 4 Oct 2026 15:51:37 +0100 Subject: [PATCH 1/5] docs(cli): bound public self-profile editing --- .../initiatives/WS-CLI-001/WS-CLI-001-02.md | 114 ++++++++++++++++++ 1 file changed, 114 insertions(+) create mode 100644 .commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md new file mode 100644 index 000000000..469bcabc7 --- /dev/null +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md @@ -0,0 +1,114 @@ +# WS-CLI-001-02 — Public caller profile editing + +- Initiative: `WS-CLI-001` +- Durable disposition: `Complete` +- Intended merge outcome: The Go CLI edits or clears only caller-owned profile + display fields through the existing public PATCH contract. + +## Intent + +Complete the first self-service write for humans and agents, without copying +Workstream identity or authorization into the client. The user authorized this +slice after the two public reads in PR #471 merged. + +## Current behavior + +`cli/internal/command/command.go` provides profile and project reads. +`backend/app/api/routes/auth.py:update_current_actor_profile` already publishes +`PATCH /api/v1/actors/me`. `ActorProfileUpdateRequest` in +`backend/app/modules/actors/schemas.py` accepts only `display_name` and +`contact_email`: omitted fields are unchanged, null clears, text is validated +and normalized by the server. `ActorService.update_self` persists only supplied +fields. There is no public optimistic-version or replay-key contract for this +write; the CLI must not invent one. + +## Bounded change + +### Allowed + +- `cli/internal/api/client.go`: one fixed PATCH operation using the existing + transport/strict profile response validation; bounded update document. +- `cli/internal/command/command.go`: `profile update`, set/clear flags and shared + profile output. Built-process tests under `cli/tests/integration/`. +- CLI/root README, affected roadmap statements, this record, CLI overview and + the existing index row. + +### Not allowed + +- No backend/MCP/product/permission/schema change or private API access. +- No new dependency, endpoint dispatcher, local JWT verifier, credentials in + flags/files, role inference, authority edits, actor selector or background job. +- No automatic retry, invented idempotency/version header, extra preflight GET, + prompt/TUI, release packaging, unit suite or coverage quota. +- No workflow changes or weakening/skipping existing tests. + +## Design and decisions + +`workstream profile update` offers `--display-name`, `--contact-email`, +`--clear-display-name`, and `--clear-contact-email`. A set and clear for the +same field, or no selected field, fails as invalid arguments without making a +request. A typed update +request preserves omission versus explicit null. Server-owned validation is +not duplicated: invalid text receives the actual bounded API failure. + +Reuse the existing transport and profile decoder, and return the validated API +profile through the same text/JSON renderer. Only caller token forwarding is +performed. No second authentication or actor lookup is introduced. + +A PATCH transport failure, truncated/unreadable response, malformed success, +redirect, or server failure cannot prove whether the write committed. Such +failures retain a nonzero exit and report `outcome_unknown: true` in JSON (a +safe explanation in text). A complete 4xx response is a known denial/validation +failure, not uncertain commit. Do not retry; use `whoami` to inspect current +state, acknowledging that concurrent later writes remain possible. Successful +local output is not a global write-order guarantee. GET failures are unchanged. + +Alternatives: a JSON-file editor adds another input path for only two fields; +an interactive editor blocks agent automation. Neither is needed here. + +## Acceptance criteria + +- [ ] One exact public PATCH, caller bearer unchanged, JSON Content-Type and + only selected display fields; omission preserves, null clears. +- [ ] Invalid/conflicting flags cause no network write; no actor or + authority-field flags are available. +- [ ] Strict profile response decoding, terminal escaping and exact successful + JSON are reused for reads and writes; malformed success cannot report success. +- [ ] A dropped post-send response is nonzero, explicitly uncertain and never + retried. Complete denials retain safe metadata; redirect cannot forward body + or bearer. Existing read proofs remain intact. +- [ ] Real public API/PostgreSQL proof verifies stored edits, normalization, + omitted/null semantics, field limits, caller separation and suspended denial. +- [ ] Documentation reconciles three operations, write uncertainty and public + source availability without claiming distribution or lifecycle completion. + +## Risk and review routing + +- Risk class: `L1` (credential-bearing public mutation and uncertain execution). +- Required reviewers: `security`, `architecture`, `qa`, `test_delta`, + `documentation`; CI integrity only if workflow behavior changes. +- Human review focus: omission/null, one-request/no-retry semantics, truthful + unknown outcome, public-only client and retained backend authority. +- No additional human design decision or permission is required in scope. + +## Evidence + +| Claim | Command or proof | Result | Remaining uncertainty | +|---|---|---|---| +| Plan feasibility | Inspect actual public PATCH, request schema, update owner and existing subprocess harness; follow omitted/null and lost-response cases | Planned | Runtime proof must still execute | +| HTTP process contract | Extend `test_http_boundary.py` with PATCH payload/flag/redirect/uncertain-outcome assertions | Planned | Controlled server does not prove real authorization | +| Real write behavior | Extend `test_installed_cli_uses_only_public_profile_and_project_context` with stored profile edits, validation, isolation and suspended denial | Planned | Local Flow-compatible issuer does not certify deployment | +| Package and proof | Go verify/tidy/vet/build; Ruff; `run_isolated_tests.py --timeout-seconds 240 -- python -m pytest -q ../cli/tests/integration`; existing required hosted Backend proof | Planned | Binary release remains separate | + +## Review findings + +Record material findings and repaired behavior here; current review/CI freshness +belongs to the PR rather than this durable record. + +## Reconciliation + +- Current-source reconciliation: main `89dd8c91` includes the two-read CLI and + the existing public self-profile PATCH; no new product endpoint is needed. +- Next usable boundary: later role-specific public journeys, not hidden routes. +- Remaining risks: no server optimistic-update/replay contract is claimed; + production Flow and binary distribution remain separate release proof. From f44088119a2b616e714e6639517159f283bd730d Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Sun, 4 Oct 2026 16:05:14 +0100 Subject: [PATCH 2/5] feat(cli): edit caller profile through the public API --- .commitrail/INDEX.md | 2 +- .../initiatives/WS-CLI-001/OVERVIEW.md | 18 ++- .../initiatives/WS-CLI-001/WS-CLI-001-02.md | 32 ++-- README.md | 5 +- cli/README.md | 36 ++++- cli/internal/api/client.go | 94 ++++++++++-- cli/internal/command/command.go | 59 +++++-- cli/tests/integration/test_http_boundary.py | 145 ++++++++++++++++++ .../integration/test_public_self_service.py | 104 ++++++++++++- docs/roadmap_status.md | 8 +- 10 files changed, 453 insertions(+), 50 deletions(-) diff --git a/.commitrail/INDEX.md b/.commitrail/INDEX.md index cd6c19c63..ce2aa9f73 100644 --- a/.commitrail/INDEX.md +++ b/.commitrail/INDEX.md @@ -8,7 +8,7 @@ for current product capability. |---|---|---| | [WS-DB-002](initiatives/WS-DB-002/OVERVIEW.md) | Complete | Shared UUIDv7 record generation, native-UUID relationships and fresh v0.1 baseline; natural-owner retry custody and aligned CI/local setup | | [WS-MCP-002](initiatives/WS-MCP-002/OVERVIEW.md) | Planned | Three self-service tools delivered through WS-MCP-002-02; 24 tools remain and WS-MCP-002-03 administrative reads are next | -| [WS-CLI-001](initiatives/WS-CLI-001/OVERVIEW.md) | Planned | Two public Go CLI reads delivered through WS-CLI-001-01; profile editing and later public workflows remain | +| [WS-CLI-001](initiatives/WS-CLI-001/OVERVIEW.md) | Planned | Public Go CLI reads and human profile editing delivered through WS-CLI-001-02; role-specific public journeys and binary distribution remain | | [WS-ARCH-001](initiatives/WS-ARCH-001/OVERVIEW.md) | Planned | Source storage, inert AUTH contracts, TASK request reservation and hidden exact AUTH preparation are delivered; CON-07/shared acceptance prerequisites are next for the selected automated path, followed by hidden handlers and atomic routing activation. True admission does not depend on CON/shared acceptance. | | [WS-ART-001](initiatives/WS-ART-001/OVERVIEW.md) | Planned | Exact checker input/output custody and packet foundations are delivered; routing integration, remediation and public intake remain. | | [WS-AUTH-001](initiatives/WS-AUTH-001/OVERVIEW.md) | Planned | AUTH-19A commitments, TASK request reservation and ARCH-04E2-A hidden strict PREP matching are delivered; the action remains unavailable, with CON-07/shared acceptance, atomic receipt custody and scoped activation still required for the automated path. | diff --git a/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md b/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md index 59df57799..256699e83 100644 --- a/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md +++ b/.commitrail/initiatives/WS-CLI-001/OVERVIEW.md @@ -5,7 +5,8 @@ Workstream operations without creating another identity, authority, or product lifecycle implementation. - Delivered boundary: [WS-CLI-001-01](WS-CLI-001-01.md), caller-owned profile and - exact-project authorization context through the public REST API. + exact-project authorization context; [WS-CLI-001-02](WS-CLI-001-02.md), human + self-profile editing through the public REST API. ## Current boundary @@ -16,8 +17,11 @@ The backend exposes `GET /api/v1/actors/me` and those operations, but is not a certification of every public or hidden route. The independent [MCP adapter](../../../mcp_server/README.md) forwards caller bearers to the same API; it is not a CLI client library. The independent Go -package under `cli/` implements `whoami` and `project access PROJECT_ID`, with -text/JSON output and built-binary integration proof. Further public workflows +package under `cli/` implements `whoami`, `project access PROJECT_ID`, and +`profile update` for caller-owned human display fields. All have text/JSON +output and built-binary integration proof. Mutations preserve omitted/null +semantics and explicitly report uncertain outcomes without automatic retries. +Further public workflows and binary distribution remain proposed below. ## Design @@ -41,16 +45,16 @@ completion. Bubble Tea is a candidate for a later, bounded TUI change, not a dependency of the foundation. Do not add Python, TypeScript, or Rust duplicate CLIs. Keep the package independent of backend and MCP runtime dependencies. -## Proposed PR boundaries +## Delivered and later boundaries 1. **WS-CLI-001-01:** Independent Go package, safe caller-token transport, exact self-profile and project-authorization reads, human/JSON output, built-binary integration proof, package CI, and documentation. `GET /actors/me` can cause server-owned first admission and last-seen updates; the CLI must not describe it as side-effect free. -2. **Later self-service writes:** Profile editing and any further public - self-service operation, with explicit omission/null and uncertain-mutation - behavior. Define its own bounded record when started. +2. **WS-CLI-001-02:** Human profile editing through the existing public PATCH, + with explicit omission/null and uncertain-mutation behavior. It changes no + identity, authority or backend policy. 3. **Later governed-work commands:** Add project setup, task, submission, review, revision, and contribution reads/writes only as their actual public contracts and authority boundaries become available. Split by user journey, diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md index 469bcabc7..0e73f409f 100644 --- a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md @@ -7,8 +7,8 @@ ## Intent -Complete the first self-service write for humans and agents, without copying -Workstream identity or authorization into the client. The user authorized this +Complete human self-profile editing for interactive and agent-driven terminals, +without copying Workstream identity or authorization into the client. The user authorized this slice after the two public reads in PR #471 merged. ## Current behavior @@ -47,9 +47,10 @@ write; the CLI must not invent one. `workstream profile update` offers `--display-name`, `--contact-email`, `--clear-display-name`, and `--clear-contact-email`. A set and clear for the same field, or no selected field, fails as invalid arguments without making a -request. A typed update -request preserves omission versus explicit null. Server-owned validation is +request. A typed update request preserves omission versus explicit null. Server-owned validation is not duplicated: invalid text receives the actual bounded API failure. +The encoded request is capped at 8 KiB. Service-actor profile editing is not +supported by the existing human-only public contract. Reuse the existing transport and profile decoder, and return the validated API profile through the same text/JSON renderer. Only caller token forwarding is @@ -68,18 +69,18 @@ an interactive editor blocks agent automation. Neither is needed here. ## Acceptance criteria -- [ ] One exact public PATCH, caller bearer unchanged, JSON Content-Type and +- [x] One exact public PATCH, caller bearer unchanged, JSON Content-Type and only selected display fields; omission preserves, null clears. -- [ ] Invalid/conflicting flags cause no network write; no actor or +- [x] Invalid/conflicting flags cause no network write; no actor or authority-field flags are available. -- [ ] Strict profile response decoding, terminal escaping and exact successful +- [x] Strict profile response decoding, terminal escaping and exact successful JSON are reused for reads and writes; malformed success cannot report success. -- [ ] A dropped post-send response is nonzero, explicitly uncertain and never +- [x] A dropped post-send response is nonzero, explicitly uncertain and never retried. Complete denials retain safe metadata; redirect cannot forward body or bearer. Existing read proofs remain intact. -- [ ] Real public API/PostgreSQL proof verifies stored edits, normalization, +- [x] Real public API/PostgreSQL proof verifies stored edits, normalization, omitted/null semantics, field limits, caller separation and suspended denial. -- [ ] Documentation reconciles three operations, write uncertainty and public +- [x] Documentation reconciles three operations, write uncertainty and public source availability without claiming distribution or lifecycle completion. ## Risk and review routing @@ -95,16 +96,19 @@ an interactive editor blocks agent automation. Neither is needed here. | Claim | Command or proof | Result | Remaining uncertainty | |---|---|---|---| -| Plan feasibility | Inspect actual public PATCH, request schema, update owner and existing subprocess harness; follow omitted/null and lost-response cases | Planned | Runtime proof must still execute | -| HTTP process contract | Extend `test_http_boundary.py` with PATCH payload/flag/redirect/uncertain-outcome assertions | Planned | Controlled server does not prove real authorization | -| Real write behavior | Extend `test_installed_cli_uses_only_public_profile_and_project_context` with stored profile edits, validation, isolation and suspended denial | Planned | Local Flow-compatible issuer does not certify deployment | -| Package and proof | Go verify/tidy/vet/build; Ruff; `run_isolated_tests.py --timeout-seconds 240 -- python -m pytest -q ../cli/tests/integration`; existing required hosted Backend proof | Planned | Binary release remains separate | +| Plan feasibility | Inspect actual public PATCH, request schema, update owner and existing subprocess harness; follow omitted/null and lost-response cases | Reviewed; human-only wording clarified | Plan inspection alone is not execution | +| HTTP process contract | `test_http_boundary.py`: PATCH payload/flag/redirect/uncertain-outcome assertions | Passed in local process execution | Controlled server does not prove real authorization | +| Real write behavior | `test_installed_cli_uses_only_public_profile_and_project_context`: stored profile edits, validation, isolation and suspended denial | Passed with isolated PostgreSQL and verified cleanup | Local Flow-compatible issuer does not certify deployment | +| Package and proof | Go verify/tidy/vet/build; Ruff; `run_isolated_tests.py --timeout-seconds 240 -- python -m pytest -q ../cli/tests/integration`; existing required hosted Backend proof | Local checks pass; exact-head hosted evidence lives in the PR | Binary release remains separate | ## Review findings Record material findings and repaired behavior here; current review/CI freshness belongs to the PR rather than this durable record. +- `PLAN-SEC-001`: Narrowed human/agent wording to human self-profile editing + usable by noninteractive clients; no service-actor support is introduced. + ## Reconciliation - Current-source reconciliation: main `89dd8c91` includes the two-read CLI and diff --git a/README.md b/README.md index 43c0b08c6..b8e4bb868 100644 --- a/README.md +++ b/README.md @@ -296,8 +296,9 @@ implementation history until their owning migrations replace the runtime. ## Terminal Client The independent [Go CLI](cli/README.md) provides `workstream whoami` and -`workstream project access PROJECT_ID` through the currently public REST API. -Both support human-readable and JSON output, using the caller's Flow token. +`workstream project access PROJECT_ID`, plus `workstream profile update` for +caller-owned human display fields, through the currently public REST API. +All support human-readable and JSON output, using the caller's Flow token. The first source package is buildable; further workflow commands and published binaries remain planned. diff --git a/cli/README.md b/cli/README.md index d4a3fff27..721b7b8f8 100644 --- a/cli/README.md +++ b/cli/README.md @@ -1,11 +1,13 @@ # Workstream CLI An independent Go client for Workstream's public REST API, for humans and -agents using the terminal. The first slice provides two self-service reads: +agents using the terminal. It provides human self-profile reads and editing, +plus one exact-project authority read: | Command | Public API | |---|---| | `workstream whoami` | `GET /api/v1/actors/me` | +| `workstream profile update` | `PATCH /api/v1/actors/me` | | `workstream project access PROJECT_ID` | `GET /api/v1/actors/me/authorization-context?project_id=PROJECT_ID` | Workstream verifies the caller's Flow bearer and owns identity resolution, @@ -45,6 +47,8 @@ terminal control characters in API text. `--output json` (or `-o json`) writes the successful API object to stdout without a wrapper. Failures leave stdout empty and write bounded error metadata to stderr; JSON errors use an `error` object with `code`, optional HTTP `status`, and optional `correlation_id`. +For machine-readable argument errors, place `--output json` before the command; +flag parsing can stop at an invalid argument before reading later flags. Raw error bodies and transport exceptions are not printed. Server error codes and correlation headers containing the caller's bearer are suppressed, including case-only reflections. Success responses require @@ -58,6 +62,32 @@ responses are bounded to 64 KiB and requests are not automatically retried. Use `--help`, `--version` and `completion bash|zsh|fish|powershell` without a credential or network connection. +## Edit your profile + +```sh +workstream profile update --display-name 'Ada' --contact-email 'ada@example.test' +workstream profile update --clear-contact-email --output json +``` + +Only human caller-owned `display_name` and `contact_email` are writable. +An omitted flag leaves its field unchanged; a clear flag sends explicit JSON +null. You can also use `--clear-display-name`. Select at least one field; setting +and clearing the same field is invalid. Text must be valid UTF-8 and the JSON +request is capped at 8 KiB. Workstream validates and normalizes the text: +display name has a 200-character limit, contact text 320, and blank or NUL text +is rejected. Contact text does not change your Flow login or identity. +Service-actor editing and authority/lifecycle changes are not CLI operations. + +A successful update prints the validated API profile, using the same text/JSON +output as `whoami`. No preflight read or automatic retry is performed, and no +idempotency/version mechanism is invented. If the server might have received +the update but no trustworthy result arrives (including lost connection, +malformed success, redirect or server error), exit status is nonzero and JSON +includes `error.outcome_unknown: true`; text explains the uncertainty. Do not +assume rollback or blindly retry: use `workstream whoami` to inspect the current +profile. That observation cannot establish global order against concurrent +later edits. Complete API denials and validation failures remain known errors. + ## Verification Behavior tests invoke the built executable from outside the repository, with @@ -65,6 +95,10 @@ no import of Go internals. One suite uses a controlled HTTP server to exercise credential/destination safety, output and failure boundaries. The other uses the current FastAPI app with isolated real PostgreSQL to prove first admission, profile fields, authorized exact-project context and foreign-project denial. +It also proves persisted profile edits, normalization, omission/null semantics, +field limits, caller isolation and suspended denial. The HTTP fixture proves +the exact PATCH body, invalid local input, redirect refusal and no-retry behavior +when a response is lost after body receipt. Local Flow-compatible tokens are test fixtures, not deployed-provider proof. No coverage percentage or test-count target is used. diff --git a/cli/internal/api/client.go b/cli/internal/api/client.go index a92300db3..56c57b705 100644 --- a/cli/internal/api/client.go +++ b/cli/internal/api/client.go @@ -1,6 +1,7 @@ package api import ( + "bytes" "context" "encoding/hex" "encoding/json" @@ -21,6 +22,7 @@ import ( ) const maxResponseBytes = 64 * 1024 +const maxUpdateBytes = 8 * 1024 var bearerValue = regexp.MustCompile(`^[A-Za-z0-9\-._~+/]+=*$`) var safeCode = regexp.MustCompile(`^[A-Za-z][A-Za-z0-9_]{0,99}$`) @@ -29,12 +31,18 @@ var safeCorrelation = regexp.MustCompile(`^[0-9a-fA-F-]{36}$`) // Failure contains only bounded, public error metadata. It never includes a // credential, URL, response body, or transport exception. type Failure struct { - Code string `json:"code"` - Status int `json:"status,omitempty"` - CorrelationID string `json:"correlation_id,omitempty"` + Code string `json:"code"` + Status int `json:"status,omitempty"` + CorrelationID string `json:"correlation_id,omitempty"` + OutcomeUnknown bool `json:"outcome_unknown,omitempty"` } func (f *Failure) Error() string { + if f.OutcomeUnknown { + known := *f + known.OutcomeUnknown = false + return known.Error() + "; update outcome unknown; read whoami before retrying" + } if f.CorrelationID != "" { return fmt.Sprintf("%s (HTTP %d; correlation %s)", f.Code, f.Status, f.CorrelationID) } @@ -78,6 +86,15 @@ type Result[T any] struct { Value T } +// ProfileUpdate distinguishes absent fields from explicit clearing. Validation +// of text and authority remains with the public Workstream API. +type ProfileUpdate struct { + DisplayName *string + ContactEmail *string + ClearDisplayName bool + ClearContactEmail bool +} + func New(origin, token string) (*Client, error) { normalized, err := validateOrigin(origin) if err != nil { @@ -134,12 +151,17 @@ func validateOrigin(raw string) (string, error) { func (c *Client) Profile(ctx context.Context) (Result[Profile], error) { var result Result[Profile] - raw, err := c.get(ctx, "/api/v1/actors/me", "") + raw, err := c.request(ctx, http.MethodGet, "/api/v1/actors/me", "", nil) if err != nil { return result, err } + return decodeProfile(raw) +} + +func decodeProfile(raw json.RawMessage) (Result[Profile], error) { + var result Result[Profile] value := Profile{Domains: []string{"contributor"}, AdminRoles: []string{}, ProjectRoleGrants: []string{}} - err = decode(raw, &value, []string{ + err := decode(raw, &value, []string{ "actor_profile_id", "actor_kind", "status", "display_name", "contact_email", "created_at", "updated_at", "last_seen_at", }, []string{"domains", "admin_roles", "project_role_grants"}) @@ -154,13 +176,52 @@ func (c *Client) Profile(ctx context.Context) (Result[Profile], error) { return Result[Profile]{Raw: raw, Value: value}, nil } +func (c *Client) UpdateProfile(ctx context.Context, update ProfileUpdate) (Result[Profile], error) { + var result Result[Profile] + if (update.DisplayName != nil && !utf8.ValidString(*update.DisplayName)) || + (update.ContactEmail != nil && !utf8.ValidString(*update.ContactEmail)) { + return result, errors.New("profile fields must be valid UTF-8") + } + if (update.DisplayName != nil && update.ClearDisplayName) || + (update.ContactEmail != nil && update.ClearContactEmail) { + return result, errors.New("cannot set and clear the same profile field") + } + fields := make(map[string]*string) + if update.DisplayName != nil || update.ClearDisplayName { + fields["display_name"] = update.DisplayName + } + if update.ContactEmail != nil || update.ClearContactEmail { + fields["contact_email"] = update.ContactEmail + } + if len(fields) == 0 { + return result, errors.New("select at least one profile field") + } + body, err := json.Marshal(fields) + if err != nil || len(body) > maxUpdateBytes { + return result, errors.New("profile update exceeds the request size limit") + } + raw, err := c.request(ctx, http.MethodPatch, "/api/v1/actors/me", "", body) + if err == nil { + result, err = decodeProfile(raw) + } + if err != nil { + var failure *Failure + if errors.As(err, &failure) { + // Complete 4xx replies are known denials. Every other failed + // write response conservatively leaves commit outcome unknown. + failure.OutcomeUnknown = failure.Status < 400 || failure.Status >= 500 || failure.Code == "invalid_api_response" + } + } + return result, err +} + func (c *Client) AuthorizationContext(ctx context.Context, projectID string) (Result[AuthorizationContext], error) { var result Result[AuthorizationContext] if projectID == "" || !utf8.ValidString(projectID) || utf8.RuneCountInString(projectID) > 100 || strings.ContainsRune(projectID, '\x00') { return result, errors.New("PROJECT_ID must be a nonempty project selector of at most 100 characters") } query := url.Values{"project_id": {projectID}}.Encode() - raw, err := c.get(ctx, "/api/v1/actors/me/authorization-context", query) + raw, err := c.request(ctx, http.MethodGet, "/api/v1/actors/me/authorization-context", query, nil) if err != nil { return result, err } @@ -238,25 +299,32 @@ func (c *Client) safeMetadata(value string) bool { return !strings.Contains(strings.ToLower(value), strings.ToLower(c.token)) } -func (c *Client) get(ctx context.Context, path, query string) (json.RawMessage, error) { +func (c *Client) request(ctx context.Context, method, path, query string, body []byte) (json.RawMessage, error) { target := c.origin + path if query != "" { target += "?" + query } - req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil) + var reader io.Reader + if body != nil { + reader = bytes.NewReader(body) + } + req, err := http.NewRequestWithContext(ctx, method, target, reader) if err != nil { return nil, &Failure{Code: "invalid_request"} } req.Header.Set("Authorization", "Bearer "+c.token) req.Header.Set("Accept", "application/json") req.Header.Set("Accept-Encoding", "identity") + if body != nil { + req.Header.Set("Content-Type", "application/json") + } response, err := c.http.Do(req) if err != nil { return nil, &Failure{Code: "service_unavailable"} } defer response.Body.Close() - body, err := io.ReadAll(io.LimitReader(response.Body, maxResponseBytes+1)) - if err != nil || len(body) > maxResponseBytes { + responseBody, err := io.ReadAll(io.LimitReader(response.Body, maxResponseBytes+1)) + if err != nil || len(responseBody) > maxResponseBytes { return nil, &Failure{Code: "invalid_api_response", Status: response.StatusCode} } correlation := response.Header.Get("X-Correlation-ID") @@ -276,15 +344,15 @@ func (c *Client) get(ctx context.Context, path, query string) (json.RawMessage, Code string `json:"code"` } `json:"error"` } - if json.Unmarshal(body, &envelope) == nil && safeCode.MatchString(envelope.Error.Code) && c.safeMetadata(envelope.Error.Code) { + if json.Unmarshal(responseBody, &envelope) == nil && safeCode.MatchString(envelope.Error.Code) && c.safeMetadata(envelope.Error.Code) { code = envelope.Error.Code } } return nil, &Failure{Code: code, Status: response.StatusCode, CorrelationID: correlation} } mediaType, _, err := mime.ParseMediaType(response.Header.Get("Content-Type")) - if err != nil || mediaType != "application/json" || !jsontext.Value(body).IsValid() { + if err != nil || mediaType != "application/json" || !jsontext.Value(responseBody).IsValid() { return nil, &Failure{Code: "invalid_api_response", Status: response.StatusCode, CorrelationID: correlation} } - return json.RawMessage(body), nil + return json.RawMessage(responseBody), nil } diff --git a/cli/internal/command/command.go b/cli/internal/command/command.go index 6090febd3..e6961a28d 100644 --- a/cli/internal/command/command.go +++ b/cli/internal/command/command.go @@ -66,18 +66,47 @@ func Run(args []string, stdout, stderr io.Writer, getenv environment, version st if err != nil { return err } - if output == "json" { - return writeJSON(stdout, result.Raw) - } - p := result.Value - _, err = fmt.Fprintf(stdout, - "Actor: %s\nStatus: %s\nName: %s\nEmail: %s\nAdmin roles: %s\nProject grants: %s\n", - safeText(p.ActorProfileID), safeText(p.Status), optional(p.DisplayName), optional(p.ContactEmail), - list(p.AdminRoles), list(p.ProjectRoleGrants)) - return err + return writeProfile(stdout, output, result) }, }) + profile := &cobra.Command{Use: "profile", Short: "Caller-owned profile operations"} + var displayName, contactEmail string + var clearDisplayName, clearContactEmail bool + update := &cobra.Command{ + Use: "update", + Short: "Set or clear your human profile display fields", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, _ []string) error { + fields := api.ProfileUpdate{ClearDisplayName: clearDisplayName, ClearContactEmail: clearContactEmail} + if cmd.Flags().Changed("display-name") { + fields.DisplayName = &displayName + } + if cmd.Flags().Changed("contact-email") { + fields.ContactEmail = &contactEmail + } + if (fields.DisplayName != nil && clearDisplayName) || (fields.ContactEmail != nil && clearContactEmail) || + (fields.DisplayName == nil && fields.ContactEmail == nil && !clearDisplayName && !clearContactEmail) { + return commandError{"invalid_arguments", "select profile fields; do not set and clear the same field"} + } + apiClient, err := client() + if err != nil { + return err + } + result, err := apiClient.UpdateProfile(cmd.Context(), fields) + if err != nil { + return err + } + return writeProfile(stdout, output, result) + }, + } + update.Flags().StringVar(&displayName, "display-name", "", "Set your display name") + update.Flags().StringVar(&contactEmail, "contact-email", "", "Set your profile contact text (not your login identity)") + update.Flags().BoolVar(&clearDisplayName, "clear-display-name", false, "Clear your display name") + update.Flags().BoolVar(&clearContactEmail, "clear-contact-email", false, "Clear your profile contact text") + profile.AddCommand(update) + root.AddCommand(profile) + project := &cobra.Command{Use: "project", Short: "Project-scoped public operations"} project.AddCommand(&cobra.Command{ Use: "access PROJECT_ID", @@ -139,6 +168,18 @@ func writeJSON(w io.Writer, raw json.RawMessage) error { return err } +func writeProfile(w io.Writer, output string, result api.Result[api.Profile]) error { + if output == "json" { + return writeJSON(w, result.Raw) + } + p := result.Value + _, err := fmt.Fprintf(w, + "Actor: %s\nStatus: %s\nName: %s\nEmail: %s\nAdmin roles: %s\nProject grants: %s\n", + safeText(p.ActorProfileID), safeText(p.Status), optional(p.DisplayName), optional(p.ContactEmail), + list(p.AdminRoles), list(p.ProjectRoleGrants)) + return err +} + func writeFailure(w io.Writer, output string, f api.Failure) { if output == "json" { _ = json.NewEncoder(w).Encode(struct { diff --git a/cli/tests/integration/test_http_boundary.py b/cli/tests/integration/test_http_boundary.py index 0fa7ff589..b3878846d 100644 --- a/cli/tests/integration/test_http_boundary.py +++ b/cli/tests/integration/test_http_boundary.py @@ -43,6 +43,8 @@ def http_fixture(): "body": json.dumps(PROFILE).encode(), "headers": {"Content-Type": "application/json"}, "delay": 0, + "drop": False, + "updates": [], } class Handler(BaseHTTPRequestHandler): @@ -50,6 +52,9 @@ def do_GET(self): # noqa: N802 - standard HTTP handler interface requests.append( (self.command, self.path, self.headers.get("Authorization")) ) + if response["drop"]: + self.close_connection = True + return time.sleep(response["delay"]) self.send_response(response["status"]) for key, value in response["headers"].items(): @@ -60,6 +65,13 @@ def do_GET(self): # noqa: N802 - standard HTTP handler interface except (BrokenPipeError, ConnectionResetError): pass + def do_PATCH(self): # noqa: N802 - standard HTTP handler interface + body = self.rfile.read(int(self.headers.get("Content-Length", "0"))) + response["updates"].append( + (self.headers.get("Content-Type"), json.loads(body)) + ) + self.do_GET() + def do_CONNECT(self): # noqa: N802 - captures attempted HTTPS proxy use requests.append( (self.command, self.path, self.headers.get("Authorization")) @@ -86,6 +98,139 @@ def assert_failure(result, code, exit_code=1): assert json.loads(result.stderr)["error"]["code"] == code +def test_profile_update_sends_only_selected_fields(cli): + with http_fixture() as (origin, response, requests): + for flags, expected in ( + (("--display-name", "Ada é"), {"display_name": "Ada é"}), + ( + ("--contact-email", "contact@example.test"), + {"contact_email": "contact@example.test"}, + ), + (("--clear-display-name",), {"display_name": None}), + (("--clear-contact-email",), {"contact_email": None}), + ( + ("--display-name", "Ada", "--clear-contact-email"), + {"display_name": "Ada", "contact_email": None}, + ), + ( + ("--clear-display-name", "--clear-contact-email"), + {"display_name": None, "contact_email": None}, + ), + ): + result = cli(origin, TOKEN, "profile", "update", *flags, "-o", "json") + assert result.returncode == 0 and result.stderr == "" + assert json.loads(result.stdout) == PROFILE + assert response["updates"][-1] == ("application/json", expected) + assert requests == [("PATCH", "/api/v1/actors/me", "Bearer " + TOKEN)] * 6 + response["body"] = json.dumps({**PROFILE, "display_name": "Ada\x1b"}).encode() + text = cli(origin, TOKEN, "profile", "update", "--display-name", "Ada") + assert text.returncode == 0 and "Name: Ada\\u001B\n" in text.stdout + + +def test_profile_update_bad_arguments_never_write(cli): + with http_fixture() as (origin, response, requests): + for flags in ( + (), + ("--clear-display-name=false",), + ("--display-name", "Ada", "--clear-display-name"), + ("--contact-email", "x", "--clear-contact-email"), + ("--actor-profile-id", ACTOR), + ("--admin-roles", "access_administrator"), + ("unexpected",), + ("--display-name", "x" * 9000), + ("--display-name", b"\xff"), + ): + result = cli(origin, TOKEN, "-o", "json", "profile", "update", *flags) + assert_failure(result, "invalid_arguments", exit_code=2) + assert requests == [] and response["updates"] == [] + + +def test_profile_update_uncertainty_and_known_denials(cli): + with http_fixture() as (origin, response, requests): + # Record body receipt, then lose the connection before status headers. + response["drop"] = True + result = cli( + origin, TOKEN, "profile", "update", "--display-name", "Ada", "-o", "json" + ) + assert_failure(result, "service_unavailable") + assert json.loads(result.stderr)["error"]["outcome_unknown"] is True + assert len(requests) == len(response["updates"]) == 1 + response["drop"] = False + for status, body, headers, code, unknown in ( + ( + 403, + b'{"error":{"code":"actor_suspended"}}', + {}, + "actor_suspended", + False, + ), + ( + 422, + b'{"error":{"code":"validation_error"}}', + {}, + "validation_error", + False, + ), + ( + 503, + b'{"error":{"code":"service_unavailable"}}', + {}, + "service_unavailable", + True, + ), + (200, b'{"actor_profile_id":"bad"}', {}, "invalid_api_response", True), + ( + 200, + json.dumps(PROFILE).encode(), + {"Content-Length": "99999"}, + "invalid_api_response", + True, + ), + (200, b"x" * 65537, {}, "invalid_api_response", True), + ): + response.update( + status=status, + body=body, + headers={"Content-Type": "application/json", **headers}, + ) + before = len(requests) + result = cli( + origin, + TOKEN, + "profile", + "update", + "--clear-contact-email", + "-o", + "json", + ) + assert_failure(result, code) + assert ( + json.loads(result.stderr)["error"].get("outcome_unknown", False) + is unknown + ) + assert len(requests) == before + 1 + response.update( + status=503, body=b"{}", headers={"Content-Type": "application/json"} + ) + text = cli(origin, TOKEN, "profile", "update", "--clear-display-name") + assert text.returncode == 1 and text.stdout == "" + assert "update outcome unknown; read whoami before retrying" in text.stderr + + +def test_profile_update_redirect_does_not_forward_body_or_bearer(cli): + with ( + http_fixture() as (sink, _, sink_requests), + http_fixture() as (origin, response, requests), + ): + response.update(status=307, body=b"", headers={"Location": sink}) + result = cli( + origin, TOKEN, "profile", "update", "--display-name", "Ada", "-o", "json" + ) + assert_failure(result, "redirect_refused") + assert json.loads(result.stderr)["error"]["outcome_unknown"] is True + assert len(requests) == 1 and sink_requests == [] + + def test_public_commands_and_noninteractive_output(cli): with http_fixture() as (origin, response, requests): raw = response["body"].decode() diff --git a/cli/tests/integration/test_public_self_service.py b/cli/tests/integration/test_public_self_service.py index 466e297d5..43f6d2994 100644 --- a/cli/tests/integration/test_public_self_service.py +++ b/cli/tests/integration/test_public_self_service.py @@ -1,4 +1,4 @@ -"""Installed CLI parity with two public Workstream self-service operations.""" +"""Installed CLI parity with public Workstream self-service operations.""" from __future__ import annotations @@ -108,6 +108,7 @@ async def test_installed_cli_uses_only_public_profile_and_project_context( assert "get" in specification.json()["paths"][path], ( "CLI route must remain public" ) + assert "patch" in specification.json()["paths"]["/api/v1/actors/me"] profiles: dict[str, dict] = {} for name, token in tokens.items(): result = cli(origin, token, "whoami", "--output", "json") @@ -131,6 +132,80 @@ async def test_installed_cli_uses_only_public_profile_and_project_context( len({profile["actor_profile_id"] for profile in profiles.values()}) == 3 ) + manager_headers = {"Authorization": f"Bearer {tokens['cli-manager']}"} + # No project/admin grant is required for these caller-owned fields. + for flags, name, email in ( + ( + ( + "--display-name", + " Ada é ", + "--contact-email", + " contact@example.test ", + ), + "Ada é", + "contact@example.test", + ), + (("--display-name", "N" * 200), "N" * 200, "contact@example.test"), + (("--contact-email", "E" * 320), "N" * 200, "E" * 320), + (("--clear-display-name",), None, "E" * 320), + (("--clear-contact-email",), None, None), + ): + edited = cli( + origin, + tokens["cli-manager"], + "profile", + "update", + *flags, + "-o", + "json", + ) + assert edited.returncode == 0, edited.stderr + saved = await direct.get("/api/v1/actors/me", headers=manager_headers) + assert saved.status_code == 200 + actual = json.loads(edited.stdout) + assert ( + actual["actor_profile_id"] + == profiles["cli-manager"]["actor_profile_id"] + ) + assert actual["display_name"] == saved.json()["display_name"] == name + assert actual["contact_email"] == saved.json()["contact_email"] == email + assert {k: v for k, v in actual.items() if k not in touched} == { + k: v for k, v in saved.json().items() if k not in touched + } + assert ( + actual["admin_roles"] == [] and actual["project_role_grants"] == [] + ) + for flags in ( + ("--display-name", ""), + ("--display-name", " "), + ("--display-name", "N" * 201), + ("--contact-email", "E" * 321), + ): + rejected = cli( + origin, + tokens["cli-manager"], + "profile", + "update", + *flags, + "-o", + "json", + ) + assert rejected.returncode == 1 and rejected.stdout == "" + assert json.loads(rejected.stderr)["error"]["status"] == 422 + assert "outcome_unknown" not in json.loads(rejected.stderr)["error"] + saved = await direct.get("/api/v1/actors/me", headers=manager_headers) + assert ( + saved.json()["display_name"] is None + and saved.json()["contact_email"] is None + ) + outsider_profile = await direct.get( + "/api/v1/actors/me", + headers={"Authorization": f"Bearer {tokens['cli-outsider']}"}, + ) + assert { + k: v for k, v in outsider_profile.json().items() if k not in touched + } == {k: v for k, v in profiles["cli-outsider"].items() if k not in touched} + _bootstrap(profiles["cli-admin"]["actor_profile_id"], env) grant = await direct.post( "/api/v1/admin-role-grants", @@ -242,6 +317,33 @@ async def test_installed_cli_uses_only_public_profile_and_project_context( ) assert after_revocation.returncode == 1 and after_revocation.stdout == "" assert json.loads(after_revocation.stderr)["error"]["status"] == 404 + suspended = await direct.post( + f"/api/v1/actors/{profiles['cli-manager']['actor_profile_id']}/suspend", + headers={ + "Authorization": f"Bearer {tokens['cli-admin']}", + "Idempotency-Key": str(uuid4()), + }, + json={"reason": "CLI self-update lifecycle denial proof"}, + ) + assert suspended.status_code == 200, suspended.text + denied = cli( + origin, + tokens["cli-manager"], + "profile", + "update", + "--display-name", + "must not persist", + "-o", + "json", + ) + assert denied.returncode == 1 and denied.stdout == "" + assert json.loads(denied.stderr)["error"]["code"] == "actor_suspended" + assert "outcome_unknown" not in json.loads(denied.stderr)["error"] + stored = await direct.get( + f"/api/v1/actors/{profiles['cli-manager']['actor_profile_id']}", + headers={"Authorization": f"Bearer {tokens['cli-admin']}"}, + ) + assert stored.status_code == 200 and stored.json()["display_name"] is None finally: api.terminate() try: diff --git a/docs/roadmap_status.md b/docs/roadmap_status.md index 41e157dd4..f632c705c 100644 --- a/docs/roadmap_status.md +++ b/docs/roadmap_status.md @@ -129,7 +129,8 @@ compensation effects, operations and release proof complete v0.1. The [independent MCP package](../mcp_server/README.md) implements three self-service tools. The [Go CLI](../cli/README.md) provides caller-profile and -exact-project authorization reads with human/JSON output. These source packages +exact-project authorization reads plus human self-profile editing, with text/JSON +output. CLI write uncertainty is explicit and never automatically retried. These packages do not claim hosted deployment, published CLI binaries or the complete proposed workflow catalogue. @@ -276,7 +277,10 @@ cannot be reused as post-submission review-gate evidence. See the authentication adapter, not a public deployment or a 27-tool release. The [CLI foundation](../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-01.md) provides `whoami` and `project access PROJECT_ID` through those public REST - contracts. Built-binary HTTP integration and isolated real-API proof accompany + contracts. [CLI profile editing](../.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md) + adds caller-owned display-field set/clear through public PATCH, with explicit + uncertain-outcome reporting and no automatic retry. Built-binary HTTP + integration and isolated real-API proof accompany the package. Further public commands, optional TUI and binary distribution remain planned; CLI reads do not complete unfinished product lifecycle work. The public-client drill targets currently usable APIs only; hidden and From 334c8e8113d5d41ee41ee68bf1d58b5dfd5ac419 Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Sun, 4 Oct 2026 16:18:20 +0100 Subject: [PATCH 3/5] fix(cli): classify write uncertainty by failure provenance --- .../initiatives/WS-CLI-001/WS-CLI-001-02.md | 5 +++++ cli/internal/api/client.go | 21 +++++++++++-------- cli/tests/integration/test_http_boundary.py | 7 +++++++ 3 files changed, 24 insertions(+), 9 deletions(-) diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md index 0e73f409f..de54ed569 100644 --- a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md @@ -108,6 +108,11 @@ belongs to the PR rather than this durable record. - `PLAN-SEC-001`: Narrowed human/agent wording to human self-profile editing usable by noninteractive clients; no service-actor support is introduced. +- `CLI-SEC-003`: Removed server error-code text from write-certainty decisions. + Transport/read/success-decoding failures mark uncertainty at their actual + failure sites; complete 4xx replies remain known even when their public code + equals `invalid_api_response`. The process regression distinguishes this + defect from the repaired implementation. ## Reconciliation diff --git a/cli/internal/api/client.go b/cli/internal/api/client.go index 56c57b705..912ed64f4 100644 --- a/cli/internal/api/client.go +++ b/cli/internal/api/client.go @@ -201,15 +201,15 @@ func (c *Client) UpdateProfile(ctx context.Context, update ProfileUpdate) (Resul return result, errors.New("profile update exceeds the request size limit") } raw, err := c.request(ctx, http.MethodPatch, "/api/v1/actors/me", "", body) - if err == nil { - result, err = decodeProfile(raw) + if err != nil { + return result, err } + result, err = decodeProfile(raw) if err != nil { var failure *Failure if errors.As(err, &failure) { - // Complete 4xx replies are known denials. Every other failed - // write response conservatively leaves commit outcome unknown. - failure.OutcomeUnknown = failure.Status < 400 || failure.Status >= 500 || failure.Code == "invalid_api_response" + // A malformed successful reply cannot establish the write outcome. + failure.OutcomeUnknown = true } } return result, err @@ -320,12 +320,12 @@ func (c *Client) request(ctx context.Context, method, path, query string, body [ } response, err := c.http.Do(req) if err != nil { - return nil, &Failure{Code: "service_unavailable"} + return nil, &Failure{Code: "service_unavailable", OutcomeUnknown: method == http.MethodPatch} } defer response.Body.Close() responseBody, err := io.ReadAll(io.LimitReader(response.Body, maxResponseBytes+1)) if err != nil || len(responseBody) > maxResponseBytes { - return nil, &Failure{Code: "invalid_api_response", Status: response.StatusCode} + return nil, &Failure{Code: "invalid_api_response", Status: response.StatusCode, OutcomeUnknown: method == http.MethodPatch} } correlation := response.Header.Get("X-Correlation-ID") if !safeCorrelation.MatchString(correlation) || !c.safeMetadata(correlation) { @@ -348,11 +348,14 @@ func (c *Client) request(ctx context.Context, method, path, query string, body [ code = envelope.Error.Code } } - return nil, &Failure{Code: code, Status: response.StatusCode, CorrelationID: correlation} + // A fully received 4xx response is a known denial regardless of its + // server-controlled error code. Incomplete replies were handled above. + return nil, &Failure{Code: code, Status: response.StatusCode, CorrelationID: correlation, + OutcomeUnknown: method == http.MethodPatch && (response.StatusCode < 400 || response.StatusCode >= 500)} } mediaType, _, err := mime.ParseMediaType(response.Header.Get("Content-Type")) if err != nil || mediaType != "application/json" || !jsontext.Value(responseBody).IsValid() { - return nil, &Failure{Code: "invalid_api_response", Status: response.StatusCode, CorrelationID: correlation} + return nil, &Failure{Code: "invalid_api_response", Status: response.StatusCode, CorrelationID: correlation, OutcomeUnknown: method == http.MethodPatch} } return json.RawMessage(responseBody), nil } diff --git a/cli/tests/integration/test_http_boundary.py b/cli/tests/integration/test_http_boundary.py index b3878846d..4ff9e561c 100644 --- a/cli/tests/integration/test_http_boundary.py +++ b/cli/tests/integration/test_http_boundary.py @@ -178,6 +178,13 @@ def test_profile_update_uncertainty_and_known_denials(cli): "service_unavailable", True, ), + ( + 422, + b'{"error":{"code":"invalid_api_response"}}', + {}, + "invalid_api_response", + False, + ), (200, b'{"actor_profile_id":"bad"}', {}, "invalid_api_response", True), ( 200, From 58040db1f77d4f9d33ee1db9a4af47fbd05f91ee Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Sun, 4 Oct 2026 16:36:05 +0100 Subject: [PATCH 4/5] fix(cli): retain uncertainty for unrecognized gateway denials --- .commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md | 13 ++++++++++--- cli/README.md | 5 ++++- cli/internal/api/client.go | 10 ++++++---- cli/tests/integration/test_http_boundary.py | 9 +++++++++ 4 files changed, 29 insertions(+), 8 deletions(-) diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md index de54ed569..fb296c2e0 100644 --- a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md @@ -59,8 +59,10 @@ performed. No second authentication or actor lookup is introduced. A PATCH transport failure, truncated/unreadable response, malformed success, redirect, or server failure cannot prove whether the write committed. Such failures retain a nonzero exit and report `outcome_unknown: true` in JSON (a -safe explanation in text). A complete 4xx response is a known denial/validation -failure, not uncertain commit. Do not retry; use `whoami` to inspect current +safe explanation in text). A complete 4xx with a parseable API error envelope +(nonempty string `error.code`) is a known denial/validation failure, independently +of that code's spelling or redaction. Gateway 4xx replies without this envelope +remain uncertain. Do not retry; use `whoami` to inspect current state, acknowledging that concurrent later writes remain possible. Successful local output is not a global write-order guarantee. GET failures are unchanged. @@ -110,9 +112,14 @@ belongs to the PR rather than this durable record. usable by noninteractive clients; no service-actor support is introduced. - `CLI-SEC-003`: Removed server error-code text from write-certainty decisions. Transport/read/success-decoding failures mark uncertainty at their actual - failure sites; complete 4xx replies remain known even when their public code + failure sites; complete API 4xx replies remain known even when their public code equals `invalid_api_response`. The process regression distinguishes this defect from the repaired implementation. +- `CLI-EXT-004`: A plain gateway 4xx did not establish an API denial but was + marked known. The existing error decoder now distinguishes a received API + error envelope from an unrecognized reply without coupling certainty to + metadata safety or code spelling. Process proof covers gateway 408/429, + retained code-collision denial and credential-reflection suppression. ## Reconciliation diff --git a/cli/README.md b/cli/README.md index 721b7b8f8..b6611b2e6 100644 --- a/cli/README.md +++ b/cli/README.md @@ -86,7 +86,10 @@ malformed success, redirect or server error), exit status is nonzero and JSON includes `error.outcome_unknown: true`; text explains the uncertainty. Do not assume rollback or blindly retry: use `workstream whoami` to inspect the current profile. That observation cannot establish global order against concurrent -later edits. Complete API denials and validation failures remain known errors. +later edits. Complete 4xx replies with a parseable Workstream error envelope +(a nonempty string `error.code`) remain known denials or validation failures, +even when sensitive metadata is suppressed. A gateway 4xx without that envelope +is uncertain too; HTTP status alone does not establish a Workstream denial. ## Verification diff --git a/cli/internal/api/client.go b/cli/internal/api/client.go index 912ed64f4..7f3d93cf3 100644 --- a/cli/internal/api/client.go +++ b/cli/internal/api/client.go @@ -336,6 +336,7 @@ func (c *Client) request(ctx context.Context, method, path, query string, body [ } if response.StatusCode != http.StatusOK { code := "api_error" + knownEnvelope := false if response.StatusCode >= 300 && response.StatusCode < 400 { code = "redirect_refused" } else { @@ -344,14 +345,15 @@ func (c *Client) request(ctx context.Context, method, path, query string, body [ Code string `json:"code"` } `json:"error"` } - if json.Unmarshal(responseBody, &envelope) == nil && safeCode.MatchString(envelope.Error.Code) && c.safeMetadata(envelope.Error.Code) { + knownEnvelope = json.Unmarshal(responseBody, &envelope) == nil && envelope.Error.Code != "" + if knownEnvelope && safeCode.MatchString(envelope.Error.Code) && c.safeMetadata(envelope.Error.Code) { code = envelope.Error.Code } } - // A fully received 4xx response is a known denial regardless of its - // server-controlled error code. Incomplete replies were handled above. + // A complete API 4xx envelope is a known denial, independent of code + // spelling/redaction. A gateway reply without it cannot prove rollback. return nil, &Failure{Code: code, Status: response.StatusCode, CorrelationID: correlation, - OutcomeUnknown: method == http.MethodPatch && (response.StatusCode < 400 || response.StatusCode >= 500)} + OutcomeUnknown: method == http.MethodPatch && (response.StatusCode < 400 || response.StatusCode >= 500 || !knownEnvelope)} } mediaType, _, err := mime.ParseMediaType(response.Header.Get("Content-Type")) if err != nil || mediaType != "application/json" || !jsontext.Value(responseBody).IsValid() { diff --git a/cli/tests/integration/test_http_boundary.py b/cli/tests/integration/test_http_boundary.py index 4ff9e561c..31b645f7e 100644 --- a/cli/tests/integration/test_http_boundary.py +++ b/cli/tests/integration/test_http_boundary.py @@ -185,6 +185,15 @@ def test_profile_update_uncertainty_and_known_denials(cli): "invalid_api_response", False, ), + (408, b"gateway timeout", {}, "api_error", True), + (429, b"{}", {}, "api_error", True), + ( + 422, + json.dumps({"error": {"code": TOKEN}}).encode(), + {}, + "api_error", + False, + ), (200, b'{"actor_profile_id":"bad"}', {}, "invalid_api_response", True), ( 200, From 276b9e302b4493bb3a12515f4813133444fbf09b Mon Sep 17 00:00:00 2001 From: Commitrail Probe Date: Sun, 4 Oct 2026 16:49:06 +0100 Subject: [PATCH 5/5] fix(cli): reuse strict error envelope decoding --- .../initiatives/WS-CLI-001/WS-CLI-001-02.md | 7 +++- cli/internal/api/client.go | 2 +- cli/tests/integration/test_http_boundary.py | 37 ++++++++++++++++++- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md index fb296c2e0..680c6346f 100644 --- a/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md +++ b/.commitrail/initiatives/WS-CLI-001/WS-CLI-001-02.md @@ -120,11 +120,16 @@ belongs to the PR rather than this durable record. error envelope from an unrecognized reply without coupling certainty to metadata safety or code spelling. Process proof covers gateway 408/429, retained code-collision denial and credential-reflection suppression. +- `QA-CLI-004`: Reused the existing strict JSON decoder for error envelopes, + removing case-folded and duplicate-member recognition. Independent outer/ + inner-case and duplicate-key process cases preserve uncertainty without a + new parsing abstraction. The pre-fix binary fails the case-folding regression. ## Reconciliation -- Current-source reconciliation: main `89dd8c91` includes the two-read CLI and +- Current-source reconciliation: main `bcd0bd49` includes the two-read CLI and the existing public self-profile PATCH; no new product endpoint is needed. + The shared client ledger retains the merged nine-tool MCP delivery. - Next usable boundary: later role-specific public journeys, not hidden routes. - Remaining risks: no server optimistic-update/replay contract is claimed; production Flow and binary distribution remain separate release proof. diff --git a/cli/internal/api/client.go b/cli/internal/api/client.go index 7f3d93cf3..a670a0dcc 100644 --- a/cli/internal/api/client.go +++ b/cli/internal/api/client.go @@ -345,7 +345,7 @@ func (c *Client) request(ctx context.Context, method, path, query string, body [ Code string `json:"code"` } `json:"error"` } - knownEnvelope = json.Unmarshal(responseBody, &envelope) == nil && envelope.Error.Code != "" + knownEnvelope = jsonv2.Unmarshal(responseBody, &envelope) == nil && envelope.Error.Code != "" if knownEnvelope && safeCode.MatchString(envelope.Error.Code) && c.safeMetadata(envelope.Error.Code) { code = envelope.Error.Code } diff --git a/cli/tests/integration/test_http_boundary.py b/cli/tests/integration/test_http_boundary.py index 31b645f7e..94fa3865e 100644 --- a/cli/tests/integration/test_http_boundary.py +++ b/cli/tests/integration/test_http_boundary.py @@ -187,6 +187,41 @@ def test_profile_update_uncertainty_and_known_denials(cli): ), (408, b"gateway timeout", {}, "api_error", True), (429, b"{}", {}, "api_error", True), + ( + 408, + b'{"Error":{"Code":"gateway_timeout"}}', + {}, + "api_error", + True, + ), + ( + 408, + b'{"error":{"Code":"gateway_timeout"}}', + {}, + "api_error", + True, + ), + ( + 408, + b'{"Error":{"code":"gateway_timeout"}}', + {}, + "api_error", + True, + ), + ( + 408, + b'{"error":{"code":"a"},"error":{"code":"b"}}', + {}, + "api_error", + True, + ), + ( + 408, + b'{"error":{"code":"a","code":"b"}}', + {}, + "api_error", + True, + ), ( 422, json.dumps({"error": {"code": TOKEN}}).encode(), @@ -219,11 +254,11 @@ def test_profile_update_uncertainty_and_known_denials(cli): "-o", "json", ) - assert_failure(result, code) assert ( json.loads(result.stderr)["error"].get("outcome_unknown", False) is unknown ) + assert_failure(result, code) assert len(requests) == before + 1 response.update( status=503, body=b"{}", headers={"Content-Type": "application/json"}