feat: native .dipx bundle support#209
Merged
Merged
Conversation
TestResolveUnderRoot_AcceptsAbsoluteInside and AcceptsNonexistentInRoot compared resolveUnderRoot's output against the raw t.TempDir() path, which on macOS sits under /var/folders (a symlink to /private/var). resolveUnderRoot correctly returns the evaluated form, so the prefix check would fail unconditionally on macOS. Bug was in the test, not the helper. The helper's symlink-resolution behavior is intentional and what production safety relies on. Tests now eval the tempdir's symlinks the same way before comparing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures the request from the Pipelines team plus the validated design
for native .dipx loading, identity-stamped audit trail, and strict
bundle-identity verification on resume.
Decisions:
- IR-direct loader path via dipx.Open + bundle.Entry/Lookup;
LoadDippinWorkflow splits into a shared FromIR tail so the .dip
and .dipx paths converge after parse.
- Identity threads into Checkpoint, jsonlLogEntry, RunSummary, and
tracker.Result. Plain .dip runs leave the fields empty.
- Strict resume verification with --force-bundle-mismatch escape
hatch; downgrade and upgrade are also rejected by default.
- dippin-lang v0.23.0 → v0.24.0 bump as the first commit of the PR.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
17 TDD-cycled tasks chaining from the dippin-lang v0.24.0 bump through loader + audit-trail threading + strict resume verification + CHANGELOG. Each task is bite-sized (2-5 minutes), self-contained, and follows the project's test-first + commit-per-task discipline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups from the Task 2 code review: - MinimalDip is moved from the test file to pack.go and exported. Downstream tests (Tasks 4, 5, 9, 15, 16) can now import the helper instead of re-embedding .dip source inline — preventing recurrence of the YAML-shape parse bug the plan originally specified. - PackTestBundle now closes the file explicitly and fails the test on close error rather than swallowing it via defer. Matches the fail-fast pattern used by the rest of the function.
…ence test Follow-up from the Task 3 code review. The original test compared only 4 fields (Name, StartNode, len(Nodes), DippinValidated), missing ExitNode/Edges/Attrs/NodeOrder. Since Task 4 (LoadDipxBundle) will run the IR path on every workflow in a bundle with no source-string fallback, any silent divergence between the two paths would propagate invisibly. Deep-equal locks the refactor invariant in tightly.
Follow-ups from the Task 4 code review: - The original tests only ran the entry-only path; the manifest.Files-walk + bundle.Lookup + LoadDippinWorkflowFromIR subgraph branch had zero direct coverage. Tasks 5/9/15/16 lean on this branch — close the gap before the CLI integration starts exercising it for real. - Add a one-line comment in the hash-mismatch test explaining why offset 100 is the right tamper location (deflate-compressed payload region of MinimalDip-sized bundles). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds loadPipelineAndBundle as a sibling to the tightly-scoped loadPipeline chokepoint. The new entry point branches on extension: .dipx routes to pipeline.LoadDipxBundle (Task 4) for sealed bundle loading with pre-resolved subgraphs and content-addressed identity; everything else falls through to loadPipeline + loadSubgraphs and returns a zero-value BundleInfo. loadAndValidatePipeline now returns BundleInfo as a third value so downstream tasks can wire the bundle identity through to the engine. Both call sites in run.go take the extra return as _ for now; Task 9 threads it through to tracker.Run. validate's loadPipelineForValidation also dispatches through the new entry point so 'tracker validate sprint.dipx' works end-to-end without trying to parse ZIP bytes as .dip source. isExplicitFilePath in resolve.go now recognizes .dipx alongside .dip/.dot so bare paths to bundles bypass the workflow-name lookup. dipxtest moved from pipeline/internal/dipxtest to internal/dipxtest so it can be imported from cmd/tracker tests. The Go internal-visibility rule limited the pipeline/internal/ location to packages under pipeline/.
Follow-up from the Task 5 spec review: simulate.go had its own loader path (readPipelineSource → os.ReadFile → tracker.ValidateSource) that bypasses loadAndValidatePipeline / loadPipelineAndBundle. On a .dipx input, it read ZIP bytes as .dip text and failed with "validation error(s) in inline.dip" — the exact bug the request reports. The fix routes .dipx through loadPipelineAndBundle to get a *Graph directly, then drives the simulate library API with the pre-loaded graph. Plain .dip flow is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…on bundles Follow-ups from the Task 5 code review: - validate had no regression test for the .dipx dispatch path (simulate had two). Adds TestValidateDipxBundle so a future refactor that re-routes validate back to loadPipeline+os.ReadFile fails loudly. - simulate's .dipx path was skipping lint warnings on the assumption that the bundle was validated at pack time. True for errors, but dippin lint warnings (DIP101–DIP115) are useful at inspect time — especially for third-party bundles. Restores parity with the .dip path via pipeline.ValidateAllWithLint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds PipelineEvent.BundleIdentity and the matching jsonl bundle_identity field. Introduces Engine.bundleIdentity + WithBundleIdentity option, and stamps the identity in the e.emit chokepoint so every engine-emitted event (and therefore every line of activity.jsonl) carries .dipx bundle provenance. The emit helper preserves any explicitly-set value so future child/subgraph paths can supply their own identity.
Follow-up from the Task 7 spec review. Handler-package emissions in
pipeline/handlers/parallel.go and pipeline/handlers/manager_loop.go
bypass Engine.emit and called eventHandler.HandlePipelineEvent
directly — landing in activity.jsonl without bundle identity. Task 16's
end-to-end check ("every activity.jsonl line carries identity") would
have failed.
Fix wraps cfg.pipelineEvents in a stampingHandler at registry config
time when WithHandlerBundleIdentity is set. Wires the identity through
from cmd/tracker/run.go's two callers of loadAndValidatePipeline (which
already had BundleInfo as a return value from Task 5).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two follow-ups from the Task 7 code review: 1. WriteAgentEvent and WriteLLMEvent on JSONLEventHandler bypassed all event-handler chokepoints — agent/llm lines in activity.jsonl would have appeared without bundle_identity. Task 16's "every line" check would fail on the first agent line. Adds JSONLEventHandler.bundleIdentity (private) with SetBundleIdentity public setter, wired from cmd/tracker/run.go. Stamps in both write paths, with the same empty-guard pattern as Engine.emit and the registry's stamping wrapper. 2. BundleIdentityStamper (renamed from unexported stampingHandler) is exported for library-API parity. Task 9 will add tracker.Config .BundleIdentity for external callers; they can now apply the same stamping to their own registries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The engine's bundleIdentity (set via WithBundleIdentity in Task 7) is now written into Checkpoint.BundleIdentity on every saveCheckpoint call. The omitempty JSON tag on the field keeps plain .dip checkpoints clean (empty identity is not serialized). This is what strict-resume verification (Task 15) will read back to fail-fast on bundle drift between resume runs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a BundleIdentity string field to tracker.Config so embedded integrations can thread .dipx bundle provenance through the library API without going through the CLI. When set, the engine is constructed with pipeline.WithBundleIdentity so every checkpoint save and every emitted PipelineEvent carries the identity. Empty (the default) is a no-op and matches plain .dip behavior. The library API does not own the activity log (JSONLEventHandler); the field doc directs callers that build one separately to also call activityLog.SetBundleIdentity(cfg.BundleIdentity) so agent/llm writes outside the engine event chain carry the same provenance. Round-trip is pinned by TestRun_Config_BundleIdentity_FlowsToEngine (checkpoint loaded after Run reads back the identity) and TestRun_Config_BundleIdentity_EmptyByDefault (no stamp when unset). Task 9 of native .dipx bundle support. The CLI path was wired in Task 7; this completes the library-API surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-up from the Task 9 code review: - buildRegistry now passes handlers.WithHandlerBundleIdentity when Config.BundleIdentity is set. Without it, embedded integrations with parallel or manager_loop nodes would receive unstamped EventStage* events on their cfg.EventHandler — re-introducing the bypass Task 7 closed at the CLI. - Slimmed the 14-line Config field comment that leaked the internal option name into the public API doc.
Add `Bundle:` line to the `tracker audit` header block (printed by `printAuditHeader`) showing the full bundle identity (`sha256:<64 hex>`) for `.dipx` runs. Plain `.dip` runs omit the line entirely. `AuditReport.BundleIdentity` is populated from `checkpoint.BundleIdentity` in `Audit()` so the printer has access to it. JSON tag uses `omitempty` so plain `.dip` runs produce no `bundle_identity` field in JSON output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a boolean flag (off by default) that allows resume to proceed even when the .dipx bundle's content-addressed identity differs from the original run. This is plumbing only — Task 15 will wire the resume identity verification logic that consumes the flag. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When resuming a run with `tracker -r <runID>`, the resume flow now verifies the checkpoint's stored BundleIdentity against the current pipeline source. Any mismatch — including .dipx→.dip downgrades and .dip→.dipx upgrades — aborts the resume unless the user explicitly passes --force-bundle-mismatch. Adds EventBundleMismatchForced for future activity.jsonl audit emission; this commit limits the surface to a stderr warning on force-override so the change stays focused on verification logic. Event emission can be wired later without re-opening the flag/verifier contract. Empty-vs-empty (plain .dip resume against .dip) is still silently allowed, preserving existing behavior for non-bundle workflows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Follow-ups from the Task 15 code review: - currentBundleIdentity had no direct test coverage. Adds five cases: real .dipx (via dipxtest.PackTestBundle), .dip / .dot / no-extension / .txt fall-through to empty identity, and missing .dipx file producing the "resume verification" wrap. - EventBundleMismatchForced declared but unused — added a TODO comment so its deferred wiring is visible to grep.
Drives a real .dipx bundle through tracker.Run and asserts the content-addressed identity propagates onto every audit surface: Result.BundleIdentity, Checkpoint.BundleIdentity, every line of activity.jsonl, and RunSummary.BundleIdentity (via ListRuns). The pipeline is the minimal start→exit passthrough produced by dipxtest.MinimalDip — completes without LLM calls, pins the full integration end-to-end so a future regression on any one surface (stamping bypass, unwired registry, forgotten Result mirror) is caught here even if the unit tests for each surface continue to pass.
Critical fix from the final code review. LoadDipxBundle returned a subgraphs map keyed by canonical bundle paths (e.g., "workflows/sub.dip"), but the entry graph's nodes kept their author-written subgraph_ref values (e.g., "sub.dip"). validateSubgraphRefs then failed to find the subgraph by source ref, breaking every multi-file .dipx — including the canonical sprint_runner_dr.dipx use case the original request describes. Fix walks the loaded entry and subgraphs after IR-to-Graph conversion and rewrites each subgraph_ref attr via bundle.Resolve(ref, parentPath) so it matches the subgraphs map key. Also strengthens TestLoadDipxBundle_WithSubgraph to assert ref-to-map consistency, and adds a CLI-integration test that exercises the full validateSubgraphRefs path with a real subgraph-containing bundle.
Two fixes from the final code review: 1. tracker doctor was the one CLI command not migrated to handle .dipx — the extension whitelist rejected it and the loader read the ZIP bytes as .dip text. Routes through pipeline.LoadDipxBundle now. 2. dippin's DIP126 lint (subgraph ref doesn't exist on disk) fired on every load of a bundle with subgraphs, because the bundle's file paths live inside the ZIP, not on the filesystem. Filtered out for the .dipx loader path. .dip loader path is unchanged.
Final code review caught that the original Task 17 entry omitted tracker doctor from the affected-commands list. The C2 follow-up fix landed doctor support; this completes the CHANGELOG's command list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the two deferred items from the final code review: - EventBundleMismatchForced was declared but never emitted. Now resolveRunCheckpoint returns a resumeInfo struct carrying the forced-mismatch detail; run.go and runTUI write the event to activity.jsonl via the new JSONLEventHandler.WriteBundleMismatchForced once the activity log is constructed. The audit trail can now record that a run executed against a different bundle than its checkpoint claimed. - Added CLI integration tests covering resume identity match, mismatch-abort, and mismatch-with-force paths through resolveRunCheckpoint end-to-end (previously only the inner verifyResumeBundle and currentBundleIdentity were tested). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds native ChangesNative .dipx Bundle Support
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/tracker/commands.go (1)
233-242:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate usage text to reflect
.dipxsupport.Both
validateandsimulateusage messages still imply only.dipinput, which conflicts with the new native.dipxsupport.Suggested patch
- return fmt.Errorf("usage: tracker validate <pipeline.dip>") + return fmt.Errorf("usage: tracker validate <pipeline.dip|bundle.dipx>") ... - return fmt.Errorf("usage: tracker simulate <pipeline.dip>") + return fmt.Errorf("usage: tracker simulate <pipeline.dip|bundle.dipx>")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/tracker/commands.go` around lines 233 - 242, The usage strings for the validate and simulate commands still reference only ".dip"; update the error messages returned when cfg.pipelineFile == "" in the function that calls runValidateCmd and in executeSimulate to mention that both .dip and .dipx are accepted (e.g., "usage: tracker validate <pipeline.dip|pipeline.dipx>" and similarly for "tracker simulate"), so users know .dipx is supported; locate the two return fmt.Errorf(...) calls and change their message text accordingly.
🧹 Nitpick comments (2)
cmd/tracker/main.go (1)
44-47: ⚡ Quick winRefresh stale task-scoped comment text.
Line [44]-Line [47] says there is “no behavior wiring yet,” which is now outdated and may confuse future readers.
Proposed comment update
- // Consumed by resume identity verification (Task 15); Task 14 only - // plumbs the flag — no behavior wiring yet. + // Consumed by resume identity verification to allow an explicit + // operator override when bundle identities differ.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/tracker/main.go` around lines 44 - 47, The comment for the forceBundleMismatch flag is stale: remove "no behavior wiring yet" and update it to reflect that the flag is now wired into resume identity verification; specifically change the block referencing Task 14/Task 15 to state that Task 14 plumbs the flag and Task 15 (resume identity verification) consumes it, and briefly describe its effect (allows resume to proceed when the .dipx bundle identity differs) so readers understand current behavior; edit the comment above the forceBundleMismatch declaration to this updated wording.pipeline/events_jsonl.go (1)
285-294: 💤 Low valueConsider extracting
displayIdentityForLogto a shared package.The comment on line 286 notes this duplicates
cmd/tracker/resume_bundle.go:displayIdentity. While the duplication is acknowledged and both implementations are simple, consolidating into a shared utility (e.g., a newinternal/bundleutilpackage or adding to an existing internal package) would eliminate the maintenance burden of keeping both in sync.🔧 Suggested consolidation approach
If consolidation is desired, create a shared helper in a common package accessible to both
pipelineandcmd/tracker(e.g.,internal/bundleutil), then import and use it in both locations. However, given the simplicity of the function and the low likelihood of divergence, accepting the duplication is also reasonable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pipeline/events_jsonl.go` around lines 285 - 294, The function displayIdentityForLog is duplicated across packages (pipeline and cmd/tracker); extract it into a shared internal package (e.g., internal/bundleutil) as a single exported helper (e.g., DisplayIdentityForLog) and replace the local implementations by importing that package and calling the shared function from both pipeline/events_jsonl.go and cmd/tracker/resume_bundle.go; ensure the new helper preserves the exact behavior for empty strings and update imports and any references accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/tracker/resume_bundle.go`:
- Around line 26-30: The error message constructed with fmt.Errorf using
errBundleIdentityMismatch, displayIdentity(checkpointIdentity), and
displayIdentity(currentIdentity) ends with a period which triggers staticcheck
ST1005; update the format string in resume_bundle.go to remove the trailing
period (so the final sentence "To resume against a different bundle, pass
--force-bundle-mismatch" does not end with a period) while keeping the same
fmt.Errorf call and inserted values (errBundleIdentityMismatch,
displayIdentity(checkpointIdentity), displayIdentity(currentIdentity)).
In `@cmd/tracker/resume_dipx_test.go`:
- Around line 227-235: The test currently reassigns the process-global os.Stderr
(origStderr/devnull/os.Stderr), which can cause cross-test flakiness; remove the
os.Stderr reassignment and teardown, and instead either (A) accept/ignore the
warning output in this test, or (B) refactor the production code under test to
accept an io.Writer for warnings and pass a bytes.Buffer or testing logger from
the test to capture/inspect output; update the test to use the injected writer
(or simply not suppress os.Stderr) rather than mutating
origStderr/devnull/os.Stderr.
In `@docs/plans/2026-05-11-native-dipx-bundle-support-design.md`:
- Around line 51-57: The markdown fences in the design doc are missing language
identifiers and trigger MD040; update each unlabeled triple-backtick fence
around examples (e.g., the block showing loadPipeline(filename, formatOverride)
that references loadDipxPipeline and the CLI/flow diagrams including
resolveRunCheckpoint and tracker.Result.BundleIdentity) to include a language
tag such as text (replace ``` with ```text) so markdownlint passes; apply the
same change to all other unlabeled fences mentioned (lines 140-181 and 187-199).
In `@pipeline/dipx_load_test.go`:
- Around line 179-195: This test directly reads node.Attrs["subgraph_ref"]
inside the loop over graph.Nodes (the block that sets foundRef and validates
subgraphs/ prefixes), which violates the rule to use typed accessors; add a
concise inline comment immediately above the direct access explaining why the
exception is acceptable here (e.g., "test verifies canonicalization after parse
so we intentionally read raw Attrs for strict-parse check"), referencing
node.Attrs and the for loop/validation logic so future readers know this is a
deliberate, allowed direct attribute read in test code.
In `@pipeline/dipx_load.go`:
- Around line 110-121: The helper canonicalizeSubgraphRefs currently
reads/writes node.Attrs["subgraph_ref"] directly; change it to use the pipeline
Node typed accessors (or add them) instead: call the Node accessor to read the
subgraph ref (e.g., Node.GetSubgraphRef() or equivalent) and skip when empty,
call bundle.Resolve(...) as before, and then set the canonical value via a Node
setter (e.g., Node.SetSubgraphRef(canonical)); update canonicalizeSubgraphRefs
signature to accept/return any types needed and keep the same error wrapping.
Ensure you reference the Graph, canonicalizeSubgraphRefs, dipx.Bundle, and the
"subgraph_ref" field in your changes and do not access node.Attrs directly.
In `@pipeline/engine_test.go`:
- Around line 92-95: The tests create a shared slice named captured and a
PipelineEventHandlerFunc handler that appends to it without synchronization;
protect access by adding a sync.Mutex (or sync.RWMutex) and lock/unlock around
any append/read of captured inside the handler and in the test assertions so
concurrent event emissions are safe; update both test occurrences that declare
captured and handler (the PipelineEventHandlerFunc closures) to use the mutex
when modifying or iterating the captured []PipelineEvent.
In `@pipeline/handlers/stamping.go`:
- Around line 25-30: Guard against a nil receiver and nil Inner in
BundleIdentityStamper.HandlePipelineEvent to avoid panics: at the start of
HandlePipelineEvent check if s == nil and return immediately; then perform the
existing evt.BundleIdentity assignment using s.Identity, and before calling
s.Inner.HandlePipelineEvent(evt) check if s.Inner != nil and only call it when
non-nil. This uses the existing BundleIdentityStamper, HandlePipelineEvent,
Inner and Identity symbols.
---
Outside diff comments:
In `@cmd/tracker/commands.go`:
- Around line 233-242: The usage strings for the validate and simulate commands
still reference only ".dip"; update the error messages returned when
cfg.pipelineFile == "" in the function that calls runValidateCmd and in
executeSimulate to mention that both .dip and .dipx are accepted (e.g., "usage:
tracker validate <pipeline.dip|pipeline.dipx>" and similarly for "tracker
simulate"), so users know .dipx is supported; locate the two return
fmt.Errorf(...) calls and change their message text accordingly.
---
Nitpick comments:
In `@cmd/tracker/main.go`:
- Around line 44-47: The comment for the forceBundleMismatch flag is stale:
remove "no behavior wiring yet" and update it to reflect that the flag is now
wired into resume identity verification; specifically change the block
referencing Task 14/Task 15 to state that Task 14 plumbs the flag and Task 15
(resume identity verification) consumes it, and briefly describe its effect
(allows resume to proceed when the .dipx bundle identity differs) so readers
understand current behavior; edit the comment above the forceBundleMismatch
declaration to this updated wording.
In `@pipeline/events_jsonl.go`:
- Around line 285-294: The function displayIdentityForLog is duplicated across
packages (pipeline and cmd/tracker); extract it into a shared internal package
(e.g., internal/bundleutil) as a single exported helper (e.g.,
DisplayIdentityForLog) and replace the local implementations by importing that
package and calling the shared function from both pipeline/events_jsonl.go and
cmd/tracker/resume_bundle.go; ensure the new helper preserves the exact behavior
for empty strings and update imports and any references accordingly.
🪄 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: CHILL
Plan: Pro
Run ID: a4c25c68-9b8f-4907-ad81-c2bd56d3567c
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (45)
CHANGELOG.mdagent/tools/completer_test.gocmd/tracker/audit.gocmd/tracker/audit_test.gocmd/tracker/commands.gocmd/tracker/flags.gocmd/tracker/loading.gocmd/tracker/loading_dipx_test.gocmd/tracker/main.gocmd/tracker/resolve.gocmd/tracker/resume_bundle.gocmd/tracker/resume_dipx_test.gocmd/tracker/run.gocmd/tracker/simulate.gocmd/tracker/simulate_test.gocmd/tracker/validate.gocmd/tracker/validate_test.godocs/plans/2026-05-11-native-dipx-bundle-support-design.mddocs/plans/2026-05-11-native-dipx-bundle-support.mddocs/requests/native-dipx-bundle-support.mdgo.modinternal/dipxtest/pack.gointernal/dipxtest/pack_test.gopipeline/checkpoint.gopipeline/checkpoint_test.gopipeline/dippin_load.gopipeline/dippin_load_test.gopipeline/dipx_load.gopipeline/dipx_load_test.gopipeline/engine.gopipeline/engine_checkpoint.gopipeline/engine_test.gopipeline/events.gopipeline/events_jsonl.gopipeline/events_jsonl_test.gopipeline/handlers/registry.gopipeline/handlers/stamping.gopipeline/handlers/stamping_test.gotracker.gotracker_audit.gotracker_audit_test.gotracker_dipx_e2e_test.gotracker_doctor.gotracker_doctor_test.gotracker_test.go
Comment on lines
+110
to
+121
| func canonicalizeSubgraphRefs(g *Graph, bundle *dipx.Bundle, parentBundlePath string) error { | ||
| for _, node := range g.Nodes { | ||
| ref := node.Attrs["subgraph_ref"] | ||
| if ref == "" { | ||
| continue | ||
| } | ||
| canonical, err := bundle.Resolve(ref, parentBundlePath) | ||
| if err != nil { | ||
| return fmt.Errorf("resolve subgraph ref %q from %s: %w", ref, parentBundlePath, err) | ||
| } | ||
| node.Attrs["subgraph_ref"] = canonical | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Avoid direct node.Attrs access in pipeline code.
This helper directly reads/writes node.Attrs["subgraph_ref"]. Please route this through typed node config accessors (or add a dedicated accessor) to match pipeline config access conventions.
As per coding guidelines pipeline/**/*.go: “Node configuration reads must go through typed accessors on *pipeline.Node ... Do not read node.Attrs[...] directly except for strict-parse helpers with inline comments explaining why”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pipeline/dipx_load.go` around lines 110 - 121, The helper
canonicalizeSubgraphRefs currently reads/writes node.Attrs["subgraph_ref"]
directly; change it to use the pipeline Node typed accessors (or add them)
instead: call the Node accessor to read the subgraph ref (e.g.,
Node.GetSubgraphRef() or equivalent) and skip when empty, call
bundle.Resolve(...) as before, and then set the canonical value via a Node
setter (e.g., Node.SetSubgraphRef(canonical)); update canonicalizeSubgraphRefs
signature to accept/return any types needed and keep the same error wrapping.
Ensure you reference the Graph, canonicalizeSubgraphRefs, dipx.Bundle, and the
"subgraph_ref" field in your changes and do not access node.Attrs directly.
- cmd/tracker/resume_bundle.go: drop trailing period on mismatch
error message (staticcheck ST1005).
- cmd/tracker/resume_dipx_test.go: drop the os.Stderr swap; the
forced-mismatch warning is informational and the assertions
only inspect the returned resumeInfo. Removes a flake risk
under `go test -p N`.
- docs/plans/2026-05-11-native-dipx-bundle-support-design.md:
label the three unlabeled fenced code blocks as `text`
(markdownlint MD040).
- pipeline/dipx_load_test.go: add a comment explaining why the
subgraph_ref read is intentionally raw — no typed accessor
exists on *pipeline.Node, matching the convention in
pipeline/subgraph.go, cmd/tracker/loading.go, etc.
- pipeline/dipx_load.go: add a parallel comment in
canonicalizeSubgraphRefs documenting the same convention.
Leaves the direct Attrs read as-is per the decision rule
("if no typed accessor exists, document the gap").
- pipeline/engine_test.go: guard the two captured []PipelineEvent
slices with a sync.Mutex so the tests stay correct if the
engine ever emits events from goroutines (the simple graph
used here doesn't trigger that today, but parallel /
manager_loop handlers can).
- pipeline/handlers/stamping.go: add defensive nil guards on
BundleIdentityStamper.HandlePipelineEvent so a nil receiver
or nil Inner doesn't panic.
- cmd/tracker/commands.go: update validate / simulate usage
strings to include `|bundle.dipx`.
- cmd/tracker/main.go: refresh the forceBundleMismatch comment
to reflect that Task 15 already wired the behavior.
- internal/bundleid/format.go (new): shared DisplayForLog helper
used by both pipeline/events_jsonl.go and
cmd/tracker/resume_bundle.go + cmd/tracker/commands.go.
Replaces two duplicated displayIdentity / displayIdentityForLog
functions with one canonical implementation.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class support for content-addressed .dipx bundles across Tracker’s CLI and library API, threading a bundle’s SHA-256 identity through checkpoints, activity logs, and audit/list surfaces, and enforcing identity checks on resume.
Changes:
- Introduces
.dipxloading viadipx.Openwith IR-direct conversion topipeline.Graphplus subgraph canonicalization. - Adds bundle identity propagation/stamping across engine events, handler-registry emissions, JSONL agent/LLM writes, and surfaces it in
Result, checkpoints,list, andaudit. - Implements strict resume-time bundle identity verification with
--force-bundle-mismatchand an auditablebundle_mismatch_forcedJSONL entry.
Reviewed changes
Copilot reviewed 46 out of 47 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tracker.go | Adds Config.BundleIdentity and mirrors it into Result.BundleIdentity; threads identity into engine/registry options. |
| tracker_test.go | Adds library-surface tests ensuring BundleIdentity flows into engine/checkpoints/events/results. |
| tracker_doctor.go | Updates pinned dippin version and adds .dipx validation path in doctor via bundle loader. |
| tracker_doctor_test.go | Adds regression test ensuring doctor accepts .dipx bundles. |
| tracker_dipx_e2e_test.go | Adds an end-to-end-ish test asserting identity appears across audit surfaces. |
| tracker_audit.go | Adds BundleIdentity to AuditReport/RunSummary and populates from checkpoint. |
| tracker_audit_test.go | Tests bundle identity propagation into Audit() and ListRuns(), including empty identity behavior. |
| pipeline/handlers/stamping.go | Adds BundleIdentityStamper wrapper for handler-emitted events that bypass Engine.emit. |
| pipeline/handlers/stamping_test.go | Unit tests for stamping wrapper + registry option wiring. |
| pipeline/handlers/registry.go | Adds WithHandlerBundleIdentity and wraps pipeline event handler when configured. |
| pipeline/events.go | Adds PipelineEvent.BundleIdentity and new EventBundleMismatchForced event type. |
| pipeline/events_jsonl.go | Adds bundle_identity to JSONL entries; supports stamping for agent/LLM writes; adds WriteBundleMismatchForced. |
| pipeline/events_jsonl_test.go | Tests JSONL bundle_identity behavior for pipeline/agent/LLM entries and forced-mismatch entry shape. |
| pipeline/engine.go | Adds WithBundleIdentity and stamps identity in Engine.emit. |
| pipeline/engine_test.go | Tests engine stamping behavior and checkpoint persistence of bundle identity. |
| pipeline/engine_checkpoint.go | Persists engine bundle identity into checkpoint saves. |
| pipeline/dipx_load.go | Adds LoadDipxBundle to open/verify/convert bundles and return identity + subgraphs. |
| pipeline/dipx_load_test.go | Tests bundle loading, hash mismatch detection, subgraph ref canonicalization, and DIP126 suppression. |
| pipeline/dippin_load.go | Refactors to add LoadDippinWorkflowFromIR to reuse validate/lint/convert tail for bundles. |
| pipeline/dippin_load_test.go | Verifies source-vs-IR loaders produce equivalent graphs; nil IR guard test. |
| pipeline/checkpoint.go | Adds Checkpoint.BundleIdentity with omitempty for backward compatibility. |
| pipeline/checkpoint_test.go | Tests BundleIdentity JSON roundtrip and backward compat load. |
| internal/dipxtest/pack.go | Adds test helper for packing real .dipx bundles via dipx.Pack. |
| internal/dipxtest/pack_test.go | Tests PackTestBundle produces a valid openable .dipx. |
| internal/bundleid/format.go | Adds shared formatting helper for identity display in logs/errors. |
| cmd/tracker/loading.go | Adds .dipx dispatch via loadPipelineAndBundle and pipeline.LoadDipxBundle. |
| cmd/tracker/loading_dipx_test.go | Tests .dipx dispatch and subgraph ref validation for bundles. |
| cmd/tracker/validate.go | Routes validate through loadPipelineAndBundle so .dipx works. |
| cmd/tracker/validate_test.go | Adds .dipx validate regression test. |
| cmd/tracker/simulate.go | Adds .dipx simulation path by loading bundle graphs and simulating directly. |
| cmd/tracker/simulate_test.go | Adds .dipx simulate regression tests (plan output + lint warnings behavior). |
| cmd/tracker/run.go | Plumbs bundle identity into JSONL handler stamping, engine opts, registry opts, and forced-mismatch audit entry emission. |
| cmd/tracker/resolve.go | Updates explicit file detection to include .dipx. |
| cmd/tracker/main.go | Adds forceBundleMismatch flag field to runConfig. |
| cmd/tracker/flags.go | Adds --force-bundle-mismatch and includes it in usage output. |
| cmd/tracker/resume_bundle.go | Adds strict bundle identity verification helper for resume. |
| cmd/tracker/resume_dipx_test.go | Tests resume verification logic and resolveRunCheckpoint behavior with real bundles. |
| cmd/tracker/commands.go | Implements resume-time identity verification, warning output on forced mismatch, and wires resume info to run path. |
| cmd/tracker/audit.go | Adds Bundle column to tracker list output and Bundle line to tracker audit header. |
| cmd/tracker/audit_test.go | Tests Bundle column truncation and Bundle header printing behavior. |
| agent/tools/completer_test.go | Adjusts tests for macOS symlink behavior in temp dirs (unrelated robustness fix). |
| go.mod | Bumps github.com/2389-research/dippin-lang to v0.24.0; updates golang.org/x/text. |
| go.sum | Updates checksums for bumped dependencies. |
| CHANGELOG.md | Adds release notes for native .dipx bundle support and dippin-lang bump. |
| docs/requests/native-dipx-bundle-support.md | Adds upstream feature request document being closed by this PR. |
| docs/plans/2026-05-11-native-dipx-bundle-support.md | Adds detailed implementation plan (large design/plan artifact). |
| docs/plans/2026-05-11-native-dipx-bundle-support-design.md | Adds design doc describing approach and surfaces. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+44
to
+68
| entry := bundle.Entry() | ||
| entryGraph, diags, err := LoadDippinWorkflowFromIR(entry, manifest.Entry) | ||
| for _, d := range filterBundleLintNoise(diags) { | ||
| fmt.Fprintln(os.Stderr, d.String()) | ||
| } | ||
| if err != nil { | ||
| return nil, nil, BundleInfo{}, fmt.Errorf("load bundle %s: entry %s: %w", path, manifest.Entry, err) | ||
| } | ||
| if err := canonicalizeSubgraphRefs(entryGraph, bundle, manifest.Entry); err != nil { | ||
| return nil, nil, BundleInfo{}, fmt.Errorf("load bundle %s: entry %s: %w", path, manifest.Entry, err) | ||
| } | ||
|
|
||
| subgraphs := make(map[string]*Graph) | ||
| for _, file := range manifest.Files { | ||
| if file.Path == manifest.Entry { | ||
| continue | ||
| } | ||
| wf, err := bundle.Lookup(file.Path) | ||
| if err != nil { | ||
| return nil, nil, BundleInfo{}, fmt.Errorf("load bundle %s: lookup %s: %w", path, file.Path, err) | ||
| } | ||
| sub, subDiags, err := LoadDippinWorkflowFromIR(wf, file.Path) | ||
| for _, d := range filterBundleLintNoise(subDiags) { | ||
| fmt.Fprintln(os.Stderr, d.String()) | ||
| } |
Comment on lines
+18
to
+22
| Timestamp string `json:"ts"` | ||
| Source string `json:"source"` // "pipeline", "agent", "llm" | ||
| Type string `json:"type"` | ||
| RunID string `json:"run_id,omitempty"` | ||
| NodeID string `json:"node_id,omitempty"` |
Comment on lines
+19
to
+23
| // TestE2E_DipxIdentityFlowsThroughAllAuditSurfaces drives a real .dipx bundle | ||
| // through tracker.Run and asserts the content-addressed identity appears on | ||
| // every audit surface: Result.BundleIdentity, Checkpoint.BundleIdentity, | ||
| // EVERY line of activity.jsonl, and RunSummary.BundleIdentity (via ListRuns). | ||
| // |
Collaborator
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Resolves CHANGELOG.md conflict with v0.25.1 (Gemini SSE / Bedrock
Gateway polish landed on main while this branch was open) by stacking
the [Unreleased] dipx entry above the v0.25.1 section.
Addresses two unresolved Copilot review items:
- pipeline.LoadDipxBundle no longer writes to os.Stderr. The library
function now returns []validator.Diagnostic alongside the graph; the
CLI wrapper (cmd/tracker/loadDipxPipeline) prints them to stderr,
mirroring how cmd/tracker/loadDippinPipeline handles the .dip path.
tracker doctor surfaces them as CheckStatusWarn details. This keeps
the pipeline package free of process-global stderr side effects so
embedded callers can route diagnostics through their own logger.
- jsonlLogEntry.Source comment in pipeline/events_jsonl.go was missing
the "cli" value emitted by WriteBundleMismatchForced. Updated to
list all four sources with one-line descriptions.
- tracker_dipx_e2e_test.go docstring overclaimed scope ("drives a
real .dipx bundle through tracker.Run"); the run actually executes
raw .dip source, with the bundle only used to compute wantIdentity.
Reworded to accurately describe what the test pins (the cross-
surface identity *stamping* path) and point readers at the bundle-
loader tests in pipeline/dipx_load_test.go for the execution path.
The unresolved CodeRabbit item on pipeline/dipx_load.go:110
(canonicalizeSubgraphRefs reads node.Attrs["subgraph_ref"] directly)
is intentionally left as-is; the inline comment documents that this
matches the existing convention in pipeline/subgraph.go,
cmd/tracker/loading.go:loadSubgraphsRecursive, cmd/tracker/nodes.go,
and the manager_loop handler — there is no typed accessor for
subgraph_ref in the codebase, and adding one is out of scope for this
PR.
CodeRabbit's "extract displayIdentityForLog to shared package"
nitpick is marked Low value by CodeRabbit itself and is skipped.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment on lines
+915
to
+937
| func checkPipelineBundle(bundlePath string) CheckResult { | ||
| out := CheckResult{Name: "Pipeline File"} | ||
| entry, subgraphs, info, diags, err := pipeline.LoadDipxBundle(context.Background(), bundlePath) | ||
| if err != nil { | ||
| out.Status = CheckStatusError | ||
| out.Message = fmt.Sprintf("%s: bundle load failed: %v", bundlePath, err) | ||
| out.Hint = "run `tracker validate " + bundlePath + "` for full details" | ||
| return out | ||
| } | ||
| for _, d := range diags { | ||
| out.Details = append(out.Details, CheckDetail{ | ||
| Status: CheckStatusWarn, | ||
| Message: d.String(), | ||
| }) | ||
| } | ||
| out.Details = append(out.Details, CheckDetail{ | ||
| Status: CheckStatusOK, | ||
| Message: fmt.Sprintf("%s valid (%d nodes, %d edges, %d subgraph(s), identity %s)", | ||
| bundlePath, len(entry.Nodes), len(entry.Edges), len(subgraphs), info.Identity), | ||
| }) | ||
| out.Status = CheckStatusOK | ||
| out.Message = fmt.Sprintf("%s is valid", bundlePath) | ||
| return out |
| func executeValidate(cfg runConfig) error { | ||
| if cfg.pipelineFile == "" { | ||
| return fmt.Errorf("usage: tracker validate <pipeline.dip>") | ||
| return fmt.Errorf("usage: tracker validate <pipeline.dip|bundle.dipx>") |
| func executeSimulate(cfg runConfig) error { | ||
| if cfg.pipelineFile == "" { | ||
| return fmt.Errorf("usage: tracker simulate <pipeline.dip>") | ||
| return fmt.Errorf("usage: tracker simulate <pipeline.dip|bundle.dipx>") |
- cmd/tracker/commands.go validate/simulate usage strings dropped .dot when .dipx was added. Both commands still support .dot pipelines via detectPipelineFormat, so the hint was misleading. Restored .dot. - tracker_doctor.go checkPipelineBundle now matches checkPipelineFile's warn-promotion behavior: when LoadDipxBundle returns lint diagnostics (or when the bundled entry graph trips tracker's ValidateAllWithLint), the overall check status is now CheckStatusWarn with a "valid with N warning(s)" message instead of a bare OK. Also runs ValidateAllWithLint on the entry graph so .dipx gets the same handler-aware semantic validation coverage as the .dip path — dipx.Open verified structural integrity at load time, this adds tracker's own checks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment on lines
38
to
44
| @@ -38,7 +39,7 @@ func loadPipelineForValidation(pipelineFile, formatOverride string) (*pipeline.G | |||
| graph, err = loadEmbeddedPipeline(info) | |||
| displayName = info.Name | |||
| } else { | |||
| graph, err = loadPipeline(resolved, formatOverride) | |||
| graph, _, _, err = loadPipelineAndBundle(resolved, formatOverride) | |||
| displayName = resolved | |||
| } | |||
Copilot caught that loadPipelineForValidation routed .dip/.dot through loadPipelineAndBundle (added for .dipx support), which eagerly walks and parses every transitively-referenced subgraph file and then discards them. That's wasted I/O on every validate, and worse, surfaces new "missing subgraph file" failures on .dip pipelines that previously validated fine (validate has historically validated the entry file only — subgraph ref validation happens at run time). Split the loader into three branches: embedded → loadEmbeddedPipeline, .dipx → loadDipxPipeline (still validates hashes + canonicalizes refs since the bundle loader is doing that work either way), .dip/.dot → loadPipeline (pre-PR behavior, no subgraph walk). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 12, 2026
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
Tracker now accepts content-addressed
.dipxbundles (produced bydippin pack) natively across every command —validate,simulate,run,doctor, and-r <runID>resume. The bundle's SHA-256 identity is threaded through every audit surface (activity.jsonlevery line,checkpoint.json,tracker listnew Bundle column,tracker auditnew Bundle header,tracker.Result.BundleIdentity,tracker.RunSummary.BundleIdentity), and resume strictly verifies that the current bundle matches the one the checkpoint was started against — mismatch aborts unless--force-bundle-mismatchis set (which logs both a stderr warning and anEventBundleMismatchForcedline toactivity.jsonl).Loader uses dipx's pre-parsed
*ir.Workflowdirectly (no re-parse of bundled sources) and bypasses the filesystem subgraph walker since dipx already verified ref closure + acyclicity onOpen. Bundle identity stamping flows through three composable layers —Engine.emit,BundleIdentityStamperregistry wrapper for handlers that bypass the engine chokepoint (parallel, manager_loop), andJSONLEventHandler.SetBundleIdentityfor agent/llm writes that bypass both — so every line of audit output carries provenance.Closes
docs/requests/native-dipx-bundle-support.md(Pipelines team feature request).github.com/2389-research/dippin-langv0.23.0 → v0.24.0 for thedipxpackageTest plan
go build ./...cleango test ./... -short— 18 packages pass (was 17, plus newinternal/dipxtest).dipx, runtracker validate,simulate,doctoron it — all succeedtracker run smoke.dipx --autopilot laxagainst a trivial pipeline → checkpoint persistsBundleIdentity, everyactivity.jsonlline carriesbundle_identity,tracker listshows truncated Bundle column,tracker audit <runID>showsBundle:header--force-bundle-mismatchallows resume and writesbundle_mismatch_forcedtoactivity.jsonldippin doctorexamples/*.dip still A grade🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmithwith what you need.Summary by CodeRabbit
New Features
Tests
Chores