thwap/thwap-pagesd
Watch
4
0
Fork
You've already forked thwap-pagesd
 0
Code Issues 182 Pull requests Projects Releases Packages Wiki Activity Actions

feat(tpagectl): add context support for multiple instances #629

Merged
fuzzy merged 1 commit from feat/tpagectl-context into main 2026-08-10 15:13:44 +00:00
Conversation 1 Commits 1 Files changed 17 +615 -3
automation commented 2026-08-10 15:01:32 +00:00
Owner
Copy link

What

Implements Phase 10 multi-instance support (roadmap #319), the final CLI-ROADMAP item.

  • Config (#318) — new contexts map (name → {url, token}) plus an active context key. config.ApplyContext resolves a named context into server.url/server.token (unknown context → config error, exit 2); config.SetActiveContext(name) persists the active name via the existing atomic 0600 config write.
  • --context flag + context command (#317) —
    • Root --context NAME selects a context for a single invocation (precedence: --server/--token flags > selected context > server.* defaults > config context).
    • New internal/tpagectl/context group: context list (NAME/URL/ACTIVE table, active marked *) and context use NAME (validates the name against the configured contexts, then persists it).

Why

Roadmap task #319 — completing the entire CLI-ROADMAP.md.

Testing

  • Config: contexts + active parsing; ApplyContext known/unknown + token-only merge; SetActiveContext write → Load round-trip (XDG temp)
  • Root: --context prod resolves URL/token; active context: from config applied; --server overrides context; unknown context → exit 2
  • context list table (NAME/URL/ACTIVE + *); context use persists and round-trips; unknown name and missing arg → validation errors
  • context --help wiring; root-help golden + docs regenerated
  • go test -race ./... passes (36 packages); make coverage = 84.9% (≥80% gate)
  • golangci-lint run clean; pre-commit hooks pass

Breaking Changes

None. Adds a context command group, a root --context flag, and new contexts/context config keys (additive).

Notes

No thwap-actions action applies (CLI feature). This closes out the CLI roadmap (all phases 1–10 complete).

Closes #317
Closes #318
Closes #319

## What Implements Phase 10 multi-instance support (roadmap #319), the final CLI-ROADMAP item. - **Config (#318)** — new `contexts` map (`name → {url, token}`) plus an active `context` key. `config.ApplyContext` resolves a named context into `server.url`/`server.token` (unknown context → config error, exit 2); `config.SetActiveContext(name)` persists the active name via the existing atomic 0600 config write. - **`--context` flag + `context` command (#317)** — - Root `--context NAME` selects a context for a single invocation (precedence: `--server`/`--token` flags > selected context > `server.*` defaults > config `context`). - New `internal/tpagectl/context` group: `context list` (NAME/URL/ACTIVE table, active marked `*`) and `context use NAME` (validates the name against the configured `contexts`, then persists it). ## Why Roadmap task #319 — completing the entire `CLI-ROADMAP.md`. ## Testing - [x] Config: `contexts` + active parsing; `ApplyContext` known/unknown + token-only merge; `SetActiveContext` write → Load round-trip (XDG temp) - [x] Root: `--context prod` resolves URL/token; active `context:` from config applied; `--server` overrides context; unknown context → exit 2 - [x] `context list` table (NAME/URL/ACTIVE + `*`); `context use` persists and round-trips; unknown name and missing arg → validation errors - [x] `context --help` wiring; root-help golden + docs regenerated - [x] `go test -race ./...` passes (36 packages); `make coverage` = 84.9% (≥80% gate) - [x] `golangci-lint run` clean; pre-commit hooks pass ## Breaking Changes None. Adds a `context` command group, a root `--context` flag, and new `contexts`/`context` config keys (additive). ## Notes No `thwap-actions` action applies (CLI feature). This closes out the CLI roadmap (all phases 1–10 complete). Closes #317 Closes #318 Closes #319
feat(tpagectl): add context support for multiple instances
Some checks failed
CI / docker (pull_request) Successful in 3m7s
Details
CI / build (pull_request) Successful in 6m7s
Details
CI / lint (pull_request) Successful in 5m56s
Details
CI / test (push) Successful in 5m45s
Details
CI / docker (push) Failing after 1m14s
Details
CI / test (pull_request) Successful in 19m25s
Details
CI / build (push) Successful in 5m40s
Details
CI / lint (push) Successful in 5m54s
Details
d9df415302 
Support multiple thwap-pagesd instances:

- config: new `contexts` map (name -> {url, token}) and an active
  `context` key; config.ApplyContext resolves a named context into
  server.url/token (unknown context -> error), and SetActiveContext
  persists the active name via the atomic 0600 config write.
- Root --context NAME selects a context for a single invocation
  (flag > config context > server defaults; --server/--token still win).
- New `context` command group: `context list` (NAME/URL/ACTIVE table) and
  `context use NAME` (validates the name and persists it).

Documented in the README and regenerated command docs.

Closes #317
Closes #318
Closes #319
fuzzy scheduled this pull request to auto merge when all checks succeed 2026-08-10 15:02:13 +00:00
the.auditor approved these changes 2026-08-10 15:13:38 +00:00
the.auditor left a comment
Copy link

Clean multi-instance support that closes out the CLI roadmap. The contexts/context config keys, ApplyContext merge semantics, SetActiveContext persistence via the existing atomic 0600 write, root --context flag, and context list/use group are all implemented cleanly with good tests and regenerated docs. Approving; two findings filed.

Suggestions

  1. cmd/tpagectl/main.go – ApplyContext runs after config.Load, overwriting cfg.Server.URL/Token that viper already set from TPAGECTL_SERVER_URL/TPAGECTL_SERVER_TOKEN, so an active context (a config-file value) silently beats the documented env-above-file precedence. Apply context only when the env var didn't set the field, or document the new precedence. Filed as #630.
  2. internal/tpagectl/context/context.go:84 – context use checks URL == "" rather than map membership, rejecting token-only contexts that ApplyContext explicitly supports, and an unknown context exits 5 via use vs 2 via --context — inconsistent classification. Check membership and align exit codes. Filed as #631.

Praise

  • Precedence (--server/--token flags > selected context > server.* defaults) is correctly ordered and tested.
  • context list sorts names and marks the active one with * using the shared output.RenderTable.
  • SetActiveContext reuses the atomic 0600 config writer, keeping the token-file security posture consistent.
Clean multi-instance support that closes out the CLI roadmap. The `contexts`/`context` config keys, `ApplyContext` merge semantics, `SetActiveContext` persistence via the existing atomic 0600 write, root `--context` flag, and `context list`/`use` group are all implemented cleanly with good tests and regenerated docs. Approving; two findings filed. ## Suggestions 1. `cmd/tpagectl/main.go` – `ApplyContext` runs after `config.Load`, overwriting `cfg.Server.URL/Token` that viper already set from `TPAGECTL_SERVER_URL`/`TPAGECTL_SERVER_TOKEN`, so an active context (a config-file value) silently beats the documented env-above-file precedence. Apply context only when the env var didn't set the field, or document the new precedence. Filed as #630. 2. `internal/tpagectl/context/context.go:84` – `context use` checks `URL == ""` rather than map membership, rejecting token-only contexts that `ApplyContext` explicitly supports, and an unknown context exits 5 via `use` vs 2 via `--context` — inconsistent classification. Check membership and align exit codes. Filed as #631. ## Praise - Precedence (`--server`/`--token` flags > selected context > `server.*` defaults) is correctly ordered and tested. - `context list` sorts names and marks the active one with `*` using the shared `output.RenderTable`. - `SetActiveContext` reuses the atomic 0600 config writer, keeping the token-file security posture consistent.
fuzzy merged commit d9df415302 into main 2026-08-10 15:13:44 +00:00
fuzzy deleted branch feat/tpagectl-context 2026-08-10 15:13:45 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
the.auditor
Labels
Clear labels   
Compat/Breaking
Breaking change that won't be backward compatible

  
Kind/Bug
Something is not working

  
Kind/Documentation
Documentation changes

  
Kind/Enhancement
Improve existing functionality

  
Kind/Feature
New functionality

  
Kind/Security
This is security issue

  
Kind/Testing
Issue or pull request related to testing

  
Priority
Critical
 The priority is critical

  
Priority
High
 The priority is high

  
Priority
Low
 The priority is low

  
Priority
Medium
 The priority is medium

  
Reviewed
Confirmed
 Issue has been confirmed

  
Reviewed
Duplicate
 This issue or pull request already exists

  
Reviewed
Invalid
 Invalid issue

  
Reviewed
Won't Fix
 This issue won't be fixed

  
Status
Abandoned
 Somebody has started to work on this but abandoned work

  
Status
Blocked
 Something is blocking this issue or pull request

  
Status
Need More Info
 Feedback is required to reproduce issue or to continue work

No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
thwap/thwap-pagesd!629
No description provided.