Skip to content
Open
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
64 changes: 64 additions & 0 deletions PLAN.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# fleet-e2e-toy — Implementation Plan

> Add a CLI tool (`./tool`) supporting `--version`, `--help`, and input validation for blank arguments. Three features, one new source file, zero new dependencies.

---

## Tasks

### Phase 1: CLI Tool

All three sprint requirements share the same new files (`src/cli.ts`, `tool`, `tests/cli.test.ts`). They are cohesive — each builds on the arg-parsing structure established in Task 1. Ordered cheap → cheap → cheap; the riskiest assumption (the tool can be invoked via `./tool` and tested via the exported `main()` function) is validated in Task 1.

#### Task 1: Create CLI entry point and implement `--version` / `-v` flag
- **Change:** Create `src/cli.ts` with an exported `main(argv: string[]): number` function. Recognize `--version` and `-v`; print `fleet-e2e-toy v<version>` (tool display name hardcoded as `fleet-e2e-toy`, version read from `package.json` via `resolveJsonModule`), return 0. Add a `tool` shell script at the repo root that invokes `./node_modules/.bin/ts-node src/cli.ts "$@"` so the binary is callable as `./tool`. Create `tests/cli.test.ts` with tests for both `--version` and `-v`.
- **Files:** `src/cli.ts` (new), `tool` (new), `tests/cli.test.ts` (new)
- **Tier:** cheap
- **Done when:** `npm test` passes including new version-flag tests; `./tool --version` prints `fleet-e2e-toy v1.0.0` and exits 0; existing API tests still pass.
- **Blockers:** `ts-node` is already in devDependencies and `resolveJsonModule` is already enabled in `tsconfig.json` — no new dependencies required. `tool` must be marked executable (`chmod +x tool`).

#### Task 2: Implement help subcommand and `--help` / `-h` flag
- **Change:** Extend `main()` in `src/cli.ts` to recognise `help` as a subcommand (first positional arg) and `--help`/`-h` as flags. Print a usage block listing all available commands (`help`) and all flags (`--version`/`-v`, `--help`/`-h`), return 0. Add tests for `['help']`, `['--help']`, and `['-h']` — each must produce non-empty output and return 0.
- **Files:** `src/cli.ts`, `tests/cli.test.ts`
- **Tier:** cheap
- **Done when:** `npm test` passes including new help tests; `./tool help`, `./tool --help`, and `./tool -h` each print usage covering every command and flag.
- **Blockers:** None.

#### Task 3: Add input validation for empty or blank string arguments
- **Change:** In `main()`, before dispatching, iterate over positional arguments (those not beginning with `-`). If any argument is an empty string or contains only whitespace, print a clear error message to stderr (`Error: argument cannot be empty or blank`) and return 1. Add unit tests: empty string `""` and whitespace-only `" "` must each produce an error and non-zero return value.
- **Files:** `src/cli.ts`, `tests/cli.test.ts`
- **Tier:** cheap
- **Done when:** `npm test` passes including new validation tests; both `./tool ""` and `./tool " "` print the error and exit non-zero.
- **Blockers:** None. Validation fires only on positional args so flags (`--version`, `-h`) are unaffected.

#### VERIFY: CLI Tool
- Run full test suite (`npm test`) — all suites (API, validation, CLI) must pass
- Confirm `./tool --version`, `./tool help`, `./tool --help`, `./tool ""` each behave as specified
- Report: tests passing, any regressions in existing API tests, any issues found

---

## Risk Register

| Risk | Impact | Mitigation |
|------|--------|------------|
| `tool` script lacks execute permission after commit | Med | `chmod +x tool` in Task 1; note in done criteria |
| `fleet-e2e-toy` display name diverges from `package.json` `name` field (`noteapi`) | Low | Hardcode display name as a constant in `src/cli.ts`; read only version from package.json |
| Existing API tests broken by new CLI file | Low | CLI lives in a new file; no shared state; `beforeEach` clears the note store |
| TypeScript strict-mode rejects `package.json` import | Low | `resolveJsonModule: true` and `skipLibCheck: true` already set in `tsconfig.json` |

## Phase Sizing Rules

**Phase boundaries by cohesion, not count.** A phase is a coherent unit of work that produces a reviewable, testable increment. Group tasks into a phase when they share a data model, code path, or design decision — splitting them would produce an incoherent intermediate state or require touching the same code twice. Place a VERIFY at the natural completion boundary of that unit, not at an arbitrary task count. Phases may have 4-5 tasks (a coherent subsystem) or just 1-2 (a genuinely isolated change).

**Monotonically non-decreasing tiers within a phase.** Within a phase, order tasks cheap → standard → premium. The PM resumes the same session across tasks in a phase — a premium task can build a large context that a cheap model cannot load. The PM may group consecutive same-tier tasks into a single dispatch streak; tier transitions trigger a new dispatch. If a dependency forces a higher-tier task before a lower-tier task within a phase, split the phase at that boundary rather than violating the ordering rule. Cross-phase tier order does not matter — each phase always starts a fresh session.
```
cheap → cheap → standard → standard → premium → VERIFY [VALID]
cheap → standard → cheap → VERIFY [INVALID] (downgrade within phase — split into two phases)
```

## Notes
- Each task should result in a git commit
- Verify tasks are checkpoints — stop and report after each one
- Base branch: main
- Implementation branch: e2e-s1.2-26527886489/cli-features
129 changes: 129 additions & 0 deletions feedback.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
# e2e-s1.2-26527886489 — Code Review

**Reviewer:** reviewer
**Date:** 2026-05-27 15:30:00-04:00
**Verdict:** APPROVED

> Prior review (commit 9bc0d03) was a plan review — APPROVED with two non-blocking notes: (1) `rootDir: ./src` prevents `resolveJsonModule` import of `package.json`, and (2) the "works alongside other flags" acceptance criterion for `--version` needed verification. Both notes are addressed in this implementation — see sections below.

---

## 1. Build, Lint, and Tests

**PASS.** All three gates clear:

- **Build:** `tsc --noEmit` — zero errors. TypeScript compiles cleanly under strict mode.
- **Lint:** `eslint src/ tests/ --ext .ts` — zero warnings, zero errors.
- **Tests:** 28/28 passing across 3 suites (CLI: 7, validation: 5, notes/API: 16). No regressions in existing API or validation tests.

---

## 2. CI

**NOTE.** No CI runs exist for this branch. The workflow (`.github/workflows/ci.yml`) triggers on `push` to `main` or `feature/**` branches, and on PRs to `main`. The branch name `e2e-s1.2-26527886489/cli-features` does not match `feature/**`. This is a pre-existing CI configuration gap — not introduced by this sprint. Local build, lint, and test all pass, so this is non-blocking.

---

## 3. Task 1.1 — `--version` / `-v` Flag

**PASS.** `src/cli.ts` exports `main(argv: string[]): number` as specified. Recognizes `--version` and `-v` at the first argument position. Prints `fleet-e2e-toy v1.0.0` and returns 0.

The plan originally called for reading `package.json` via `resolveJsonModule`, but the plan review (section 10) flagged that `rootDir: ./src` would block this. The implementer correctly used `fs.readFileSync` + `JSON.parse` instead (commit 5d85532 fixed an earlier `require()` attempt). The `getVersion()` function uses `path.join(__dirname, "../package.json")` with a try/catch fallback to `"unknown"` — clean and safe.

Display name `fleet-e2e-toy` is hardcoded as a constant, separate from the `package.json` `name` field (`noteapi`), per the risk register.

The `tool` shell script is executable (`-rwxr-xr-x`), uses `exec` to replace the shell process, and passes `"$@"` correctly.

Tests verify both `--version` and `-v` produce the exact expected string `"fleet-e2e-toy v1.0.0"` and return 0.

**Re: "Works alongside other flags" AC (plan review note 2):** The implementation processes `--version` only as the first argument. `./tool somearg --version` will not print the version — it returns 0 silently. This is standard CLI behavior (similar to `node --version`, `git --version`) and is acceptable. The AC is ambiguous — "alongside other flags" more likely means `--version` doesn't conflict with the existence of other flags, not that it works in any argument position.

---

## 4. Task 1.2 — Help Subcommand and `--help` / `-h` Flag

**PASS.** `main()` recognizes `help` as a positional subcommand and `--help`/`-h` as flags. All three routes call `printHelp()`, which outputs a usage block listing:

- Commands: `help`
- Options: `--version, -v` and `--help, -h`

Returns 0 in all cases. The help output includes the display name in the usage line.

Tests cover all three invocations (`['help']`, `['--help']`, `['-h']`) and verify the output contains `"help"`, `"--version"`, and `"--help"`. Tests check return value is 0.

---

## 5. Task 1.3 — Input Validation for Empty/Blank Arguments

**PASS.** After flag dispatch, `main()` iterates all `argv` entries. For positional arguments (those not starting with `-`), it checks `arg === "" || /^\s+$/.test(arg)`. On match, prints `"Error: argument cannot be empty or blank"` to stderr and returns 1.

The validation correctly skips flag-like arguments (starting with `-`), so `--version`, `-h`, etc. are unaffected.

Tests verify both `""` and `" "` produce the exact error message on stderr and return 1. Spies target `console.error` (not `console.log`), matching the implementation.

---

## 6. Code Quality and Patterns

**PASS.** The implementation is consistent with existing project conventions:

- Uses `import * as fs from "fs"` / `import * as path from "path"` — matches the Node.js import style used elsewhere.
- Function signatures are typed. `main()` return type is explicit.
- The `if (require.main === module)` guard correctly separates testable exports from script execution.
- `process.exit(main(args))` in the entry point — clean exit with the return code.
- Test file follows the same `describe`/`it` structure and spy pattern used in `tests/validation.test.ts` and `tests/notes.test.ts`.
- Each test properly calls `mockRestore()` after assertions.

---

## 7. Test Quality

**PASS.** 7 new tests across 3 `describe` blocks. No redundant tests — each covers a distinct input. Tests verify both return values and output content:

- Version tests assert the exact output string, catching regressions in format or version number.
- Help tests assert the output contains all documented commands and flags, ensuring help text stays in sync with actual features.
- Validation tests assert the exact error message string and verify stderr (not stdout).

**NOTE.** Minor gaps that are non-blocking: no tests for `main([])` (returns 0 — benign default) or `main(["unknown"])` (returns 0 — reasonable). No test for mixed valid/invalid positional args (e.g., `["valid", ""]`). The existing tests cover the specified acceptance criteria fully; these are potential future improvements, not missing coverage.

---

## 8. Security

**PASS.** No vulnerabilities found:

- `fs.readFileSync` reads a hardcoded relative path (`../package.json`) — no user-controlled path injection.
- `JSON.parse` operates on trusted `package.json` content within a try/catch.
- Error output is a hardcoded string — no reflection of user input.
- No network calls, no eval, no shell execution from user-provided args.
- No secrets in code.

---

## 9. File Hygiene

**PASS.** Seven files changed, all justified:

| File | Justification |
|------|--------------|
| `PLAN.md` | Implementation plan (sprint artifact) |
| `requirements.md` | Sprint requirements (sprint artifact) |
| `progress.json` | Task tracking (sprint artifact) |
| `feedback.md` | Review output (sprint artifact) |
| `src/cli.ts` | New CLI entry point (T1.1, T1.2, T1.3) |
| `tests/cli.test.ts` | New CLI tests (T1.1, T1.2, T1.3) |
| `tool` | Shell script entry point (T1.1) |

No temp files, no config changes, no stale artifacts. CLAUDE.md is tracked from main but was not modified in this branch.

---

## Summary

All three sprint requirements are fully implemented and tested. Build, lint, and all 28 tests pass with zero errors. The code is clean, consistent with existing patterns, and free of security issues. The plan review's two notes were properly handled: the `rootDir` constraint was worked around with `fs.readFileSync`, and the `--version` "alongside other flags" behavior follows standard CLI conventions.

Non-blocking notes for future consideration:
1. CI trigger pattern (`feature/**`) does not match sprint branch naming (`e2e-s1.2-*/cli-features`) — CI did not run on this branch.
2. Minor test coverage gaps for edge cases (empty argv, unknown commands, mixed valid/invalid args) could be added in a future sprint.

**Verdict: APPROVED.** Phase 1 is complete and ready for merge.
41 changes: 41 additions & 0 deletions progress.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
{
"project": "fleet-e2e-toy",
"sprint": "e2e-s1.2-26527886489",
"branch": "e2e-s1.2-26527886489/cli-features",
"updated": "2026-05-27",
"tasks": [
{
"id": "T1.1",
"phase": 1,
"title": "Create CLI entry point and implement --version / -v flag",
"tier": "cheap",
"status": "completed",
"notes": "src/cli.ts created with main() function; tool executable created and marked with chmod +x; tests for --version and -v passing"
},
{
"id": "T1.2",
"phase": 1,
"title": "Implement help subcommand and --help / -h flag",
"tier": "cheap",
"status": "completed",
"notes": "Extended main() to recognize help subcommand and --help/-h flags; prints usage with all commands and flags; tests passing"
},
{
"id": "T1.3",
"phase": 1,
"title": "Add input validation for empty or blank string arguments",
"tier": "cheap",
"status": "completed",
"notes": "Added validation for empty/whitespace-only positional args; returns 1 with error message; tests for empty string and whitespace passing"
},
{
"id": "T1.V",
"phase": 1,
"type": "verify",
"title": "VERIFY: CLI Tool",
"tier": "cheap",
"status": "completed",
"notes": "Build: ✓ passed; Lint: ✓ passed (fixed fs.readFileSync); Tests: 28 passed (all API, validation, CLI suites); All prior tasks verified working correctly"
}
]
}
53 changes: 53 additions & 0 deletions requirements.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Sprint Requirements — fleet-e2e-toy

Sprint: e2e-s1.2-26527886489
Repo: https://github.com/Apra-Labs/fleet-e2e-toy
Base branch: main

---

## gh-toy-4ef · Add --version flag to CLI

**Type:** feature
**Priority:** P1
**External:** gh-1

**Description:**
The CLI tool should support a `--version` (or `-v`) flag that prints the current version string and exits with code 0.

**Acceptance Criteria:**
- Running `./tool --version` prints `fleet-e2e-toy v1.0.0`
- Exit code is 0
- Works alongside other flags

---

## gh-toy-v6z · [bug] Add input validation for empty or blank strings

**Type:** bug
**Priority:** P1
**External:** gh-2

**Description:**
When a user passes an empty string or whitespace-only string as an argument, the tool should reject it with a clear error message instead of silently proceeding or crashing.

**Acceptance Criteria:**
- Passing empty or whitespace-only string prints a user-friendly error message
- Exits with non-zero exit code
- A unit test is added covering this behaviour

---

## gh-toy-kbk · Implement a help command

**Type:** feature
**Priority:** P1
**External:** gh-3

**Description:**
Add a help subcommand (and `--help` / `-h` flag) that prints usage information for all available commands and flags.

**Acceptance Criteria:**
- `./tool help` and `./tool --help` both work
- Lists every subcommand and flag
- Exit code is 0
66 changes: 66 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import * as fs from "fs";
import * as path from "path";

const DISPLAY_NAME = "fleet-e2e-toy";

function getVersion(): string {
try {
const pkgPath = path.join(__dirname, "../package.json");
const content = fs.readFileSync(pkgPath, "utf-8");
const pkg = JSON.parse(content);
return pkg.version;
} catch {
return "unknown";
}
}

function printHelp(): void {
console.log(`Usage: ${DISPLAY_NAME} [command] [options]

Commands:
help Show this help message

Options:
--version, -v Show version
--help, -h Show help`);
}

export function main(argv: string[]): number {
if (argv.length === 0) {
return 0;
}

const firstArg = argv[0];

if (firstArg === "--version" || firstArg === "-v") {
console.log(`${DISPLAY_NAME} v${getVersion()}`);
return 0;
}

if (firstArg === "--help" || firstArg === "-h") {
printHelp();
return 0;
}

if (firstArg === "help") {
printHelp();
return 0;
}

// Validate positional arguments (those not beginning with -)
for (const arg of argv) {
if (!arg.startsWith("-")) {
if (arg === "" || /^\s+$/.test(arg)) {
console.error("Error: argument cannot be empty or blank");
return 1;
}
}
}

return 0;
}

if (require.main === module) {
const args = process.argv.slice(2);
process.exit(main(args));
}
Loading
Loading