Add aps: Swift CLI dogfooding AppState - #1
Conversation
Ship a Swift 6 ArgumentParser executable with get/set/watch/dump over a fixed demo schema (State, StoredState, FileState) and real @AppDependency injection for clock + JSON coding. Co-authored-by: Leif <leif.algo@pm.me>
Add keys/reset commands, Observation watch coverage, Linux/macOS workflows, and a smoke script that respects process-local State. Co-authored-by: Leif <leif.algo@pm.me>
Wire fledge, SpecSync, augur, and attest; author aps-cli/state-store specs; replace hosted Linux/macOS jobs with self-hosted macOS runners while the repository is private. Co-authored-by: Leif <leif.algo@pm.me>
Accept the trust adoption change after fixing Public API export tables for SpecSync effective-contract checks. Add companion requirements and targeted tests; keep fledge verify native-only so Trust can own specsync. Co-authored-by: Leif <leif.algo@pm.me>
Reopen and re-accept CHG-0001 so closing evidence matches the delivered companion files and stays exact for Trust/specsync checks. Co-authored-by: Leif <leif.algo@pm.me>
Pass an explicit base..head comparison range so CorvidLabs Trust does not treat the credentialed origin remote as an external repository. Co-authored-by: Leif <leif.algo@pm.me>
Trust installs SpecSync 5.0.1; acceptance digests produced by 5.1.1 were rejected as stale. Re-verify and re-accept with 5.0.1 so the contract gate matches CI. Co-authored-by: Leif <leif.algo@pm.me>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6d9326fd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
0xLeif
left a comment
There was a problem hiding this comment.
Review from local verification (branch checked out and exercised, not just read).
Verified locally
swift build— clean, no warningsswift test— 18/18 pass (matches the claimed count)./Scripts/smoke.sh— passspecsync check --force— 2/2 specs, 15/15 exports each, as claimed- CI: both
build-test-smokeandtrustgreen
Blocking: the cross-process watch note claim is false
The spec contract (specs/aps-cli/aps-cli.spec.md:74-78) promises:
Given
aps watch noteis running / When another process runsaps set note changed/ Then the watcher printschangedwithin one poll interval.
Tested exactly this: the watcher printed its initial value and never saw the other process's write. Root cause is upstream in AppState — FileState.value's getter populates shared.cache on first read, so the polling fallback in StateStore.watchBlocking (Sources/aps/StateStore.swift:121) re-reads the in-memory cache, never the disk. Observation only fires for in-process writes, so external writes are invisible forever.
watch flag cross-process does work (verified: watcher printed false → true), because StoredState's read path doesn't populate the cache in a process that never sets the key. So README.md:80's "including updates written by another aps process" holds for flag but not note.
Options: narrow the spec/README to claim cross-process only for StoredState; make the note watcher read the file directly instead of via FileState; or fix the caching in AppState itself. Since specsync treats these examples as contracts, the spec shouldn't keep an example the code can't satisfy.
Minor
APSError.unknownKeyis unreachable — ArgumentParser rejects bad keys beforerun()(the spec's own Error Cases section says so).JSONCoding.decodeis only exercised by tests. Harmless, but they're documented exports, so they're load-bearing dead code.- README advertises "macOS and Linux first" / "macOS 14+ or Linux", but there is no Linux CI lane — the claim is untested. Either soften it or add a Linux job.
.attest.jsonhas every requirement off and.trust.tomlprovenance issoft— fine for v1, just be aware the trust gate is currently advisory.- CI builds release, then
swift testrebuilds debug — two full builds per run. Works, just slow. - Tests and smoke write to real
UserDefaults.standardand leave~/.aps/behind on whatever machine runs them (including the self-hosted runner).
What's good
- The
MainActor.assumeIsolated+precondition(Thread.isMainThread)bridge for synchronous@mainis the right trick, and it's documented. watchflushing viaFileHandle.standardOutputfor non-TTY stdout shows someone actually tested piping.- trust.yml pins action SHAs, sets explicit
permissions: contents: read, and both workflows carry the fork-PR self-hosted warning. - Specs genuinely track the code — specsync passing isn't theater.
Verdict: approve-worthy once the watch note cross-process spec example and README line are corrected (or the behavior is actually implemented). Everything else is nits.
Address PR review: poll note.json directly so cross-process watch works, configure FileState paths from CLI boot only, and fail set note when the on-disk value does not persist. Specs/README updated; CHG-0002 accepted. Co-authored-by: Leif <leif.algo@pm.me>
|
Addressed the blocking Blocking (cross-process
Codex P2s
Nits from your review
20 tests + smoke green locally; SpecSync CHG-0002 accepted (CHG-0001 archived). CI should re-run on this push. |
0xLeif
left a comment
There was a problem hiding this comment.
Follow-up on 8a76d01 — re-verified everything locally.
Blocking finding: fixed, verified empirically
Re-ran the exact experiment from the first review: aps watch note in one process now prints changed-from-other-process when a second process writes it. The direct disk read in freshValue/readNoteFromDisk (Sources/aps/StateStore.swift:143-166) correctly bypasses the FileState cache, and the format mirror is exact — AppState writes plain JSONEncoder().encode(value) for non-Base64 FileState, so the JSONDecoder read-back is correct.
Also good in this commit:
set noteread-verifies the disk write and throwsAPSError.persistenceFailed— closes the hole where AppState swallows save errors after updating its cache.APSPaths.configure()moving fromStateStore.init()toboot()also fixes a hidden test-isolation bug: the old ordering silently clobbered the test-injected temp path, so the previous suite was actually writing to~/.aps.testNoteUsesInjectedFileStatePathlocks the fix in.- README Linux claims softened; specs and requirements updated in lockstep.
Verified
swift buildclean, 20/20 tests pass (2 new),./Scripts/smoke.shok- CI:
build-test-smokeandtrustboth green on the new commit - Cross-process
watch note: pass (experiment above)
New issue: local specsync check now fails with the current CLI
error: legacy archive baseline historical integrity is invalid:
failed to read legacy archive baseline: No such file or directory
Archiving CHG-0001 under the Trust-pinned 5.0.1 flow did not produce .specsync/archive/legacy-baseline.json, which the current specsync release (5.1.1) requires. Before this commit there was no archive/ directory and 5.1.1 passed. Net effect: the repo now only passes specsync check with exactly 5.0.1, so fledge run check breaks locally for anyone on the latest release — and the README points at the brew tap, which installs latest. CI is green because Trust pins 5.0.1, so this is a contributor-experience break, not a CI break. Either regenerate the baseline with a 5.1.x flow, or pin/document the required local specsync version.
Verdict: the blocker is genuinely resolved — the code changes look approve-worthy. The specsync version skew needs a decision before merge, since it contradicts "local specsync check --force passes" for anyone not on 5.0.1.
Remove the incomplete 5.0.1 manual archive that lacked legacy-baseline.json, rebind CHG-0002 under SpecSync 5.1.1, and wire Trust v1.0.1 with a runner-temp file:// mirror so local brew SpecSync and CI share the same tool version. Co-authored-by: Leif <leif.algo@pm.me>
|
Addressed the SpecSync version skew in 458f017 by moving to latest SpecSync 5.1.1 (not pinning 5.0.1):
Local: |
* Publish aps v1.1 goal and archive merged SpecSync v1 change Add GOAL.md for the agent-ready AppState dogfood harness milestone, link it from the README, and archive CHG-0002 under SpecSync 5.1.1 now that PR #1 has landed on main. Co-authored-by: Leif <leif.algo@pm.me> * Ship aps 0.2.0 agent-ready CLI on pre-public 0.x Rename the mistaken v1.1 milestone to 0.2.0 and deliver the APS checklist: --json on core commands, APS_HOME/--state-dir, watch --count/--timeout/--jsonl, structured profile FileState, Linux smoke CI, and matching SpecSync contracts. Co-authored-by: Leif <leif.algo@pm.me> * Fix watch --jsonl stale FileState values on cross-process writes Build jsonl events from the fresh onChange string instead of re-querying AppState's FileState cache. Stamp events with the injected APSClock, add coverage for the event builder, and run swift test in linux-smoke. Co-authored-by: Leif <leif.algo@pm.me> * Document agent:cursor and agent:kimi ticket claim labels Cursor and Kimi fan out across issues; claim with one agent label per ticket before starting work, and drop the label when the PR is up or work stops. Co-authored-by: Leif <leif.algo@pm.me> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Summary
Swift 6 CLI (
aps) dogfooding 0xLeif/AppState, with CorvidLabs Trust gating on self-hosted macOS runners.Commands
get,set,watch,dump,keys,reset/reset --allDemo keys
counter/message(State)flag(StoredState+ synchronize)note(FileState→~/.aps/note.json)Review fixes (8a76d01)
watch notepollsnote.jsondirectly (bypasses AppState FileState cache)APSPaths.configure()moved to CLIboot()so tests keep injected FileState pathsset noteread-verifies disk and throwsAPSError.persistenceFailedon mismatchSpecSync 5.1.1 (458f017)
.specsync/version→ 5.1.1legacy-baseline.json)file://mirror for SpecSync 5.1.1Trust / CI
.github/workflows/{ci,trust}.ymlon[self-hosted, macOS]