mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-14 17:40:37 +02:00
docs: define heartbeat execution design principles (#16190)
## Thinking Path > - Paperclip coordinates AI agents and their work. > - Heartbeat controls when that work starts, who owns execution, and how it settles. > - The extracted modules need shared architectural guidance as they evolve. > - Rules should protect those boundaries without freezing the current implementation. > - This PR adds scoped heartbeat principles for contributors and reviewers. > - The rule helps future changes preserve execution authority and recoverable work. ## Linked Issues or Issue Description **Issue type** Missing documentation. **Where is the issue?** The heartbeat service has a module map but no scoped architectural rule under `rules/`. **What's wrong?** Changes span admission, execution, completion, and recovery. Reviewers need shared principles for authority, ownership, and settlement across these boundaries. **Suggested fix** Add a concise heartbeat rule, route contributors to it from `AGENTS.md`, and register the same scope for Greptile. Keep implementation details in the code and existing developer documentation. Related: #16172 established scoped repository rules. #15984 completed the latest heartbeat extraction. Searches found no duplicate heartbeat-guidelines PR. ## What Changed - Add `rules/heartbeat-execution-boundaries.mdc` with a folder guide, a concise map of existing module responsibilities, and six design principles. - Keep `heartbeat.ts` focused on coordination: extend an existing owner or extract cohesive modules when new behavior would substantially grow it, with tests and explicit contracts. - Cover current execution authority, explicit ownership, distinct completion obligations, recovery intent, durable state, and visible uncertainty. - Register matching paths in `.greptile/files.json` and link the rule from `AGENTS.md` and `doc/DEVELOPING.md`. - Include behavioral review criteria and flag substantial coordinator growth or mixed responsibilities while allowing justified cohesion and ordering exceptions. ## Verification - Parsed the rule YAML and Greptile JSON. Confirmed that the rule globs exactly match the Greptile scope. - Checked 14 positive and negative scope cases with Node's `path.matchesGlob`, including nested heartbeat modules, sibling services, regression tests, and unrelated paths. - Verified local links and heading anchors, including the existing heartbeat module map, and confirmed all 20 named modules exist. Reviewed their responsibility descriptions against the current code and developer documentation. - `git diff origin/master --check` passed after rebasing onto current master. - Reviewed the principles against the current heartbeat code and tests. The diff changes four documentation and review-configuration files. - Application typecheck, runtime tests, and build were not run locally. This worktree has no installed dependencies. CI and review status are tracked below. - CI passed on `a17f5a2cc82b7bb7e4629d4a2d9e3a766e27c5c0`: 55 successful checks, two skipped, no failures or pending checks. This includes typecheck, build, server/runner tests, and browser tests. - Greptile reviewed the same commit at 5/5 with no findings or unresolved review threads. ## Risks The rule can influence later implementation and review decisions. It applies to changed heartbeat behavior and permits justified contract changes with tests. The scoped globs exclude unrelated subsystems. This PR does not change application behavior. Hosted loading of the new rule has not been demonstrated by the local configuration checks. ## Model Used OpenAI GPT-6 Astra (`gpt-6-astra`) via Codex, with repository inspection, file editing, and command execution. The context-window size was not exposed to this session. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass (scoped rule/configuration checks described above) - [x] I have added or updated tests where applicable (documentation/configuration validation; no runtime changes) - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
e3029c632b
commit
e67db42dc9
4 files changed
+144
No files matched your search
@@ -19,6 +19,16 @@
|
||||
"server/src/services/chat-channels/**",
|
||||
"server/src/services/chat-channels.ts"
|
||||
]
|
||||
},
|
||||
{
|
||||
"path": "rules/heartbeat-execution-boundaries.mdc",
|
||||
"description": "Heartbeat module ownership and review criteria for admission, cancellation, recovery, cleanup, and durable execution evidence.",
|
||||
"scope": [
|
||||
"server/src/services/heartbeat.ts",
|
||||
"server/src/services/heartbeat/**",
|
||||
"server/src/services/heartbeat-*.ts",
|
||||
"server/src/__tests__/heartbeat-*.test.ts"
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -38,6 +38,9 @@ nested `AGENTS.md` instructions.
|
||||
For `server/src/services/chat-channels.ts` and everything under
|
||||
`server/src/services/chat-channels/`, read
|
||||
[`rules/chat-channel-platform-boundaries.mdc`](rules/chat-channel-platform-boundaries.mdc).
|
||||
For `server/src/services/heartbeat.ts`, `server/src/services/heartbeat/`,
|
||||
heartbeat-prefixed sibling services, and heartbeat regression tests, read
|
||||
[`rules/heartbeat-execution-boundaries.mdc`](rules/heartbeat-execution-boundaries.mdc).
|
||||
See [`rules/README.md`](rules/README.md) for rule authoring and Greptile setup.
|
||||
|
||||
## 3. Repo Map
|
||||
|
||||
@@ -1363,6 +1363,8 @@ When a workspace service runs Paperclip for browser OAuth QA, configure its `exp
|
||||
## Heartbeat Service Extractions
|
||||
|
||||
Keep relevant heartbeat extractions and their focused tests in `server/src/services/heartbeat/`.
|
||||
Follow [Heartbeat execution boundaries](../rules/heartbeat-execution-boundaries.mdc)
|
||||
for state lifetime, admission, recovery ownership, cleanup ordering, and verification.
|
||||
|
||||
Runtime-service coordination is in `server/src/services/heartbeat/runtime-services.ts`.
|
||||
It validates service intents, starts configured services, records adapter-managed
|
||||
|
||||
@@ -0,0 +1,129 @@
|
||||
---
|
||||
description: Keep heartbeat a clear lifecycle coordinator with cohesive modules, explicit execution authority, and safe resource ownership.
|
||||
globs: "server/src/services/heartbeat.ts,server/src/services/heartbeat/**,server/src/services/heartbeat-*.ts,server/src/__tests__/heartbeat-*.test.ts"
|
||||
alwaysApply: false
|
||||
---
|
||||
|
||||
# Heartbeat execution boundaries
|
||||
|
||||
## Scope and purpose
|
||||
|
||||
This rule covers the heartbeat entry point, its extracted modules,
|
||||
heartbeat-prefixed sibling services, and heartbeat regression tests. Keep the
|
||||
code organized so a reader can follow whether work may start, who owns the
|
||||
execution, and how its outcome and resources settle.
|
||||
|
||||
## Folder structure and module responsibilities
|
||||
|
||||
`server/src/services/heartbeat.ts` is the public entry point and lifecycle
|
||||
coordinator. Heartbeat-specific modules and their focused `*.test.ts` tests belong
|
||||
in `server/src/services/heartbeat/`. Tests that exercise the assembled heartbeat
|
||||
service also live in `server/src/__tests__/heartbeat-*.test.ts`. Keep these roles
|
||||
distinct: the entry point should make the lifecycle readable, while modules own
|
||||
the behavior of a particular phase or domain.
|
||||
|
||||
Use these existing boundaries when deciding where a change belongs. Paths below
|
||||
are relative to `server/src/services/heartbeat/`:
|
||||
|
||||
| Modules | Responsibility |
|
||||
| --- | --- |
|
||||
| `queue.ts`, `scheduling.ts` | Queue admission, coalescing, and dispatch; timer and monitor scheduling. |
|
||||
| `run-preparation.ts`, `execution-configuration.ts` | Run context and tool access; effective execution configuration and credentials. |
|
||||
| `session-preparation.ts`, `run-state.ts` | Session selection and resume preparation; persisted run/session state and retrieval. |
|
||||
| `workspaces.ts`, `workspace-preparation.ts` | Workspace resolution, reuse, and validation; preparing the selected workspace and acquiring its environment. |
|
||||
| `environment-selection.ts`, `runtime-selection.ts` | Execution environment policy; selecting or restoring the execution runtime. |
|
||||
| `runtime-execution.ts` | Dispatching prepared work through the runtime and handling its execution result. |
|
||||
| `run-completion.ts`, `run-lifecycle.ts` | Success/failure settlement; shared status transitions, events, and completion handoffs. |
|
||||
| `run-control.ts`, `retries.ts`, `recovery.ts` | Stop and cancellation; retry scheduling; restart recovery and unfinished cleanup. |
|
||||
| `task-context.ts`, `task-markdown.ts` | Assembling task context and rendering assignment text. |
|
||||
| `run-output.ts`, `run-log.ts` | Per-run output handling; log formatting, redaction, and payload bounds. |
|
||||
|
||||
This is a responsibility map, not an exhaustive API inventory. See the
|
||||
[developer module map](../doc/DEVELOPING.md#heartbeat-service-extractions) for
|
||||
additional modules and implementation context. Update these descriptions when
|
||||
ownership changes.
|
||||
|
||||
## Keep the coordinator from growing back
|
||||
|
||||
- Keep `heartbeat.ts` roughly stable or smaller as features are added. Small
|
||||
wiring and lifecycle-ordering changes belong there; substantial new policy,
|
||||
preparation, provider handling, or result processing belongs in a module.
|
||||
Access to convenient local variables is not a reason to add another large
|
||||
block to the coordinator.
|
||||
- Extend the existing module that owns the behavior before creating another.
|
||||
If a new responsibility has no owner, add a clearly named module under
|
||||
`heartbeat/`. Avoid generic `helpers.ts` or `utils.ts` buckets and a separate
|
||||
file for every small function.
|
||||
- When a change would substantially grow the coordinator, extract the affected
|
||||
responsibility in the same PR, backed by characterization tests. If an extracted
|
||||
module begins accumulating unrelated responsibilities, split it at a cohesive
|
||||
boundary too. Moving a large block into another file without clarifying its
|
||||
contract is not enough.
|
||||
- Preserve visible lifecycle ordering and resource ownership across extractions.
|
||||
Keep coupled decisions together when splitting them would obscure correctness;
|
||||
explain that tradeoff when substantial growth is necessary. Size is a signal
|
||||
to review the boundary, not a reason to split an otherwise cohesive operation.
|
||||
- Keep provider protocols and environment mechanics in their adapter/runtime
|
||||
services. Heartbeat should coordinate through their contracts. Small dispatch
|
||||
branches are appropriate; backend details should not spread into shared policy.
|
||||
- Use narrow, typed contracts that make inputs, outcomes, and resource ownership
|
||||
clear. Avoid passing the whole service closure or a generic mutable context.
|
||||
Production dependencies should flow from the facade into its modules, without
|
||||
modules importing the facade back.
|
||||
|
||||
## Design principles
|
||||
|
||||
- **Execution requires current authority.** Queue acceptance is not permission
|
||||
to execute indefinitely. Admission and dispatch must respect company scope,
|
||||
responsible-user authority, budgets, pause/Stop, and task eligibility, including
|
||||
when these change during preparation. Retries and recovery must not become
|
||||
alternate paths around those controls.
|
||||
- **Ownership must be explicit.** Every execution and resource needs an owner,
|
||||
a lifetime, and a defined handoff or release condition. These contracts must
|
||||
hold across service instances, cancellation, partial startup, and restart.
|
||||
Process-local coordination cannot establish authority over another process.
|
||||
- **Completion has several independent obligations.** A result being accepted,
|
||||
a provider stopping, a workspace being saved, and cleanup finishing are distinct
|
||||
facts. Keep enough state to represent them honestly. Do not infer that resources
|
||||
are safe to destroy or that successor work may start from a terminal status alone.
|
||||
- **Recovery preserves intent and completed work.** Resume the admitted work
|
||||
under its recorded execution contract. New configuration belongs to a new
|
||||
admission decision, not a silent reinterpretation of an existing run. Repair
|
||||
incomplete persistence or cleanup without repeating already completed provider
|
||||
work. Automatic recovery should be bounded and safe to retry.
|
||||
- **Durable state governs competing effects.** Claims and transitions must remain
|
||||
correct when Stop, completion, and recovery race. A losing operation must respect
|
||||
the winning outcome. Persist authoritative state before publishing notifications;
|
||||
notification failures must not change the execution result or duplicate effects.
|
||||
- **Uncertainty stays visible.** Missing ownership or termination evidence must
|
||||
remain pending, held, or explicitly failed until reconciled. Preserve recoverable
|
||||
work through failures. Keep security and sandbox requirements intact rather than
|
||||
silently choosing a weaker execution path. Optional diagnostics must not prevent
|
||||
essential settlement.
|
||||
|
||||
## Examples and allowed exceptions
|
||||
|
||||
- Retrying workspace copy-back after an accepted result is recovery work; it
|
||||
should not start another provider turn merely to repeat the completed task.
|
||||
- A shared checkout cache and per-run preparation state have different lifetimes.
|
||||
Organize them by the ownership they need, rather than making all state local to
|
||||
whichever factory happens to use it.
|
||||
- Keep coupled admission or cleanup decisions together when splitting them would
|
||||
hide their ordering. A large cohesive module can be appropriate. Apply these
|
||||
principles to new or materially changed behavior without requiring unrelated
|
||||
restructuring or a generic orchestration framework.
|
||||
|
||||
## Verification and review
|
||||
|
||||
Test the affected contract through observable behavior, including relevant
|
||||
cancellation, concurrency, partial-failure, and restart cases. Exercise the public
|
||||
heartbeat path when a module change could alter lifecycle coordination; use an
|
||||
assembled-app journey for changes to dispatch, Stop, workspace recovery, or saved
|
||||
agent files. Assert execution and persisted outcomes, not only helper calls.
|
||||
|
||||
Flag changes that give one policy competing owners, hide lifecycle ordering,
|
||||
weaken execution authority, lose recoverable work, or confuse completion with
|
||||
resource settlement. Explain the violated principle and the responsible boundary.
|
||||
Also flag substantial coordinator growth or modules collecting unrelated behavior;
|
||||
identify the existing owner or cohesive extraction that would keep the code
|
||||
tractable. Do not freeze current variable names or helper layouts as policy.
|
||||
Reference in new issue
Block a user