Repository navigation
fix(desktop): keep OS clipboard chords (Cmd+C/V/X/A) bubbling out of v1 terminal - #3407
AytuncYildizli wants to merge 3 commits into
Conversation
…v1 terminal ParadevAI#3390 changed the v1 terminal keyboard handler so any chord not registered as an app hotkey falls through to the PTY. That regressed standard OS clipboard shortcuts: Cmd+C / Cmd+V / Cmd+X / Cmd+A on macOS and the Linux/Windows equivalents (Ctrl+Shift+C/V/X/A, Shift+Insert) are not registered as app hotkeys, so they now reach xterm and never get a chance to copy/paste via Electron's edit menu — text selection in the terminal became impossible to copy. Add an explicit short-circuit before the "fall through to PTY" branch: when the chord is exactly an OS clipboard combination, return false so the event bubbles to the host. xterm.js' selection / Electron's edit role still get to do the work; SIGINT (Ctrl+C in PTY) and other terminal-reserved chords are unaffected because they're handled earlier in the same handler. Reproduced on Superset Desktop v1.5.1 (macOS, Apple Silicon, Turkish keyboard): Cmd+C in terminal pane silently dropped before this patch, copies selection after.
📝 WalkthroughWalkthroughAdded observability system documentation establishing required telemetry contracts, field registry, event taxonomy, journey definitions, query reference, and operational triage procedures across six new documentation files. ChangesObservability System Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes a regression from #3390 where OS clipboard chords ( Key changes:
Issues found:
Confidence Score: 3/5Safe to merge for macOS users, but non-Mac clipboard handling has two concrete bugs that leave the fix incomplete on Linux/Windows. The macOS path (Cmd+C/V/X/A) is correctly implemented and solves the primary regression. However, the non-Mac path has two P1 issues: Shift+Insert paste is documented as fixed but the outer guard never matches it, and Ctrl+Delete is incorrectly bubbled — it is a terminal delete-forward-word shortcut causing a new PTY editing regression on Linux/Windows. apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts — specifically the non-Mac clipboard chord block (lines 611–628) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[KeyboardEvent in xterm handler] --> B{isShiftEnter?}
B -- yes --> C[handle / return false]
B -- no --> D{isCmdBackspace/Left/Right or Option/Ctrl nav?}
D -- yes --> E[handle nav / return false]
D -- no --> F{isTerminalReservedEvent? Ctrl+C/D/Z/S/Q}
F -- yes --> G[return true to PTY]
F -- no --> H{matches CLEAR_TERMINAL?}
H -- yes --> I[call onClear / return false]
H -- no --> J{isMac and onlyMeta and C/V/X/A?}
J -- yes --> K[return false OS clipboard bubbles]
J -- no --> L{not isMac and onlyCtrl or ctrlShiftOnly?}
L -- Ctrl+Shift+C/V/X/A --> M[return false OS clipboard bubbles]
L -- onlyCtrl+Insert --> N[return false Ctrl+Insert Copy]
L -- onlyCtrl+Delete --> O[BUG: return false breaks delete-fwd-word]
L -- Shift+Insert shiftOnly --> P[outer guard never entered falls to PTY]
L -- no match --> Q{resolveHotkeyFromEvent not null?}
Q -- registered hotkey --> R[return false bubble]
Q -- unregistered --> S[return true PTY]
Reviews (1): Last reviewed commit: "fix(desktop): keep OS clipboard chords (..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| onlyCtrl && | ||
| (event.code === "Insert" || event.code === "Delete") | ||
| ) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Ctrl+Delete is a terminal editing shortcut, not a clipboard chord
Ctrl+Delete in terminal contexts sends a "delete forward word" escape sequence (e.g. \e[3;5~ in many shells/readline configs). Making it bubble (return false) prevents that sequence from reaching the PTY, regressing forward-word-delete for non-Mac users.
The standard CUA clipboard shortcuts that warrant bubbling are:
- Copy:
Ctrl+Insert✓ - Paste:
Shift+Insert(currently missing, see sibling comment) - Cut:
Shift+Delete(separateshiftOnlycase)
Ctrl+Delete is not part of that set and should be removed from this condition:
| onlyCtrl && | |
| (event.code === "Insert" || event.code === "Delete") | |
| ) { | |
| return false; | |
| } | |
| if ( | |
| onlyCtrl && | |
| event.code === "Insert" | |
| ) { | |
| return false; | |
| } |
| if (!isMac && (onlyCtrl || ctrlShiftOnly)) { | ||
| // Linux/Windows: Ctrl+Shift+C/V/X/A or Shift+Insert / Ctrl+Insert | ||
| if ( | ||
| ctrlShiftOnly && | ||
| (event.code === "KeyC" || | ||
| event.code === "KeyV" || | ||
| event.code === "KeyX" || | ||
| event.code === "KeyA") | ||
| ) { | ||
| return false; | ||
| } | ||
| if ( | ||
| onlyCtrl && | ||
| (event.code === "Insert" || event.code === "Delete") | ||
| ) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Shift+Insert mentioned in comment but never enters the bubble block
The inline comment on line 612 and the PR description both list Shift+Insert (the Linux/Windows paste shortcut) as a case that should bubble. However, the outer guard if (!isMac && (onlyCtrl || ctrlShiftOnly)) requires ctrlKey to be set. For Shift+Insert, only shiftKey is pressed with no ctrlKey, so neither onlyCtrl nor ctrlShiftOnly is ever true — the block is never entered and Shift+Insert still falls through to the PTY path unchanged.
A shiftOnly variable (analogous to onlyCtrl) should be added to the outer condition, and Shift+Insert should be handled inside:
const shiftOnly =
event.shiftKey && !event.ctrlKey && !event.metaKey && !event.altKey;
if (!isMac && (onlyCtrl || ctrlShiftOnly || shiftOnly)) {
// Ctrl+Shift+C/V/X/A
if (ctrlShiftOnly && (event.code === "KeyC" || event.code === "KeyV" ||
event.code === "KeyX" || event.code === "KeyA")) {
return false;
}
// Ctrl+Insert = Copy
if (onlyCtrl && event.code === "Insert") { return false; }
// Shift+Insert = Paste
if (shiftOnly && event.code === "Insert") { return false; }
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`:
- Around line 611-627: The current modifier checks use onlyCtrl for
Insert/Delete which incorrectly prevents Shift+Insert from being handled and
allows Ctrl+Delete to bubble; update the condition so Insert/Delete are gated by
the Shift-only modifier instead of Ctrl (replace the onlyCtrl check with the
shift-only variable used in this keyboard handler, e.g., shiftOnly or onlyShift)
so the code becomes: if (shiftOnly && (event.code === "Insert" || event.code ===
"Delete")) return false; thereby mapping Shift+Insert/Shift+Delete to the
terminal clipboard/cut behavior and preventing Ctrl+Delete from being swallowed.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49b3a373-2400-4d92-bd71-d62f5c1e6e03
📒 Files selected for processing (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts
| if (!isMac && (onlyCtrl || ctrlShiftOnly)) { | ||
| // Linux/Windows: Ctrl+Shift+C/V/X/A or Shift+Insert / Ctrl+Insert | ||
| if ( | ||
| ctrlShiftOnly && | ||
| (event.code === "KeyC" || | ||
| event.code === "KeyV" || | ||
| event.code === "KeyX" || | ||
| event.code === "KeyA") | ||
| ) { | ||
| return false; | ||
| } | ||
| if ( | ||
| onlyCtrl && | ||
| (event.code === "Insert" || event.code === "Delete") | ||
| ) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Fix the non-mac Insert/Delete modifier mapping.
Line 611 excludes Shift+Insert, so that documented paste chord still reaches the PTY. Lines 622-625 also bubble Ctrl+Delete, which is a common terminal editing chord rather than a clipboard shortcut. If you want the Delete-based cut path too, that modifier should be Shift, not Ctrl.
🔧 Proposed fix
const onlyMeta =
event.metaKey && !event.ctrlKey && !event.altKey && !event.shiftKey;
const onlyCtrl =
event.ctrlKey && !event.metaKey && !event.altKey && !event.shiftKey;
+ const onlyShift =
+ event.shiftKey && !event.metaKey && !event.ctrlKey && !event.altKey;
const ctrlShiftOnly =
event.ctrlKey && event.shiftKey && !event.metaKey && !event.altKey;
@@
- if (!isMac && (onlyCtrl || ctrlShiftOnly)) {
+ if (!isMac && (onlyCtrl || onlyShift || ctrlShiftOnly)) {
// Linux/Windows: Ctrl+Shift+C/V/X/A or Shift+Insert / Ctrl+Insert
if (
ctrlShiftOnly &&
(event.code === "KeyC" ||
event.code === "KeyV" ||
event.code === "KeyX" ||
event.code === "KeyA")
) {
return false;
}
- if (
- onlyCtrl &&
- (event.code === "Insert" || event.code === "Delete")
- ) {
+ if (onlyCtrl && event.code === "Insert") {
+ return false;
+ }
+ if (onlyShift && event.code === "Insert") {
return false;
}
}📝 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.
| if (!isMac && (onlyCtrl || ctrlShiftOnly)) { | |
| // Linux/Windows: Ctrl+Shift+C/V/X/A or Shift+Insert / Ctrl+Insert | |
| if ( | |
| ctrlShiftOnly && | |
| (event.code === "KeyC" || | |
| event.code === "KeyV" || | |
| event.code === "KeyX" || | |
| event.code === "KeyA") | |
| ) { | |
| return false; | |
| } | |
| if ( | |
| onlyCtrl && | |
| (event.code === "Insert" || event.code === "Delete") | |
| ) { | |
| return false; | |
| } | |
| if (!isMac && (onlyCtrl || onlyShift || ctrlShiftOnly)) { | |
| // Linux/Windows: Ctrl+Shift+C/V/X/A or Shift+Insert / Ctrl+Insert | |
| if ( | |
| ctrlShiftOnly && | |
| (event.code === "KeyC" || | |
| event.code === "KeyV" || | |
| event.code === "KeyX" || | |
| event.code === "KeyA") | |
| ) { | |
| return false; | |
| } | |
| if (onlyCtrl && event.code === "Insert") { | |
| return false; | |
| } | |
| if (onlyShift && event.code === "Insert") { | |
| return false; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts`
around lines 611 - 627, The current modifier checks use onlyCtrl for
Insert/Delete which incorrectly prevents Shift+Insert from being handled and
allows Ctrl+Delete to bubble; update the condition so Insert/Delete are gated by
the Shift-only modifier instead of Ctrl (replace the onlyCtrl check with the
shift-only variable used in this keyboard handler, e.g., shiftOnly or onlyShift)
so the code becomes: if (shiftOnly && (event.code === "Insert" || event.code ===
"Delete")) return false; thereby mapping Shift+Insert/Shift+Delete to the
terminal clipboard/cut behavior and preventing Ctrl+Delete from being swallowed.
Review feedback on ParadevAI#3407 (Greptile P1 + CodeRabbit major): - Ctrl+Delete is a shell readline chord (delete-word-forward), not a clipboard shortcut. Removed it from the bubble path so it reaches the PTY again. - Shift+Insert is the documented Linux paste chord but the previous `onlyCtrl` guard excluded shift, so it never matched. Added an explicit Insert + (onlyCtrl || onlyShift) path that covers Ctrl+Insert (copy) and Shift+Insert (paste). Shift+Delete is intentionally excluded — terminals usually consume it as a readline edit, not as cut.
|
thanks @greptile-apps and @coderabbitai — both findings were valid. fixed in 9a2b3fa:
|
|
Tip For best results, initiate chat on the files or code changes.
The clipboard bubble path looks solid now. If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
|
The fix looks correct. The relevant block now reads: if (event.code === "Insert" && (onlyCtrl || onlyShift)) {
return false;
}All three points check out:
The Tip: You can customize Greptile's behavior for this repo with |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts">
<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts:611">
P2: `Shift+Insert` (standard Linux/Windows paste shortcut) is documented in the comment on line 612 and the PR description as a chord that should bubble, but the outer guard `if (!isMac && (onlyCtrl || ctrlShiftOnly))` requires `ctrlKey` to be set. For `Shift+Insert`, only `shiftKey` is pressed, so neither `onlyCtrl` nor `ctrlShiftOnly` is true — the block is never entered and `Shift+Insert` still falls through to the PTY. Add a `shiftOnly` modifier variable and include it in the outer condition.</violation>
<violation number="2" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts:624">
P2: `Ctrl+Delete` is a terminal editing shortcut (forward-word-delete, typically sends `\e[3;5~`), not a clipboard chord. Bubbling it out of the terminal handler breaks forward-word-delete for Linux/Windows users. The standard CUA clipboard shortcuts using these keys are `Ctrl+Insert` (copy) and `Shift+Insert` (paste) — `Ctrl+Delete` is not in that set. Remove `event.code === "Delete"` from this condition.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
Follow-up note: PR #3415 now exists to handle the broader terminal shortcut cases around this same area. It tightens the platform guard so Linux fallback chords do not leak onto macOS, and aligns the v1 terminal behavior more closely with VS Code's terminal clipboard handling, including macOS Link: #3415 |
|
Thank you for reporting a fixing! |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/observability/query-reference.md (2)
26-28: Expand missing-data taxonomy with concrete examples.The taxonomy ("no traffic, no failures, broken exporter…") is useful but generic. Consider adding:
- How to distinguish between "no traffic" vs. "broken exporter" (e.g., check exporter logs, verify datasource connectivity).
- Common deployment/ingestion delays (SLO expectations, cache TTLs).
- Specific label mismatches to watch for (e.g., typos in
environmentorsurfacevalues).🤖 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 `@docs/observability/query-reference.md` around lines 26 - 28, Expand the Missing-data taxonomy section to provide actionable diagnostic guidance alongside the current list of possible causes. Add specific troubleshooting steps to help users distinguish between similar scenarios, such as how to check exporter logs and verify datasource connectivity to differentiate "no traffic" from "broken exporter". Include information about common deployment and ingestion delay expectations (SLO timeframes and cache TTL values). Additionally, add examples of specific label mismatches that commonly cause issues, particularly typos in field names like "environment" or "surface" values that users should watch for when debugging empty dashboards.
3-3: Clarify what "initial repo contract" means and define completion criteria.The status line marks this as an "initial" contract awaiting datasource names, but does not say:
- Who is responsible for filling in datasource names (Grafana/OTel/PostHog/Sentry wiring)?
- What is the timeline or blocking condition for finalization?
- How will users know when the document is actionable?
Tighten the status message to be specific (e.g., "Awaiting Grafana/OTel wiring landing (tracked in issue
#XYZ)") or move this blocking work to a separate ADR/tracking issue.🤖 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 `@docs/observability/query-reference.md` at line 3, The status message at the beginning of the query-reference.md file uses vague language ("initial repo contract") without clarifying who is responsible for completing the work or what the timeline is. Replace this status line with specific, actionable language that either: identifies the exact blocking dependencies with linked issue references (e.g., "Awaiting completion of Grafana/OTel wiring, tracked in issue `#XYZ`"), or move the blocking work description to a separate tracking issue or ADR document so the status line simply reflects the document's current state. This will make it clear to users whether the document is usable and who to contact for updates.
🤖 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.
Nitpick comments:
In `@docs/observability/query-reference.md`:
- Around line 26-28: Expand the Missing-data taxonomy section to provide
actionable diagnostic guidance alongside the current list of possible causes.
Add specific troubleshooting steps to help users distinguish between similar
scenarios, such as how to check exporter logs and verify datasource connectivity
to differentiate "no traffic" from "broken exporter". Include information about
common deployment and ingestion delay expectations (SLO timeframes and cache TTL
values). Additionally, add examples of specific label mismatches that commonly
cause issues, particularly typos in field names like "environment" or "surface"
values that users should watch for when debugging empty dashboards.
- Line 3: The status message at the beginning of the query-reference.md file
uses vague language ("initial repo contract") without clarifying who is
responsible for completing the work or what the timeline is. Replace this status
line with specific, actionable language that either: identifies the exact
blocking dependencies with linked issue references (e.g., "Awaiting completion
of Grafana/OTel wiring, tracked in issue `#XYZ`"), or move the blocking work
description to a separate tracking issue or ADR document so the status line
simply reflects the document's current state. This will make it clear to users
whether the document is usable and who to contact for updates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 48903103-f41e-48f8-bb1d-fb0d5f0cf4f2
📒 Files selected for processing (6)
docs/observability/event-taxonomy.mddocs/observability/field-registry.mddocs/observability/journeys.mddocs/observability/query-reference.mddocs/observability/system-overview.mdops/runbooks/observability-triage.md
✅ Files skipped from review due to trivial changes (5)
- ops/runbooks/observability-triage.md
- docs/observability/journeys.md
- docs/observability/event-taxonomy.md
- docs/observability/field-registry.md
- docs/observability/system-overview.md
There was a problem hiding this comment.
4 issues found across 6 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/observability/field-registry.md">
<violation number="1" location="docs/observability/field-registry.md:9">
P2: The `span_id` row maps the field directly to the `traceparent` header without clarifying that the header's `parent-id` is the caller's span (parent span) on ingress and the current span only on egress. This can cause implementers to use the wrong span ID for local spans, breaking trace/span correlation.</violation>
</file>
<file name="docs/observability/system-overview.md">
<violation number="1" location="docs/observability/system-overview.md:5">
P2: Machine-specific absolute local path with username is committed in shared documentation. Replace `/Users/aytuncyildizli/superset` with a generic representation such as the repo root, a relative path, or the Git URL so the doc is accurate and portable across contributors and environments.</violation>
</file>
<file name="docs/observability/query-reference.md">
<violation number="1" location="docs/observability/query-reference.md:1">
P2: Unrelated observability documentation file added inside a focused desktop clipboard bugfix PR. This couples a docs contract change to a bugfix, increasing review/rollback risk and violating the team's keep-PRs-focused guidance.</violation>
</file>
<file name="docs/observability/event-taxonomy.md">
<violation number="1" location="docs/observability/event-taxonomy.md:10">
P2: Completion and failure journey events should require the same `environment` and `app_version` dimensions as the start event, or the document should explicitly state that the typed proxy auto-enriches them. Otherwise the taxonomy cannot support consistent end-to-end observability slicing (e.g., failure rates by version or environment).</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| | --- | --- | --- | --- | --- | --- | --- | | ||
| | `request_id` | generated id | yes | yes | yes | no | One HTTP/IPC/job operation. | | ||
| | `trace_id` | W3C trace id | `traceparent` | yes | native | no | Full journey trace. | | ||
| | `span_id` | W3C span id | `traceparent` | yes | native | no | Current operation span. | |
There was a problem hiding this comment.
P2: The span_id row maps the field directly to the traceparent header without clarifying that the header's parent-id is the caller's span (parent span) on ingress and the current span only on egress. This can cause implementers to use the wrong span ID for local spans, breaking trace/span correlation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/observability/field-registry.md, line 9:
<comment>The `span_id` row maps the field directly to the `traceparent` header without clarifying that the header's `parent-id` is the caller's span (parent span) on ingress and the current span only on egress. This can cause implementers to use the wrong span ID for local spans, breaking trace/span correlation.</comment>
<file context>
@@ -0,0 +1,25 @@
+| --- | --- | --- | --- | --- | --- | --- |
+| `request_id` | generated id | yes | yes | yes | no | One HTTP/IPC/job operation. |
+| `trace_id` | W3C trace id | `traceparent` | yes | native | no | Full journey trace. |
+| `span_id` | W3C span id | `traceparent` | yes | native | no | Current operation span. |
+| `correlation_id` | generated id | yes | yes | yes | no | One business action/retry chain. |
+| `actor_id` | hashed/pseudonymous id | yes | yes | yes | no | Never raw email/phone/customer id. |
</file context>
| | `span_id` | W3C span id | `traceparent` | yes | native | no | Current operation span. | | |
| | `span_id` | W3C span id | `traceparent` | yes | native | no | Current operation span. Inbound `traceparent.parent-id` is the parent span; the current span ID is generated locally and becomes `parent-id` in outbound `traceparent`. | |
|
|
||
| Blueprint source: company-wide `observability-blueprint.md`. Vendor stack can vary; principle IDs do not. | ||
|
|
||
| Repo: `/Users/aytuncyildizli/superset` |
There was a problem hiding this comment.
P2: Machine-specific absolute local path with username is committed in shared documentation. Replace /Users/aytuncyildizli/superset with a generic representation such as the repo root, a relative path, or the Git URL so the doc is accurate and portable across contributors and environments.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/observability/system-overview.md, line 5:
<comment>Machine-specific absolute local path with username is committed in shared documentation. Replace `/Users/aytuncyildizli/superset` with a generic representation such as the repo root, a relative path, or the Git URL so the doc is accurate and portable across contributors and environments.</comment>
<file context>
@@ -0,0 +1,17 @@
+
+Blueprint source: company-wide `observability-blueprint.md`. Vendor stack can vary; principle IDs do not.
+
+Repo: `/Users/aytuncyildizli/superset`
+Surface: agent/desktop/web app surface
+
</file context>
| @@ -0,0 +1,34 @@ | |||
| # Observability Query Reference | |||
There was a problem hiding this comment.
P2: Unrelated observability documentation file added inside a focused desktop clipboard bugfix PR. This couples a docs contract change to a bugfix, increasing review/rollback risk and violating the team's keep-PRs-focused guidance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/observability/query-reference.md, line 1:
<comment>Unrelated observability documentation file added inside a focused desktop clipboard bugfix PR. This couples a docs contract change to a bugfix, increasing review/rollback risk and violating the team's keep-PRs-focused guidance.</comment>
<file context>
@@ -0,0 +1,34 @@
+# Observability Query Reference
+
+Status: initial repo contract. Fill datasource names when Grafana/OTel/PostHog/Sentry wiring lands.
</file context>
| | Event | Journey | When | Required fields | Forbidden fields | | ||
| | --- | --- | --- | --- | --- | | ||
| | `superset.journey_started` | TBD | User/system intent accepted | `correlation_id`, `surface`, `environment`, `app_version` | raw content, token, email, phone | | ||
| | `superset.journey_completed` | TBD | Business success confirmed | `correlation_id`, `result`, `success` | raw content, token, email, phone | |
There was a problem hiding this comment.
P2: Completion and failure journey events should require the same environment and app_version dimensions as the start event, or the document should explicitly state that the typed proxy auto-enriches them. Otherwise the taxonomy cannot support consistent end-to-end observability slicing (e.g., failure rates by version or environment).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/observability/event-taxonomy.md, line 10:
<comment>Completion and failure journey events should require the same `environment` and `app_version` dimensions as the start event, or the document should explicitly state that the typed proxy auto-enriches them. Otherwise the taxonomy cannot support consistent end-to-end observability slicing (e.g., failure rates by version or environment).</comment>
<file context>
@@ -0,0 +1,11 @@
+| Event | Journey | When | Required fields | Forbidden fields |
+| --- | --- | --- | --- | --- |
+| `superset.journey_started` | TBD | User/system intent accepted | `correlation_id`, `surface`, `environment`, `app_version` | raw content, token, email, phone |
+| `superset.journey_completed` | TBD | Business success confirmed | `correlation_id`, `result`, `success` | raw content, token, email, phone |
+| `superset.journey_failed` | TBD | Bounded failure class known | `correlation_id`, `error_class`, `success` | raw error message as label |
</file context>
|
Closing: this only touches the legacy v1 UI (screens/main), which is being retired. only v1 Terminal helpers.ts; v2 has clipboard-shortcuts.ts (#3968). If the bug still shows in the v2 workspace, please open an issue. |
|
Thank you for digging into the clipboard chords, and for the careful event.code work in the sibling PR. The v1 terminal is on its way out, which is the only reason this isn't landing. |
Summary / What changed
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts: short-circuit OS clipboard chords (Cmd+C/V/X/A on macOS, Ctrl+Shift+C/V/X/A and Shift+Insert / Ctrl+Insert on other platforms) so they bubble out of the terminal handler instead of falling through to the PTY.Why
#3390 made the v1 terminal forward any chord not registered as an app hotkey to the PTY. That fixed Claude Code TUI shortcuts but regressed standard OS clipboard combos:
Cmd+C,Cmd+V,Cmd+X,Cmd+Aare not registered as app hotkeys, so they now go straight to xterm and never reach Electron's edit menu / xterm.js Selection. Result: text selected in the terminal pane can't be copied; clipboard paste also doesn't reach the input line.The patch adds an explicit allow-bubble step before the "fall through to PTY" branch. Terminal-reserved chords (Ctrl+C SIGINT, Ctrl+D EOF, etc.) are still handled earlier in the same function, so PTY semantics are unchanged.
Tests
No unit tests touched — the change is platform-conditional event routing inside the live keyboard handler. Manual repro was decisive enough.
Real-world validation
Cmd+C, nothing happens;Cmd+Vdoes nothing. After: selection copies to the system clipboard; paste reaches the PTY input line as expected.Ctrl+Cin PTY still sends SIGINT (terminal-reserved path runs first).Issue
Regression of #3390. No standalone issue yet — happy to file one if maintainers prefer that flow.
Summary by cubic
Fixes the v1 terminal swallowing OS clipboard shortcuts so they bubble to the host, restoring copy, paste, cut, and select-all in terminal panes; PTY-reserved chords are unchanged. Also seeds observability blueprint docs and a triage runbook.
Written for commit 6d65367. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Documentation
Runbooks