Uh oh!
There was an error while loading. Please reload this page.
Productize Relayfile cloud mount flow - #71
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:dfbedc593f
ℹ️ 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".
| if err != nil { | ||
| return err | ||
| } | ||
| if err := process.Signal(syscall.SIGTERM); err != nil { |
There was a problem hiding this comment.
runStop always sends syscall.SIGTERM, but on Windows os.Process.Signal only supports os.Kill, so this path returns an error and background mounts cannot be stopped there. This commit added Windows-specific daemon startup support (daemon_windows.go), so relayfile stop becoming nonfunctional on Windows is a user-visible regression for that environment.
Useful? React with 👍 / 👎.
Comment on lines
+3053
to
+3057
| switch provider { | ||
| case "slack-sage": | ||
| return "slack" | ||
| default: | ||
| return provider |
There was a problem hiding this comment.
providerRootDir only special-cases slack-sage, but fallbackIntegrationCatalog also includes slack-my-senior-dev with VFSRoot: /slack-msd. With the current mapping, status/progress probes and disconnect cleanup target /<provider-id> instead of the actual mounted root for that provider, so these flows operate on the wrong directory for that integration.
Useful? React with 👍 / 👎.
Wires the cloud-side endpoints landed in cloud#406 into the CLI so the
productized cloud-mount surface has actual consumers.
- `relayfile ops list` reads `.relay/dead-letter/<opId>.json` records
produced by the mount writeback path (contract §8.4) with text and
--json output modes.
- `relayfile ops replay <opId>` POSTs to the cloud's
`/api/v1/workspaces/{id}/ops/{opId}/replay` and removes the local
record when the queue accepts the replay.
- `relayfile status` adds the contract §7.4 warning row when a provider
reports `webhookHealthy=false` and `lagSeconds>60`, so users can see
when polling is the only thing keeping the mirror current.
- `syncProviderStatus` carries the optional `webhookHealthy` field so
the value flows through once relayfile (or the cloud sync proxy)
emits it.
Note: the syncer does not yet write `.relay/dead-letter/<opId>.json`
when a writeback fails non-retryably — that is its own follow-up. The
list command is correct against the contract today; it just shows
nothing until that producer lands.Targets the larger relayfile-side follow-ups for the productized
cloud-mount contract:
- `relayfile pull [--provider PROVIDER] [--reason TEXT]` triggers a
manual `/v1/workspaces/{id}/sync/refresh` against relayfile. With no
--provider it iterates over the connected providers from the sync
status feed.
- `relayfile ops list` now reconciles `.relay/dead-letter/<opId>.json`
against `/v1/workspaces/{id}/ops?status=dead_lettered` before
printing: new dead-lettered ops land on disk with a structured
payload (path, code, attempts, lastAttemptedAt, replayUrl), and ops
the server no longer reports as dead-lettered are pruned so the
local view does not go stale after replay or ack. `--no-refresh`
skips the round-trip for offline use.
- Integration catalog responses are now cached at
`~/.relayfile/catalog-cache.json` for 1 hour (contract verdict §A12).
A 409 from `/integrations/connect-session` invalidates the cache so
the next invocation pulls fresh provider ids; otherwise normal calls
read from the cache and skip the round trip.
5 new tests cover both pull paths, mirror reconcile + prune, and the
catalog cache hit / invalidate-and-refetch behavior.When the cloud refresh token can no longer mint access tokens, the
mount loop now enters a read-only degraded state instead of treating
it as a generic transient error:
- Add `ErrCloudRefreshExpired` sentinel returned from
`refreshCloudCredentials` for empty refresh token, 403 invalid_grant,
and 401 responses.
- Mount loop detects the sentinel, sets a `degraded` flag, persists a
human-readable stall reason ("cloud session expired — run
'relayfile login' to refresh"), and prints the recovery instruction
on a 1-minute cadence rather than every cycle.
- Reconcile cycles are skipped while degraded so we do not spam the
log or loop on doomed retries; the watcher path also short-circuits
outbound writes. Local edits stay marked dirty so the next
successful cycle after re-login picks them up.
- Each cycle re-attempts the refresh, so the loop self-recovers as
soon as `relayfile login` lands fresh credentials.
- `relayfile status` now reads the persisted state.json `stallReason`
and prints it under the daemon line so the user sees the degraded
state from the CLI.
4 new tests cover the sentinel paths (invalid_grant, 401, empty
refresh token) and the status surfacing.
The mount loop's "no successful reconcile for 10m" stall stays
suppressed while degraded so it does not clobber the more specific
recovery message.Implements contract §8.2 for schema-validation failures during writeback. Previously a `400 schema_validation_failed` from the cloud bubbled out of the bulk write path as a generic HTTPError, leaving the offending body in place and the next reconcile cycle stuck re-pushing it. - Add `ErrSchemaValidation` sentinel and a `SchemaValidationError` type that `bulkWriteErrorAsError` returns for `schema_validation_failed` and `validation_error` codes. - `handleWriteError` routes schema errors into a new `materializeSchemaInvalid` helper that: - Saves the local body to `.relay/conflicts/<path>.invalid.<ts>` (UTC timestamp, contract-specified suffix). - Restores the prior remote version into the original path so the user can edit the file without breaking the next sync cycle. - When the failing write was a CREATE (no remote to restore), removes the local file and clears tracked state so reconcile does not retry the invalid body. - Logs `schema validation failed at <path> (<reason>); local saved at <artifact>, remote restored` for visibility in `relayfile logs`. - The cycle returns nil on schema failure (consistent with conflict handling) so `relayfile mount` exits 0 per A6. 2 new tests cover the UPDATE-rejected (restore remote) and CREATE-rejected (remove local) paths against a `fakeClient` that returns `schema_validation_failed`.
Formalizes two productized cloud-mount contract acceptance tests in a dedicated `background_test.go` file (per the contract verdict's path naming) and wires the contract-mandated exit code for state mismatch. A2 — Cloud login state mismatch must surface as exit 10: - New `ErrCloudLoginStateMismatch` sentinel returned by runCloudLogin when the OAuth callback's `state` does not match the value the CLI generated. - main() maps the sentinel to a dedicated `cloudLoginStateMismatchExitCode` (10) so shells and CI can distinguish a tampered/replayed login flow from a generic CLI error (which still exits 1). - Test forks runCloudLogin in a goroutine, parses the listener URL out of stdout, forges a callback with a wrong `state`, and asserts the function returns the sentinel. A14 — Background mode pid/log + clean stop signal: - Spawns a real `sleep 30` subprocess to stand in for the daemon, populates `.relay/mount.pid` and `.relay/mount.log`, calls `relayfile stop`, and asserts the subprocess receives SIGTERM and exits within 5 s. - Companion tests cover stop's "no running mount" error path and that `relayfile logs` tails the recorded mount.log. The new test file is `//go:build !windows` because A14 relies on `sleep` and SIGTERM semantics; the Windows daemon path uses a different process-group mechanism and warrants its own test.
Adds two more productized cloud-mount contract acceptance tests in
dedicated files (per the verdict's path naming):
A10 — Initial sync gate (`cmd/relayfile-cli/setup_e2e_test.go`):
- Positive path: cloud reports `cataloging` on the first poll, then
`ready` on the second; `waitForInitialSync` returns nil after the
transition so setup exits 0.
- Timeout path: provider stays `cataloging` past the configured
deadline; `waitForInitialSync` exits 0 after printing the resume
hint ("notion still syncing in the background...") so the workspace
and mount stay usable while the sync catches up.
A12 — Catalog refresh (`cmd/relayfile-cli/catalog_test.go`):
- 409-forces-revalidation: a seeded catalog cache with a deprecated
provider triggers connect-session; the cloud responds 409 with the
fresh catalog inline; `connectCloudIntegration` invalidates the
cache file so the next invocation pulls a fresh provider list.
- TTL expiry: a backdated cache (older than the 1-hour TTL) is
treated as a miss; the freshly added `github` provider appears in
the next `loadIntegrationCatalog` call without explicit invalidation.Closes the remaining productized cloud-mount contract items that did
not require full mount-loop scaffolding.
A13 — Synced-mirror honesty (banner + --help):
- mountStartBanner now leads with "Synced mirror started at <dir>" and
rounds the interval to whole seconds, so the user sees the default
mode named per contract §3.1 instead of an unqualified "Mirror".
- printMountHelp surfaces the contract §3.6 list of synced-mirror
limitations (file handles, mtime, directory listings, fsnotify
semantics) when `relayfile mount --help` / `-h` is invoked.
- runMount detects flag.ErrHelp from the parser and prints the custom
help to stdout instead of returning an error.
A1 — Synced-mirror banner naming:
- Pure-format test that pins the banner shape ("Synced mirror started
at <dir>", "Sync interval 30s", "±20%") so future edits can't drift
away from the contract wording without breaking tests.
A3 — Setup re-run skips connect when already ready:
- Seeds .relay/integrations/<provider>.json with a connectionId,
exposes /status returning ready=true, and asserts
ensureCloudIntegration skips connect-session entirely while still
printing "<provider> already connected".
A8 — VFS token mid-mount refresh:
- Table-test for relayfileTokenNeedsRefresh covering already-expired,
near-expiry, lifetime/10 leeway, far-future tokens, and the unparseable/
empty cases that must not trigger spurious refreshes.
- Companion test exercising joinWorkspaceViaCloud against a mock cloud
that returns a fresh relayfile token + wsUrl, asserting the call
shape (bearer token, body) and a non-empty result so the in-loop
refresh path stays trustworthy.P1 — Use a supported termination signal on Windows: - Factor the daemon stop signal into per-platform helpers. Unix keeps SIGTERM (graceful, lets the mount loop flush state.json). Windows uses Process.Kill, which routes through TerminateProcess — the only outbound signal the Windows kernel surfaces via os.Process.Signal, so a hard-coded SIGTERM there always errored and `relayfile stop` was a no-op for Windows daemons. - runStop now calls signalDaemonStop(process) instead of inlining syscall.SIGTERM. While on the Windows path, also fix the pre-existing `undefined: syscall.DETACHED_PROCESS` cross-compile error by importing the constant from `golang.org/x/sys/windows` (already an indirect dep, now promoted to direct). `GOOS=windows go build` is now clean. P2 — Map Slack variants to the correct mirror root: - providerRootDir was special-casing only `slack-sage`. The fallbackIntegrationCatalog also exposes `slack-my-senior-dev` (vfsRoot `/slack-msd`) and `slack-nightcto`. With the old mapping, status probes (`/fs/tree?path=/<provider-id>`), the connect banner, and `markProviderDisconnected` cleanup all targeted the wrong directory for those integrations. - Extended the switch to cover every catalog entry. New regression test cross-checks the function against fallbackIntegrationCatalog so future catalog additions can't silently leave it behind.
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Stack
codex/relayfile-guided-cloud-setup) because this builds on the guided setup entrypoint.Tests
go test ./...npm run typecheckdocs/evidence/productized-cloud-mount-final-evidence.mdHygiene
.trajectoriesindex/untracked trajectory noise unstaged.