docs: rewrite docs site as audience-first reorg - #87
Merged
Conversation
SHIP_USE_PROBE_VERDICT and SHIP_USE_CONTINUATION_LOOP were declared in the env schema but never read from src/. Remove the schema entries, the parseBooleanEnv calls in loadConfig(), and the corresponding test assertions in test/config.test.ts. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace the flat 11-file layout with 22 focused pages organised under three audience tabs — use the bot, run the service, build on it — plus the changelog. Strip leaked spec references (FR-NNN, specs/<dated>/...) from public docs and rewrite the bot:ship documentation to reflect the post-PR-79 shepherding lifecycle only. scripts/check-docs-sync.ts gates on docs/use/workflows/*.md instead of the removed docs/BOT-WORKFLOWS.md. docs/index.md is added to .prettierignore so the Material grid-cards block keeps its 4-space indent (Prettier was collapsing the body indent and breaking the grid layout). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis pull request reorganizes documentation from a flat structure to a hierarchical model with dedicated user guides, operational runbooks, and workflow-specific pages. Concurrently, two feature flags ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 49 minutes and 10 seconds.Comment |
The repo-wide `build/` ignore rule in .gitignore (intended for project build outputs) was matching docs/build/, so the four pages under that tab — architecture.md, extending.md, conventions.md, contributing.md — were silently dropped from the previous commit. CI mkdocs strict build caught it via missing nav targets. Add `!docs/build/` negation and force-stage the four missing pages. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Rewrites the docs/ site into an audience-first structure (Use / Operate / Build), updates MkDocs navigation accordingly, and removes two unused bot:ship rollout flags from runtime config and tests.
Changes:
- Reorganized docs into
docs/use/,docs/operate/, and (intended)docs/build/, with updatedmkdocs.ymlnav and a new landing page layout. - Updated the doc-sync CI guard to require updates under
docs/use/workflows/whensrc/workflows/**changes. - Removed unused config/env flags (
SHIP_USE_PROBE_VERDICT,SHIP_USE_CONTINUATION_LOOP) and deleted their schema/test coverage.
Reviewed changes
Copilot reviewed 38 out of 40 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/config.test.ts | Removes assertions/tests for the deleted SHIP_USE_* flags. |
| src/config.ts | Drops the two unused ship feature flags from schema + env parsing. |
| scripts/check-docs-sync.ts | Updates doc-sync gating to docs/use/workflows/*.md and refreshes messaging. |
| mkdocs.yml | Rewrites nav into “Use / Run / Build” tabs and points Changelog to docs/changelog.md. |
| docs/use/workflows/triage.md | New workflow page for bot:triage. |
| docs/use/workflows/ship.md | New workflow page for bot:ship shepherding lifecycle. |
| docs/use/workflows/review.md | New workflow page for bot:review. |
| docs/use/workflows/resolve.md | New workflow page for bot:resolve. |
| docs/use/workflows/plan.md | New workflow page for bot:plan. |
| docs/use/workflows/index.md | New workflows catalog and shared rules page. |
| docs/use/workflows/implement.md | New workflow page for bot:implement. |
| docs/use/safety.md | New safety/boundaries page (destructive-action guard, ship pause/resume/abort, etc.). |
| docs/use/invoking.md | New “how to invoke” guide with trigger-router flow diagram and refusal cases. |
| docs/operate/setup.md | New local dev setup guide (commands, daemon local run, troubleshooting). |
| docs/operate/runbooks/triage.md | New operational runbook for the triage heavy-classifier. |
| docs/operate/runbooks/stuck-ship-intent.md | New runbook for diagnosing stuck bot:ship sessions, with SQL queries. |
| docs/operate/runbooks/daemon-fleet.md | New daemon fleet runbook (persistent vs ephemeral, lifecycle, scaling, constraints). |
| docs/operate/observability.md | New observability reference (common fields, ship log schema, dispatch reasons, alerts). |
| docs/operate/github-app.md | New step-by-step GitHub App creation guide and event subscription list. |
| docs/operate/deployment.md | New deployment guide (image topology, build/run, probes, shutdown, K8s requirements). |
| docs/operate/configuration.md | New env var reference grouped by concern, with mode matrix. |
| docs/index.md | New docs landing page with Material “grid cards” layout and updated intro. |
| docs/changelog.md | Adds MkDocs snippet include for root CHANGELOG.md. |
| docs/TRIAGE.md | Removes legacy flat-page triage doc (replaced by operate runbook). |
| docs/SHIP.md | Removes legacy ship doc (replaced by docs/use/workflows/ship.md). |
| docs/SETUP.md | Removes legacy setup doc (replaced by docs/operate/setup.md + github-app.md). |
| docs/OBSERVABILITY.md | Removes legacy observability doc (replaced by docs/operate/observability.md). |
| docs/EXTENDING.md | Removes legacy extending doc (intended replacement under docs/build/). |
| docs/DEPLOYMENT.md | Removes legacy deployment doc (replaced by docs/operate/deployment.md). |
| docs/DAEMON.md | Removes legacy daemon doc (replaced by docs/operate/runbooks/daemon-fleet.md). |
| docs/CONTRIBUTING.md | Removes legacy docs-only contributing pointer (intended replacement under docs/build/). |
| docs/CONFIGURATION.md | Removes legacy config doc (replaced by docs/operate/configuration.md). |
| docs/BOT-WORKFLOWS.md | Removes legacy monolithic workflows doc (replaced by docs/use/workflows/*). |
| docs/ARCHITECTURE.md | Removes legacy architecture doc (intended replacement under docs/build/). |
| .prettierignore | Ignores docs/index.md to preserve Material grid-card indentation. |
Two Copilot review nits:
- docs/operate/deployment.md listed the shared Docker base as
oven/bun:1.3.12, but Bun is pinned to 1.3.13 via .tool-versions and
both Dockerfiles. Update the table to match.
- scripts/check-docs-sync.ts header said the diff range was BASE_SHA..
HEAD_SHA (two dots, "head minus base"), but the code uses three dots
($\{base\}...$\{head\}, "head minus merge-base"). Match the comment to
the code.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
docs/operate/setup.md (1)
118-118:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd trailing newline at end of file.
The file should end with a newline character per POSIX text file conventions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/operate/setup.md` at line 118, Add a trailing newline to the end of the docs/operate/setup.md file so it ends with a newline character (POSIX convention); simply open setup.md and ensure the last line is terminated with a newline character and save the file so the file ends with "\n".docs/use/workflows/ship.md (1)
126-126:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd trailing newline at end of file.
The file should end with a newline character per POSIX text file conventions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/use/workflows/ship.md` at line 126, The file docs/use/workflows/ship.md is missing a trailing newline; update the file by adding a single newline character at the end of the file so it ends with a POSIX-compliant newline (ensure your editor or the CI linter shows the file as ending with a newline).docs/operate/runbooks/stuck-ship-intent.md (1)
128-128:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd trailing newline at end of file.
The file should end with a newline character per POSIX text file conventions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/operate/runbooks/stuck-ship-intent.md` at line 128, Add a single trailing newline character to the end of the markdown file stuck-ship-intent.md so the file ends with a blank line (POSIX-compliant text file); open the file and ensure the final line terminates with \n and save.docs/operate/runbooks/triage.md (1)
64-64:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd trailing newline at end of file.
The file should end with a newline character per POSIX text file conventions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/operate/runbooks/triage.md` at line 64, Add a single trailing newline character at the end of triage.md so the file ends with a newline per POSIX conventions; open the document (docs/operate/runbooks/triage.md) and ensure the final line is terminated with '\n' so there is a blank line break at EOF rather than the file ending mid-line.docs/operate/configuration.md (1)
135-135:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd trailing newline at end of file.
The file should end with a newline character per POSIX text file conventions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/operate/configuration.md` at line 135, Add a single trailing newline character at the end of the Markdown file (ensure the file ends with '\n' at EOF) so it conforms to POSIX text file conventions; open the configuration.md document, place the cursor after the last character, insert one newline, save the file, and commit the change.docs/use/workflows/review.md (1)
57-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd trailing newline at end of file.
The file should end with a newline character per POSIX text file conventions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/use/workflows/review.md` at line 57, Add a single trailing newline character at the end of the file review.md so the file ends with a POSIX-compliant newline; open review.md, ensure the last line is terminated with '\n' (no extra content), save and commit the change.docs/use/workflows/resolve.md (1)
42-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd trailing newline at end of file.
The file should end with a newline character per POSIX text file conventions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/use/workflows/resolve.md` at line 42, The markdown file is missing a trailing newline at EOF; open the README/markdown file in the PR and add a single newline character at the end of the file (ensure the file is saved with POSIX line endings so the last character is '\n'), then re-commit the change so the file ends with a blank line.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/operate/deployment.md`:
- Around line 106-110: The `/readyz` docs row currently lists failure as `503
shutting down` but the actual readiness handler in src/app.ts returns `503 not
ready` whenever readiness is false; update the table entry for the `/readyz`
endpoint to show failure `503 not ready` (and optionally note that this covers
startup, dependency down, or shutdown) so the documentation matches the
readiness route implementation.
In `@docs/operate/observability.md`:
- Around line 46-59: The table of event keys is out of sync with the declared
canonical source SHIP_LOG_EVENTS; either update the table to only list the exact
keys defined in SHIP_LOG_EVENTS or update SHIP_LOG_EVENTS to include the missing
keys (e.g., ship.tickle.*, ship.scoped.<verb>.daemon.completed/failed,
ship.scoped.<verb>.daemon.completed) so they match; locate the constant
SHIP_LOG_EVENTS and the table in the docs, reconcile differences by
adding/removing entries and, if you change the constant, ensure tests/log
emitters reference the new keys (search for uses of SHIP_LOG_EVENTS and emitters
like tickleScheduler or connection-handler to verify consistency).
In `@docs/operate/setup.md`:
- Line 28: Update the relative link in the heading line that currently reads
"see operate/configuration.md" to "see configuration.md" so the reference from
docs/operate/setup.md points to the correct adjacent file; modify the string in
the heading (the line starting with "# Edit .env — see ...") to use
"configuration.md" instead of "operate/configuration.md".
In `@docs/use/safety.md`:
- Line 17: The doc currently overstates enforcement by saying
`src/workflows/handlers/resolve.ts` has an "in-source assertion" that
`octokit.rest.pulls.merge` is never called; update the text to accurately
reflect that the file only contains a comment requirement (no runtime guard) —
change the wording to state that the prohibition is documented in a comment
inside `src/workflows/handlers/resolve.ts` (or in the `resolve` handler) rather
than enforced, and remove any language asserting that `octokit.rest.pulls.merge`
is prevented at runtime.
In `@docs/use/workflows/plan.md`:
- Line 34: Rewrite the ambiguous sentence "`plan` is **fresh** while a
successful row exists for the issue created **after** the most recent triage
success. Re-applying the label after either succeeded run terminates re-runs the
latest stale step." into two explicit actions: (1) state that any currently
running stale run is immediately terminated when a newer successful run exists
for the issue (clarify “terminates” and when it happens), and (2) state that
re-applying the label after a successful run causes the latest stale step to be
re-run (clarify “re-runs” and the trigger). Replace the original single sentence
with these two clear, short sentences referencing the phrase `plan` and
“re-applying the label” so operators can unambiguously see both behaviors.
In `@mkdocs.yml`:
- Around line 117-121: The nav in mkdocs.yml references missing pages (nav keys
"Architecture", "Extending", "Conventions", "Contributing" pointing to
build/*.md) which breaks the build; either add the missing Markdown files
(create build/architecture.md, build/extending.md, build/conventions.md,
build/contributing.md with appropriate content) or update the mkdocs.yml entries
to point to existing docs (replace the build/*.md paths with the correct
relative paths for the existing pages) and ensure the "Changelog: changelog.md"
entry still points correctly; locate the nav block in mkdocs.yml and update the
paths or add the files to resolve the broken home-page architecture link.
---
Outside diff comments:
In `@docs/operate/configuration.md`:
- Line 135: Add a single trailing newline character at the end of the Markdown
file (ensure the file ends with '\n' at EOF) so it conforms to POSIX text file
conventions; open the configuration.md document, place the cursor after the last
character, insert one newline, save the file, and commit the change.
In `@docs/operate/runbooks/stuck-ship-intent.md`:
- Line 128: Add a single trailing newline character to the end of the markdown
file stuck-ship-intent.md so the file ends with a blank line (POSIX-compliant
text file); open the file and ensure the final line terminates with \n and save.
In `@docs/operate/runbooks/triage.md`:
- Line 64: Add a single trailing newline character at the end of triage.md so
the file ends with a newline per POSIX conventions; open the document
(docs/operate/runbooks/triage.md) and ensure the final line is terminated with
'\n' so there is a blank line break at EOF rather than the file ending mid-line.
In `@docs/operate/setup.md`:
- Line 118: Add a trailing newline to the end of the docs/operate/setup.md file
so it ends with a newline character (POSIX convention); simply open setup.md and
ensure the last line is terminated with a newline character and save the file so
the file ends with "\n".
In `@docs/use/workflows/resolve.md`:
- Line 42: The markdown file is missing a trailing newline at EOF; open the
README/markdown file in the PR and add a single newline character at the end of
the file (ensure the file is saved with POSIX line endings so the last character
is '\n'), then re-commit the change so the file ends with a blank line.
In `@docs/use/workflows/review.md`:
- Line 57: Add a single trailing newline character at the end of the file
review.md so the file ends with a POSIX-compliant newline; open review.md,
ensure the last line is terminated with '\n' (no extra content), save and commit
the change.
In `@docs/use/workflows/ship.md`:
- Line 126: The file docs/use/workflows/ship.md is missing a trailing newline;
update the file by adding a single newline character at the end of the file so
it ends with a POSIX-compliant newline (ensure your editor or the CI linter
shows the file as ending with a newline).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8bda4346-84fb-4002-a7e6-cd050ed13292
📒 Files selected for processing (35)
.prettierignoredocs/ARCHITECTURE.mddocs/BOT-WORKFLOWS.mddocs/CONFIGURATION.mddocs/CONTRIBUTING.mddocs/DAEMON.mddocs/DEPLOYMENT.mddocs/EXTENDING.mddocs/OBSERVABILITY.mddocs/SETUP.mddocs/SHIP.mddocs/TRIAGE.mddocs/changelog.mddocs/index.mddocs/operate/configuration.mddocs/operate/deployment.mddocs/operate/github-app.mddocs/operate/observability.mddocs/operate/runbooks/daemon-fleet.mddocs/operate/runbooks/stuck-ship-intent.mddocs/operate/runbooks/triage.mddocs/operate/setup.mddocs/use/invoking.mddocs/use/safety.mddocs/use/workflows/implement.mddocs/use/workflows/index.mddocs/use/workflows/plan.mddocs/use/workflows/resolve.mddocs/use/workflows/review.mddocs/use/workflows/ship.mddocs/use/workflows/triage.mdmkdocs.ymlscripts/check-docs-sync.tssrc/config.tstest/config.test.ts
💤 Files with no reviewable changes (13)
- docs/CONTRIBUTING.md
- docs/SHIP.md
- docs/ARCHITECTURE.md
- docs/TRIAGE.md
- docs/SETUP.md
- docs/DEPLOYMENT.md
- docs/DAEMON.md
- test/config.test.ts
- src/config.ts
- docs/CONFIGURATION.md
- docs/BOT-WORKFLOWS.md
- docs/OBSERVABILITY.md
- docs/EXTENDING.md
Comment on lines
+46
to
+59
| Every shepherding emitter draws its `event` value from the typed `SHIP_LOG_EVENTS` constant in `src/workflows/ship/log-fields.ts`. Operators can grep for these literals deterministically. | ||
|
|
||
| | Event key | Where it fires | What it indicates | | ||
| | ------------------------------------- | --------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------- | | ||
| | `ship.iteration.enqueued` | `iteration.runIteration` after `enqueueJob` | A non-ready verdict bridged into the daemon `workflow_runs` pipeline. One row per iteration. | | ||
| | `ship.iteration.terminal_cap` | `iteration.runIteration` cap check | The intent hit `MAX_SHIP_ITERATIONS`. | | ||
| | `ship.iteration.terminal_deadline` | `iteration.runIteration` deadline check | The intent's `deadline_at` elapsed. | | ||
| | `ship.tickle.started` | `app.ts` boot, after `tickleScheduler.start()` | The cron tickle scheduler is scanning `ship:tickle`. | | ||
| | `ship.tickle.due` | `orchestrator.onStepComplete` early-wake **or** `session-runner.resumeShipIntent` | An intent is being re-entered. `source` discriminates `workflow_run_completion` vs scheduler. | | ||
| | `ship.tickle.skip_terminal` | `orchestrator.onStepComplete` early-wake | The hook found a `shipIntentId` but the intent is already terminal; the ZADD was skipped. | | ||
| | `ship.scoped.<verb>.enqueued` | `dispatch-scoped.ts` after `enqueueJob` | A scoped command (`rebase`, `fix_thread`, `explain_thread`, `open_pr`) was enqueued. | | ||
| | `ship.scoped.<verb>.daemon.completed` | `connection-handler.handleScopedJobCompletion` and the executor | Daemon reported `succeeded`. | | ||
| | `ship.scoped.<verb>.daemon.failed` | Same | Daemon reported `halted` or `failed`. `reason` carries the structured halt reason. | | ||
|
|
There was a problem hiding this comment.
Event-key table is inconsistent with the declared canonical source.
This section says keys come from src/workflows/ship/log-fields.ts, but the table lists keys not present in SHIP_LOG_EVENTS (e.g., ship.tickle.*, ship.scoped.<verb>.daemon.completed). Please align the table to the actual constant (or update the constant if these keys are now canonical).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/operate/observability.md` around lines 46 - 59, The table of event keys
is out of sync with the declared canonical source SHIP_LOG_EVENTS; either update
the table to only list the exact keys defined in SHIP_LOG_EVENTS or update
SHIP_LOG_EVENTS to include the missing keys (e.g., ship.tickle.*,
ship.scoped.<verb>.daemon.completed/failed, ship.scoped.<verb>.daemon.completed)
so they match; locate the constant SHIP_LOG_EVENTS and the table in the docs,
reconcile differences by adding/removing entries and, if you change the
constant, ensure tests/log emitters reference the new keys (search for uses of
SHIP_LOG_EVENTS and emitters like tickleScheduler or connection-handler to
verify consistency).
Owner
Author
There was a problem hiding this comment.
Declining — the keys are present in SHIP_LOG_EVENTS. src/workflows/ship/log-fields.ts (lines 78–81) declares ship.tickle.{started,due,skip_terminal,skip_failed_child}, and lines 85–102 declare ship.scoped.<verb>.daemon.{completed,failed} for verbs rebase, fix_thread, explain_thread, open_pr. The doc table matches the constant; no doc change needed.
There was a problem hiding this comment.
@chrisleekr, thanks for the correction — I was wrong to flag this. I'll note the full scope of SHIP_LOG_EVENTS for future reviews.
(´• ω •`) 🐇
flicks ear thoughtfully
✏️ Learnings added
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 87
File: docs/operate/observability.md:46-59
Timestamp: 2026-05-01T09:53:25.427Z
Learning: In `src/workflows/ship/log-fields.ts` (chrisleekr/github-app-playground), `SHIP_LOG_EVENTS` includes more than just FR-018 iteration-wiring keys. As of PR `#87`, lines 78–81 declare `ship.tickle.{started,due,skip_terminal,skip_failed_child}` and lines 85–102 declare `ship.scoped.<verb>.daemon.{completed,failed}` for verbs `rebase`, `fix_thread`, `explain_thread`, `open_pr`. The `docs/operate/observability.md` event-key table is aligned with these constants. Do not flag the observability doc table as out of sync with SHIP_LOG_EVENTS.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 79
File: src/workflows/ship/log-fields.ts:65-105
Timestamp: 2026-04-30T13:06:19.073Z
Learning: In `src/workflows/ship/log-fields.ts` (chrisleekr/github-app-playground), `ShipLogFieldsSchema.event` is intentionally `z.string().min(1)`. `SHIP_LOG_EVENTS` covers only the FR-018 iteration-wiring subset; the full `ship.*` event space is much larger (ship.intent.create, ship.intent.transition, ship.lifecycle.*, ship.session_started, ship.reactor.*, ship.tickle.*, ship.probe.*, ship.ready_for_review*, ship.scoped.*, and more). `test/workflows/ship/log-fields.test.ts` line 11 validates `event: "ship.intent.transition"` against the schema, which would fail if event were narrowed to SHIP_LOG_EVENTS literals. The correct tightening requires two steps: (a) decide whether SHIP_LOG_EVENTS becomes the union of ALL ship.* keys or stays FR-018-scoped, and (b) update every emitter plus the round-trip test in lockstep. This work is deferred to a follow-up PR. Do not re-flag `event: z.string().min(1)` as a schema weakness until that follow-up lands.
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 79
File: src/workflows/ship/log-fields.ts:65-105
Timestamp: 2026-04-30T08:47:26.723Z
Learning: In `src/workflows/ship/log-fields.ts` (chrisleekr/github-app-playground), `ShipLogFieldsSchema.event` is intentionally typed as `z.string().min(1)` rather than a narrow enum derived from `SHIP_LOG_EVENTS`. `ShipLogFieldsSchema` covers ALL ship log events across the entire `src/workflows/ship/` surface (e.g., `ship.intent.transition` and others emitted outside the iteration-wiring path), so tightening `event` to only the `SHIP_LOG_EVENTS` iteration-wiring literals would break existing round-trip tests and reject legitimate events. The correct fix requires first enumerating every ship-emitter event into a single canonical list; that work is deferred to a follow-up PR. Do not re-flag `event: z.string().min(1)` as a schema weakness until that follow-up lands.
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 79
File: src/workflows/ship/iteration.ts:142-222
Timestamp: 2026-04-30T13:06:21.038Z
Learning: In `src/workflows/ship/iteration.ts` (chrisleekr/github-app-playground PR `#79`), the same Postgres→Valkey non-atomic split (`insertQueued` / `enqueueJob` / `appendIteration`) also exists in `src/workflows/orchestrator.ts` (`recordWorkflowExecution` → `enqueueJob`). Fixing only the iteration path without the orchestrator dispatch site would leave the inconsistency in the larger code path. The planned follow-up introduces a `workflow_outbox` table + reconciler covering both dispatch sites uniformly. Current recovery properties: (1) dangling `workflow_runs` rows are reaped by the existing reaper/abort path; (2) a missing `ship_iterations` action row only undercounts the cap by one (does not overshoot — deadline terminates anyway); (3) the unique partial index `idx_workflow_runs_inflight` on `(workflow_name, owner, repo, number)` prevents the most damaging double-dispatch scenario.
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 0
File: :0-0
Timestamp: 2026-04-28T11:17:15.394Z
Learning: In chrisleekr/github-app-playground `src/workflows/ship/tracking-comment.ts`, the canonical tracking-comment marker format is `<!-- ship-intent:{id}
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 79
File: src/workflows/ship/iteration.ts:141-205
Timestamp: 2026-04-30T08:21:22.835Z
Learning: In `src/workflows/ship/iteration.ts` (chrisleekr/github-app-playground PR `#79`), the three cross-store writes `insertQueued` (Postgres) / `enqueueJob` (Valkey) / `appendIteration` (Postgres) are intentionally non-atomic in this slice. The proper fix — an outbox pattern where a single Postgres transaction writes `workflow_runs` + an outbox row, and a separate worker drains the outbox into Valkey — is deferred to a follow-up PR touching `runs-store.ts`, `job-queue.ts`, and a new outbox table. Current mitigations: `workflow_runs.status='queued'` is the durable record, and `findInflightShipIntentRun` in-flight guard prevents double-enqueue when Valkey reads succeed but cap accounting is stale. Do not re-flag this as a blocker for the current PR.
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 79
File: src/workflows/ship/scoped/dispatch-scoped.ts:71-77
Timestamp: 2026-04-30T08:21:25.884Z
Learning: In `src/workflows/ship/scoped/dispatch-scoped.ts` (chrisleekr/github-app-playground), `deriveTriggerCommentId` returns a sentinel value of `1` when `CanonicalCommand` has no `thread_id` (e.g., `rebase`, `open-pr` surface triggers). This is intentional and documented: downstream consumers MUST treat `1` as "unknown" rather than a real comment id. The daemon-side tracking-comment durable idempotency layer is the actual dedup boundary for these cases. A follow-up is planned to widen `CanonicalCommand` to carry the maintainer's trigger comment id for non-thread surfaces and switch `deriveTriggerCommentId` to return `number | undefined`.
Learnt from: CR
Repo: chrisleekr/github-app-playground PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-01T07:55:52.355Z
Learning: Keep documentation in `docs/` synced with code changes. Update corresponding doc page when modifying: `src/config.ts` → `docs/CONFIGURATION.md`, `src/shared/dispatch-types.ts` → `docs/OBSERVABILITY.md` + `docs/ARCHITECTURE.md`, `src/orchestrator/triage.ts` → `docs/TRIAGE.md`, `src/webhook/` → `docs/ARCHITECTURE.md`, `src/k8s/ephemeral-daemon-spawner.ts` → `docs/DAEMON.md` + `docs/DEPLOYMENT.md`, `src/daemon/` → `docs/DAEMON.md`, `src/mcp/` → `docs/EXTENDING.md`, Pino fields/metrics → `docs/OBSERVABILITY.md`
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 0
File: :0-0
Timestamp: 2026-04-27T07:48:21.117Z
Learning: In `src/workflows/handlers/triage.ts` (chrisleekr/github-app-playground), the `summary` field in `verdictSchema` / `reproductionSchema` is intentionally uncapped. `docs/BOT-WORKFLOWS.md` line 64 states the field length is "uncapped (the agent writes as much as the verdict honestly requires)". Do not suggest adding a length cap to `summary` without a corresponding docs change.
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 79
File: src/workflows/ship/iteration.ts:76-82
Timestamp: 2026-04-30T13:06:27.463Z
Learning: In `src/workflows/ship/iteration.ts` (chrisleekr/github-app-playground PR `#79`), the in-flight guard `findInflightShipIntentRun` has a race window for two concurrent `runIteration` calls picking **different** workflow names for the same intent (e.g., `failing_checks` → `resolve` vs `behind_base` → `implement`). The existing partial unique index `idx_workflow_runs_inflight` (`UNIQUE (workflow_name, target_owner, target_repo, target_number) WHERE status IN ('queued','running')`) only catches same-workflow duplicates. Three fix options are tracked for a follow-up after the outbox refactor: (1) a partial functional unique index `UNIQUE ((state->>'shipIntentId')) WHERE status IN ('queued','running')` with duplicate-key retry handling, (2) `SELECT … FOR UPDATE` on the `ship_intents` row at the top of `runIteration`, or (3) a pg advisory lock keyed on `hashtext(intent.id)`. Blast radius is bounded by the iteration cap check. Linked to outbox-refactor thread PRRT_kwDORS-R1M5-iPBA. Do not re-flag this race as a blocker for the current PR.
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 77
File: src/workflows/ship/scoped/open-pr.ts:138-158
Timestamp: 2026-04-29T09:58:46.819Z
Learning: In `src/workflows/ship/scoped/open-pr.ts` (chrisleekr/github-app-playground), the race window between `findExistingBackLink` and `createBranchAndPr` is real but latent in v1 because `createBranchAndPr` is unwired (dispatch-scoped.ts throws immediately). The correct fix is a Postgres advisory lock keyed on `(owner, repo, issue_number, 'open-pr')` to serialize concurrent webhooks at the DB layer. The two-layer Map+isAlreadyProcessed pattern from the webhook router is NOT applicable here because different webhook deliveries can both legitimately request `bot:open-pr` on the same issue, making X-GitHub-Delivery unsuitable as a dedup key. Fix is deferred to land alongside the daemon-side helper wiring.
Learnt from: chrisleekr
Repo: chrisleekr/github-app-playground PR: 77
File: src/workflows/ship/trigger-router.ts:67-83
Timestamp: 2026-04-28T21:54:28.031Z
Learning: In `src/workflows/ship/trigger-router.ts` (`withSurface` function), the sequential intermediate-object spread pattern (base → withDeadline → withEventSurface → final) is intentional. It enforces `exactOptionalPropertyTypes` correctness: optional fields (`deadline_ms`, `event_surface`, `thread_id`) are only present on the object when their source value is defined, preventing any accidental `undefined` assignment that the strict compiler setting would reject. Do not suggest collapsing this into a single object literal with conditional spreads.
Learnt from: CR
Repo: chrisleekr/github-app-playground PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-05-01T07:55:52.355Z
Learning: Validate docs builds locally with `bun run docs:build` before pushing. Flag documentation gaps in PR description if no matching doc exists yet, rather than shipping silently
- deployment.md: /readyz failure body is "503 not ready" (per src/app.ts:165), not "503 shutting down". Reword to cover startup, dependency-down, and SIGTERM cases. - setup.md: relative reference from docs/operate/setup.md to configuration.md is just "configuration.md", not "operate/configuration.md". - safety.md: tone down the resolve.ts merge guard claim — there is no runtime assertion, just a non-negotiable comment requirement and the static guard documented above it. - workflows/plan.md: rewrite the garbled re-trigger sentence into two clear statements. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Owner
Author
|
🎉 This PR is included in version 1.7.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The public docs site (
docs/) had grown 11 mixed-audience pages with leaked spec references (FR-NNN, datedspecs/<feature>/...paths) and a tiny 41-lineSHIP.mdthat no longer matched the post-PR-#79 shepherding lifecycle. This PR rewrites the site as an audience-first reorg, strips the spec leakage, and removes two dead config flags surfaced by the audit.Diagram
flowchart LR subgraph BeforeAfter["Before vs. after — docs/ layout"] direction LR OldRoot["docs/<br/>flat 11 files<br/>shouty UPPERCASE.md"]:::old OldList["ARCHITECTURE.md<br/>BOT-WORKFLOWS.md<br/>CONFIGURATION.md<br/>CONTRIBUTING.md<br/>DAEMON.md<br/>DEPLOYMENT.md<br/>EXTENDING.md<br/>OBSERVABILITY.md<br/>SETUP.md<br/>SHIP.md<br/>TRIAGE.md"]:::old OldRoot --> OldList NewRoot["docs/<br/>three audience tabs<br/>kebab-case.md"]:::new UseTab["use/<br/>invoking + 6 workflows + safety"]:::new OperateTab["operate/<br/>setup + github-app + deployment<br/>+ configuration + observability<br/>+ 3 runbooks"]:::new BuildTab["build/<br/>architecture + extending<br/>+ conventions + contributing"]:::new NewRoot --> UseTab NewRoot --> OperateTab NewRoot --> BuildTab end classDef old fill:#852020,stroke:#4d1212,color:#ffffff classDef new fill:#1a4d3a,stroke:#0d2c20,color:#ffffffChanges
Docs reorg (
docs/)use/— invoking, workflows catalog, six per-workflow pages, safetyoperate/— local setup, GitHub App creation, deployment, configuration, observability, three runbooks (daemon-fleet, triage, stuck-ship-intent)build/— architecture, extending, conventions, contributingdocs/CHANGELOG.md→docs/changelog.md(snippet-include preserved).FR-NNN/SC-NNN/Tnnn/R8/Q5mentions andspecs/<dated-feature>/...paths.bot:shipdocumentation to reflect the post-PR-feat(ship): wire ship iteration loop, tickle scheduler, and four scoped executors #79 shepherding lifecycle only; the legacy composite cascade is no longer documented.workflow_runs, comment-to-workflow router).mkdocs.yml.prettierignoredocs/index.mdso Prettier no longer collapses the Material grid-cards 4-space indent (caught as a UI regression during local screenshot pass — the cards were staircasing instead of laying out as three columns).scripts/check-docs-sync.tsDOC_PATHregex from^docs/BOT-WORKFLOWS\.md$to^docs/use/workflows/.*\.md$so future PRs touchingsrc/workflows/**are still gated against doc updates.src/config.ts+test/config.test.tsSHIP_USE_PROBE_VERDICTandSHIP_USE_CONTINUATION_LOOP. Both were declared in the env schema and parsed inloadConfig()but never read anywhere insrc/— confirmed by grep across the tree. Tests covering them removed.Related Issues
Test plan
bun run typecheck— cleanbun run lint— 0 errors (pre-existing warnings unchanged)bun run format— cleanbun run docs:build(strict) — cleanbun run check:no-destructive— cleanbun run test:fast test/config.test.ts— 40 / 40 passpython3 -m http.server+ headless Chrome — every page renders, mermaid blocks render, grid cards lay out three columns on desktop and stack on mobile, large tables wrap correctly.Summary by CodeRabbit
Documentation
Bug Fixes & Changes