Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 22 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ List of project documents and their specific purpose:
| `docs/documentation-maintenance.md` | Doc update rules | After any code change |
| `docs/skills.md` | AI agent skill format, taxonomy, authoring guide | When adding or modifying skills |
| `docs/user-guide/getting-started.md` | Installation, first context, quick start | New users, onboarding |
| `docs/user-guide/configuration.md` | Config file, env vars, context commands | When working with config/context |
| `docs/user-guide/configuration.md` | Config file, env vars, override precedence | When working with config files / env vars |
| `docs/user-guide/context.md` | `a6 context` command reference | When working with the context command |
| `docs/user-guide/version.md` | `a6 version` output and use cases | When filing bug reports / verifying installs |
| `docs/user-guide/conditional-plugins.md` | Plugins that need APISIX-side enabling (skywalking et al) and test-coverage states | When a plugin fails with "unknown plugin" or "not enabled" |
| `.github/workflows/ci.yml` | Unit test + lint CI workflow | When modifying CI |
| `.github/workflows/e2e.yml` | E2E test CI with real APISIX | When modifying e2e infrastructure |

Expand Down Expand Up @@ -54,7 +57,7 @@ a6/
├── pkg/iostreams/ # I/O abstraction (TTY detection)
├── pkg/cmdutil/ # Shared command utilities
├── pkg/tableprinter/ # Table output rendering
├── pkg/httpmock/ # Legacy HTTP mocking helpers still under migration
├── pkg/httpmock/ # HTTP mocking helpers for unit tests
├── internal/config/ # Configuration/context management
├── internal/version/ # Build version info
├── docs/ # All documentation
Expand Down Expand Up @@ -161,6 +164,23 @@ Example: feat(route): add route list command with table output
6. Run `make check` before committing. All checks must pass.
7. Do not add new mock-Admin-API command tests. Use real e2e coverage for command behavior.

### Bug-Class Audit Protocol
When a live-validation bug is found (e.g. surfaced by the GA smoke walkthrough or a real user report), don't just fix the single occurrence. Identify the **bug class** — the underlying shape that produced it — and audit the rest of the codebase for the same shape before closing.

Trigger: you reproduce a bug in production code. Before opening the fix PR:

1. **Name the class.** What is the *shape* of this bug? Examples:
- "JSON unmarshal of an APISIX response that changed shape across versions" (e.g. `nodes` becoming an object in 3.x).
- "Success message echoes a server-returned field that doesn't actually carry the resource id."
- "`validate` accepts unknown top-level keys because the typed struct has no field for them."
- "Command path assembles `<a>/<b>` and silently strips one half when the file payload disagrees."
2. **Grep for the same shape.** Use the file/symbol patterns that match the class, not just the resource that surfaced it. For an unmarshal-shape bug, grep every `*_response.go` or `Unmarshal` call site, not only the one you hit. For a success-message bug, grep every `fmt.Fprintf.*deleted\|created\|updated` line.
3. **Decide per hit.** For each match: confirm it's affected, confirm it's exempt, or convert it to a separate sub-issue. Don't bundle unrelated fixes into the same PR — list the matches in the PR description even if you defer them.
4. **Generalise the test.** When possible, add the test at the *class* level (e.g. unmarshal-shape test across two response variants) rather than only the single resource that surfaced the bug.
5. **Note the class in the PR.** Even if only one fix lands, the description must say "audited X other call sites; Y were unaffected, Z tracked as #N." Future readers should be able to tell the audit happened.

Reference: the a7 protocol that this mirrors — a7 PR #32 audited every `*/create.go` after `BUG-2/BUG-3` turned out to share a `json.RawMessage`→YAML byte-array root cause; a7 PR #33 generalised "reject empty `upstreams: []`" to "reject bare and null forms too." These are the shape and granularity expected here.

### Environment Variables
| Variable | Purpose | Default |
|---|---|---|
Expand Down
100 changes: 100 additions & 0 deletions docs/ga-ux-sweep.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# a6 GA UX Consistency Sweep

Tracks the CLI UX consistency review requested in [#41](https://github.com/api7/a6/issues/41). Mirrors a7's review pattern (a7 issues #35, #36, #37, #42, #49): for every command, scan for **doc-vs-code flag mismatches**, **silently-dropped positional args**, **inconsistent `--output` defaults**, and **confusing `--id` semantics**.

Most findings here were surfaced during the [GA Run 1 manual walkthrough](./ga-test-report.md); a few were added by widening the sweep across `--help` output for every `<resource> <action>` combination. Each finding lists its current disposition: ✅ fixed in PR, 🟡 deferred follow-up, 🟢 minor/cosmetic, 📘 PRD-side fix.

---

## Class 1 — Doc-vs-code mismatches (PRD drift)

The PRD lists commands or flags that don't exist in the binary, or that have moved.

| # | What PRD says | What the binary does | Disposition |
|---|---|---|---|
| UX-1 | `a6 consumer credential ...` (nested under consumer) | `a6 credential ...` (top-level command) | 📘 PRD update (or rename the top-level command — pick one) |
| UX-2 | `a6 ssl upload` (in §"Resource Commands") | No `upload` subcommand exists — `ssl` has only `create/delete/export/get/list/update` | 📘 PRD update or implement |
| UX-3 | `a6 schema get` (in §"Resource Commands") | No `schema` top-level command exists | 📘 PRD update or implement |
| UX-4 | `--verbose / -v` (in §"Common Flags") | The `--verbose` flag exists but no `-v` shorthand is registered | 📘 PRD update or wire shorthand |

These are doc/spec follow-ups, not code regressions, but they bite first-time users who reach for the documented command. The fix is one of: update PRD, implement the command, or rename/alias the implementation.

---

## Class 2 — Silently-dropped positional args / unknown subcommands

| # | Reproducer | Expected | Actual |
|---|---|---|---|
| UX-5 | `a6 route bogus` (typo'd subcommand) | non-zero exit, error message | exit=0, prints `route --help`. Same shape for any `<resource> <typo>` |
| UX-6 | `a6 route deletex --force` (typo'd subcommand, plausible-looking flag) | error: unknown subcommand `deletex` | exit=0, prints `route --help`; `--force` silently swallowed |

🟡 **Tracked as a follow-up.** Cobra has `SilenceErrors` defaults that can be tightened (use `cobra.OnlyValidArgs` or set `SilenceUsage: false` + return an error from the parent's `RunE`). Affects every parent command — fix is centralised in `pkg/cmd/root/root.go` and the per-resource parent files.

In the manual walkthrough, no resource was found to silently drop positionals on **valid** subcommands (Cobra's `ExactArgs` / `MaximumNArgs` reject extras correctly). The silent-success failure mode is specific to **unknown** subcommands.

---

## Class 3 — Inconsistent `--output` defaults

Verified by sweeping `--help` text across every `<resource> <action>` combination:

| Action group | `--help` advertises | Actual behaviour |
|---|---|---|
| `<resource> list`, `<resource> delete` | `json, yaml, table` | All three formats work |
| `<resource> get`, `<resource> create`, `<resource> update` | `json, yaml` | `-o table` returns `unsupported output format: table` |

🟡 **BUG-6** in the [Run 1 report](./ga-test-report.md). Deferred from PR #51 — fix needs a shared per-resource table renderer lifted out of each `list.go`. Touches 12 `get.go` files plus the matching `create.go` / `update.go` if we want table for those too.

Additional symptom of the same class: **TTY default differs between `get` and `list`.** `get` defaults to `yaml` in TTY (because table is unsupported); `list` defaults to `table`. A user piping `a6 route get | grep` and `a6 route list | grep` gets different formats — not wrong per se, but unexpected.

---

## Class 4 — `--id` / positional-id semantics

The sweep across every resource confirms there is **no `--id` flag** anywhere — ids are always positional, which is consistent with Cobra conventions. The convention split that matters is the **bracket style** in `Use:` strings:

| Resource group | Pattern | Convention | Required? |
|---|---|---|---|
| `route`, `service`, `upstream`, `ssl`, `plugin-config`, `global-rule`, `stream-route`, `proto`, `consumer-group` | `<resource> get [id]` | Square brackets | Optional — falls back to interactive selector when omitted in TTY |
| `consumer` | `consumer get [username]` | Square brackets | Optional (username is the id, just renamed) |
| `secret` | `secret get [manager/id]` | Square brackets | Optional; positional must include the `manager/` prefix |
| `plugin-metadata` | `plugin-metadata get <plugin_name>` | Angle brackets | Required |
| `credential` | `credential get <id>` | Angle brackets, **but also needs `--consumer`** flag | Required |

🟢 The square-vs-angle distinction is semantically correct (Cobra: `<x>` required, `[x]` optional). No bug here, but the cognitive load is real — readers have to remember that `plugin-metadata` and `credential` require the positional while everyone else doesn't.

Real `--id`-shape bug surfaced during Run 1 and fixed: **secret create** rejected mismatched ids in body vs. positional. See [Run 1 report — BUG-4](./ga-test-report.md#bug-4--secret-create-vaultid-rejected-by-apisix-when-file-has-a-bare-id).

One related observation that survived Run 1: **global-rule `--id`** (where the resource id is forced to equal the single plugin key inside `plugins:`). a7 logged the same finding and treated it as a real bug. We didn't separately fix it on a6 — but `a6 global-rule create -f file.yaml` with a mismatched `id:` should clearly reject rather than silently letting APISIX compute the id from the plugin name. 🟡 Tracked.

---

## Class 5 — Other UX inconsistencies surfaced during Run 1

| # | Symptom | Notes |
|---|---|---|
| UX-7 | `<resource> delete <id>` (no `--force`) **silently deletes in non-TTY** | PRD says destructive ops "must require confirmation unless `--force`." In non-TTY there's no way to confirm interactively, so a6 just deletes. Either the PRD needs to allow this carve-out or the CLI should bail out asking for `--force` when stdin isn't a TTY. |
| UX-8 | `a6 config sync --dry-run` and `a6 config diff` produce **identical output** | A `Dry-run — no changes applied.` banner on the sync side would help operators in shared shells distinguish them at a glance. |
| UX-9 | `a6 config diff` emits a line per known section even when empty | `consumer_groups: create=0 update=0 delete=0 unchanged=0` etc. on an empty diff is verbose; collapse zero-change sections by default with a `--verbose` opt-in. |
| UX-10 | `credential delete` output read `Credential 1 deleted` (literal "1") before PR #51 | ✅ Fixed in PR #51 as BUG-3. |
| UX-11 | `upstream health` couldn't parse APISIX 3.x healthcheck response shape | ✅ Fixed in PR #51 as BUG-1. |
| UX-12 | `upstream health` ignored `A6_CONTROL_URL` env var | ✅ Fixed in PR #51 as BUG-2. |
| UX-13 | `config validate` accepted unknown top-level sections silently | ✅ Fixed in PR #51 as BUG-5. |

---

## Summary

| Class | Total findings | Already fixed (PR #51) | Open / deferred |
|---|---|---|---|
| 1 — PRD drift | 4 | 0 | 4 (📘 PRD-side) |
| 2 — Silent unknown subcommand | 2 | 0 | 1 (centralised fix) |
| 3 — `--output` inconsistency | 1 class spanning ~36 commands | 0 | 1 (BUG-6 in #51 report) |
| 4 — `--id` semantics | 1 cross-resource + 1 global-rule case | 1 (BUG-4) | 1 (global-rule body-id) |
| 5 — Other UX | 7 items | 4 (BUG-1/2/3/5) | 3 |

**Net: 10 open UX findings.** All non-blocking for GA from a correctness standpoint; together they're the polish layer for the imperative CLI surface.

## Next step for #41

Per the issue body, each finding becomes a discrete sub-issue. After this PR lands, file 10 GitHub issues under #33 — one per open row — and reference this document so the audit trail stays linked. Closed-by-#51 items don't need new issues.
20 changes: 10 additions & 10 deletions docs/roadmap.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ This document defines the per-PR development plan for the a6 CLI. Each PR is sel

**Audience**: AI coding agents and human developers. Each PR section contains enough detail to be implemented autonomously.

> **Status: ✅ Phase 1-3 COMPLETE (27 PRs)** — Phase 4 (AI Agent Skills, PR-28 through PR-36) is in progress.
> **Status: ✅ Phase 1-4 COMPLETE (36 PRs)** — All phases shipped. Phase 4 (AI Agent Skills, PR-28 through PR-36) merged as GitHub PRs #1 to #9 in March 2026. Post-merge work tracked under the GA-readiness epic ([#33](https://github.com/api7/a6/issues/33)).

---

Expand Down Expand Up @@ -1369,12 +1369,12 @@ Phase 4 adds AI agent skill files (`SKILL.md`) that enable AI coding agents to w
| PR-25 | Bulk Operations | 3 | ✅ | Bulk delete/export by label |
| PR-26 | Auto-Update | 3 | ✅ | Version + update check |
| PR-27 | CLI Extensions | 3 | ✅ | Extension management lifecycle |
| PR-28 | Skills Infra + Shared Skill | 4 | 🚧 | CI validation |
| PR-29 | Auth Plugin Skills | 4 | | CI validation |
| PR-30 | Security + Rate Limiting Skills | 4 | | CI validation |
| PR-31 | Traffic + Transformation Skills | 4 | | CI validation |
| PR-32 | Operational Recipe Skills | 4 | | CI validation |
| PR-33 | AI Gateway Skills | 4 | | CI validation |
| PR-34 | Observability Skills | 4 | | CI validation |
| PR-35 | Advanced Plugin Skills | 4 | | CI validation |
| PR-36 | Advanced Recipes + Personas | 4 | | CI validation |
| PR-28 | Skills Infra + Shared Skill | 4 | | CI validation (GitHub PR #1) |
| PR-29 | Auth Plugin Skills | 4 | | CI validation (GitHub PR #2) |
| PR-30 | Security + Rate Limiting Skills | 4 | | CI validation (GitHub PR #3) |
| PR-31 | Traffic + Transformation Skills | 4 | | CI validation (GitHub PR #4) |
| PR-32 | Operational Recipe Skills | 4 | | CI validation (GitHub PR #5) |
| PR-33 | AI Gateway Skills | 4 | | CI validation (GitHub PR #6) |
| PR-34 | Observability Skills | 4 | | CI validation (GitHub PR #7) |
| PR-35 | Advanced Plugin Skills | 4 | | CI validation (GitHub PR #8) |
| PR-36 | Advanced Recipes + Personas | 4 | | CI validation (GitHub PR #9) |
61 changes: 61 additions & 0 deletions docs/user-guide/conditional-plugins.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# Conditional Plugins

Most APISIX plugins are enabled by default and "just work" once you reference them from a route, service, consumer, or global rule. A few plugins are **conditional**: they ship with APISIX but are commented out in the stock `config.yaml`, and APISIX rejects requests that reference them until you enable them in the `plugins:` list.

If a plugin isn't enabled, you'll see one of these errors when you try to use it through a6:

```text
API error (status 400): unknown plugin [<name>]
API error (status 400): plugin <name> is not enabled
```

This page lists the conditional plugins a6 has surfaced during real-environment validation, how to enable them, and how that affects the test/skills coverage you'll see in this repo.

## Plugins that need enabling

| Plugin | APISIX docs | Why it's off by default | Notes |
|---|---|---|---|
| `skywalking` | [docs](https://apisix.apache.org/docs/apisix/plugins/skywalking/) | Requires an external SkyWalking OAP collector; off so APISIX doesn't ship a noisy default. | Enabled in this repo's e2e config (`test/e2e/apisix_conf/config.yaml`, `config-docker.yaml`). |
| `skywalking-logger` | [docs](https://apisix.apache.org/docs/apisix/plugins/skywalking-logger/) | Same external dependency as `skywalking`. | Enabled in this repo's e2e config. |
| `ai-content-moderation` | [docs](https://apisix.apache.org/docs/apisix/plugins/ai-content-moderation/) | Requires an external moderation backend (e.g. AWS Comprehend). | Not enabled in the e2e config; the matching skill case is `Skip()`-gated. |
| `chaitin-waf` | [docs](https://apisix.apache.org/docs/apisix/plugins/chaitin-waf/) | Requires the Chaitin SafeLine WAF sidecar. | Enabled in plugin list but no upstream to talk to in tests. |
| `splunk-hec-logging`, `kafka-logger`, `tcp-logger`, etc. | various | Require a real external sink. | Enabled in plugin list but harmless without a configured destination. |

This list reflects what a6's own e2e walkthrough has run into. APISIX's full conditional set is broader — when in doubt, check your APISIX deployment's `config.yaml` against the [stock APISIX config](https://github.com/apache/apisix/blob/master/conf/config.yaml.example) to see which plugins are commented out.

## Enabling a conditional plugin

Edit your APISIX deployment's `config.yaml` and add the plugin name to the `plugins:` list, then restart or hot-reload APISIX:

```yaml
plugins:
- real-ip
- cors
- key-auth
- skywalking # <-- add the line
```

```bash
apisix reload
# or, with Docker:
docker restart <apisix-container>
```

After the reload, the previously-failing a6 command should succeed.

## How this maps to the test suite

The a6 e2e suite distinguishes three states for each plugin scenario:

| State | What you'll see |
|---|---|
| **Verified in real env** | Tests run against a live APISIX with the plugin enabled. These cases run on every CI pass — no skip. |
| **Conditional / skipped** | Tests are gated behind a `Skip(...)` call (see `test/e2e/`). They run if the plugin is enabled in the test APISIX, otherwise they skip with an explanatory message. Currently this covers `ai-content-moderation`, `chaitin-waf` flows where the dependency isn't installed, and a small number of license-gated scenarios. |
| **Not covered** | Plugins for which a6 ships no skill or test. They may still work — the gateway accepts them — but a6 makes no guarantees beyond the generic CRUD path. |

The full list of currently-skipped scenarios is tracked in [issue #36](https://github.com/api7/a6/issues/36). Each skip will be either un-skipped (by enabling the dependency) or removed (by dropping the scenario from the supported set) before GA.

## Related

- [Plugin commands](./plugin.md) — `a6 plugin list` / `a6 plugin get` for plugin introspection.
- [Skills](../skills.md) — the AI-agent skill format, including how plugin skills declare their conditional dependencies.
Loading
Loading