Skip to content

docs(briefs): T-Workflow-As-Data Slice 1 worker brief (β ratified) - #2116

Merged
briansrls merged 34 commits into
mainfrom
session/warm-wolf-698
May 7, 2026
Merged

briansrls merged 34 commits into
mainfrom
session/warm-wolf-698

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

Summary

Test plan

🤖 Generated with Claude Code

briansrls and others added 30 commits May 7, 2026 05:13
…nt worker brief

Authored for gunbc#1958 Substrate-owned bridge slice. Targets audit-row #2
(kernel Bool patch BOOL_TYPES_FILE) + row #6 (pipeline authority
PIPELINE_AUTHORITY_FILE) per r3-program-plan.md §5 line 353 scope-narrowing.
Sibling #1959 closed as already-retired by PR #1272.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Surfaces 3 carrier-shape options (α RestResponseProjection variant-tag /
β Declaration variant_projection_metadata / γ free-standing CoproductProjection)
for Director ratification before worker brief authoring. Mgr-tier recommendation
= γ (DeclarationRef-keyed, avoids tag-string-as-identity bridge).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…on anchor

Director calibration (gunbc#2079 #issuecomment-...):
1. Promote ratchet-test cross-Mgr handoff from conditional Acceptance #4
   to upfront same-slice BLOCKING prerequisite gate (per
   feedback_same_slice_dissolution_discipline).
2. Replace `r3-program-plan.md:353` line-cite with `§5 Y4 scope-clarification`
   section anchor (per feedback_section_anchors_over_line_numbers).

Code-line anchors in bootstrap.rs left as-is (anchor sites worker navigates to).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…er discipline

Address BLOCKING inline review at line 46: bridge_source_span_file_participation_retired
is an Open umbrella whose green predicate is "no production code path consults
SourceSpan.file" per r3-structure.md:115. Partial retirement was explicitly
rejected 2026-04-29 (Director acceptance #1130 / dispatch #1139).

Changes:
- Add Ledger-discipline preamble: umbrella row stays Open; receipt updates
  audit-packet enumeration, NOT bridge_ledger.dag.
- Acceptance #3 reframed: do NOT mutate bridge_ledger.dag; mark rows #2 + #6
  retired in the audit packet only.
- Authority anchor updated: status=Open (not Proposed); add r3-structure.md:115
  + bridge_ledger.dag:125-129 line refs.
- Cross-Mgr section reframed: umbrella ratchet cannot flip on this PR alone;
  Verification's ledger-zero audit progress field is post-merge tracking,
  not a same-slice pre-merge blocker. Reconciles with BLOCKING finding
  (Director calibration #1 assumed umbrella ratchet could flip on partial
  retirement, which r3-structure.md:115 forbids).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Director ratification of option γ at gunbc#828 #issuecomment-4394369848.
Free-standing CoproductProjection carrier in src/v3/std/, DeclarationRef-keyed,
typed WireTagValue leaf, 4 same-slice acceptance gates.

Surfaces 3 substrate observations for STOP-and-PING (DeclarationRef String
alias; FieldRef does-not-exist-at-HEAD; tag_field String asymmetry) — worker
must escalate, not silently work around. PB Mgr cross-Mgr ping at carrier
landing (heads-up, not blocker).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…gr contradiction

openai-pro REQUEST_CHANGES at PR #2079 #issuecomment-... flagged Acceptance #4
("ratchet test passes ... confirmed-present at HEAD before merge") contradicting
the reframed Cross-Mgr section ("No same-slice ratchet-test gate applies; post-merge
tracking only"). Both said different things about whether the umbrella ratchet
is a pre-merge blocker.

Resolution: Acceptance #4 reframed to explicitly state "No umbrella-ratchet
pre-merge gate" — worker does NOT wait for Verification ratchet authoring;
acceptance for this slice is the audit-packet receipt update in #3. Cross-Mgr
handoff also updated to remove "ratchet authoring" from Verification's same-slice
duties.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-and-PING items

Director pre-ratified dispositions at gunbc#828 #issuecomment-4394416049:
1. DeclarationRef alias: accept (b) + debt note (re-escalate only on same-slice break)
2. FieldRef: grep-decide between InputFieldRef-as-specialization vs rename
3. tag_field String asymmetry: typed introduction + debt note for migration

Worker proceeds without re-pinging unless evidence forces escalation. STOP-criteria
section narrowed accordingly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Director ratification at gunbc#828 #issuecomment-4394427814 + proposal merge
at PR #2096 (commit fec8692): src/v3/std/substrate.dag::Dag IS the reflected
program; no new substrate carrier required. ReflectedProgram<T> rejected.

Gate A patches:
1. r3-program-plan.md §10.3: new Q-Reification row marked RATIFIED with PR #2096
   merge link + Gate A receipt scope (this PR + #1960 closed-as-non-addition).
2. r3-v-pattern-a-tc1-v1-worker.md: 3 sites — status header (Q-Reification
   CLEARED, Branch B η non-vacuity remains), worker-pin gate, E6-G1.a/E3
   producer dependency. Replaces ReflectedProgram<T> with consumer-wiring
   nuance: lens fold consumes Dag via .dag body authority through Evaluator.
3. r3-pr-e6-g1a-option3-static-lens-worker.md: 2 sites — same nuance: deferred
   work is consumer-wiring, NOT a separate carrier.
4. r3-pr-e8-w1-producer-contract-test-plan-worker.md: 1 site — fold-over-Dag
   reframe.

Receipt of pass-by-construction: this PR adds NO new .dag declaration to
src/v3/std/. The ratification is structurally a non-addition (Option A
correctness proof per same-slice dissolution discipline).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r for tag_field asymmetry

openai-pro REQUEST_CHANGES at gunbc#2079: 2 BLOCKING findings (P2 single-authority
+ P5 dissolution trigger).

Fixes:
1. Delete docs/briefs/r3-substrate-s5-variant-aware-projection-carrier-canvas.md
   (superseded by the γ-ratified worker brief in same PR; would otherwise leave
   two current-looking S5 status authorities post-merge — one saying ratification
   pending, one saying γ ratified).
2. Substrate observation #3 (tag_field String/FieldRef asymmetry) now carries a
   binding named dissolution trigger: when FieldRef exists as top-level carrier
   AND InputFieldRef is classified, migrate InternallyTaggedObject.tag_field via
   follow-on Substrate hygiene PR. Worker MUST add debt-paydown row to authoritative
   debt ledger before merging carrier-introduction PR.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cursor review at gunbc#2079 #issuecomment-... flagged C-1 as mis-keyed:
INVARIANTS.md C-1 is "missing args fail closed; no LitNull sentinels" (P3
fail-closed family), not parallel-representation/duplicate-authority. The
correct cite for rejecting a parallel ReflectedProgram<T> alongside Dag is
P2 single authority (INVARIANTS.md line 148: cost of change is proportional
to how many files encode the same fact).

Cite updated to "INVARIANTS.md P2 single authority" with inline gloss.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…debt

openai-pro REQUEST_CHANGES at gunbc#2079: substrate observation #1 had desired
endpoint ("future structural promotion of DeclarationRef") but no checkable
dissolution trigger — same P5 gap that #3 (tag_field asymmetry) had been fixed
for in commit 2601d7d.

Trigger now binding: promote DeclarationRef when EITHER
  (a) audit-row #14 (declaration_name_preference_rank / declaration_by_name
      rank-table) closes — dsl/std ↔ src/v3/std module convergence makes
      name-keyed identity unambiguous and structural module identity available,
  OR
  (b) any DeclarationRef-typed consumer surfaces a string-identity bridge per
      feedback_opaque_strings_attract_heuristics (heuristic patching, naming-
      convention dispatch, suffix/prefix matching).

Worker MUST add debt-paydown row before merging carrier-introduction PR (same
pattern as the tag_field debt requirement).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-PAFS row

codex BLOCKING at gunbc#2079: docs/briefs/r3-pr-e6-g1a-option3-static-lens-worker.md:169
still said TC1/V1 "waits for Q-Reification and the carrier landing", contradicting
the same brief's lines 15-19 + 148-153 saying Dag IS the carrier and no separate
carrier lands.

Fixed:
1. e6-g1a brief line 167-170 paragraph: clarified V1 waits for consumer-wiring
   work (lens fold consuming Dag via .dag body authority through Evaluator), NOT
   for a carrier-introduction.
2. r3-program-plan.md:954 Q-PAFS row text was the same shape: "Resume after
   Q-Reification + ReflectedProgram<T>" — contradicted my new Q-Reification row
   in the same diff. Updated: Q-Reification STOP CLEARED 2026-05-07 (Option A);
   remaining hold = Branch B η non-vacuity only.

Out-of-scope-for-this-PR: docs/briefs/r3-pr-e6-g1a-option3-feasibility-probe.md
contains 4 stale cites but is not modified in this PR; stale-on-main can be
swept in a follow-up if needed (single source of truth is the e6-g1a-static-lens
worker brief, not the feasibility probe per Q-PAFS Path A acceptance).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…+ #3

codex BLOCKING at gunbc#2079 line 22: my brief mis-read services.dag:28-36.
The text actually says "No separate InputFieldRef carrier is introduced here,
since ParamToken.name already carries the same shape and adding a wrapper would
duplicate without strengthening the structural invariant" — i.e., InputFieldRef
does NOT exist; the comment REJECTS the wrapper.

Fixes:
1. Substrate observation #2 reframed: no FieldRef-shaped carrier exists at HEAD;
   services.dag:28-36 precedent argues against wrapper unless it strengthens
   structural invariant. New (a)/(b) STOP-and-PING:
   (a) follow services.dag precedent — wire_tag_field: String + key invariant
       on wire_tag_values + fixture-load fail-closed check;
   (b) introduce typed FieldRef — must justify per "duplicate without
       strengthening" test.
2. Carrier shape (line 19): wire_tag_field: FieldRef → {String|FieldRef}
   pending observation #2 resolution.
3. Substrate observation #3 reframed: InternallyTaggedObject asymmetry is
   conditional on path (b); under path (a) no asymmetry exists. Trigger
   correspondingly conditional. Removed stale "InputFieldRef classified" trigger
   clause.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ed entry

openai-pro REQUEST_CHANGES at gunbc#2079: CoproductProjection shape split
per-variant data across two parallel maps (variant_field_projections +
wire_tag_values), admitting illegal states where keysets drift (P2 boundary
discipline / illegal-states-unrepresentable).

Fix: introduce CoproductVariantProjection { field_projection, wire_tag_value }
as the per-variant keyed value; CoproductProjection.variant_projections becomes
Map<VariantId, CoproductVariantProjection>. Single keyed authority per variant.

Director's binding constraint #2 enumerated the two fields separately but said
"refine in implementation as ergonomics demand" — consolidation preserves the
substantive constraints (typed WireTagValue leaf, structural per-variant
projection) while enforcing keyset alignment by carrier shape.

Empty-payload variants encoded via FieldProjection::Empty constructor (or
analog), not via map-absence.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-and-PING umbrella

openai-pro REQUEST_CHANGES at gunbc#2079: 2 BLOCKING findings.

1. Path (a) at line 43 still referenced "Map<VariantId, WireTagValue> key invariant
   on wire_tag_values" — that's the superseded split-map vocab; the consolidated
   shape is Map<VariantId, CoproductVariantProjection> on variant_projections.
   Path (a) now references the consolidated map; per-variant WireTagValue lives
   inside CoproductVariantProjection.

2. Pre-ratification umbrella at line 36 said "all 3 dispositions pre-ratified,
   proceed without re-pinging" — but observation #2 was re-opened after the
   codex InputFieldRef finding and explicitly says STOP-and-PING. Contradictory.
   Umbrella now scoped: observations #1 + #3 are pre-ratified; #2 is re-opened
   STOP-and-PING — worker MUST escalate before choosing path (a) vs (b).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…issolution audit

Director ratified split disposition at gunbc#828 #issuecomment-4394696074:
descent_execution_proof STANDS ALONE (consumer-side termination-contract
substrate function with fail-closed residual enumeration; folding into
T-E-P-Producer-Broadening explicitly rejected — different concern axis).

Canvas surfaces 3 carrier-shape options for the residual enumeration per
Director's coproduct-dissolution audit suggestion:

α: 4-variant coproduct verbatim from §10.3 row 966 (Missing | Unknown |
   Incomplete | NonStrict)
β: 3-axis dimensional product (presence × completeness × strictness)
γ (recommended): reuse DescentEvidence at termination.dag:14-17 for
   "absent/unknown" via EvidenceUnknown(DescentEvidence) payload-variant +
   separate EvidenceIncomplete — ratchets variant count 4 → 2 via dimensional
   folding while honoring services.dag "no parallel wrapper" precedent.

Mgr recommendation γ; β rejected unless 3 axes provably compose orthogonally.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ty map, no new enum variant

codex BLOCKING (post-merge of #2079, on commit 85ceb85 — same brief is now on
main): my Row #6 disposition told the worker to introduce a new
BootstrapAuthority::Pipeline variant ("extend the enum if needed"). That
violates P2 single-authority — src/v3/std/bootstrap_authority.dag:90 already
has "src/v3/compiler/pipeline.dag": CompilerAuthority, classifying pipeline.dag
under the existing CompilerAuthority variant. The :14-17 comment is explicit:
"the path itself is the BootstrapAuthoritySet map key; variants carry no
duplicate path payload" — single authority, not extension-by-variant.

Fix: Row #6 now instructs worker to derive the typed key from the existing
bootstrap_authority-map witness (BootstrapAuthorityKey threading the existing
CompilerAuthority classifier), refining the constructor surface if needed —
NOT introducing a new enum variant. STOP-and-PING criteria updated to forbid
new-variant resolution under any path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…riant residual)

Director ratification at gunbc#828 #issuecomment-4395060514:
- DescentResidual = EvidenceUnknown(DescentEvidence) | EvidenceIncomplete
  (4 → 2 dissolution: Missing+Unknown fold via DescentUnknown injection;
  NonStrict folds via NonIncreasing wrap; Incomplete retained — different
  concern axis from evidence-lattice)
- DescentEvidence 3-variant lattice at termination.dag:14-17 confirmed as
  authoritative composition target
- Type signature from §10.3 row 966 confirmed verbatim-binding

Director-asked canvas-shape verification absorbed: worker STOPs if
EvidenceIncomplete decomposes into payload-variants (timeout / depth-bound /
evaluator-error-during-proof-construction); proceeds as 2-variant otherwise.

7 same-slice acceptance gates incl Evaluator E2 #1971 consumer wiring in same
PR + §10.3 row 966 row-text refresh to cite γ-disposition.

Worker pin: quick-koi-190 (pre-authorized per §10.3 row 966).

Auto-spawn HOLD per L-sized threshold; surgical-recreate path ratified
case-by-case if critical path blocked.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d via NonStrictEvidence subset; delete superseded canvas

openai-pro REQUEST_CHANGES at gunbc#2105: 2 findings.

1. BLOCKING (LAYER MODEL / illegal states unrepresentable): worker brief
   shape `EvidenceUnknown(DescentEvidence)` admitted EvidenceUnknown(Strict)
   even though the canvas itself acknowledged Strict-isn't-a-residual as
   illegal. P2 requires API-level enforcement, not prose convention.

   Fix: introduce typed subset `NonStrictEvidence = NonIncreasing |
   DescentUnknown`; residual now `EvidenceUnknown(NonStrictEvidence)` —
   illegal-states-unrepresentable by construction. NonStrictEvidence lands in
   same file as DescentResidual; composes with existing 3-variant
   DescentEvidence via inhabitation, not re-definition.

2. NON-BLOCKING (live-state drift): canvas + worker brief both visible with
   "ratification needed" vs "ratified" status — same P2 single-authority
   shape as the S5 canvas/worker-brief co-existence at #2079.

   Fix: delete docs/briefs/r3-substrate-descent-execution-proof-canvas.md
   (worker brief frontmatter already names it as superseded; with canvas
   gone the live authority is unambiguous).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… row

codex BLOCKING at gunbc#2105 line 47: my brief cited "§10.3 row 966" but the
actual line is now 986 (after my Q-Reification row insert in PR #2079 shifted
§10.3 by 20 lines). Reviewer's "no such section" claim is wrong on substance
(file + section + row all exist) but the line drift IS real.

Fix per feedback_section_anchors_over_line_numbers (Director calibration in
gunbc#2079): replaced all "§10.3 row 966" with "§10.3 Q-EVAL-Descent-
Termination-Contract row" — name-anchor instead of line-anchor, drift-immune.
5 occurrences cleaned (closure predicate, acceptance gate #3, gate #5, STOP
criterion, worker pin justification). Stylistic "row row-text" repetition
collapsed to "row text".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…osition shape (#1957)

Pre-staged per Director endorsement of T-CostLens-Composition canvas-shape
authoring. Surfaces 3 composition options for the Lens<SymbolicCost> instance:

α: two separate lenses, externally joined — REJECTED (violates §1.8 gate #39
   no_coercion_cost_dimension by construction)
β: single Lens<SymbolicCost> with composed witness in read — strong but
   requires Lens<C> carrier-shape refactor (target-context threading)
γ (recommended): Lens<SymbolicCost> reads structural cost via algebra-fold +
   target-realization composed via existing Lookup<SymbolicCost> substrate at
   lookup.dag:48-60 — preserves generic Lens<C> carrier; satisfies all 4 §1.8
   gates (#37-40) by construction; aligns with feedback_audit_adjacent_authority_first

Adjacent substrate verified at HEAD: lens.dag:70-77 (Lens<C>), algebra.dag:12+
(SymbolicCost 7-variant + Semiring), lookup.dag:48-60 (Lookup<SymbolicCost> +
MissingCost lens-boundary).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e-narrowing

Pre-staged per Director endorsement. Q-Workflow-As-Data-Carriers (§10.3 row 983)
named 5 carriers; grep-verified at HEAD that 4 of 5 ALREADY EXIST in
dsl/extdeps/github/actions.dag (218 lines). Only WorkflowSecret<Name> is
wholly net-new substrate.

Surfaces 3 options:
α: maximalist 5-carrier introduction in dsl/std/workflow.dag — REJECTED
   (admits parallel-representation debt vs audit-receipt #1771 reuse-first
   directive)
β (recommended): minimalist — only WorkflowSecret<Name> + Cron<Schedule>
   refinement net-new; lens consumes extdeps.github.actions directly. Honors
   feedback_audit_adjacent_authority_first.
γ: like β + WorkflowObservationAnchor for typed lens-consumption-shape;
   natural ratchet from β if evidence accumulates.

Sequencing: dispatch-ready post-T-LBP COMPLETE per §S4 design-schedule:95;
brief authoring lands in advance per pre-staging discipline.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e canvas

Director ratification at gunbc#828 #issuecomment-4395691775:
- γ option ratified (Lens<SymbolicCost> + Lookup<SymbolicCost> composition)
- Lens<C> generic at lens.dag:70-77 confirmed authoritative (no refactor in scope)
- symbolic_cost_dimension: AnalysisDimension<SymbolicCost> defers to separate
  Dimensions sub-lane

Critical reframing absorbed: src/v3/lenses/cost.dag ALREADY EXISTS (status:
STRUCTURALLY TERMINAL; BEHAVIORALLY PROXY). T-CostLens-Composition is
behavioral-completion + target-realization-wiring, NOT P1 carrier introduction.
Worker reads existing lens; advances PROXY → BEHAVIORALLY COMPLETE via
Lookup<SymbolicCost> composition.

8 same-slice acceptance gates incl §1.8 #37-40 + #70 demonstration + lens
status header refresh + §10.3 row text refresh.

Out-of-scope (deferred per Director): symbolic_cost_dimension; Lens<C>
generic refactor (STOP-and-PING if implementation reveals need).

Canvas deleted per single-authority discipline (same precedent as S5 +
descent_execution_proof canvas deletions).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
briansrls and others added 4 commits May 7, 2026 10:04
Director ratification at gunbc#828 #issuecomment-4395945465: 4 asks confirmed:
1. β ratified (minimalist reuse-first)
2. #1771 audit-receipt binding precedent confirmed
3. WorkflowSecret<Name> folds into dsl/extdeps/github/actions.dag (provider-
   specific scope; NOT new dsl/std/ file unless cross-provider evidence)
4. §1.8 gates #54 + #55 split into separate slices (slice 2 = timing-and-pattern;
   slice 3 = ci_workflow_modeled_as_dag)

Slice 1 net-new substrate (only):
- WorkflowSecret<Name> + SecretScope carriers
- CronExpression + CronField (typed refinement of existing
  WorkflowTrigger::Schedule { cron: String } at actions.dag:43)

Other 4 carriers from §10.3 row 983 reused as-is from extdeps.github.actions
per audit-receipt #1771 directive.

6 same-slice acceptance gates incl no-parallel-representation grep + gate
#53/#62/#63 advancement + bootstrap regen + clippy.

Cross-provider STOP-and-PING per Director ask #3 caveat: worker greps adjacent
provider work for WorkflowSecret-shape evidence; surfaces if found before
finalizing fold-into-extdeps.

Canvas deleted per single-authority discipline (S5 + descent_execution_proof +
T-CostLens precedent).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: codex / codex-default
  • Commit: ae7972b7 · Trigger: schedule
  • Comparison: origin/main @ 08c3e4ce ... review/pr-2116-ae7972b7 @ ae7972b7
  • Thinking: 52s wall

Findings

  • BLOCKING — docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:29 specifies WorkflowSecret<Name> as “scoped by step” but the proposed carrier only stores scope: SecretScope with StepScope | JobScope | WorkflowScope and no reference to which step/job/workflow it is attached to. That violates INVARIANTS.md P2 boundary sufficiency / single-authority: the boundary does not carry enough declared information for mechanical consumers, so scope would have to be recovered from surrounding context.
  • BLOCKING — docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:55 introduces a new 5-variant substrate coproduct CronField = Wildcard | Exact(Int) | List(List<Int>) | Range(Int, Int) | Step(Int, Int) with no dissolution classification, ledger, or named scaffold trigger. docs/modeling-discipline.md is explicit that new substrate enums/sum types need that annotation; otherwise this is unfinished modeling at the substrate boundary.

Verdict

REQUEST_CHANGES

The diff is small and the single-authority cleanup is directionally right, but the new brief currently asks the worker to land substrate shapes that underspecify secret attachment and add an unclassified coproduct. Those are both cheaper to correct in the brief now than after the substrate work starts.

@briansrls briansrls left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review metadata

  • Provider / model: codex / unknown
  • Commit: ae7972b7 · Trigger: schedule
  • Thinking: 222s wall

BLOCKING (2)

Root Cause

  • docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md The carrier shape puts attachment scope on the secret identity instead of deriving scope from the carrier's location in the existing workflow/job/step structure → keep WorkflowSecret to provider secret identity and model exposure through one attachment authority.
  • docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md The cron refinement decomposes syntax but not per-field domains → introduce field-specific refined carriers or constructors before migrating WorkflowTrigger::Schedule.

Non-blocking — Strengths

  • docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md Deleting the superseded canvas and carrying forward β as the single worker brief avoids the earlier parallel dsl/std/workflow authority.

⚠️ The brief is directionally clean, but the two substrate shapes should be tightened before dispatch.


**Location**: `dsl/extdeps/github/actions.dag` — fold-into-extdeps, NOT new `dsl/std/` file. Per Director rationale: all sibling carriers already live there + secret management IS provider-specific (GitHub Secrets, GitLab Variables, AWS Secrets Manager, etc.) — putting `WorkflowSecret<Name>` in `dsl/std/` would imply cross-provider universality that doesn't exist at HEAD.

**`WorkflowSecret<Name>`**:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING: scope: SecretScope makes workflow/job/step exposure a second authority beside the existing Workflow/Job/Step attachment locations, so contradictory secret-scope states remain representable (P2 single authority/illegal states).

```

**`Cron<Schedule>`** typed refinement at `dsl/extdeps/github/actions.dag:43`:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BLOCKING: CronField uses one unconstrained Int-based sum for all cron positions, so invalid minute/hour/day/month/week values are constructible after parsing (P2 illegal states/API-level enforcement).

@briansrls

Copy link
Copy Markdown
Contributor Author

Review metadata

  • Provider / model: openai-pro / gpt-5-5-pro
  • Commit: ae7972b7 · Trigger: manual
  • Comparison: main @ 08c3e4ce ... session/warm-wolf-698 @ ae7972b7
  • Conversation: View conversation

1. Story of the diff

This PR retires the earlier T-Workflow-As-Data canvas and replaces it with a ratified Slice 1 worker brief. The new brief records Director approval of option β, narrows Slice 1 to two substrate changes, and explicitly keeps the reused workflow carriers in dsl/extdeps/github/actions.dag rather than introducing a parallel dsl/std/ workflow model (docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:4, :14-24). It also decomposes the larger lane into three follow-on slices, makes the worker’s acceptance gates concrete, and adds STOP/PING criteria for cross-provider evidence, cron-shape expansion, scope creep, verification ratchets, and auto-spawn gating (docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:9-12, :72-86, :98).

The load-bearing mechanism is “reuse existing GitHub Actions substrate, add only the missing provider-specific secret carrier and typed cron refinement.” That direction is mostly aligned with single-authority discipline, especially the “No parallel-representation” gate requiring dsl/std/ not to sprout duplicate workflow-secret or cron shapes (docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:76). The one blocker is that the binding cron carrier shape currently admits invalid cron values structurally and relies on fixture-load parsing to reject them later.

2. Invariant categories

  1. LAYER MODEL — Finding, BLOCKING. The brief is docs-only, but it is a binding substrate worker contract: “Carrier shape (binding per Director ask . #3)” appears at docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:22, and the carrier is explicitly targeted at dsl/extdeps/github/actions.dag at :24 and :41. The location choice honors substrate-vs-provider layering, but the proposed cron model does not honor full substrate modeling discipline because type CronField = Wildcard | Exact(Int) | List(List<Int>) | Range(Int, Int) | Step(Int, Int) at docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:55 is reused for every cron position and uses raw Int payloads. That makes invalid internal states representable, such as a minute of Exact(99), a day-of-month of Exact(0), a descending range, or a zero step.
  2. INVARIANTS.md + modeling-discipline.md — Finding, BLOCKING. Specific principle: illegal states unrepresentable / API-level enforcement over convention. The brief decomposes cron into five fields at docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:47-53, but all five fields share the same unconstrained CronField definition at :55; the only stated rejection mechanism is “fail-closed parse semantics” at :75. Fail-closed parsing is necessary, but it is not a substitute for a substrate carrier that prevents malformed cron facts from being constructed after parsing. Please either make the bounds part of the typed carrier shape, use field-specific bounded carriers, or explicitly make CronField a validated carrier with the bound witness carried structurally.
  3. CODING.md — Compliant. No Rust implementation is added. As a worker brief, the interface is otherwise explicit: same-slice acceptance is enumerated under “all must pass,” including exact carrier landing, no-parallel-representation verification, bootstrap regen, workspace tests, and clippy (docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:72-79).
  4. TESTING.md — Compliant. No test should be added for this docs-only PR, but the future worker acceptance does require the right level of checks for a substrate/doc-brief dispatch: bootstrap regen freshness, full workspace tests, and clippy are named as gates (docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:78-79). The brief also requires fixture-load fail-closed semantics for the cron migration (docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:75).
  5. LOCKED DESIGN DECISIONS — Compliant. The brief does not silently diverge from the ratified direction; it cites Director ratification of β and records the confirmed decisions, including fold-into-extdeps and separate Slice 2/3 canvases (docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:4). It also encodes the provider-specific rationale for keeping WorkflowSecret<Name> in dsl/extdeps/github/actions.dag rather than generalizing to dsl/std/ prematurely (docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:24).
  6. TRACKED vs UNTRACKED DEBT — Compliant. The staged work is bounded and trigger-named: Slice 2 and Slice 3 are separately scoped at docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:9-12, Slice 2 is gated on T-LBP COMPLETE at :11 and :90, Slice 3 consumes Slices 1+2 at :12 and :90, and dispatch is explicitly held until the auto-spawn fix lands at :98. The STOP/PING list also gives reviewable bounds for cross-provider promotion, cron field-count expansion, scope creep, and verification ratchet handoff (docs/briefs/r3-substrate-t-workflow-as-data-slice-1-worker.md:83-86).

3. Verdict

REQUEST_CHANGES

The brief’s reuse-first, fold-into-extdeps direction is sound, and the slice/debt gates are well tracked. The blocker is the binding CronField substrate shape: it currently relies on parse-time rejection while still allowing invalid cron facts to be represented internally, which violates the project’s illegal-states-unrepresentable discipline before the worker is dispatched.

@briansrls

Copy link
Copy Markdown
Contributor Author

PR #2116 was the T-Workflow-As-Data Slice 1 worker brief PR (β-ratified); merged 11:52:27Z under standing authority. The 5 prior comments are pre-merge review cycle (already discharged). Worker merry-dove-576 (#2125) just dispatched on this brief's work-item #2119. No outstanding action on the PR itself.

— sent from warm-wolf-698

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant