MCP: preserve EC2 runtime and publisher identity - #23
Open
huronat wants to merge 8 commits into
Open
Conversation
Persist the resolved machine id and reuse it when init renders login-time tracking, keeping activation and edge stream names identical. Report the explicit server bucket from synty_status even when a supervised MCP pod has no workstation config. The default 304-scenario suite and the 325-scenario cloud/HTTP/Athena matrix pass.
Generate the fake raw events relative to the scenario start so the seven-day production guard does not make the test expire with wall-clock time. The complete 325-scenario S3, GCS, HTTP MCP, and Athena matrix passes.
📝 WalkthroughWalkthroughInitialization now resolves and persists machine identities for autostart and stream alignment. Installed services keep state under ChangesRuntime identity and state consistency
Sequence Diagram(s)sequenceDiagram
participant InitRun
participant Config
participant Autostart
participant Unit
InitRun->>Config: persist resolved machine
InitRun->>Autostart: enable with resolved machine
Autostart->>Unit: write machine-scoped unit
sequenceDiagram
participant MCPClient
participant Server
participant StatusView
participant Config
MCPClient->>Server: call synty_status
Server->>StatusView: pass configured bucket
StatusView->>Config: load fallback bucket
StatusView-->>Server: return Status
Server-->>MCPClient: return health output
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/config.rs`:
- Around line 37-40: Add a scenario-style unit test in the existing #[cfg(test)]
block covering config save/load round-tripping with machine set and confirming
the value is preserved, plus deserialization of a legacy config without machine
yielding None. Reuse the existing config construction and save/load helpers
rather than testing serialization in isolation.
In `@src/trace_athena.rs`:
- Around line 1140-1145: Update the fixture setup around the timestamp variables
so the `days` partition value is derived from the original `DateTime<Utc>`
`started` value, before it is converted to RFC3339. Replace the hard-coded
`2026-07-22` value near the fixture row with that matching date while preserving
the generated `started`, `called`, and `completed` timestamps.
🪄 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 Plus
Run ID: 92904ab4-4c0a-4ffe-9747-b8eb72900adc
📒 Files selected for processing (8)
README.mddocs/design.mdsrc/config.rssrc/init.rssrc/mcp.rssrc/trace_athena.rssrc/track.rssrc/view.rs
Treat capture_repos as repository identity input as well as upload policy. This lets a local-only checkout such as sie-harness pass the fail-closed upload boundary from its cwd without requiring a Git remote, while unlisted local sessions remain denied.
Run installed launchd and systemd trackers from the user's home because every persistent path already includes .synty. This keeps config, cursors, corpus, uploads, and logs in one state directory instead of silently creating .synty/.synty.
Exercise the same path-backed save and load helpers used by runtime configuration. The scenario verifies that a resolved machine survives a round trip and that a pre-machine config still deserializes with no identity.
Keep the raw-row reconstruction scenario's physical day aligned with its runtime-generated timestamp. The fixture now remains partition-consistent across calendar boundaries instead of depending on a historical hard-coded date.
Stamp the canonical repository into session starts while the edge checkout and capture policy are available, then consume that metadata during remote reconstruction. This keeps repo-filtered Athena traces usable for local-only repositories without weakening the unknown-session upload boundary.
Render relative tracker output below .synty when the managed service runs from the user's home, while preserving repository-local corpus paths in development checkouts. This keeps the installed corpus on the persistent state volume without reintroducing nested config paths.
huronat
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/trace.rs`:
- Around line 485-492: Update the repository assignment in fold_context so the
top-level ev.payload["repo"] value is accepted only when processing the trusted
session-start event; otherwise leave ctx.repo available for the cwd-based or
later session-start fallback. Add a scenario covering a non-session event
containing repo and verify it does not establish authoritative session context.
In `@src/track.rs`:
- Around line 647-653: Update unit_output_for to accept explicit installed-mode
state instead of inferring it from cwd == home, and use that state when deciding
whether to prepend .synty/. Update all callers, including the related flow
around the additional referenced lines, and add a scenario-style unit test
covering a development checkout rooted at $HOME where relative --out
corpus/local remains unchanged.
🪄 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 Plus
Run ID: 9a318d81-5982-41e7-8694-01654d94b9c4
📒 Files selected for processing (3)
README.mdsrc/trace.rssrc/track.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
Comment on lines
+485
to
+492
| if ctx.repo.is_empty() { | ||
| ctx.repo = ev.payload["repo"].as_str().unwrap_or("").to_string(); | ||
| } | ||
| if let Some(cwd) = event_cwd(&ev.payload).filter(|cwd| !cwd.is_empty()) { | ||
| if ctx.cwd.is_empty() { | ||
| ctx.repo = crate::units::resolve_repo(&cwd, known); | ||
| if ctx.repo.is_empty() { | ||
| ctx.repo = crate::units::resolve_repo(&cwd, known); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Only accept repository stamps from the trusted session-start event.
fold_context runs for every event, so any event with a top-level repo field can become authoritative session context. In a bounded remote trace where session_start is absent or arrives later, this can misattribute the session and all downstream spans/units, while also preventing the cwd or later session-start fallback from correcting it. Gate this assignment on the trusted session-start event (and add a scenario covering a non-session event with repo).
Suggested fix
- if ctx.repo.is_empty() {
+ if ev.kind == "session_start" && ctx.repo.is_empty() {
ctx.repo = ev.payload["repo"].as_str().unwrap_or("").to_string();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ctx.repo.is_empty() { | |
| ctx.repo = ev.payload["repo"].as_str().unwrap_or("").to_string(); | |
| } | |
| if let Some(cwd) = event_cwd(&ev.payload).filter(|cwd| !cwd.is_empty()) { | |
| if ctx.cwd.is_empty() { | |
| ctx.repo = crate::units::resolve_repo(&cwd, known); | |
| if ctx.repo.is_empty() { | |
| ctx.repo = crate::units::resolve_repo(&cwd, known); | |
| } | |
| if ctx.repo.is_empty() { | |
| ctx.repo = ev.payload["repo"].as_str().unwrap_or("").to_string(); | |
| } | |
| if let Some(cwd) = event_cwd(&ev.payload).filter(|cwd| !cwd.is_empty()) { | |
| if ctx.cwd.is_empty() { | |
| if ctx.repo.is_empty() { | |
| ctx.repo = crate::units::resolve_repo(&cwd, known); | |
| } |
🤖 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 `@src/trace.rs` around lines 485 - 492, Update the repository assignment in
fold_context so the top-level ev.payload["repo"] value is accepted only when
processing the trusted session-start event; otherwise leave ctx.repo available
for the cwd-based or later session-start fallback. Add a scenario covering a
non-session event containing repo and verify it does not establish authoritative
session context.
Comment on lines
+647
to
+653
| fn unit_output_for(cwd: &Path, home: &Path, out: &str) -> String { | ||
| let path = Path::new(out); | ||
| if path.is_absolute() || out.starts_with(".synty/") || cwd != home { | ||
| out.to_string() | ||
| } else { | ||
| format!(".synty/{out}") | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not infer installed mode from cwd == $HOME.
A development checkout rooted at $HOME also satisfies this condition, so a relative --out corpus/local is rewritten to .synty/corpus/local instead of remaining repository-local. Pass explicit installed-mode state into this helper and add a $HOME-rooted checkout scenario.
As per coding guidelines, every behavioral change must include a scenario-style unit test written from user expectations.
Also applies to: 1089-1114
🤖 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 `@src/track.rs` around lines 647 - 653, Update unit_output_for to accept
explicit installed-mode state instead of inferring it from cwd == home, and use
that state when deciding whether to prepend .synty/. Update all callers,
including the related flow around the additional referenced lines, and add a
scenario-style unit test covering a development checkout rooted at $HOME where
relative --out corpus/local remains unchanged.
Source: Coding guidelines
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
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
units
synty_statusreport the explicit MCP server bucket when no workstationconfig exists
remote
persistent
$HOME/.synty/state directoryreconstruction preserves repo attribution
and matching physical partition day
Why
Live EC2 onboarding activated the requested machine marker but originally
rendered the watcher with machine
local; bucket-backed MCP status alsoreported local state. After correcting those, a real publisher test exposed two
more runtime gaps: a no-remote
sie-harnesscheckout failed the uploadallowlist, and installed path resolution created nested state or moved the
corpus off the persistent volume. Finally, the uploaded trace was queryable by
machine but not by repository because the canonical edge attribution was not
carried in the immutable session metadata.
These changes make the machine marker, stream, upload policy, persisted state,
MCP status, and remote trace identity agree without rewriting existing S3
objects.
Validation
cargo test— 309 passedcargo test --features s3,gcs,mcp-http,athena— 330 passedby scenario tests and marked addressed
WorkingDirectory=/home/ec2-user--out .synty/corpus/local--machine sie-dev-cuda-rustchunks (80,564 and 36,335 bytes), with exact day/object partition metadata
synty_statusspans from
edge-sie-dev-cuda-rust-codexrepo: sie-harness; no existing raw S3object was rewritten
Summary by CodeRabbit
SESSION_STARTwith the resolved repo, enabling remote/edge attribution.