Conversation
New() returns Result, accepts CoreOption functionals. Restores v0.3.3 service registration contract: - WithService(factory func(*Core) Result) — service factory receives Core - WithOptions(Options) — key-value configuration - WithServiceLock() — immutable after construction Services registered in New() form the application conclave with shared IPC access. Each Core instance has its own bus scope. Co-Authored-By: Virgil <virgil@lethean.io>
…. Check: 1) API contract ...' (#29) from agent/review-pr--28--read-claude-md-first--che into dev
- WithOptions copies the Options slice (constructor isolation regression) - WithService auto-discovers service name from package path via reflect - WithService auto-registers HandleIPCEvents if present (v0.3.3 parity) - Add test for failing option short-circuit in New() Co-Authored-By: Virgil <virgil@lethean.io>
- HandleIPCEvents only auto-registered for services the factory didn't
register itself (prevents double handler registration)
- Auto-discovery only creates Service{} placeholder when factory didn't
call c.Service() — factories that register themselves keep full lifecycle
Addresses Codex review findings 1 and 2 from third pass.
Co-Authored-By: Virgil <virgil@lethean.io>
WithService is now a simple factory call — no reflect, no auto-registration. New() calls discoverHandlers() after all opts run, scanning Config for service instances that implement HandleIPCEvents. This eliminates both double-registration and empty-placeholder issues: - Factories wire their own lifecycle via c.Service() - HandleIPCEvents discovered once, after all services are registered - No tension between factory-registered and auto-discovered paths Co-Authored-By: Virgil <virgil@lethean.io>
Restores v0.3.3 service manager capabilities: - RegisterService(name, instance) stores the raw instance - Auto-discovers Startable/Stoppable interfaces → wires lifecycle - Auto-discovers HandleIPCEvents → wires to IPC bus - ServiceFor[T](c, name) for typed instance retrieval - Service DTO gains Instance field for instance tracking WithService is a simple factory call — no reflect, no magic. discoverHandlers removed — RegisterService handles it inline. No double-registration: IPC wired once at registration time. Co-Authored-By: Virgil <virgil@lethean.io>
Options is now a proper struct with New(), Set(), Get(), typed accessors.
Result gains New(), Result(), Get() methods on the struct.
WithOption("key", value) convenience for core.New().
options_test.go: 22 tests passing against the new contract.
Other test files mechanically updated for compilation.
Co-Authored-By: Virgil <virgil@lethean.io>
App.New() creates from Options. App.Find() locates programs on PATH. Both are struct methods — no package-level functions. 8 tests passing. Co-Authored-By: Virgil <virgil@lethean.io>
Cli struct unchanged — already conforms. Tests use WithOption() convenience. 9 tests passing. Co-Authored-By: Virgil <virgil@lethean.io>
Cli{}.New(c) replaces &Cli{core: c} in contract.go.
9 tests passing.
Co-Authored-By: Virgil <virgil@lethean.io>
Cli as service with ServiceRuntime, incomplete. Need to properly port v0.3.3 service_manager, message_bus, WithService with full name/IPC discovery. Co-Authored-By: Virgil <virgil@lethean.io>
…tration
- WithService now calls factory, discovers service name from package path via
reflect/runtime (last path segment, _test suffix stripped, lowercased), and
calls RegisterService — which handles Startable/Stoppable/HandleIPCEvents
- If factory returns nil Value (self-registered), WithService returns OK without
a second registration
- Add contract_test.go with _Good/_Bad tests covering all three code paths
- Fix core.go Cli() accessor: use ServiceFor[*Cli](c, "cli") (was cli.New())
- Fix pre-existing })) → }}) syntax errors in command_test, service_test, lock_test
- Fix pre-existing Options{...} → NewOptions(...) in core_test, data_test,
drive_test, i18n_test (Options is a struct, not a slice)
Co-Authored-By: Virgil <virgil@lethean.io>
…tration WithService now: calls factory, discovers service name from instance's package path via reflect.TypeOf, discovers HandleIPCEvents method, calls RegisterService. If factory returns nil Value, assumes self-registered. Also fixes: Cli() accessor uses ServiceFor, test files updated for Options struct. Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
…arity Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
WithService: full name discovery + IPC handler auto-registration via reflect WithName: explicit service naming RegisterService: Startable/Stoppable/HandleIPCEvents auto-discovery MustServiceFor[T]: panics if not found WithServiceLock: enable/apply split (v0.3.3 parity) Cli: registered as service via CliRegister, accessed via ServiceFor @todo Codex: Fix data_test.go and embed_test.go — embed path resolution after Options changed from []Option to struct. Mount paths need updating. Co-Authored-By: Virgil <virgil@lethean.io>
…ixes
Root cause: Result.New didn't mark single-value results as OK=true,
breaking Mount/ReadDir/fs helpers that used Result{}.New(value, err).
Also: data_test.go and embed_test.go updated for Options struct,
doc comments updated across data.go, drive.go, command.go, contract.go.
All tests green. Coverage 82.2%.
Co-Authored-By: Virgil <virgil@lethean.io>
Config.New() initialises ConfigOptions. Fs.New(root) sets sandbox root. ErrorLog uses Default() fallback — no explicit init needed. contract.go uses constructors instead of struct literals. All tests green. Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
Co-Authored-By: Virgil <virgil@lethean.io>
…' (#28) from feat/service-options into dev
- Run() uses context.Background() for shutdown (c.context is cancelled) - Stoppable closure uses context.Background() for OnShutdown - WithService delegates HandleIPCEvents to RegisterService only Fixes Codex review findings 1, 2, 3. Co-Authored-By: Virgil <virgil@lethean.io>
…(#36) from fix/codex-review-findings into dev
Tests: Run, RegisterService, ServiceFor, MustServiceFor _Bad/_Ugly variants. Fix: Lock map is now per-Core instance, not package-level global. This prevents deadlocks when multiple Core instances exist (e.g. tests). Coverage: 82.4% → 83.6% Co-Authored-By: Virgil <virgil@lethean.io>
… (#37) from feat/test-coverage into dev
Standard llm.txt with package layout, key types, service pattern. Points to CLAUDE.md and docs/RFC.md for full specs. Co-Authored-By: Virgil <virgil@lethean.io>
Section 19 (API streams) and Section 21 (Entitlements) now implemented. 483 tests, 84.7% coverage, 100% AX-7 naming. Remaining v0.8.0 blockers: consumer alignment (go-process, core/agent). Consumer RFCs written and ready for implementation. Co-Authored-By: Virgil <virgil@lethean.io>
All 16 Known Issues replaced with resolved summary table. All 12 passes (108 findings) replaced with findings summary table. Full discovery detail preserved in git history. What remains: 21 feature sections (the API contract), Design Philosophy, AX Principles, root cause synthesis, consumer RFCs, versioning. No "Planned" tags. No unresolved findings. No v0.9.0 deferrals. Co-Authored-By: Virgil <virgil@lethean.io>
…ource
Plans 1-5 are implemented. Plan 6 (ecosystem sweep) is in consumer RFCs.
RFC.plan.md, RFC.plan.1.md, RFC.plan.2.md served their purpose as
session continuity docs — the work they described is done.
RFC.md (1935 lines) is now the single source of truth for core/go.
Removed:
- RFC.implementation.{1-6}.md
- RFC.plan.md, RFC.plan.1.md, RFC.plan.2.md
Co-Authored-By: Virgil <virgil@lethean.io>
- Version header: v0.7.0+ → v0.8.0 - Section 2.4: type Task any removed, PERFORM replaced by PerformAsync - Section 3.3+3.5: Startable/Stoppable return Result not error - Section 4.3: PERFORM → PerformAsync with named action + Options - Section 1.3: Added Process, API, Action, Task, Entitled, RegistryOf - Section 8: Added WriteAtomic, NewUnrestricted, Root - Section 9: Added Command.Managed field - Description updated to include "permission" Co-Authored-By: Virgil <virgil@lethean.io>
- Section 1.2: Added RunE() to lifecycle, clarified defer ServiceShutdown - Section 4.1: Handler results are ignored, panic recovery per handler - Section 6: Documented Data embeds Registry[*Embed] - Section 7: Documented Drive embeds Registry[*DriveHandle] - Section 14: Added core.ID(), ValidateName(), SanitisePath() Co-Authored-By: Virgil <virgil@lethean.io>
…detail Section 17 was 240 lines of design spec with old signatures. Now 30 lines matching the actual Process primitive. Consumer detail (ProcessHandle, IPC messages) lives in go-process/docs/RFC.md. Section 18 stale items identified for next pass. Co-Authored-By: Virgil <virgil@lethean.io>
Section 18: Removed PERFORM references, ActionDef→Action, TaskDef→Task, OnStartup returns Result, removed aspirational patterns (Parallel, Conditional, Scheduled), kept what's implemented. Section 19: Removed old struct definition, Stream returns Result not (Stream, error), RemoteAction uses c.RemoteAction() not c.Action(), removed stale subsystem map, added Web3 snider.lthn example. Section 20: Removed migration history (20.1 The Problem, 20.6-20.8), kept the API contract. Added 20.6 "What Embeds Registry" reference. 4193 → 1519 lines (64% reduction total). Co-Authored-By: Virgil <virgil@lethean.io>
- c.Registry("x") → c.RegistryOf("x") (3 occurrences)
- serviceRegistry → ServiceRegistry in export rules
- Removed speculative core/go-cli from layer list
- Entitlement "implementation pending" → "implemented" (3 occurrences)
- Removed Section 21.13 implementation plan (done)
- Cleaned changelog — one line per era, not per discovery step
- Fixed double blank line
Co-Authored-By: Virgil <virgil@lethean.io>
…rename - AX principle 5: PERFORM removed, now ACTION/QUERY + named Actions - Findings table: TaskDef → Task - Root Cause 2: removed stale v0.8.0 framing, Entitlement→Entitled method name - Root Cause 3: TaskDef→Task, linked to core/agent RFC not deleted plan file - Root Cause 4: Run()→RunE() in code example - Root Cause 5: Updated to show resolved items vs open items - Fixed triple blank line, cleaned whitespace Co-Authored-By: Virgil <virgil@lethean.io>
- Priority table: Phase references → Done status - Root Cause 5: "designed" → "Done" - Cross-ref table: c.Entitlement→c.Entitled, bool→Entitlement - Removed c.Secret() (not implemented) from examples - Cadence: future tense → present tense (process description) - Requirements: ActionDef/TaskDef rename cruft removed - Test count: 456→483 - Simplified entitlement example block Co-Authored-By: Virgil <virgil@lethean.io>
… spec tag Findings are converging — 4 items this pass vs 7 last pass. Co-Authored-By: Virgil <virgil@lethean.io>
…ecret() - "Open Problems" → "Ecosystem RFCs" (they're resolved) - c.Secret(name) removed (not implemented — future primitive) - P11-2 resolution: Fs.NewUnrestricted() not TIM - Simplified table columns 3 items found — diminishing returns confirmed. Co-Authored-By: Virgil <virgil@lethean.io>
CLAUDE.md was telling agents NOT to use WithService (the actual API). Tests path was wrong (tests/ vs root *_test.go). core.New() example showed deleted DTO pattern. PERFORM listed as current. Missing 6 subsystem accessors. llm.txt had pkg/core/ path (doesn't exist), PERFORM reference, missing 8 key types. Both now match v0.8.0 implementation. Co-Authored-By: Virgil <virgil@lethean.io>
README showed core.New(core.Options{...}) (deleted pattern),
RegisterTask (removed), PERFORM (removed), type Task any (removed).
Quick example would not compile.
Also found 6 docs/ files with same stale patterns — tracked for
next session (getting-started, index, messaging, primitives, testing).
Co-Authored-By: Virgil <virgil@lethean.io>
…rted, testing All PERFORM/RegisterTask/type Task any references replaced with named Action patterns. Every code example now uses the v0.8.0 API. - docs/messaging.md: full rewrite — ACTION/QUERY + named Actions + Task - docs/primitives.md: full rewrite — added Action, Task, Registry, Entitlement - docs/index.md: full rewrite — updated surface table, quick example, doc links - docs/getting-started.md: 2 RegisterTask+PERFORM blocks → Action pattern - docs/testing.md: 1 RegisterTask+PERFORM block → Action pattern An agent reading any doc file now gets compilable code. Co-Authored-By: Virgil <virgil@lethean.io>
Removed ~200 lines of progress-report content: - Known Issues (16 resolved items — history, not spec) - Findings Summary (108 findings table — discovery report) - Synthesis (5 root causes narrative — architectural history) - What v0.8.0 Requires (Done checklist — project management) - What Blocks / What Does NOT Block (status tracking) All preserved in git history. The RFC now describes what v0.8.0 IS, not how we built it. 4193 → 1278 lines (70% reduction from original). Co-Authored-By: Virgil <virgil@lethean.io>
core.JSONMarshal(), JSONMarshalString(), JSONUnmarshal(), JSONUnmarshalString() wrap encoding/json so consumers don't import it directly. Same guardrail pattern as string.go wraps strings. api.go Call() now uses JSONMarshalString instead of placeholder optionsToJSON. 7 AX-7 tests. 490 tests total, 84.8% coverage. Co-Authored-By: Virgil <virgil@lethean.io>
…ciple 2) core/go was violating its own RFC-025 Principle 2: every exported function must have a comment showing HOW with real values. 37 functions had no comments — mostly one-liner accessors on Core, Config, ServiceRuntime, IPC, and Options. Now every exported function in every source file has a usage-example comment. AX Principle 2 compliance: 0/37 → 37/37 (100%). Co-Authored-By: Virgil <virgil@lethean.io>
Go Example functions serve triple duty: - Run as tests (count toward coverage) - Show in godoc (usage documentation) - Seed for Sonnet to write user guides Covers: New, Options, Result, Action (register/invoke/list), Task, Registry (Set/Get/Lock/Seal), Entitlement (default/custom/NearLimit), Process (permission model), JSON, ID, ValidateName, SanitisePath, Command, Config, Error. 512 tests total, 84.8% coverage. Co-Authored-By: Virgil <virgil@lethean.io>
…h, service, error, array 33 new examples across 8 dedicated files. Removed phantom CleanPath (in RFC spec but never implemented — spec drift caught by examples). 545 tests total, 84.8% coverage. Every major primitive has compilable examples that serve as test, documentation seed, and godoc content. Co-Authored-By: Virgil <virgil@lethean.io>
CleanPath existed all along (path.go:118) — earlier grep had a stray quote that hid it. Example now demonstrates actual behaviour: redundant separator removal and .. resolution. Removed duplicate CleanPath_Good test that conflicted with existing. 546 tests, all pass. Co-Authored-By: Virgil <virgil@lethean.io>
New example files: entitlement, task, lock, log, drive, config, command, info. Every major source file now has a dedicated *_example_test.go with compilable, tested examples. 561 tests, 84.8% coverage. Co-Authored-By: Virgil <virgil@lethean.io>
Replaced all errors.New() with core.NewError() and errors.Is() with core.Is() across error_test.go, error_example_test.go, utils_test.go. The "errors" stdlib import is now zero across all test files. Examples teach agents core.NewError() and core.Is() — not errors.New(). Dogfooding: Core's own tests use Core's own primitives. Co-Authored-By: Virgil <virgil@lethean.io>
Replaced all filepath.Join() with core.Path() across fs_test.go, fs_example_test.go, core_test.go, path_test.go. core.Path() IS the path traversal security boundary. Agents using filepath.Join bypass it. Tests now demonstrate the Core way. "path/filepath" import is now zero across all test files. Co-Authored-By: Virgil <virgil@lethean.io>
Eliminated ALL disallowed imports from test files:
- os.WriteFile → Fs.Write()
- os.ReadFile → Fs.Read()
- os.ReadDir → Fs.List()
- os.MkdirAll → Fs.EnsureDir()
- os.MkdirTemp → Fs.TempDir()
- os.DirFS → core.DirFS()
- os.UserHomeDir → Env("DIR_HOME")
- os.Exit/Args/Environ → removed subprocess tests (RunE covers behaviour)
- Added Fs.TempDir() and core.DirFS() primitives
558 tests, 84.6% coverage. Core's tests now exclusively use Core's
own primitives. The quality gate from RFC-025 Principle 9 has zero
violations in Core's own codebase.
Co-Authored-By: Virgil <virgil@lethean.io>
…ives New primitives: - core.ReadAll(reader) — reads all from any reader, closes, returns Result - core.WriteAll(writer, content) — writes to any writer, closes, returns Result - core.CloseStream(v) — closes any value implementing io.Closer Replaced all io.ReadCloser/io.WriteCloser/io.ReadAll type assertions in fs_test.go and data_test.go with Core primitives. "io" import is now zero across all test files. 558 tests, 84.5% coverage. Remaining stdlib imports (all legitimate test infrastructure): testing, fmt, context, time, sync, embed, io/fs, bytes, gzip, base64 Co-Authored-By: Virgil <virgil@lethean.io>
…h everywhere
New primitive: core.Println() wraps fmt.Println.
Replaced across all test + example files:
- fmt.Println → Println (17 example files)
- fmt.Sprintf → Concat + Sprint
- dir + "/file" → Path(dir, "file") (path security)
- "str" + var → Concat("str", var) (AX consistency)
"fmt" import is now zero across all test files.
String concat with + is zero across all test files.
Remaining 9 stdlib imports (all Go infrastructure):
testing, context, time, sync, embed, io/fs, bytes, gzip, base64
558 tests, 84.5% coverage.
Co-Authored-By: Virgil <virgil@lethean.io>
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis pull request comprehensively restructures the CoreGO framework API: introducing named action and task composition, remote API/streaming support, entitlement-based permissions, process primitives, and a unified generic registry infrastructure. Documentation, lifecycle contracts, and service semantics were overhauled to reflect these changes. Changes
No sequence diagrams are generated, as the changes span multiple independent subsystems (documentation updates, refactoring of existing patterns, test reorganisation, and new utility functions) rather than introducing a cohesive new feature with complex multi-component interactions. |
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
Test plan
go test ./...— all pass🤖 Generated with Claude Code
Co-Authored-By: Virgil virgil@lethean.io
Summary by CodeRabbit
New Features
Refactor
Documentation