Skip to content

Commit 02ff321

Browse files
authored
docs(spdd): add safeguards, edge cases, ops order, norms, and requirements to five spec files (#46283)
1 parent f8a0b36 commit 02ff321

5 files changed

Lines changed: 128 additions & 7 deletions

File tree

scratchpad/agents/hierarchical-agents-quickstart.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,23 @@ Track these to measure effectiveness:
311311
- Faster issue resolution
312312
- Better resource utilization
313313

314+
## Requirements
315+
316+
The following table traces each major Quick-Start step to the specification or acceptance criterion that governs it. Every step must satisfy its linked requirement before the feature is considered complete.
317+
318+
| Quick-Start Step | Specification / Acceptance Criterion |
319+
|------------------|--------------------------------------|
320+
| Check Workflow Health Dashboard pinned issue for critical issues | Workflow Health Manager must create or update a pinned issue on each run; spec: `scratchpad/agents/hierarchical-agents.md` §Workflow Health Manager |
321+
| Address P0/P1 issues immediately | Priority labels `priority-p0` and `priority-p1` must be applied to all issues created by meta-orchestrators; spec: `scratchpad/agents/hierarchical-agents.md` §Key Principles |
322+
| Review Campaign Manager report if campaigns are active | Campaign Manager must generate a discussion report on each daily run when ≥1 active campaign exists; spec: `scratchpad/agents/hierarchical-agents.md` §Campaign Manager |
323+
| Review Agent Performance Report discussion | Agent Performance Analyzer must publish a performance-report discussion on each daily run; spec: `scratchpad/agents/hierarchical-agents.md` §Agent Performance Analyzer |
324+
| Check Agent Performance report for best practices before creating a workflow | Agent Performance Analyzer must surface at least the top-3 performing agents with actionable patterns; spec: `scratchpad/agents/hierarchical-agents.md` §Agent Performance Analyzer Success Metrics |
325+
| Check Campaign Manager for potential conflicts when starting a campaign | Campaign Manager must detect and report cross-campaign conflicts in each portfolio report; spec: `scratchpad/agents/hierarchical-agents.md` §Campaign Manager Responsibilities |
326+
| Recompile workflow with `gh aw compile <workflow>.md` after schedule changes | Compilation must succeed without errors and regenerate the `.lock.yml` file; spec: `DEVGUIDE.md` §Compilation |
327+
| Increase `safe-outputs.create-issue.max` if meta-orchestrators hit limits | Safe output limits must be configurable in workflow frontmatter; spec: `scratchpad/safe-outputs-specification.md` |
328+
329+
These requirements are derived from the feature specifications in `scratchpad/agents/hierarchical-agents.md` and the safe-outputs contract in `scratchpad/safe-outputs-specification.md`. Any change to a Quick-Start step must be accompanied by a corresponding update to the linked specification.
330+
314331
---
315332

316333
**Remember:** Meta-orchestrators are here to help, not to replace human judgment. Use their insights to make better decisions, but always apply context and expertise when acting on recommendations.

scratchpad/agents/hierarchical-agents.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,30 @@ Meta-orchestrators use a shared repository memory branch (`memory/meta-orchestra
158158
- Workflow Health Manager identifies that Agent Performance Analyzer found quality issues in a specific workflow
159159
- Agent Performance Analyzer notes that Campaign Manager deprioritized a campaign, reducing urgency of related improvements
160160

161+
### Safeguards
162+
163+
The shared memory path `/tmp/gh-aw/repo-memory-default/memory/meta-orchestrators/` is ephemeral and may be absent or unreadable at runtime. Meta-orchestrators **must** treat it as optional and degrade gracefully under the following failure modes:
164+
165+
1. **Memory path missing (fresh environment):** The `/tmp/gh-aw/repo-memory-default/` directory may not exist on the first run or in a freshly provisioned runner. Each meta-orchestrator must check for path existence before reading and continue without historical context if the directory is absent. No error should be surfaced to the user; instead, the orchestrator starts with an empty baseline.
166+
167+
2. **Memory branch unreadable (checkout failure):** If the `memory/meta-orchestrators` branch fails to check out (e.g., the branch was deleted, a network error occurred, or the repo-memory action is unavailable), the orchestrator must log a warning and proceed with in-session data only. Avoid writing to a failed checkout path, as doing so could corrupt subsequent reads.
168+
169+
3. **Stale or corrupted memory file:** A shared file (e.g., `shared-alerts.md`) may contain truncated content due to a previous run crashing mid-write. Each orchestrator must validate that the file is parseable before consuming it. If parsing fails, the file should be treated as absent and overwritten with a fresh snapshot at the end of the run.
170+
171+
4. **Concurrent write collision:** All three meta-orchestrators run on the same daily schedule and could write to the same memory branch simultaneously. Memory writes must be treated as best-effort: if a push fails due to a concurrent update, the orchestrator logs a warning and proceeds without blocking. Consumers must tolerate reading a version written by a sibling run from the previous cycle.
172+
173+
5. **Permission denied on memory branch:** If the GitHub token used for the run lacks permission to push to the `memory/meta-orchestrators` branch (e.g., PR-scoped token restrictions), the orchestrator must skip the write step and emit a single warning rather than failing the entire run.
174+
175+
### Operations Order
176+
177+
When all three meta-orchestrators are triggered simultaneously (e.g., on the same daily schedule), the following order governs their execution and memory access:
178+
179+
1. **Workflow Health Manager** — runs first. It has no dependency on the other orchestrators' outputs and produces the foundational health snapshot (`workflow-health-latest.md`) that the other two may read.
180+
2. **Campaign Manager** — runs second. It reads the workflow health snapshot to cross-reference failing workflows against active campaigns before writing `campaign-manager-latest.md`.
181+
3. **Agent Performance Analyzer** — runs third. It reads both workflow health and campaign data to provide context-aware quality assessments and writes `agent-performance-latest.md`.
182+
183+
**Concurrency contract:** Because the three workflows are scheduled independently and GitHub Actions does not guarantee serial execution, they may overlap. Each orchestrator must be designed to produce a correct result using only the memory snapshot present at its *start*, without assuming that sibling orchestrators have already completed. The ordering above is the *recommended* sequence for coordinated scheduling; it is not enforced by the platform.
184+
161185
## How Meta-Orchestrators Work
162186

163187
### Discovery

scratchpad/architecture.md

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,3 +111,18 @@ CLI entry points → core packages (cli, workflow, parser, console) → domain h
111111
| `importinpututil` | Util | Import path / sub-key resolver |
112112
| `testutil` | Util | Test helpers (test-only, not used in production) |
113113
| `colorwriter` | Util | Color-profile-aware io.Writer (NO_COLOR/COLORTERM/TERM aware) |
114+
115+
## Maintenance
116+
117+
### Sync Policy
118+
119+
The diagram and package table above must stay in sync with the actual Go package layout under `pkg/`. Without an explicit policy, new packages added to the codebase will silently diverge from this document.
120+
121+
**Sync trigger:** Any PR that adds or removes a Go package under `pkg/` **must** update this document. Reviewers should verify that:
122+
1. The ASCII diagram reflects the new package and its correct layer (Core / Domain / Util).
123+
2. A corresponding row is added to (or removed from) the Package Reference table.
124+
3. The `Last updated` date on line 3 is refreshed.
125+
126+
**Tracking:** A CI lint rule or OWNERS note enforcing this policy is tracked in a future issue. Until that check is automated, the PR author is responsible for updating the diagram as part of their change.
127+
128+
**Ownership:** The `architecture.md` document is owned by the gh-aw maintainer team. Any contributor may propose updates via PR; changes to the diagram layers or the package categorization require at least one maintainer approval.

scratchpad/artifact-naming-compatibility.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,20 @@ gh aw logs <run-id> --artifacts firewall --json
9797

9898
> **⚠️ Common mistake:** Downloading `agent-artifacts` or `agent` and expecting to find `token-usage.jsonl` there. Token usage data lives in the `firewall-audit-logs` artifact, not in the agent artifact.
9999
100+
## Risks / Edge Cases
101+
102+
The `flattenSingleFileArtifacts()` function assumes that artifacts containing exactly one file are safe to flatten to the root directory. The following edge cases present data-loss or correctness risks and should be handled explicitly:
103+
104+
1. **Empty artifact directory:** An artifact directory that was created but contains zero files will not trigger flattening. However, if a caller iterates over the directory expecting to find a specific file (e.g., `aw_info.json`), it will silently receive nothing. The function must skip empty directories without error, but callers must not treat a missing file as an empty file — they must surface a "not found" error.
105+
106+
2. **Unexpected multi-file artifact where single-file was expected:** If an artifact that is always expected to contain one file (e.g., `aw-info`) unexpectedly contains multiple files after a future schema change, `flattenSingleFileArtifacts()` will skip flattening and leave the directory intact. Downstream CLI code that looks for `aw_info.json` at the root will then fail with a confusing "file not found" error rather than a meaningful "artifact has unexpected structure" diagnostic. A guard should log a warning when a nominally single-file artifact is encountered with multiple files.
107+
108+
3. **File name collision at root:** If two separate artifact directories each contain a file with the same name (e.g., both `aw-info/aw_info.json` and a legacy root-level `aw_info.json` exist in the same download directory), flattening would overwrite the pre-existing file. The function must check for conflicts before moving files and abort with an error rather than silently overwriting.
109+
110+
4. **Artifact name matches an existing root file:** A new artifact type introduced in a future version might coincidentally produce a file whose name collides with one already written by an older artifact. The flattening logic must detect this case (same as collision above) and surface it explicitly instead of masking the conflict.
111+
112+
5. **Non-directory entry in artifact list:** If `gh run download` creates a file (not a directory) at the artifact level — for example, due to a platform-specific behavior — `flattenSingleFileArtifacts()` must skip it gracefully without attempting to read its contents as a directory.
113+
100114
## Testing
101115

102116
Tests ensure compatibility:

scratchpad/breaking-cli-rules.md

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,25 @@ Remove deprecated `--old-flag` option
203203
**Reason**: The option was deprecated in v0.X.0 and has been removed to simplify the CLI.
204204
```
205205

206+
**Schema breaking change example** (removing a top-level frontmatter field):
207+
208+
```markdown
209+
---
210+
"gh-aw": major
211+
---
212+
213+
Remove deprecated top-level `defaults` field from workflow frontmatter schema
214+
215+
**⚠️ Breaking Change**: The `defaults` field has been removed from the workflow frontmatter schema.
216+
217+
**Migration guide:**
218+
- Workflows that declare `defaults:` at the top level must be updated.
219+
- Move any `defaults.run.shell` settings into the individual step definitions.
220+
- Run `gh aw compile` after updating; compilation will fail with a schema validation error if `defaults:` is still present, making affected workflows easy to identify.
221+
222+
**Reason**: The `defaults` block was deprecated in v0.24.0 when per-step shell configuration was introduced. It is now removed to reduce schema surface area.
223+
```
224+
206225
### Changeset Format for Non-Breaking Changes
207226

208227
For new features:
@@ -237,15 +256,21 @@ Reviewers should verify:
237256

238257
## Exit Code Standards
239258

240-
The CLI uses standard exit codes:
259+
The CLI uses the following exit codes. Codes are defined in `pkg/cli/exit_code_error.go` (`ExitCodeError`) and set at specific call sites:
241260

242-
| Exit Code | Meaning | Breaking to Change |
243-
|-----------|---------|-------------------|
244-
| 0 | Success | No (adding is fine) |
245-
| 1 | General error | No (for new errors) |
246-
| 2 | Invalid usage | No (for new checks) |
261+
| Exit Code | Meaning | Subcommand(s) | Source Reference | Breaking to Change |
262+
|-----------|---------|---------------|-----------------|-------------------|
263+
| 0 | Success — no action needed | `gh aw upgrade` (already up to date) | `pkg/cli/upgrade_command.go` | Yes — changing a success to non-zero breaks scripts |
264+
| 1 | General / processing error | `gh aw fix` (codemod processing failure) | `pkg/cli/fix_command.go` | Yes — changing an error scenario to 0 masks failures |
265+
| 2 | Manual intervention required | `gh aw fix` (issues require human fixes) | `pkg/cli/fix_command.go` | Yes — scripts may branch on code 2 vs 1 |
266+
| 124 | Timeout | `gh aw forecast` (computation deadline exceeded) | `pkg/cli/forecast.go` | Yes — timeout semantics are relied on by CI wrappers |
267+
| 130 | User cancellation (SIGINT) | Any interactive secret-collection prompt | `pkg/cli/engine_secrets.go` | Yes — shells use 130 to detect Ctrl-C; changing it breaks shell handlers |
247268

248-
**Breaking**: Changing the exit code for an existing scenario (e.g., changing from 1 to 2 for a specific error type).
269+
> **Note:** Source references point to the Go files where these codes are set via `ExitCodeError`. Line numbers are not pinned here as they shift over time; search for `ExitCodeError{Code: <N>}` in the referenced file to locate the exact call site.
270+
271+
**Breaking**: Changing the exit code for an *existing* scenario (e.g., changing from 1 to 2 for a specific error type already in production).
272+
273+
**Not Breaking**: Adding a new exit code for a *new* scenario that did not previously exist.
249274

250275
## JSON Output Standards
251276

@@ -265,6 +290,32 @@ Special consideration for strict mode changes:
265290
- **Changing strict mode defaults** is breaking (e.g., v0.31.0)
266291
- **Adding new strict mode validations** is not breaking (strictness is opt-in initially)
267292

293+
## Approvals / Norms
294+
295+
### Who Approves Major-Version Bumps
296+
297+
A changeset marked `major` (breaking change) requires explicit review and approval before merge:
298+
299+
- **Minimum quorum:** At least **2 maintainer approvals** on the PR. A single maintainer approval is insufficient for breaking changes, regardless of how small the change appears.
300+
- **Maintainer role:** A maintainer is any contributor listed in `CODEOWNERS` with write or admin access to the repository. For internal contributors, this maps to the `@github/gh-aw-maintainers` team. External contributors can identify current maintainers via the `CODEOWNERS` file at the repository root.
301+
- **Author exclusion:** The PR author does not count toward the quorum even if they are a maintainer.
302+
- **Review window:** Breaking change PRs must remain open for a minimum of **48 hours** after the first maintainer approval to allow the team to surface objections.
303+
304+
### Escalation Path
305+
306+
If consensus cannot be reached within the normal review process:
307+
308+
1. Open a discussion in the `gh-aw` repo tagged `breaking-change-decision`.
309+
2. Any maintainer may call a synchronous review meeting if the change is time-sensitive.
310+
3. The final decision rests with the repository owner if the team is deadlocked.
311+
312+
### Documentation Requirements
313+
314+
Every major changeset **must** include:
315+
- A CHANGELOG entry with migration guidance.
316+
- An updated help text (if the changed surface is user-visible).
317+
- A link to a tracking issue or discussion if the breaking change was previously discussed.
318+
268319
## References
269320

270321
- **Changeset System**: See `scratchpad/changesets.md` for version management

0 commit comments

Comments
 (0)