Enforce bounded operation guardrails
Assistant: codex Assistant-Model: gpt-5.6-sol Assistant-Session: 01a028de-e2c8-7732-8521-46a7fc5db82f
This commit is contained in:
parent
c384f60530
commit
26934e25b9
51 changed files with 1843 additions and 472 deletions
|
|
@ -143,14 +143,11 @@ Three trigger types are supported:
|
|||
### Immediate
|
||||
|
||||
- activity-core's `INTENT.md` and `SCOPE.md` are rewritten to reflect this architecture.
|
||||
- The `task_instances` Postgres table is reclassified as a **spawn audit trail** —
|
||||
it records the act of spawning (what was created, when, which issue-core reference)
|
||||
but is not the authoritative task record. Authoritative lifecycle state lives in
|
||||
issue-core.
|
||||
- A task emission adapter interface (`src/activity_core/issue_sink.py`) replaces any
|
||||
direct Postgres writes to `task_instances` with calls through the adapter.
|
||||
- The `TaskExecutorWorkflow` stub from WP-0001 is replaced with the actual adapter
|
||||
call in WP-0003.
|
||||
- `task_spawn_log` is the local **spawn audit trail**; authoritative work-item
|
||||
lifecycle state lives downstream.
|
||||
- A task emission adapter (`src/activity_core/issue_sink.py`) owns downstream
|
||||
creation. The unused `TaskExecutorWorkflow` and `task_instances` compatibility
|
||||
surface was retired by ACTIVITY-WP-0035.
|
||||
|
||||
### Medium term
|
||||
|
||||
|
|
|
|||
|
|
@ -227,7 +227,7 @@ trusted_fields:
|
|||
- event.attributes.domain
|
||||
- event.attributes.tags
|
||||
model: claude-sonnet-4-6
|
||||
review_required: false
|
||||
review_advisory: false
|
||||
prompt: |
|
||||
A new repository has been registered in the Coulomb organization.
|
||||
|
||||
|
|
|
|||
|
|
@ -185,7 +185,7 @@ trusted_fields: # REQUIRED — explicit allowlist of payload
|
|||
- event.attributes.domain
|
||||
- event.attributes.tags
|
||||
model: claude-sonnet-4-6
|
||||
review_required: false # true | false — curator gate for output
|
||||
review_advisory: false # true | false — advisory evidence, not a gate
|
||||
prompt: |
|
||||
{prompt template — only trusted_fields may be interpolated}
|
||||
output_schema: {path to JSON schema file}
|
||||
|
|
@ -218,23 +218,19 @@ created from unvalidated output**.
|
|||
Structured output mode (tool_use / JSON mode) is used where the model supports
|
||||
it. The output schema must define `List[TaskSpec]` or a compatible envelope.
|
||||
|
||||
#### `review_required: true`
|
||||
#### `review_advisory: true`
|
||||
|
||||
When set today, the instruction's task/report output is marked with
|
||||
`review_required=true` in activity-core audit metadata. For report-producing
|
||||
instructions, this flag is also persisted in configured report sinks so an
|
||||
operator can distinguish validated-but-review-worthy output from routine
|
||||
output.
|
||||
ACTIVITY-WP-0035 selected advisory-only semantics because no downstream owner
|
||||
currently exposes an acknowledged proposal/decision/release contract. The
|
||||
instruction's task/report evidence is marked `review_advisory=true` and
|
||||
`review_gate_applied=false` so an operator can distinguish review-worthy output
|
||||
without interpreting it as held for approval.
|
||||
|
||||
activity-core does **not** currently route proposed tasks to a pending review
|
||||
queue. That queue must be owned by issue-core, because issue-core owns task
|
||||
lifecycle state. Until issue-core exposes a review contract, `review_required`
|
||||
is metadata only; it must not be treated as evidence that live task creation was
|
||||
held for approval.
|
||||
|
||||
Future issue-core review integration may use the same field, but that change
|
||||
must update the issue sink contract and tests before any ActivityDefinition
|
||||
relies on queue routing.
|
||||
The legacy input name `review_required` is accepted during migration and
|
||||
normalized to `review_advisory`; new definitions must not use it. activity-core
|
||||
does **not** route proposals to a pending-review queue and does not own review
|
||||
lifecycle state. A future hold/release design requires a named downstream owner,
|
||||
an idempotent release reference, an ADR update, and fail-closed emission tests.
|
||||
|
||||
#### Evaluation semantics
|
||||
|
||||
|
|
@ -259,11 +255,20 @@ Every task emission records:
|
|||
| `prompt_hash` | — | SHA-256 of rendered prompt |
|
||||
| `model` | — | model ID used |
|
||||
| `output_validated` | — | `true` / `false` |
|
||||
| `review_required` | — | `true` / `false` |
|
||||
| `review_advisory` | — | `true` / `false`; no gate applied |
|
||||
|
||||
The audit trail is written to the `task_spawn_log` table in activity-core's database
|
||||
and referenced from the task record in issue-core.
|
||||
|
||||
The rendered prompt and provider response are deliberately not persisted. The
|
||||
prompt hash proves equality when an authorized operator can reconstruct the
|
||||
same input, but the audit contract does not promise reconstruction after source
|
||||
definitions or upstream event retention have changed. `activity_runs` retains
|
||||
the definition version and bounded context snapshot; reports may retain
|
||||
allowlisted route/usage metadata. Prompts, messages, tool output, credential
|
||||
fields, and provider blobs are excluded from run, progress, and public API
|
||||
evidence.
|
||||
|
||||
### Testing strategy
|
||||
|
||||
**Rules**: every rule can and should be unit-tested with fixture event payloads.
|
||||
|
|
|
|||
|
|
@ -103,7 +103,7 @@ Implemented in `src/activity_core/rules/executor.py`:
|
|||
pretty-printed and NDJSON output, attempts a best-effort `_try_repair` on a
|
||||
truncated tail, validates each recovered object against the item schema, and
|
||||
keeps the valid ones. Survivors are emitted with `output_validated=true`,
|
||||
`partial=true`, and `review_required=true`.
|
||||
`partial=true`, and `review_advisory=true` (`review_gate_applied=false`).
|
||||
- **Producer guardrails (`_partition_items`, applied on both the recovery and the
|
||||
happy path).** Per recommendation: structural type → schema → structural caps
|
||||
(`_MAX_DEPTH`, `_MAX_STRING_LEN`) → reference allow-list → count cap (top-N by
|
||||
|
|
|
|||
|
|
@ -66,6 +66,11 @@ Canon already allows a DB-only exception for “runtime operations data (logs,
|
|||
metrics, run histories, token events)” (`work-record-types_v0.1.md`). This ADR
|
||||
names that exception for **ops runs**.
|
||||
|
||||
This is also the precise meaning of INTENT's no-task-lifecycle boundary:
|
||||
`ops_run` claim/lease/outcome is durable runtime delivery state, not work-item
|
||||
state. It may not grow assignment, commitments, dependencies, project phases,
|
||||
or manually managed task status.
|
||||
|
||||
## Decision
|
||||
|
||||
### 1. Two planes for “work,” one vocabulary for people
|
||||
|
|
|
|||
|
|
@ -126,6 +126,11 @@ The attribution references we carry (`assignment_ref`, `role_ref`, `duty_ref`,
|
|||
here. Their vocabulary belongs to `info-tech-canon`; activity-core must not
|
||||
invent org roles (ACTIVITY-WP-0029 responsibility map).
|
||||
|
||||
ACT-ADR-007's bounded-operation exception does not alter this decision. A
|
||||
code-registered fixed maintenance operation is not a Glas/rein execution
|
||||
constellation, and the exception cannot be used to run an agent loop or execute
|
||||
an `ops_run` inside activity-core.
|
||||
|
||||
## Consequences
|
||||
|
||||
- `ops_runs` grows `harness_profile_ref` and the attribution refs; the emission
|
||||
|
|
|
|||
128
docs/adr/adr-007-bounded-operations.md
Normal file
128
docs/adr/adr-007-bounded-operations.md
Normal file
|
|
@ -0,0 +1,128 @@
|
|||
---
|
||||
id: ACT-ADR-007
|
||||
type: architecture-decision-record
|
||||
title: "Code-registered bounded operations are the only local mutation exception"
|
||||
status: accepted
|
||||
state_hub_decision_id: "2b1f0c01-1d6f-4fdc-b3e7-537ce0a0a1f8"
|
||||
owner: activity-core
|
||||
revision: "accepted-1"
|
||||
last_reviewed: "2026-08-23"
|
||||
review_interval: 6m
|
||||
decided_by: Bernd Worsch
|
||||
date: "2026-08-23"
|
||||
scope: repo
|
||||
affects:
|
||||
- activity-core
|
||||
- sbom-nexus
|
||||
- railiance-platform
|
||||
tags:
|
||||
- architecture
|
||||
- activity-core
|
||||
- bounded-operation
|
||||
- execution-boundary
|
||||
---
|
||||
|
||||
# ACT-ADR-007: Code-registered Bounded Operations
|
||||
|
||||
## Status
|
||||
|
||||
**Accepted** (2026-08-23) for ACTIVITY-WP-0035.
|
||||
|
||||
## Context
|
||||
|
||||
The Event Bridge principle says activity-core answers when, what, and where and
|
||||
does not execute domain work. Production nevertheless contains three useful
|
||||
operations whose complete outcome is a small scheduled maintenance action:
|
||||
|
||||
- ingest at most three Repo Manager-selected immutable sources into SBOM Nexus;
|
||||
- prune Forgejo package versions under live-image protection; and
|
||||
- invoke the fixed CNPG Option A backup tool for an explicit target list.
|
||||
|
||||
Historically the latter two ran through a generic `shell` context resolver.
|
||||
That made a mutating subprocess look like a read and left extension policy to
|
||||
convention. Removing the operations would recreate bespoke cron; accepting
|
||||
arbitrary shell would make activity-core a general executor.
|
||||
|
||||
## Decision
|
||||
|
||||
### 1. Bounded operations are a narrow implementation exception
|
||||
|
||||
The governing when/what/where responsibility does not gain a general “how.” A
|
||||
bounded operation is allowed only when the operation itself is the declared
|
||||
automation outcome and all admission requirements below are code-reviewable.
|
||||
|
||||
The initial allowlist is exactly:
|
||||
|
||||
1. `sbom_nexus_ingest`
|
||||
2. `forgejo_package_prune`
|
||||
3. `cnpg_option_a_backup`
|
||||
|
||||
Adding an operation requires updating the code-owned registry, this ADR (or a
|
||||
successor), tests, credential route, and evidence contract. A string in a
|
||||
definition cannot register an operation.
|
||||
|
||||
### 2. Admission is fail closed during file sync
|
||||
|
||||
Each registry entry declares:
|
||||
|
||||
- source type and query;
|
||||
- whether mutation intent is `apply` or `dry_run` and that it is explicit;
|
||||
- fixed or maximum target bounds;
|
||||
- canonical implementation and allowed configuration;
|
||||
- idempotency and Temporal retry semantics;
|
||||
- maximum execution timeout;
|
||||
- credential owner/route; and
|
||||
- mandatory non-secret evidence mode.
|
||||
|
||||
Unknown `shell` queries are refused unless separately registered as read-only.
|
||||
Known operation queries with missing, malformed, or over-limit safety fields
|
||||
are refused before database projection or Temporal schedule reconciliation.
|
||||
|
||||
### 3. Context resolution is read-only
|
||||
|
||||
The workflow first resolves and freezes context. Mutations then run in an
|
||||
explicit bounded-operation stage and merge only normalized outcomes into the
|
||||
snapshot before evidence and rule/instruction evaluation.
|
||||
|
||||
SBOM selection remains a read in the context phase; its fixed selection is the
|
||||
input to the operation stage. Package prune and backup have no discovery read
|
||||
inside activity-core and bind a pending marker until their operation completes.
|
||||
|
||||
### 4. Retry behavior is operation-specific
|
||||
|
||||
- SBOM ingest uses stable per-run/per-repository idempotency keys and heartbeat
|
||||
checkpoints, so Activity retries resume the frozen batch.
|
||||
- Forgejo prune and CNPG backup have no activity-core-verifiable remote
|
||||
idempotency receipt. Their operation activity therefore has one Temporal
|
||||
attempt; a failure remains visible for operator reconciliation rather than
|
||||
risking an automatic second mutation.
|
||||
|
||||
This does not preclude future safe retries after the platform tools expose a
|
||||
durable operation receipt.
|
||||
|
||||
### 5. Evidence is mandatory and bounded
|
||||
|
||||
Every operation must produce an allowlisted summary through a configured
|
||||
report/evidence sink. Raw subprocess output, tokens, provider payloads, archive
|
||||
URLs, kubeconfigs, and credential material are not evidence.
|
||||
|
||||
## Rejected alternatives
|
||||
|
||||
- **Keep mutating shell resolvers.** Rejected because resolution should be a
|
||||
replayable read and the generic dispatcher hides mutation admission.
|
||||
- **Generic command activity.** Rejected because command text/path from a
|
||||
definition is remote code execution by configuration.
|
||||
- **Move every operation to a rein.** Rejected for these fixed platform
|
||||
operations; it adds an agent execution constellation without judgement or
|
||||
repository work. Operations that exceed this ADR's bounds do belong there.
|
||||
- **Remove all local operations.** Rejected because it recreates scattered cron
|
||||
and loses Temporal/evidence guarantees for established maintenance.
|
||||
|
||||
## Consequences
|
||||
|
||||
- Definition parsing gains a central operation-policy validator.
|
||||
- The workflow gains an explicit operation stage.
|
||||
- The generic `shell` resolver becomes read-only.
|
||||
- Existing definitions migrate without widening targets or permissions.
|
||||
- The registry is intentionally small and architectural review is required to
|
||||
expand it.
|
||||
|
|
@ -18,7 +18,6 @@ extension point `af654abb`).
|
|||
| Queue name | Registered workers |
|
||||
|---|---|
|
||||
| `orchestrator-tq` | `RunActivityWorkflow` and all its activities (`load_activity_definition`, `resolve_context`, `log_run`) |
|
||||
| `task-execution-tq` | Legacy `TaskExecutorWorkflow` stub only when `ACTIVITY_CORE_ENABLE_TASK_EXECUTOR_STUB=true`; real execution belongs in per-repo workers / agent-harness |
|
||||
|
||||
**Rule:** a workflow and its activities must be registered on the same task queue.
|
||||
Cross-queue activity calls require an explicit `task_queue` argument on
|
||||
|
|
@ -32,7 +31,6 @@ See `docs/idempotency.md` for the full workflow ID strategy.
|
|||
|
||||
Summary:
|
||||
- `RunActivityWorkflow`: `activity-{activity_id}:{trigger_key}`
|
||||
- `TaskExecutorWorkflow`: `task-{run_id}:{task_type}:{index}`
|
||||
- Temporal Schedules: `activity-schedule-{activity_id}`
|
||||
|
||||
---
|
||||
|
|
@ -56,15 +54,8 @@ Each worker process registers:
|
|||
- **Workflows**: `worker.register_workflow(WorkflowClass)`
|
||||
- **Activities**: `worker.register_activity(activity_function)`
|
||||
|
||||
A single process may run workers for multiple task queues, but each `Worker`
|
||||
instance is bound to one task queue. Use separate `Worker` instances for
|
||||
`orchestrator-tq` and `task-execution-tq`.
|
||||
|
||||
`TaskExecutorWorkflow` is not a production execution surface for activity-core.
|
||||
It exists only as a compatibility/idempotency stub that writes a synthetic
|
||||
`task_instances` row in older tests and dev flows. Do not add concrete task
|
||||
execution logic here; execution ownership belongs to per-repo workers or a
|
||||
future execution-owned repo/workplan.
|
||||
The activity-core process registers one `Worker` on `orchestrator-tq`. Execution
|
||||
ownership belongs to downstream consumers and approved execution services.
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -39,6 +39,10 @@ ops_run being `open` is an **automation fire**. Those are different planes
|
|||
(ACT-ADR-005). Mapping State Hub `POST /execution/launch-requests` onto
|
||||
“create a workplan execution row here” would collapse them again.
|
||||
|
||||
The bounded-operation registry in ACT-ADR-007 is a separate, code-reviewed
|
||||
maintenance exception. It neither consumes this queue nor turns an `ops_run`
|
||||
into a workplan task.
|
||||
|
||||
## Replacement for State Hub routes
|
||||
|
||||
| State Hub route | Replacement |
|
||||
|
|
|
|||
|
|
@ -59,23 +59,10 @@ stable ID must be assigned one by the ingress boundary before entering the syste
|
|||
|
||||
---
|
||||
|
||||
## Task instance idempotency
|
||||
|
||||
Each `TaskInstance` spawned by `RunActivityWorkflow` gets its own unique workflow ID:
|
||||
|
||||
```
|
||||
task-{run_id}:{task_type}:{index}
|
||||
```
|
||||
|
||||
This ensures that if `RunActivityWorkflow` is replayed by Temporal (e.g. after a worker
|
||||
restart), it does not re-spawn task instances that were already started.
|
||||
|
||||
---
|
||||
|
||||
## Database idempotency
|
||||
|
||||
`activity_runs` uses `run_id` as the primary key (UUID). The `log_run` activity
|
||||
uses an upsert (`INSERT ... ON CONFLICT DO NOTHING`) so that Temporal activity retries
|
||||
do not produce duplicate run records.
|
||||
|
||||
`task_instances` similarly uses an upsert on `id`.
|
||||
do not produce duplicate run records. Task emission idempotency uses the
|
||||
consumer reference recorded in `task_spawn_log`; runtime delivery uses the
|
||||
unique `ops_runs.idempotency_key`.
|
||||
|
|
|
|||
|
|
@ -427,11 +427,13 @@ Default: **`ISSUE_SINK_TYPE=state-hub`** (ACTIVITY-WP-0022). See
|
|||
| `null` | Dry-run |
|
||||
| `rest` | Intentional issue-core / external tracker only |
|
||||
|
||||
`TaskExecutorWorkflow` is **disabled** unless
|
||||
`ACTIVITY_CORE_ENABLE_TASK_EXECUTOR_STUB=true` (legacy tests only).
|
||||
The legacy `TaskExecutorWorkflow`, its feature flag, and `task_instances` table
|
||||
were removed by ACTIVITY-WP-0035 after production inventory proved zero use.
|
||||
|
||||
`review_required` on instructions is **metadata only** until a downstream
|
||||
review queue exists (issue-core / work-record lane) — see ACTIVITY-WP-0023-T09.
|
||||
`review_advisory` on instructions is **advisory evidence only** and report
|
||||
projections state `review_gate_applied=false`. Legacy `review_required` input is
|
||||
normalized for compatibility; no pending-review queue or hold occurs here. See
|
||||
ACT-ADR-003 and ACTIVITY-WP-0035.
|
||||
|
||||
## Ops run claim queue (ACTIVITY-WP-0026)
|
||||
|
||||
|
|
@ -736,7 +738,7 @@ the next scheduled fire is the next authoritative run.
|
|||
### Multiple worker replicas
|
||||
|
||||
Temporal workers are stateless and horizontally scalable. Run additional worker
|
||||
processes to increase throughput on `orchestrator-tq` and `task-execution-tq`.
|
||||
processes to increase throughput on `orchestrator-tq`.
|
||||
|
||||
Each worker registers the same workflows/activities — Temporal distributes tasks
|
||||
across all pollers automatically.
|
||||
|
|
|
|||
|
|
@ -95,8 +95,8 @@ and still post the progress event — spawn without completion leaves `due=true`
|
|||
|
||||
## Anti-patterns
|
||||
|
||||
- Using `TaskExecutorWorkflow` in activity-core for real work (disabled by
|
||||
default; ACTIVITY-WP-0023-T08).
|
||||
- Adding an in-repo task executor or reviving the retired
|
||||
`TaskExecutorWorkflow`/`task_instances` surface.
|
||||
- Global `ISSUE_SINK_TYPE=rest` for all definitions (reintroduces Forgejo spam).
|
||||
- Treating `task_spawn_log` or State Hub `activity_task_spawn` as claim authority
|
||||
(use `POST /ops-runs/claim` — ACT-ADR-005).
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue