## UI Updates for Perforce Depots and Git Repos
Fixes SRCH-530
**NOTE: This PR is a refactor of an earlier
[PR](https://github.com/sourcegraph/sourcegraph/pull/64014) that was
reverted. For that reason, the PR description is largely the same.**
This PR introduces changes to the UI to differentiate between Perforce
Depots and Git repositories. Below are the key changes included in this
commit:
### 1. Dynamic Top-Level Navigation
**For Perforce Depots:**
**For Git Repos:**
### 2. Tabs on Revision Picker
**For Perforce Depots:**
Since we only need one tab for changelists, no tabs are shown.
**For Git Repos:**
We have tabs for Branches, Tags, and Commits.
### 3. Commits/Changelists Page
**For Git Repos:**
The page displays Git commits.
**For Perforce Depots:**
The page displays Perforce changelists.
### 4. Vocabulary Adjustments
- We display either Git commit SHAs or Changelist IDs based on the
project type.
- For authorship, we use "submitted by" for Perforce and "committed by"
for Git.
- We refer to "Commits" for Git projects and "Changelists" for Perforce
projects.
**Examples:**
- **For Git Commits:**
- **For Perforce Changelists:**
### 5. URL Mapping
URLs are now structured differently based on the project type:
- **Commits Page:**
- Git: `/[repo-name]/-/commits`
- Perforce: `/[repo-name]/-/changelists`
- **Individual Item Page:**
- Git: `/[repo-name]/-/commit/[commit-hash]`
- Perforce: `/[depot-name]/-/changelist/[changelist-ID]`
When viewing a specific commit or changelist:
- **Git:** `/[repo-name]@[git-commit-hash]`
- **Perforce:** `/[repo-name]@changelist/[changelist-id]`
_NOTE: The value displayed in the search field will also change
accordingly._
### What is left to be done?
**On repo search results, when searching a revision, we still show the
git commit SHA instead of the changelist ID for perforce depots:**
I plan to make a follow-up issue for this and begin work on it
immediately. It's a little trickier than the other changes because in
the RepositoryMatch type, there is no value that can help us determine
whether a project is a depot or a repo. We need to find another way to
fetch that data.
### Request for reviewers:
1. Please try to break these new features and tell me what you find. I
stumbled on a number of little gotchas while working on this, and I'm
sure I've missed some.
## Test plan
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
- Manual/Visual testing
- Adjust e2e and integration tests to obtain a passing CI
- Test directly visiting a URL versus getting there via click
- Add unit tests for new/updated helper functions
---------
Co-authored-by: Camden Cheek <camden@ccheek.com>
We're currently evaluating only pushing on Dockerhub when releasing, as
everything else uses GCR, but until then, we lower the concurrency to 4
when pushing there, and we keep 8 on the others.
Follow-up to https://github.com/sourcegraph/devx-support/issues/1163
## Test plan
CI
ALTER TABLE with a foreign key constraint will _always_ add the
constraint, which means we always require a table lock even if this
migration has run. So we check if the column exists first.
The motivation for this change was me noticing slow migrations against
S2, even though it had been migrated already. This speeds it up since it
avoids a bunch of locks.
In testing I kept running into issues with the lsif_* tables. We agreed
we will only migrate them later.
Test Plan: ran against my local instance and S2 multiple times and CI.
Also manually ran the down and up migration against dotcom
This PR makes `WithPrefix` visible on the `KeyValue` interface.
Previously, we had other interface implementations that did not support
`WithPrefix`, but now `redisKeyValue` is the only implementation.
`WithPrefix` is currently just used in tests, but it's broadly useful
and will help clean up a bunch of places that wrap Redis and manually
add key prefixes.
The improvement to the triggers I tried to ship yesterday failed on
dotcom. This is because it has the same issue as the tenant_id column
where we can't acquire the ShareRowExclusiveLock for multiple important
tables in a transaction without deadlocking. We use the same technique
where we commit and restart the transaction. This is safe to do since
even if it fails, the migration is idempotent and partial migration
still works.
Additionally after reading the documentation on triggers more deeply
[1], I noticed we can add in a WHEN clause to triggers. We already have
this in the functions, but this should be more efficient.
I didn't want to remove the condition from the functions just yet, since
that makes the migration more risky under partial failure.
## Test Plan
CI. Also added BEGIN and COMMIT to the migration than ran locally with
``` shell
psql -f migrations/frontend/1723557836_faster_soft_delete_trigger_for_permissions/up.sql
```
Then inspected the DB state for repo and had a good looking trigger come
back
```
\d+ repo
...snip
Triggers:
trig_delete_user_repo_permissions_on_repo_soft_delete AFTER UPDATE
OF deleted_at ON repo FOR EACH ROW WHEN (new.deleted_at IS NOT NULL
AND old.deleted_at IS NULL) EXECUTE FUNCTION
delete_user_repo_permissions_on_repo_soft_delete()
```
Additionally before landing I am going to manually run this migration on
S2 first, then on dotcom to see how it behaves when there is lots of
traffic and scale.
This test ensures that newly added tables also have the tenant_id
column. We will later extend this test to also check that RLS policies
exist, once we created them.
Test plan: Test passes, when I modify the new migration that adds
tenant_id everywhere to skip a table it fails with a nice error message.
Fixes CODY-3085
Fixes CODY-3086
Previously, there was no way for OpenAI clients to list the available
models on Sourcegraph or query metadata about a given model ID ("model
ref" using our internal terminology). This PR fixes that problem AND
additionally adds infrastructure to auto-generate Go models from a
TypeSpec specification.
[TypeSpec](https://typespec.io/) is an IDL to document REST APIs,
created by Microsoft. Historically, the Go code in this repository has
been the single source of truth about what exact JSON structures are
expected in HTTP request/response pairs in our REST endpoints. This new
TypeSpec infrastructure allows us to document these shapes at a higher
abstraction level, which has several benefits including automatic
OpenAPI generation, which we can use to generate docs on
sourcegraph.com/docs or automatically generate client bindings in
TypeScript (among many other use-cases).
I am planning to write an RFC to propose we start using TypeSpec for new
REST endpoints going forward. If the RFC is not approved then we can
just delete the new `tools/typespec_codegen` directory and keep the
generated code in the repo. It won't be a big difference in the end
compared our current manual approach of writing Go structs for HTTP
APIs.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
## Test plan
See test cases. I additionally wrote a basic python script with the
official OpenAI client to test that it works with this endpoint. First,
I ran `sg start minimal`. Then I wrote this script
```py
import os
from openai import OpenAI
from dotenv import load_dotenv
import httpx
load_dotenv()
openai = OpenAI(
# base_url="https://api.openai.com/v1",
# api_key=os.getenv("OPENAI_API_KEY"),
base_url="https://sourcegraph.test:3443/api/v1",
api_key=os.getenv("SRC_ACCESS_TOKEN"),
http_client=httpx.Client(verify=False)
)
def main():
response = openai.models.list()
for model in response.data:
print(model.id)
if __name__ == "__main__":
main()
```
Finally, I ran
```
❯ python3 models.py
anthropic::unknown::claude-3-haiku-20240307
anthropic::unknown::claude-3-sonnet-20240229
fireworks::unknown::starcoder
```
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
* New `GET /.api/llm/models` and `GET /.api/llm/models/{modelId}` REST
API endpoints to list available LLM models on the instance and to get
information about a given model. This endpoints is compatible with the
`/models` and `/models/{modelId}` endpoints from OpenAI.
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
We want to discourage direct usage of the Redis pool in favor of routing
all calls through the main `KeyValue` interface. This PR removes several
usages of `KeyValue.Pool`. To do so, it adds "PING" and "MGET" to the
`KeyValue` interface.
Docs here: https://github.com/sourcegraph/docs/pull/561
This PR adds support for using Bitbucket Server OAuth2 application links
for sign-in and permission syncing.
When used for permission syncing, the user's oauth token is used to
fetch user permissions (and now permissions are fetched via the server).
## Test plan
Tests added and updated.
## Changelog
- Sourcegraph now supports Bitbucket Server OAuth2 application links for
user sign-in and permission syncing.
Did a ripgrep to see if there are any other `console.log` statements
that should be removed but this was the only one.
## Test plan
code inspection, trivial change
Closes DINF-182
Before `sg start otel` would fail if `.bin` did not exist. We now create
the destination directory if it doesn't exist
## Test plan
1. `rm -rf .bin`
2. `sg start otel`
and unit tests + CI
## Changelog
* when downloading files, ensure the directory we download to exists
**chore(appliance): version list obtained from backend**
Instead of calling release registry directly from the frontend. This
commit is just preparation for a fallback mechanism for users that do
not want the external dependency on release registry.
**feat(appliance): optionally load pinned releases file**
Instead of calling release registry. This is a fallback mechanism for
airgap users.
**feat(appliance): respect pinned release versions during self-update**
This PR removes the `redispool.RedisKeyValue` constructor in favor of
the `New...KeyValue` methods, which do not take a pool directly. This
way callers won't create a `Pool` reference, allowing us to track all
direct pool usage through `KeyValue.Pool()`.
This also simplifies a few things:
* Tests now use `NewTestKeyValue` instead of dialing up localhost
directly
* We can remove duplicated Redis connection logic in Cody Gateway
Closes #srch-906
This commit adds the cody chat page to svelte. This is simply reusing
the existing wrapper around the React component and renders it in a
standalone page.
When merged this will cause the cody chat page to be handled by new web
app on dotcom by default.
## Test plan
- Verified that chat loads and the tabs are clickable.
- Verified that the scroll behavior works as in the React app.
- Verified that the sidebar chat still works as expected (scroll
behavior, default context loading/switching)
- The time.Second frequency is too frequent to be checking if the job
should be run, I think I set this in testing
- Adjust the Slack messaging to say `Active license`
- Adjust the Slack messaging to include the Salesforce subscription ID
## Test plan
Tests
This is a partial revert commit of
cbd12608b5.
We added support for NoTransaction but it isn't needed anymore. In fact
avoiding transactions leads to issues like poisoning connections.
Test Plan: CI
We believe the migration to add tenant_id is failing on dotcom is due to
this trigger which runs for any possible change to a row. Instead it
only needs to run if the deleted_at column changes. So this migration
just adjusts each trigger to only run if deleted_at is changed.
We don't need to change the trigger function, since the trigger
functions only read deleted at.
If this doesn't resolve our issue, we will need to look into a statement
level trigger for all our triggers.
Test Plan: CI and then will test safely on larger instances.
Fixes srch-882
I noticed that the button wasn't shown anymore (neither in Svelte nor in
React). It seems that this broke when we switched from using the file
extension to getting the language(s) from the server.
The server sends back capitalized names which we compare against
lowercase IDs.
If there there is a new/modern way to find out whether a language
support 'find implementation' or not, please let me know. For the time
being this seems to be a simple fix to get it working again like it
before. Alternatively we can also compare the stylized name.
## Test plan
Hovering over a Go interface (e.g.
https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/internal/conf/server.go?L12)
shows the 'find implementations' button in the hovercard again)
Fixes srch-744
It seems that the code for testing whether the target element is part of
the layout didn't work in hovercards because it (possibly?) runs before
the hovercard is rendered.
Moving the logic to `onMount` + `await tick()` seem to work, although
that might still be a coincidence.
## Test plan
Hovering over the 'precise' badge shows the corresponding tooltip.
We dived into our go postgres driver and when executing a migration it
is executed as a "simple query". Postgres in this case automatically
wraps the collection of statements in a transaction, unless it contains
transaction statements. So our last attempt at removing the transaction
failed.
In this attempt we use COMMIT AND CHAIN after each table alter. What
this does is commit the current transaction and then starts it up again.
From the perspective of the go driver, it is as if there was only one
transaction. We then switch the migration to using a transaction to
ensure the go drivers clean up the postgres connection in case of
failure.
IE if a query manually starts a transaction and does not clean up, the
connection will be marked as broken for the next person who gets the
connection from the pool. By wrapping in go's transaction code the
connection will be properly cleaned up.
Test Plan: All continuous environments have already succeeded or failed
on this migration number. So we will manually run this again against
them with the migrator code to ensure the same code paths. If they
succeed we will keep code as is, otherwise we will rollback.
Additionally we did lots of adhoc testing to understand the
characteristics of go and transaction handling.
Co-authored-by: Erik Seliger <erikseliger@me.com>
Previously, it took ~6 seconds for a single edit/test/debug feedback
loop in the `llmapi` module. After this change, it's now 1-2s.
The reason the feedback loop was slow was that we depended on the
`//cmd/frontend/internal/modelconfig` target, which transitively brings
in `graphqlbackend` and all the migration code, which adds huge overhead
to Go link times. It was relatively easy to untangle this dependency so
I went ahead and removed it to boost my local feedback loop.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
## Test plan
Green CI.
To measure the timing, I ran the tests, made a tiny change and ran the
tests against to measure the total time to build+test.
```
# Before
❯ time go test -timeout 30s github.com/sourcegraph/sourcegraph/cmd/frontend/internal/llmapi
ok github.com/sourcegraph/sourcegraph/cmd/frontend/internal/llmapi 2.394s
go test -timeout 30s 4.26s user 4.73s system 166% cpu 5.393 total
# After
❯ time go test -timeout 30s github.com/sourcegraph/sourcegraph/cmd/frontend/internal/llmapi
ok github.com/sourcegraph/sourcegraph/cmd/frontend/internal/llmapi 0.862s
go test -timeout 30s 1.20s user 1.21s system 135% cpu 1.774 total
```
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Currently, build-tracker keeps track of consecutive build failures
through an in-memory store of failed builds. As this gets deployed more
frequently on MSP, we lose state more frequently which would result in
incorrect results. Instead, we can use redis as our external store as
well as for locking using redsync
## Test plan
Unit tests have been updated, but proper testing will require live
traffic
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Fixes CODY-3269
Previously, the OpenAI-compatible API endpoints had paths like
`/api/v1/chat/completions`, which went against an existing convention of
keeping all APIs under the `/.api` prefix. We have a fair amount of
internal tooling centered around the assumption that APIs have the
`/.api` prefix so this PR corrects the mistake and moves the
`/api/v1/chat/completions` endpoint to `/.api/llm/chat/completions`.
I went with the prefix `/.api/llm` since these allow clients to interact
with LLM features like `/chat/completions` or listing model information.
These APIs happen to be compatible with OpenAI APIs but I think it will
be more confusing to add something like "openai" or "openaicomptable" in
the API endpoint. We can just document on our website that these
endpoints are compatible with OpenAI clients.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
## Test plan
Green CI. I also manually confirmed that I was able to use an OpenAI
client to send requests to the new APIs.
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
* Moved `/api/v1/chat/completions` to `/.api/llm/chat/completions`. The
functionality is unchanged.
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
When we added a migration that actually relies on the promised property
of migrations always running inside a transaction, we noticed that the
memoryStore implementation we use in the dbtest package does not
actually start a transaction. This implements transactions for the
memorystore as well, to make it pass.
Test plan: Our migration now passes, and dbtest performance with and
without this change is the same.
Fixes CODY-3267
Previously, requests to `/api/` matched the new `publicrestapi` module,
which meant that requests to the GraphQL `/api/console` no longer
worked. This PR fixes the problem by narrowing the prefix-match to
`/api/v1` so that it no longer matches `/api/console`.
I kept the scope of this PR narrow and only fixed the /api/console bug.
I will share a separate RFC to seek input on the tradeoffs between
/api/v1 and /.api. I can make that change separately if there's wide
consensus in #wg-architecture that we want to keep all API endpoints
(public and internal-facing) under /.api.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
## Test plan
Ran `sg start minimal` and confirmed that I'm able to visit the URL
https://sourcegraph.test:3443/api/console
On the main branch, the same URL leads to a 404 page.
I also confirmed that the `/api/v1/chat/completions` endpoint still
works as expected.
```hurl
POST https://sourcegraph.test:3443/api/v1/chat/completions
Content-Type: application/json
Authorization: Bearer {{token}}
{
"model": "anthropic::unknown::claude-3-sonnet-20240229",
"messages": [
{
"role": "user",
"content": [
{
"type": "text",
"text": "Respond with \"no\" and nothing else"
}
]
}
],
"temperature": 1,
"max_tokens": 256,
"top_p": 1,
"frequency_penalty": 0,
"presence_penalty": 0
}
```
```sh
❯ hurl hurl-scratchpad/openai-sg.hurl
{"id":"chat-1727acdf-6850-4387-950b-2e89850071fa","choices":[{"finish_reason":"end_turn","index":0,"message":{"content":"no","role":"assistant"}}],"created":1723536215,"model":"anthropic::unknown::claude-3-sonnet-20240229","system_fingerprint":"","object":"chat.completion","usage":{"completion_tokens":0,"prompt_tokens":0,"total_tokens":0}}
```
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Our redis `KeyValue` interface has a built-in way to prefix all keys.
This is helpful for integration tests, to contain all keys within a test
namespace.
This PR solidifies the implementation by pushing down the prefixing as
much as possible. In doing so, it fixes a bug with the `Keys()` method
where we forgot to prefix the pattern. This makes it easier to add new
methods to the interface without forgetting to add the prefix.
We hit a deadlock when deploying this migration to s2. This is because
within our transaction of the migration we likely didn't obtain table
locks in the same order as a transaction in our application code.
So this commit introduces a new migration metadata field
"noTransaction". The documentation for migrator says you should create a
migration per needed transactions. However, this would require us to
create 100s of migrations. We believe the better approach is introducing
this field and barely advertising it.
When reading the code which actually runs migrations, there is no extra
logic done outside of BEGIN; run_migration; COMMIT; so this change is
safe.
We update the migrations to avoid duplicating the function name we
introduce in case something goes wrong (now that the transaction could
leak out the function name).
Test Plan: The actual migrations are tested by go test. I added a test
assertion that we don't call Transact, but to be honest that is super
sketchy. However, we couldn't actually find any test fixtures which
actually run against the DB. So that would require a much deeper
investment for how simple the code change is.
Co-authored-by: Erik Seliger <erikseliger@me.com>
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
This PR adds the ability to use `--type minor` when running `sg release
create` during the release process.
For the time being this step is the _same_ as `--type patch` which is
the default, however this allows us to differentiate the two types now
and prepares for when/if the two types diverge. This also clears up the
some confusion as the `sg release` command _can_ accept `--type minor`
already and one would expect that to be the choice when you are in fact
cutting a minor type.
Closes:
https://linear.app/sourcegraph/issue/REL-351/sourcegraphsourcegraph64377-fixrelease-add-minor-step-to-internal
## Test plan
Tested locally with `--type minor` tag.
```shell
➜ sourcegraph git:(08-08-jdp_release_minor-flag-addition) sg release create --version 5.6.877 --type minor
👉 [ setup] Finding release manifest in "."
[ setup] No explicit branch name was provided, assuming current branch is the target: 08-08-jdp_release_minor-flag-addition
[ setup] Found manifest for "sourcegraph" (github.com/sourcegraph/sourcegraph)
[ meta] Owners: @sourcegraph/release
[ meta] Repository: github.com/sourcegraph/sourcegraph
👉 [ vars] Variables
[ vars] version="v5.6.877"
[ vars] tag="5.6.877"
[ vars] config="{\"version\":\"v5.6.877\",\"inputs\":\"\",\"type\":\"minor\"}"
[ vars] git.branch="08-08-jdp_release_minor-flag-addition"
[ vars] is_development="false"
.... Stuff here
[ buildkite] Build created, see:
[ buildkite] "https://buildkite.com/sourcegraph/sourcegraph/builds/287192"
[ step] Step "buildkite" succeeded
```
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
Internal change, N/A
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
In codenav, we use very few APIs from gitserver, so let's use a more
narrow interface (which we can potentially fake in the future) instead
of depending on gitserver.Client directly.
In the future this will allow us to attribute stack traces collected by
pprof to a tenant. This only sets it for the http middleware. I am
unsure how to achieve the same thing for grpc, since that uses
propogators.
Test Plan: captured a goroutine profile and saw some goroutines with a
tenant label
---------
Co-authored-by: Erik Seliger <erikseliger@me.com>
I was tired of seeing this warning when running gazelle:
gazelle: finding module path for import github.com/go-critic/go-critic/framework/linter: go: downloading github.com/go-critic/go-critic v0.11.4
go: module github.com/go-critic/go-critic@upgrade found (v0.11.4), but does not contain package github.com/go-critic/go-critic/framework/linter
This updated go-critic to the latest version which makes the warning go
away since it now uses the new package path.
Test Plan: gazelle is happy and CI is happy
This change ensure we correctly handle:
1. In Enterprise Portal, where no active license is available, we return
ratelimit=0 intervalduration=0, from the source `PLAN` (as this is
determined by the lack of a plan)
2. In Cody Gateway, where intervalduration=0, we do not grant access to
that feature
## Test plan
Unit tests
This change caches what we download from google storage.
Before this change on my desktop computer this test would timeout after
10 minutes. It now takes 4s.
Test Plan: go test ./internal/database/migration/stitch
These all fail and are only run outside of bazel (which is why we didn't
notice it). I believe the root cause is google getting more strict with
the URLs you pass it.
Should we consider removing these tests given no one has reported it not
working? Or should we get it working in Bazel?
Test Plan go test ./internal/database/migration/stitch/
The goal of this PR is to make search blitz usable for customers who
want to run it against their instance for continuous performance
analysis.
Specifically, this does two things:
1) Enables configuring the set of queries to run with the
`SEARCH_BLITZ_QUERY_FILE` env var
2) Packages the image in our standard format so we can publish it
Instead of adding instrumentation for each place we expect a tenant, we
instead assume that we always expect a tenant if FromContext is called.
This should allow us to not miss any places.
Note: this does remove the logging we had in diskcache, but it seems
more reasonable to just use pprof for something that can be noisy?
Test Plan: just gonna rely on CI since this defaults to off.
The `DeleteAllKeysWithPrefix` method is now only used in tests to ensure
the test keyspace is clear. This PR makes it clear this method is only
used in tests, and simplifies the implementation so it no longer needs a
script + direct Redis connection.
Another tiny Redis refactor to prepare for multi-tenancy work.
This PR passes the 'boost' annotation down to searcher, so that it can
apply phrase boosting. For now, we just pass the boost to the Zoekt
query in hybrid search, which already gives a nice benefit since the
Zoekt results are streamed back first.
Note: this doesn't completely implement boosting in searcher, but it was
really simple and seemed worth it. We're purposefully not investing in
big searcher ranking improvements, since we think a better investment is
to unify logic across Zoekt + searcher.
Adds a super simple E2E test suite that must be run with `sg test
enterprise-portal-e2e` against a locally running Enterprise Portal
instance. This is not intended to be filled with super granular
assertions - it simply tests that everything is wired up correctly and
runs end-to-end.
Caught at ~3 issues with this already, amended various downstack PRs
with the fix 😆
Closes
https://linear.app/sourcegraph/issue/CORE-229/enterprise-portal-basic-manual-e2e-testing-setup
## Test plan
```
sg start dotcom
sg test enterprise-portal-e2e
```
No additional configuration required, the defaults work as-is
What: This PR does the bare minimum to migrate the current community
search pages to Svelte. A better strategy for managing them is needed in
the medium/long term.
How: The community pages live at the root (e.g. `/kubernetes`) which
complicates things, but I'll get to that later. The page is implemented
as a single parameterized route. A parameter matcher is used to validate
the community name. Because these pages should only be accessible on
dotcom the matcher also validates whether or not we are on dotcom (if
not, the path will be matched against a different route).
The page config is stored in a separate module so that it's no included
in every page and so that it can be used in the integration test.
The loader and page implementation themselves are straightforward. I
made a couple of changes in other modules to make implementation easier:
- Extracted the parameter type of the `marked` function so that it can
be used as prop type.
- Added an `inline` option to `marked` that allows formatting markdown
as 'inline', i.e. without `p` wrapper.
- Added a `wrap` prop to `SyntaxHighlightedQuery.svelte` to configure
line wrapping of syntax highlighted search queries (instead of having to
overwrite styles with `:global`).
- Extended the route code generator to be able to handle single
parameter segments and the `communitySearchContext` matcher.
Because the community routes should only be available on dotcom I added
a new tag to the code generator that allows it include routes only for
dotcom.
Once we change how all this works and have community search pages live
under a different path we can simplify this again.
Result:
| React | Svelte |
|--------|--------|
|
|
|
## Test plan
- New integration tests.
- Verified that `/kubernetes` shows a 'repo not found error' when
running against S2.
- Verified that `/kubernetes` shows the community page when running
against dotcom.
- Verified that `window.context.svelteKit.enabledRoutes` contains the
community page route in enterprise mode but not in dotcom mode.
This adds support for navigating between search results with keyboard
shortcuts. Similar to the React app, `j` or `down` means "next result",
and `k` or `up` means previous results. To accompany this change, when a
search is submitted, the first result is focused by default to
facilitate iterating over results with the keyboard.
The first version of the file tree for the revision panel had flat file
names. This meant that files were not organized, they were very
horizontal-space-sensitive, and you could not filter to a directory
(only repo and file name).
This updates the file filter to be a full tree, which I find much easier
to use.
Required to build an updated subscriptions management UI.
Most of the diff is generated proto for some reason
Closes https://linear.app/sourcegraph/issue/CORE-226
## Test plan
Integration tests
Backcompat test suite works by checking out the code at the previous
minor release, injects the database schema from `HEAD` and run all Go
tests. This ensures that the old code can run against the new schema,
thus being backward compatible.
We forgot to update this the last time, that's why I'm bumping by two
tags here.
## Test plan
CI
This reverts commit 43e06cef0c.
This migration deadlocked on S2, we didn't expect that we have processes
taking table locks but apparently so.. Reverting for now and will
manually fix up S2 once the build went through. We probably need to make
this one migration per table :rip:
## Test plan
Revert.
Previously, `sg` would give no notice that an auto-update of itself was
happening in the background, due to background output being buffered.
This would be confusing when an invocation hangs/doesnt terminate as
quickly as expected due to the update process still being in progress.
Instead, we should print at least certain output from the process
immediately
## Test plan
`go run ./dev/sg -- <some command>` with a time.Sleep
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Connecting to the wrong DSN leads to a cryptic error:
```
Message: {"SeverityText":"FATAL","Timestamp":1723196934012096886,"InstrumentationScope":"init db (syntactic-codeintel-worker)","Caller":"shared/shared.go:87","Function":"github.com/sourcegraph/sourcegraph/cmd/syntactic-code-intel-worker/shared.initCodeintelDB","Body":"Failed to connect to codeintel database","Resource":{"service.name":"syntactic-code-intel-worker","service.version":"286647_2024-08-08_5.6-34a7914fb884","service.instance.id":"syntactic-code-intel-worker-7bb9ccc75c-mkzpk"},"Attributes":{"error":"database schema out of date"}}
```
## Test plan
- existing tests should continue to pass
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
In https://github.com/sourcegraph/sourcegraph/pull/63946 we reworked how
we handle assets, but we overlooked updating the target in
`sg.config.yaml`.
## Test plan
CI + locally tested (made some changes on the home page, got reloaded as
expected)
Fixes CODY-3081
This is the first PR for the project [⏭️ Launch Cody API
Experimental](https://linear.app/sourcegraph/project/launch-cody-api-experimental-8fd5ec338bf4),
which falls under the umbrella of moving Cody's brains to the cloud.
Previously, there was no publicly available REST API for our customers
to interact with Cody. This is a frequently requested feature from
customers and prospects.
This PR adds a new `POST /api/v1/chat/completions` endpoint, which
should be compatible with existing OpenAI clients. The OpenAI API format
is increasingly becoming an industry standard so this seems like a good
first step towards exposing a stable publicly facing API for our
customers.
The goal is to add more Cody-specific APIs in the coming weeks to send
chat messages and reference context.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
## Test plan
See added test cases.
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
* API: new publicly available `/api/v1/chat/completions` REST endpoint
that is compatible with OpenAI clients with some restrictions. The detailed list of restrictions will eventually be documented on sourcegraph.com/docs
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
We introduce the tenant_id column to every table without any magic
enforcement or magic default value. Instead we default to NULL and plan
to have an out of band migration. From out testing this change is
instant since it is only a table metadata change. According to the
documentation this sort of ALTER TABLE is instant since Postgres 11.
Test Plan: ran this migration against a clone of the s2 database.
---------
Co-authored-by: Keegan Carruthers-Smith <keegan.csmith@gmail.com>
Currently, this uses context.Background, because the initial context
might've been canceled.
However, that causes us to lose any context involved here, so traces
won't include this query and tenant information would be lost.
This uses the new-ish context.WithoutCancel to avoid this issue and
stores the context used to create the transaction to close it later.
Test plan: Ran a local instance and didn't see errors, also integration
and E2E tests pass.
To ease the transition from older search semantics to keyword search, we
[boost matches on
phrases](https://github.com/sourcegraph/sourcegraph/pull/59940). This
phrase boosting applied to all search types, including repo, and symbol
searches. However, these search backends don't effectively handle the
extra boosted OR clause, so it can hurt performance and ranking.
This PR limits the phrase boosting to text searches (`type:file` and
`type:path`), as they benefit most from the boosting and can handle it
efficiently.
closes:
https://linear.app/sourcegraph/issue/SRCH-622/remove-old-cody-web-entirely
Removes the old Cody Web completely.
## Test plan
- sg start
- visit /cody/chat and you should see the new unified panel Cody Web
without having to turn on any feature flags.
- check the blob view sidebar for the same.
## Changelog
- The new Cody Web is enabled by default and the older version of Cody
Web is completely removed from both standalone chat and sidebar.
Some analysis phase performance improvements in rules_js 2.0.0-rc9 that
are worth picking up. rules_js is very close to 2.0.0 final now. Waiting
on one last improvement requiring and API change for bzlmod.
## Test plan
CI
## Changelog
On double-checking the various Cody Gateway access management CRUD I
noticed that I did not implement the ability to remove the override
entirely properly - right now, removing a rate limit with `field_mask`
and no values results in a zero-value rate limit rather than a null one
as desired.
This change makes it so that if you provider a nil `*_rate_limit`
object, all field paths for that object is also set to nil, removing the
override and allowing the default active-license-plan-based rate limits
to apply.
This also adds a minor adjustment to Cody Gateway to correctly handle
zero durations for rate limit intervals, which should be treated as "no
access".
## Test plan
Unit/integration tests, and a manual test:
- Check out https://github.com/sourcegraph/sourcegraph/pull/64090 and
run `sg start dotcom`
- Go to a subscription, add a rate limit override
- Click the 🗑️ button to remove the rate limit override
- Repeat above on each property
Sourcegraph's GitHub discussions have been defunct for awhile, but links
to them persisted in READMEs and documentation.
## Test plan
Documentation update only.
This change follows
https://github.com/sourcegraph/sourcegraph/pull/63858 by making the Cody
Access APIs _read_ from the Enterprise Portal database, instead of
dotcomdb, using the data that we sync from dotcomdb into Enterprise
Portal.
As part of this, I also expanded the existing "compatibility" test suite
that compares the result of our dotcomdb queries against the existing
GraphQL resolvers in dotcom to also compare the results of our new Cody
Access APIs, to validate that they return the same access.
> [!WARNING]
> There is one behavioural change, which is that hashes of _expired
licenses_ will no longer be valid as access tokens. This shouldn't be an
issue if customers use zero-config (implied access token from their
license key) - I will do some outreach before rolling this out.
Subsequent PRs will implement write APIs.
Part of https://linear.app/sourcegraph/issue/CORE-218
Part of https://linear.app/sourcegraph/issue/CORE-160
## Test plan
Integration and unit tests at various layers
This adds a new profile output for when a sql query does not have a
tenant set. This can be downloaded like other pprof profiles and
inspected by "go tool pprof". This allows us to easily see the most
common stack traces and sources of missing context.
Test Plan: downloaded from the debugserver the new profile type and
inspected with "go tool pprof"
---------
Co-authored-by: Erik Seliger <erikseliger@me.com>
Fixes srch-867
This commit adds a new command, `web-sveltekit-prod`, which simplies
builds the SvelteKit app and exits.
This command is now run by default when using `sg start`, so the assets
will be available locally.
Note however that the build process is quite slow, which is why I didn't
make it rebuild on file changes. Using the vite dev server, e.g. via `sg
start enterprise-sveltekit`, delivers a much better experience.
To make the integration a bit more ergonomic, I made additional changes:
- When run via `sg` (determined by checking `DEPLOY_TYPE`) compiler
warnings are not printed. That reduces noise in the `sg` output.
- I made similar changes to the svelte check bazel target to make it
easier to spot actuall errors (warnings are still important to fix but
since they don't cause the target to fail they just make it difficult to
spot the actual error).
- Since I finally found out about the `onwarn` handler, I made it so
that warnings about missing declarations are ignored. These warnings
occur for every icon because the Svelte compiler doesn't respect ambient
d.ts files.
- And since I made icon warning related changes I also documented a bit
how icons work in the app.
## Test plan
- `sg start` runs the `web-sveltekit-prod` command and the Svelte app is
served when going to `https://sourcegraph.test:3443/search`.
- The output of `bazel test //client/web-sveltekit:svelte-check` now
only contains errors.
- The output of `pnpm dev` (vite devserver) shows warnings that are not
related to icons.
Fixes srch-859
Problem: Currently the sidebar can reopen in the following scenario:
- Go to search home and submit a search
- On the search results page click the menu icon to open the sidebar and
click 'Code Search' to go back to search home.
- Execute a different query
- -> this brings you back to search results but with the menu sidebar
open
Looking at the code it's understandable that this happens: We never
close the sidebar. But this problem wasn't very visible because every
entry in the nav sidebar either brings you to the React app or to a page
with inline nav menu.
So this commit fixes the issue by closing the sidebar whenever we
navigate to a different page.
## Test plan
Manual testing. I added a temporary nav entry that would bring me to the
sourcegraph repo page. Clicking on it from the search results page
caused the sidebar to close.
Give a more descriptive error when we are unable to find a merge base
and can't find the files that have changed.
Closes DINF-162
## Test plan
1. `git fetch -v --prune --depth=100 -- origin
ccbaba24b72f3c6f4524b3f560ca839143ea463b`
2. `git merge-base HEAD origin/main` --- you'll get nothing since there
is no merge base in the current history
Increase the repo history
1. `git fetch --unshallow`
2. `git merge-base HEAD origin/main`
```
0a6e509af3
```
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Co-authored-by: Bolaji Olajide <bolaji.olajide@sourcegraph.com>
In some deployments, these are running in separate instances. This
migration makes sure that we can still reference the tenants from the
tenant_id column reliably everywhere.
It's critical that this migration is a noop on instances that already
have this table, so please double check me here during code review.
Test plan: Integration and E2E tests still pass, as this should be a
noop.
Unless tenants are enforced, we use the old code path. If they are
enforced:
- zips are stored in `<cache-dir>/tenants/<tenantID>/<key>`
- evict runs globally
I also added another test helper to create tenants with fresh IDs for
every call
## Test plan:
- Updated unit tests
- Manual testing: I verified that unindexed symbol and text search work
and that a `/tenant/1` dir is created in both cache dirs
(`/tmp/searcher-archive` and `/tmp/symbols-cache`).
The utils are currently used outside of cmd/frontend, so to break the
internal -> cmd import I'm moving and merging the utils package with
internal/gqlutil.
Test plan: Just moved around and renamed a package, the go compiler
doesn't complain.
Now that https://github.com/sourcegraph/sourcegraph/pull/64339 is
merged, we can tell users more about what to expect with `sg start
single-program-experimental-blame-sqs`. And as it's been in flight for a
while now, it's safe to say that's it's time to give it a shorter name
😊.
So it's been renamed from `single-program-experimental-blame-sqs` to
`minimal`. And to ensure nobody is getting confused, a `deprecated`
attribute has been added on the command sets, which is used here to
indicate that the new alternative is `sg start`.
❓ Thoughts about `sg start minimal`? `sg start single` perhaps?
Running the old commandset: (ignore the yellow message, that's just a
local warning from `sg`)
Running the new commandset, with the preamble explaining what to expect:
## Test plan
Locally tested.
Fixes srch-858
Currently the menu icon rotates when hovering over it. The animation
should only target the Sourcegraph logo.
Additional changes:
- Move icon color style prop into CSS declaration (avoids inserting an
additional element)
- Let the animation target the link instead of the icon. There doesn't
seem to be reason why we actually need to target the `svg` element via
`:global`.
## Test plan
Manual testing.
This PR adds a basic table for storing tenants in the database, and automatically inserts a tenant 1 for existing instances.
Test plan: Adds a new, not yet used table. The migration is very simple, so shouldn't cause any issues to existing instances.
This PR aims to craft the /internal/tenant package for use by all Sourcegraph cluster-internal services to properly scope data visibility to the correct requesting tenant.
For now, we only expose methods we know we will DEFINITELY need.
This PR also adds the required middlewares so we can start to tinker with it in implementations.
## Test plan
CI passes. We don't enforce anything for now except not passing unparseable tenant IDs, which should be fine.
There currently isn't a 'one step' way to start a local Sourcegraph
instance with the SvelteKit app.
This commit adds `sg start enterprise-sveltekit` to fix that.
The changes in https://github.com/sourcegraph/sourcegraph/pull/64272
allow us to run the vite dev server in front of the Sourcegraph
instance, instead of building the assets and having them served by
frontend.
This gives us the benefit of hot module reloading and in general seems
to be a less fragile approach.
It's basically the same what we do with the React app in development
mode.
## Test plan
`sg start enterprise-sveltekit` starts the vite dev server as well as
the sourcegraph instance. Navigating to
`https://sourcegraph.test:3443/search` opens the Svelte app (when
enabled and logged in). Making a change in a source file updates the web
page immediately.
`sg start web-sveltekit-standalone` still works
I don't know why but having `overflow-x` causes the content of the
hovercard to not fully expand to the edges. It has something to do with
the sizing of the `hr` element.
## Test plan
Manual testing.
There was a todo comment that said we want to consolidate them but didn't yet to keep another diff smaller - so now we're doing that.
Test plan: Integration tests for search still pass.
To prevent cross-cmd imports in the future, moving the backend package into internal.
Test plan: Just moved a package around, Go compiler doesn't complain and CI still passes.
These functions are not required to be called outside of frontend, so there's no need to reexport them. Instead, we consolidate the signout cookie logic in the session package.
Test plan: Just moved some code around, go compiler doesn't complain.
This should resolve the Workflows delivery failure seen in
https://buildkite.com/sourcegraph/sourcegraph/builds/286482#01912eb8-a1a9-48c6-81b0-4f5a09448bbb
after landing the rules_oci 2 upgrade:
```
· Error: failed to compute runfiles hash for manifest: error concurrently hashing file /mnt/ephemeral/output/sourcegraph/__main__/execroot/__main__/bazel-out/k8-opt-ST-d57f47055a04/bin/dev/build-tracker/image_underlying/blobs/sha256/104f4630c01017c52e968bfe039f6eb7622ef1ad9d44d94d784cc9c39992961b: failed to open file /mnt/ephemeral/output/sourcegraph/__main__/execroot/__main__/bazel-out/k8-opt-ST-d57f47055a04/bin/dev/build-tracker/image_underlying/blobs/sha256/104f4630c01017c52e968bfe039f6eb7622ef1ad9d44d94d784cc9c39992961b for hashing: open /mnt/ephemeral/output/sourcegraph/__main__/execroot/__main__/bazel-out/k8-opt-ST-d57f47055a04/bin/dev/build-tracker/image_underlying/blobs/sha256/104f4630c01017c52e968bfe039f6eb7622ef1ad9d44d94d784cc9c39992961b: no such file or directory
```
The issue was that the runfiles of the underlying oci_image
(`image_underlying` target) were not being forwarded through the custom
`oci_image_cross` rule and therefor were not being built or fetched from
the remote cache and layed out on disk for the delivery step.
In this repo, the `oci_image` rule is a macro that is an underlying
`oci_image` with the main target being an `oci_image_cross`:
```
# Apply a transition on oci_image targets and their deps to apply a transition on platforms
# to build binaries for Linux when building on MacOS.
def oci_image(name, **kwargs):
_oci_image(
name = name + "_underlying",
tars = kwargs.pop("tars", []) + ["//internal/version:stamps"],
**kwargs
)
oci_image_cross(
name = name,
image = ":" + name + "_underlying",
platform = select({
"@platforms//os:macos": Label("@zig_sdk//platform:linux_amd64"),
"//conditions:default": Label("@platforms//host"),
}),
visibility = kwargs.pop("visibility", ["//visibility:public"]),
)
```
## Test plan
CI
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
This PR makes the calls to create the OIDC provider explicit, so that we don't need to implicitly need to call a Refresh method, even if we might end up not needing the `p.oidc`.
This is a start toward being able to create providers on the fly cheaply vs having a globally managed list of providers in memory.
We did call refresh everywhere we do now anyways to the best of my understanding, so a passive goroutine in the background to my best understanding didn't add a lot here.
Test plan: Auth with SAMS locally still works.
2nd attempt of #63111, a follow up
https://github.com/sourcegraph/sourcegraph/pull/63085
rules_oci 2.0 brings a lot of performance improvement around oci_image
and oci_pull, which will benefit Sourcegraph. It will also make RBE
faster and have less load on remote cache.
However, 2.0 makes some breaking changes like
- oci_tarball's default output is no longer a tarball
- oci_image no longer compresses layers that are uncompressed, somebody
has to make sure all `pkg_tar` targets have a `compression` attribute
set to compress it beforehand.
- there is no curl fallback, but this is fine for sourcegraph as it
already uses bazel 7.1.
I checked all targets that use oci_tarball as much as i could to make
sure nothing depends on the default tarball output of oci_tarball. there
was one target which used the default output which i put a TODO for
somebody else (somebody who is more on top of the repo) to tackle
**later**.
## Test plan
CI. Also run delivery on this PR (don't land those changes)
---------
Co-authored-by: Noah Santschi-Cooney <noah@santschi-cooney.ch>
This commit adds the necessary logic to open the explorer tab for
multiple references and for implementations. I basically copied what
@camdencheek did for the references tab.
## Test plan
Hover over `adapt` in
http://localhost:5173/github.com/sveltejs/kit@65931f276ac2102032e3032c864a472eee19b7bb/-/blob/packages/kit/src/exports/vite/index.js?L890
and click 'got to definition'. The explore panel with should open with
the definitions tab selected.
I wasn't able to test the find implementations logic because I don't
know for which language we have this implemented.
Turns out, the `dev` tag is not a reliable indicator of whether a
license is for dev use only - let's just import all subscriptions into
prod, and drop the customer records from dev later.
## Test plan
CI
Adds a background job that can periodically import subscriptions,
licenses, and Cody Gateway access from dotcom.
Note that subscriptions and licenses cannot be deleted, so we don't need
to worry about that. Additionally licenses cannot be updated, so we only
need to worry about creation and revocation.
The importer can be configured with `DOTCOM_IMPORT_INTERVAL` - if zero,
the importer is disabled.
Closes https://linear.app/sourcegraph/issue/CORE-216
## Test plan
```
DOTCOM_IMPORT_INTERVAL=10s sg start dotcom
```
Look for `service.importer` logs. Play around in
https://sourcegraph.test:3443/site-admin/dotcom/product/subscriptions/
to create and edit subscriptions, licenses, and Cody Gateway access.
Watch them show up in the database:
```
psql -d sourcegraph
sourcegraph# select * from enterprise_portal_susbscriptions;
sourcegraph# select * from enterprise_portal_susbscription_licenses;
sourcegraph# select * from enterprise_portal_cody_gateway_access;
```
---------
Co-authored-by: James Cotter <35706755+jac@users.noreply.github.com>
The screenshot images for the welcome overlay (both in Svelte and React)
are quite large.
In Svelte specifically, because they are implemented as Svelte
components, this adds a lot of code to the root layout and makes the
bundle quite large.
This commit converts the Svelte components to image files and passes
them (and the React ones) through [svgo](https://github.com/svg/svgo).
On one hand that makes the images much smaller, on the other hand in
Svelte the images are now independent of the JS code and can be loaded
separately (the React app was already using an `img` element).
Before svgo:
```
$ ls src/lib/assets/
total 3.1M
5.1K Aug 7 18:45 sourcegraph-logo.svg
1.5M Aug 7 18:46 welcome-screenshot-dark.svg
1.6M Aug 7 18:46 welcome-screenshot-light.svg
```
After svgo:
```
$ ls src/lib/assets/
total 1.4M
5.1K Aug 7 18:45 sourcegraph-logo.svg
667K Aug 7 18:50 welcome-screenshot-dark.svg
667K Aug 7 18:50 welcome-screenshot-light.svg
```
## Test plan
I temporarily changed the code so that the welcome dialog is always
shown in the Svelte app and inspected the image. It still seems to look
the same.
This makes some requested changes to the welcome banner.
Notably:
- We no longer show the dialog unless the user switches into the svelte
version from react
- We don't show the dialog on every switch
- The dialog is accessible in the React via the "Learn more" button in
the menu
- Copy is updated to be less verbose
- The request for feedback is only shown once when switching out of the
svelte webapp
Fixes srch-851
A consequence of https://github.com/sourcegraph/sourcegraph/pull/64272
is that redirecting to the cody marketing page on dotcom didn't work.
That's because `/cody` is not provided by the server as a known page.
A simple fix would be to mark the link as external, but we'd have to
keep in mind to do this in all places (present and future).
A more "central" fix is to add this page to a hardcoded list of known
pages that are not provided by the server.
## Test plan
Manual testing
Currently the link element doesn't extend to the full width of the
sidebar. You can click outside of the actual text to navigate to the
file, but it won't be preloaded.
Now the link occupies the full width.
## Test plan
Manual testing.
Closes srch-838
This commit extends the search progress popover and adds the search jobs
section.
In the React version the UI for the popover differs slightly when search
jobs are enabled vs not enabled.
I decided to ignore the differences and only impolemented the style used
for the 'search jobs enabled' version, which makes the popover a bit
more compact.
The logic for validating and creating a search job is encapsulated in a
class which makes it easy to conditionally show the search jobs UI.
Additional changes:
- Skipped items is now a list and uses `<details>` elements which is
semantically more correct. We loose the styling of the chevron but I
think that's OK.
- LoadingSpinner was updated to work inline.
- Logging was fixed (?) in the React version to send the correct
meta-data.
- Added integration tests for the search job popover behavior
## Test plan
Integration tests and manual testing.
Under high contention, the updating of execution logs query:
```
UPDATE
lsif_indexes
SET
execution_logs = execution_logs || $1::json
WHERE
id = $2
AND worker_hostname = $3
AND state = $4 RETURNING ARRAY_LENGTH(execution_logs, $5)
```
Was taking multiple seconds due to lock contention on the
lsif_indexes_state index.
Running `EXPLAIN ANALYZE` on Sourcegraph.com under lower contention uses
the primary key index on id, so we don't have an easy way to test the
high contention scenario. Try this alternate query form to see if that fixes the issue.
Closes SRCH-802
After a long and unsuccessful hunt of eventual consistency errors, I
noticed that we don't need the GraphQL fragments that are causing
problems.
I introduced a simplified request, which succeeds reliably. What
customers should see now is one error (see second picture below), which
is followed by a retry, and then success.
Previously we've seen this error, sometimes less often, sometimes more,
but customers saw it so often that it caused their batch changes to
fail.
<img width="1042" alt="Screenshot 2024-08-06 at 14 02 00"
src="https://github.com/user-attachments/assets/c5a099da-d474-43db-ac12-ac7c4f22d4d3">
Customers may still see an initial error, which I've seen to reliably
disappear on the first retry. It should not be a problem
<img width="1039" alt="Screenshot 2024-08-06 at 14 02 48"
src="https://github.com/user-attachments/assets/99f278d4-4020-48ea-b4ac-32cbdfce455d">
## Test plan
Manual testing
## Changelog
- fix(batches): improve GitHub Apps integration reliability by
simplifying the data requested from GitHub
Consolidates the two test helper/util modules, and moves one test file
to align with the file it's actually testing.
## Test plan
Tests continue to pass
Previously, the QueuedCount method was confusing because:
1. By default, it actually returned the count for both the 'queued' and
'errored' states (despite the name just have 'Queued').
2. There was an additional boolean flag for also returning entries in
the 'processing' state, but reduced clarity at call-sites.
So I've changed the method to take a bitset instead, mirroring the
just-added Exists API, and renamed the method to a more
generic 'CountByState'.
While this does make call-sites a bit more verbose, I think the
clarity win makes the change an overall positive one.
This change should not change any behaviour, but it prepares for syntactic/search-based pagination.
Both syntactic and search based usages now return a "NextCursor", which is `Some` if there are more results of their provenance to be had. For now they always return `None` which preserves the current behaviour. The logic in `root_resolver` implements a state machine that transitions the cursor through the provenances and makes sure to skip provenances if the cursor in the request starts a certain state.
## Test plan
Tests continue to pass, no behavioral changes
Closes DINF-89
Gazelle sometimes have trouble processing glob expressions, and this
isn't reported as a failure even though it ultimately results in the
`BUILD.bazel` not being correctly updated.
## Test plan
* Manual testing
In `client/web/BUILD.bazel`, add a new `src` to the `web_lib` ts_project
target that includes a glob pattern.
```
...
ts_project(
name = "web_lib",
srcs = glob(["!src/playwright/*.spec.ts"]) + [
"src/Index.tsx",
"src/LegacyLayout.tsx",
"src/LegacyRouteContext.tsx",
"src/LegacySourcegraphWebApp.tsx",
"src/PageError.tsx",
"src/SearchQueryStateObserver.tsx",
"src/SourcegraphWebApp.tsx",
...
```
When you run `go run ./dev/sg bazel configure`, the command should fail
with an error message instead of returning exit 0.
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
---------
Co-authored-by: Jean-Hadrien Chabran <jean-hadrien.chabran@sourcegraph.com>
I noticed that non-svelte repo subpages haven't been redirected to the
React app in local development (it works as expected in production).
That's because the proxy doesn't know about them since they are not part
of `knownRoutes`.
We could update the server code to include those routes as well but that
seems heavey handed just to make local development work.
I think in this case it's fine to have manual entries for the handful of
repo subpages that have not been migrated to Svelte yet.
## Test plan
Manual testing.
::sigh:: the problem here is super in the weeds, but ultimately this
fixes a problem introduced when using AWS Bedrock and Sourcegraph
instances using the older style "completions" config.
## The problem
AWS Bedrock has some LLM model names that contain a colon, e.g.
`anthropic.claude-3-opus-20240229-v1:0`. Cody clients connecting to
Sourcegraph instances using the older style "completions" config will
obtain the available LLM models by using GraphGL.
So the Cody client would see that the chat model is
`anthropic.claude-3-opus-20240229-v1:0`.
However, under the hood, the Sourcegraph instance will convert the site
config into the newer `modelconfig` format. And during that conversion,
we use a _different value_ for the **model ID** than what is in the site
config. (The **model name** is what is sent to the LLM API, and is
unmodified. The model ID is a stable, unique identifier but is sanitized
so that it adheres to naming rules.)
Because of this, we have a problem.
When the Cody client makes a request to the HTTP completions API with
the model name of `anthropic.claude-3-opus-20240229-v1:0` or
`anthropic/anthropic.claude-3-opus-20240229-v1:0` it fails. Because
there is no model with ID `...v1:0`. (We only have the sanitized
version, `...v1_0`.)
## The fix
There were a few ways we could fix this, but this goes with just having
the GraphQL component return the model ID instead of the model name. So
that when the Cody client passes that model ID to the completions API,
everything works as it should.
And, practically speaking, for 99.9% of cases, the model name and model
ID will be identical. We only strip out non-URL safe characters and
colons, which usually aren't used in model names.
## Potential bugs
With this fix however, there is a specific combination of { client,
server, and model name } where things could in theory break.
Specifically:
Client | Server | Modelname | Works |
--- | --- | --- | --- |
unaware-of-modelconfig | not-using-modelconfig | standard | 🟢 [1] |
aware-of-modelconfig | not-using-modelconfig | standard | 🟢 [1] |
unaware-of-modelconfig | using-modelconfig | standard | 🟢 [1] |
aware-of-modelconfig | using-modelconfig | standard | 🟢 [3] |
unaware-of-modelconfig | not-using-modelconfig | non-standard | 🔴 [2] |
aware-of-modelconfig | not-using-modelconfig | non-standard | 🔴 [2] |
unaware-of-modelconfig | using-modelconfig | non-standard | 🔴 [2] |
aware-of-modelconfig | using-modelconfig | non-standard | 🟢 [3] |
1. If the model name is something that doesn't require sanitization,
there is no problem. The model ID will be the same as the model name,
and things will work like they do today.
2. If the model name gets sanitized, then IFF the Cody client were to
make a decision based on that exact model name, it wouldn't work.
Because it would receive the sanitized name, and not the real one. As
long as the Cody client is only passing that model name onto the
Sourcegraph backend which will recognize the sanitized model name / ID,
all is well.
3. If the client and server are new, and using model config, then this
shouldn't be a problem because the client would use a different API to
fetch the Sourcegraph instance's supported models. And within the
client, natively refer to the model ID instead of the model name.
Fixes
[PRIME-464](https://linear.app/sourcegraph/issue/PRIME-464/aws-bedrock-x-completions-config-does-not-work-if-model-name-has-a).
## Test plan
Added some unit tests.
## Changelog
NA
This PR sets the defaults for "Sourcegraph supplied LLM models".
## When will these "defaults" be used?
These models will _only_ be used IFF the Sourcegraph instance is
_explicitly_ using the newer "modelConfiguration" site configuration
data. (And opts into using Sourcegraph-supplied LLM models.)
If the Sourcegraph instance is using the older "completions"
configuration blob, then _only_ the user-supplied models will be used.
(Or, based on the specific defaults defined in the code for the
completions provider.)
## What about Cody Free or Cody Pro?
😬 yeah, we're going to need to deal with that later. Currently
Sourcegraph.com is _not_ using the newer "modelConfiguration" site
configuration, and instead we have some hacks in the code to ignore the
internal modelconfig. See this "super-shady hack":
e5178a6bc0/cmd/frontend/internal/httpapi/completions/get_model.go (L425-L455)
So we are just erring on the side of having Cody Free / Cody Pro "do
whatever they do now", and this PR won't have any impact on that.
We _do_ want Sourcegraph.com to only return this data, but there are a
few things we need to get straightened out first. (e.g. Cody Gateway
being ware of mrefs, and having Cody Clients no longer using `dotcom.ts`
to hard-code Cody Pro LLM models.)
## What does this PR actually _do_?
1. It updates the code in `cmd/cody-gateway-config` so that it will
produce a new "supported-models.json" file.
2. I then ran the tool manually, the output of which was then written to
`internal/modelconfig/embedded/models.json`.
3. That's it.
For any Sourcegraph releases after this PR gets merged, the "Sourcegraph
supplied LLM models" will be the newer set defined in `models.json`.
(i.e. having these new defaults, and including
"fireworks::v1::starcoder".)
## Test plan
~~I tested things locally, and unfortunately it doesn't look like any
clients are filtering based on the model capabilities. So "StarCoder" is
showing up in the Cody Web UI, despite failing at runtime.~~
Update: This was a problem on my end. This isn't an issue.
## Changelog
NA?
This closes a pretty big gap in the query validation for Search Jobs. We
don't support repo search yet and searcher returned errors during
execution, complaining about missing patterns. With this PR we fail
during validation so users cannot even create these kinds of jobs. See
new test cases.
## Test plan
Updated unit tests
Running `COUNT(*)` on hot tables like lsif_indexes on Sourcegraph.com
shows up on profiles when the number of executors is bumped to
30+, and when the table has 100K+ jobs. So avoid `COUNT(*)`
where possible.
Reduce the exposed surface area with a smaller interface
minimalRepoStore, and make sure we're using fakes in the
tests which better document the intent, instead of using a
mock repo store. The fake makes sure that the methods
are mutually consistent, which is easier to do when working
with a smaller interface.
Fixes srch-765
When navigating to `/cody/dashboard` and the new web app is enabled, the
user gets a "Not found" error. This doesn't happen in the React app.
The page doesn't exist in the new web app, hence the server should fall
back to the old web app but that doesn't happen.
Why? Because the server doesn't know about `/cody/dashboard` either. It
only exists in the React app.
Instead the server interprets this path as a (non-existing) repository
page. When the new web app is enabled, repository pages are handled by
the new web app, but since neither the repo nor the page exist in the
new web app, the aforementioned error is thrown.
Configuring this route on the server makes it so that the React app is
served instead.
## Test plan
Manual testing. Going to https://sourcegraph.test:3443/cody/dashboard
loads the React app.
Follow up to #64207. In our old search semantics, quotes were
interpreted literally. So a query like `"sourcegraph"` would match only
strings like `fmt.Println("sourcegraph")`. Now, both single and double
quotes are used for escaping, and mean that the contents should be
searched exactly.
This PR makes sure to boost matches on quoted terms in result ranking.
This way, users familiar with the old syntax are more likely to find
what they're after.
## Test plan
Adapted unit tests. Re-tested all queries from #64207 manually, plus
these ones:
* `'sourcegraph'`
* `"sourcegraph"`
A long time ago, we dropped the requirement for frontend to have any
disk for caching. Our helm deployments use read-only rootFSes, so this
wouldn't even work.
This PR aims to make that clearer by removing some last remnants of
those times.
Test plan: Frontend starts locally and integration tests pass in CI.
In #64215, we removed support for smart search. We continued to allow
the `smart` search mode, but just execute with `precise` behavior.
However, we started to throw an error for `patterntype:lucky` (which is
the old way of specifying smart search). It turns out that `lucky` made
its way into some search links and settings, causing those searches to
fail.
This PR restores support for `lucky`, and remaps it to `standard`. This
preserves the semantics as much as possible (since smart search attempts
a 'standard' search as its first rule).
Closes SRCH-840
tl;dr: Everything is derived from `window.context.svelteKit.knownRoutes`
*buckle up*
### Context
After the new web app was enabled by default on dotcom I looked at
dotcom data in Sentry to see if there was anything out of the ordinary.
I noticed a high number of errors related to resolving repository
information. The most common non-existing repository that was reported
was `-/sign-out`.
Of course this shouldn't happen. `/-/sign-out` is the sign out URL and
shouldn't be interpreted as a repository.
Currently we prevent SvelteKit from interpreting specific paths as
repositories by using the `reporev` parameter validator:
```ts
// src/params/reporev.ts
const topLevelPaths = [
'insights',
'search-jobs',
'saved-searches',
// ... many more here
]
const topLevelPathRegex = new RegExp(`^(${topLevelPaths.join('|')})($|/)`)
// This ensures that we never consider paths containing /-/ and pointing
// to non-existing pages as repo name
export const match: ParamMatcher = param => {
// Note: param doesn't have a leading slash
return !topLevelPathRegex.test(param) && !param.includes('/-/')
}
```
`-/sign-out` was not in that list, including others which have been
added since this file was originally created.
Adding it would have been the simple solution but having to maintain
this list manually has always irked me. Now we have a better way...
### Production changes
#### Client side
It turns out we are already sending a list of known routes to the client
in `window.context.svelteKit.knownRoutes` to basically do the same
thing: test whether a given path is a page or a repository.
```ts
// src/lib/navigation.ts
let knownRoutesRegex: RegExp | undefined
function getKnownRoutesRegex(): RegExp {
if (!knownRoutesRegex) {
knownRoutesRegex = new RegExp(`(${window.context?.svelteKit?.knownRoutes?.join(')|(')})`)
}
return knownRoutesRegex
}
// ...
export function isRouteEnabled(pathname: string): boolean {
let foundRoute: SvelteKitRoute | undefined
// [...]
if (foundRoute) {
if (foundRoute.isRepoRoot) {
// Check known routes to see if there is a more specific route than the repo root.
// If yes then we should load the React app (if the more specific route was enabled
// it would have been found above).
return !getKnownRoutesRegex().test(pathname)
}
return true
}
return false
}
```
Why do we have the `reporev` validator and `isRouteEnabled`? They
basically do the same thing (check whether a path is a known page or a
repository) the first (`reporev`) is used by SvelteKit to determine
which route a path corresponds to (i.e. to navigate to the repository
page or whether the page doesn't exist) and the second one
(`isRouteEnabled`) is used after the route resolution but before route
loading. It's used to trigger a full page refresh to fetch the React
version if necessary.
Anyways, since we already have a list of known pages from the server,
the parameter validator should just use it too. Except it didn't work,
which made the following server side changes necessary.
#### Server
We register routes in multiple places. Right now `knownRoutes` is
populated from the router created in
`cmd/frontend/internal/app/ui/router.go`, but this does not include
`/-/sign-out`. This route (and others) are defined in
`cmd/frontend/internal/app/router/router.go` (I don't know what the
difference is). I extended the sveltekit route registration code so
that's possible to register routes from multiple routers.
I couldn't test it yet however because I currently can't get Sourcegraph
to run locally (Camden tested it; it works).
### Development mode changes
After the above changes, navigating to a React only page in development,
e.g. `/insights` would show a 'repo not found error' because
`window.context.svelteKit.knownRoutes` was empty in development. So
every non-existing page would be interpreted as missing repository.
Hardcoding a list of known pages *again* just for development seemed to
be a step backwards. So I spent quite a bit of time to find a way to
extract the JS context object from the HTML page returned by the origin
server and inject it into the HTML page generated by SvelteKit, similar
to how we do it for the React app.
Additionally the value of `window.context.svelteKit.knownRoutes` is now
used to setup the proxy to non-supported pages from the server, so we
don't have to maintain this regex anymore either:
```ts
// Proxy requests to specific endpoints to a real Sourcegraph
// instance.
'^(/sign-(in|out)|/.assets|/-|/.api|/.auth|/search/stream|/users|/notebooks|/insights|/batch-changes|/contexts)|/-/(raw|own|code-graph|batch-changes|settings)(/|$)': { ... }
```
This means that requesting non-svelte pages will also return the
corresponding React version in development.
## Test plan
Manual testing.
Relates to #64186
With this PR we only show `83 out of 120 tasks` if the search job is
currently processing. In all other states, we don't show this stat. This
is a consequence of the janitor job I recently added, because after
aggregation, this data is not available anymore. User's can still
inspect the logs and download results to get a detailed view of which
revisions were searched.
I also remove an unnecessary dependency of the download links on the job
state.
## Test plan:
I ran a search job locally and confirmed that the progress message is
only visible while the job is processing and that logs and downloads are
always available.
## Changelog
- Show detailed progress only while job is in status "processing"
- Remove dependency of download links on job state
Instead of re-generating and re-compiling the mocks 3 times,
consolidate them into a single storemocks package.
This should also provide better code navigation in the editor.
Previously, for metrics reporting, we were querying the number of
queued+errored records every 5 seconds. For large queues +
heavy contention, this scan itself takes a few seconds, despite using
an index. Reduce the frequency of this scan as we
don't need updates to the queue size every 5 seconds.
## Test plan
Check that frequency of `COUNT(*)` is reduced on Sourcegraph.com once
this patch makes its way to Sourcegraph.com
The current logging is outdated. This PR is not a complete rewrite. It
tries to keep semantics mostly as-is but it is more generous when it
comes to what we log. (For example we didn't log `(AND foo bar)` before)
IMO this is a good compromise we can ship quickly and then take some
time to figure out how to distinguish suggestions, searches, code-intel
and whether it is a good idea to mix search types and result types.
Updates:
- Prefer to log result types, fall back to pattern type. (This is
largely the same logic as before but we don't bail for complex queries)
- Don't guess repo and file results types if not explicitly specified.
We loose a bit of information here but it keeps the code much cleaner.
- Allow "AND" and "OR" operators.
The updated test case gives a good overview of how we would log things
in the future.
Kept as-is:
- I left "new" and "legacy" logging in place. I am not sure how we use
the target stores right now so I wanted to keep this as-is for now.
However we should see if we can retire the legacy logging.
- The previous and current logic translates `type:file` queries to
pattern types. I guess this makes sense from the perspective of how
things are implemented in the backend.
- "type:path" is logged as "file".
## Test plan:
Updated unit test
This PR fixes Cody autocomplete in some situations, fixing
[PRIME-426](https://linear.app/sourcegraph/issue/PRIME-426/autocomplete-completely-broken-in-main-with-and-sometimes-without-the).
Our story begins with this PR:
https://github.com/sourcegraph/sourcegraph/pull/63886. The PR updated
the `CodyLLMConfigurationResolver.Provider` GraphQL endpoint to no
longer return the "provider" that was put into the site configuration,
but to instead return the _display name_ of the provider, or if multiple
were configured the string "various".
```diff
- func (c *codyLLMConfigurationResolver) Provider() string {
- return string(c.config.Provider)
- }
+ func (c *codyLLMConfigurationResolver) Provider() string {
+ if len(r.modelconfig.Providers) != 1 {
+ return "various"
+ }
+ return r.modelconfig.Providers[0].DisplayName
+}
```
This change was wrong on several levels, and unfortunately stemmed from
the original author (me) not tracking down and understanding how the
GraphQL endpoint was actually used.
Once we discovered the problem, we quickly rectified this by changing
the behavior to the more correct version of returning the Provider ID of
the code completion model with
https://github.com/sourcegraph/sourcegraph/pull/64165:
```go
func (r *codyLLMConfigurationResolver) Provider() string {
return string(r.modelconfig.DefaultModels.CodeCompletion.ProviderID())
}
```
However, after some more testing we discovered yet another edge case. We
didn't realize that the provider "sourcegraph" (which is how you
configure Sourcegraph to use Cody Gateway, using the older site config
method) was required in some scenarios on the Cody client.
So the new logic, of returning the Provider ID was incorrect. (Because
we report the _model_ provider, e.g. "anthropic". Not the _API_
provider, e.g. "sourcegraph"/"Cody Gateway".)
With this change, we should just have the behavior we had initially:
returning whatever the admin had configured in the
"completions.provider" section of the site config. If only the newer
modelconfig settings were used, we return "sourcegraph" if using Cody
Gatway. And if not, just return the Provider ID. (Though in some
situations even that will lead to incorrect results, because ultimately
we need to update the client here.)
## Test plan
I wasn't able to test this super-well manually, and am relying on
Stephen and Taras' knowledge of how the client uses this today.
## Changelog
NA
When I added this, for some reason I thought it was site admin only. As
it turns out, it shouldn't be gated to site admins only, and it makes it
particularly annoying on dotcom.
Reverts sourcegraph/sourcegraph#64014
We decided that including this in the August 7th release would be rushed
after diving deeper into the project. Reverting this in order to
re-write the functionality, especially the data loading components.
## Test Plan
- this is a revert PR. Everything will work as before.
There is no need to have a separate image file for dark mode. The SVG
file can handle color switching itself.
## Test plan
Switched theme in the user menu and in the browser dev tools.
Nothing outside frontend needs to import this package, to properly signify that we move it to internal where 90% of the other packages reside, and avoid ambiguity on "what service is running what code exactly".
Test plan: Just moved a package, Go compiler doesn't complain.
This PR simplifies and consolidates the remaining parts of the various registry packages, and moves everything under cmd/frontend/internal for better encapusation.
This also removes the need to _ import packages which feels very brittle.
Test plan: Go compiler doesn't complain about moved code.
To make things more explicit and remove the global variable, this is now passed down to where it's needed.
It is a bit messy right now, since it's used deep in the serve-handler but that just highlights better where
it's actually used IMO. As a next step, I want to get rid of the requirement to indicate server-restart
required, so we should be able to drop a bunch of the prop drilling here.
Test plan: Still compiles, E2E test works.
The problem was that we were incorrectly not doing
a "stack push" operation when encountering a comment,
but we were doing a pop which triggered a change in the
source range for the next source range (this is done to
avoid overlap between the current occurrence and the
subsequent occurrence that will be emitted).
This is fixed by not ignoring comment markers. As a consequence, we will
emit a few more occurrences than earlier, but I don't think it should be a
big problem in practice. We can also fuse these if needed.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
Closes
https://linear.app/sourcegraph/issue/CODY-2758/[autocomplete-latency]-add-circuit-breaker-in-cody-gateway-to-handle
This PR introduces an in-memory model availability tracker to ensure we
do not send consequent requests to the currently unavailable LLM
providers.
The tracker maintains a history of error records for each model. It
evaluates these records to determine whether the model is available for
new requests. The evaluation follows these steps:
1. For every request to an upstream provider, the tracker records any
errors that occur. Specifically, it logs timeout errors (when a request
exceeds its deadline) and responses with a 429 status code (Too Many
Requests).
2. These error records are stored in a circular buffer for each model.
This buffer holds a fixed number of records, ensuring efficient memory
usage.
3. The tracker calculates the failure ratio by analyzing the stored
records. It checks the percentage of errors within a specified
evaluation window and compares this against the total number of recent
requests.
4. Based on the calculated failure ratio, the tracker decides whether
the model is available:
- Model Unavailable: If the ratio of failures (timeouts or 429 status
codes) exceeds a predefined threshold (X%), the model is marked as
unavailable. In this state, the system does not send new requests to the
upstream provider.
When a model is unavailable, the system immediately returns an error
status code, typically a 503 Service Unavailable, to the client. This
informs the client that the service is temporarily unavailable due to
upstream issues.
- Model Available: If the failure ratio is within acceptable limits, the
system proceeds with sending the request to the upstream provider.
This PR suggests considering a model unavailable if **95% of the last
100 requests within the past minute** either time out or return a 429
status code. I am not sure about these exact values and suggest them as
a starting point for discussion
## Test plan
- Added unit tests
- CI
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
This only contains one commit which reduces how often we call scan in
indexserver on dotcom.
- acacc5eda1 shards: only trigger rescan on .zoekt files changing
Test Plan: tested in zoekt CI
Searcher doesn't speak to the database nor has it for a long time. See
https://github.com/sourcegraph/sourcegraph/pull/61463
Test Plan: The following command is empty
go run ./dev/depgraph/ summary internal/database | grep 'cmd/searcher'
The `getSourceRange` function can have a range translation failure,
in which case it would be wrong to propagate the range directly
without the commit information. So skip the ranges in that case.
Introduces basic (and incomplete) UI support for perforce, including displaying changelist ids,
listing changelists, and removing references to commits and branches.
The URL path for streaming blame is ambiguous when the file path
contains `/stream/`. The pattern looks like
`/blame/<repo_name>@<revision>/stream/file/path.txt`. However, if the
file contains `/stream/`, the revision is matched greedily up until the
last stream, so we end up with a revision that looks like
`81af3g/stream/my/path`, which is in invalid revision that results in a
404.
This makes the URL pattern unambiguous by adding a `/-/` element after
the revision, which is not allowed in a revision name and acts as a path
separator. So now, the pattern looks like
`/blame/<repo_name>@<revision>/-/stream/file/path.txt`.
Note, this is a public-facing breaking change, but I don't think it
really matters since I don't expect any customers use our streaming
blame API
Automatically generated PR to update package lockfiles for Sourcegraph
base images.
Built from Buildkite run
[#285676](https://buildkite.com/sourcegraph/sourcegraph/builds/285676).
## Test Plan
- CI build verifies image functionality
Co-authored-by: Buildkite <buildkite@sourcegraph.com>
With SRCH-802 we saw error messages when batch changes may still retry
and resolve the problem. To give a better distinction between resolvable
and non-resolvable errors, I'm changing the colour variation from error
to warning if the changeset has not been marked as `FAILED` yet.
Before:
<img width="913" alt="Screenshot 2024-08-02 at 12 44 27"
src="https://github.com/user-attachments/assets/b192c5e9-d23b-460c-82a7-a039edbca3f5">
After:
<img width="1355" alt="Screenshot 2024-08-02 at 12 36 23"
src="https://github.com/user-attachments/assets/02e231a7-168a-4fe9-bd3b-014d810fd236">
## Test plan
Manual testing
## Changelog
- Batch changes that are still retrying now show a warning instead of an
error.
ResolveRevision additionally adds a "^0" to the spec to make sure we
actually check if it exists. This is important, since most uses of the
ChangedFiles API pass in commit shas which do not get resolved by git
unless the "^0" is present.
I noticed this since hybrid search in searcher started to fail if the
commit was missing. In particular Zoekt for empty repositories uses a
fake SHA of 404aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa so when a repo
started getting cloned hybrid search would fail until it was indexed.
Hybrid search explicitly checks for the NotFound error, so hybrid search
failing was a a regression from moving away from the SymbolDiff API.
I noticed we were using a much older version, so code nav
is a bit janky on S2 as the old version had index data from
lsif-go and not scip-go, causing cross-repo nav from
sg/sg to sg/log to not work.
When determining the set of visible uploads at a given commit,
we group uploads from scip-K and lsif-K for ranking purposes
to allow them to shadow each other. Generally, scip-K will end
up shadowing lsif-K, avoiding sending the client a mix of data
from a much older lsif-K index when newer data from a scip-K
index is available.
Fixes https://linear.app/sourcegraph/issue/GRAPH-787
The logic requires some final tie-breaking to avoid non-determinism
in case when there is a scip-K and lsif-K index at the same distance,
which I've done based on the upload_id in descending order
(upload IDs are monotonically increasing). Earlier, some code was
doing tie-breaking based on upload_id in _ascending_ order,
so I've changed that for consistency. This caused some test failures,
so I've updated the corresponding struct literals.
I've left some PR review comments to describe each modification
to the existing test cases.
## Test plan
Added a new dedicated test checking for shadowing behavior.
## Changelog
Fixes a bug where old LSIF uploads would also be used for code
navigation even when newer SCIP uploads were available for the
same language, potentially leading to duplicate results in the reference
panel. With this change, scip-go uploads shadow the uploads
for lsif-go, and similarly for other indexers following the
scip-X/lsif-X naming
convention.
- Update the "getting started" link in the quickstart guide to point to
the correct URL
- Remove the outdated developer help links as they are no longer
relevant
## Test plan
Test the links manually
When the `modelconfig` system was introduced, the code was written in a
way to go out of its way to ensure that the configuration data was
correct. Unfortunately, this doesn't interact well with the reality of
the Sourcegraph instance and how site configuration is set.
- Failing to initialize the `modelconfig` component if the site config
is invalid prevents the `frontend` binary from starting up. Leading to
it crash looping / an outage. Not good.
- The site config can be forced-in, regardless of any validators that
are registered. So it's definitely possible for the model configuration
to be invalid.
- Some of our validation checks were a bit too onerous anyways.
This PR refactors the `modelconfig/init.go` file to try really hard to
not fail on application start up. And instead just do its best to
initialize the `modelconfig` service.
- If the site configuration data is invalid, it will just be ignored.
And therefor default to the Sourcegraph instance using Cody Gateway /
Sourcegraph-supplied models.
- If _rendering_ the LLM model configuration fails (e.g. merging the
static configuration and admin-supplied site config), then we just
initialize the modelconfig service with 0 providers, and models. (But
still let things start up.)
These changes should fix
[PRIME-449](https://linear.app/sourcegraph/issue/PRIME-449), but this
still isn't enough for a good user experience. (Since we are now just
swallowing/ignoring modelconfig errors.)
So my next PR will be to add the site configuration validation logic, so
that we surface any of the configuration errors to site admins.
## Test plan
Tested this manually, and could no longer get my `frontend` job to fail
on startup.
## Changelog
NA
Due to CORS rules, cody web cannot make requests to arbitrary URLs.
Because of this, mentions for URLs do not currently work in Cody Web.
This adds a GraphQL endpoint to the cody context resolvers that resolves
the contents of a mentioned URL.
Closes
https://linear.app/sourcegraph/issue/SRCH-561/add-support-for-file-ranges-in-cody-web
## Test plan
- Check that you can mention files with ranges
- Check that as you open Cody chat on the blob UI page and you have
selected lines in blob UI then chat should have an initial context
mention with line range
- Check that as you change line selection it updates mention file chip
range (only if you don't have any other text)
In order to reduce the cost of calls to auth-gated endpoints, cache
valid admin passwords in-memory. The appliance's frontend calls
auth-gated endpoints in a tight loop, and bcrypt checking is
intentionally an expensive operation.
This could occasionally cause the appliance-frontend to disconnect from
the backend. We observed frontend's nginx reporting an upstream
connection close, and exec'ing into its pod and curling the backend
regularly hung.
When the admin has first installed Code Search Suite, the appliance
waits for the admin to click an "I'm ready" button. This causes the
appliance to unblock a background thread that periodically checks the
health of sg-frontend. When it is healthy, it ensures that the
ingress-facing frontend is pointed to sg-frontend. And when it is not,
it points to the appliance-frontend. Pointing to the appliance-frontend
is its initial state pre-install, and given that we've just installed
sg, the appliance switches the service over quickly.
Meanwhile, clicking this button transitions the frontend to a "refresh"
state (this being one of the states in its state machine). This causes
the UI to reload the web page. The reason we have to do this is that it
is a way to "redirect to yourself". If the ingress-facing service has
been repointed, refreshing like this will show site-admin, which is the
desired behavior. The issue this commit fixes, is that this is racy:
upon refresh, the browser tab queries the appliance (via an nginx proxy
hosted on the same domain serving appliance-frontend) for its state. We
have to store state on the backend (specifically, we use a ConfigMap
annotation), so that the appliance can do the right thing if it has been
rebooted at any time. This will help power future features such as
UI-driven upgrades. The race occurs if, upon refresh, the ingress-facing
service has been flipped over to sg-frontend. The appliance API that
answered the state questions is no longer available!
In general, we can't tell the difference between this expected turn of
events, and a state in which the backend can't be reached. This commit
mitigates the race by setting the appliance UI to refresh if it cannot
reach the appliance API. This looks no different to a "disconnected"
state if things really are broken, but in the expected path, it will
resolve the race by retrying.
This commit reliably causes the appliance-driven installation flow to
redirect to site-admin after clicking "ready", according to my
experimentation in minikube. I suspect that this would be the case even
without https://github.com/sourcegraph/sourcegraph/pull/64213, which
fixes an unrelated performance issue. I suspect we need both, otherwise
the appliance UI will regularly disconnect for prolonged periods of
time, which is confusing.
Closes
https://linear.app/sourcegraph/issue/REL-308/appliance-frontend-seems-to-disconnect-the-backend-during-installation
This change removes the backend smart search logic. After this, searches
with smart search enabled (`sm=1`) will be executed in the default
'precise' mode (`sm=0`). For old searches that use `sm=1` and
`patterntype=standard`, it's possible that they will now return no
results.
Looking at telemetry, only 0.1% of searches on dot com trigger any smart
search rule. So this change should only affect a small percentage of
usage. To mitigate the impact on these rare cases, this PR adds an alert
whenever there are no results and smart search is enabled, suggesting
users switch to keyword search. (This will help in the majority of
cases, since the most frequent smart search rule rewrites literal
queries to use 'AND' between terms).
Closes SPLF-92
This PR brings the most recent version of @sourcegraph/cody-web package
- Improvements in how we fetch providers and debounce logic for their
query
- Fix with switching-chat actions and providers list
## Test plan
- Manual checks over cody web (React and Svelte version)
Part of
[GRAPH-759](https://linear.app/sourcegraph/issue/GRAPH-759/issue-with-apex-extension-not-appearing-for-langapex)
Linguist only supports a subset of the file extensions often used for
the Apex programming languages. This PR adds support for the main set
commonly used.
**Key changes**
1. Adds all extensions for Apex
2. Update our logic to handle multiple extensions for one language
3. Update tests to ensure we only manually map languages if they don't
exist OR have different extensions in go-enry (prevents us from
duplicating entries completely from go-enry)
## Test plan
- [x] Update unit tests
- [x] Validate locally by testing the language filter
Fixes SPLF-119
This adds a background job to Search Jobs that periodically scans for
finished jobs to aggregate the status, upload logs, and clean up the
tables. This drastically reduces the size of the tables and improves the
performance of the Search Jobs GQL API.
For example, with this change, a finished search job on .com only has 1
entry in the database, whereas before it could have several millions if
we searched each repository.
Notes:
- the diff seems larger than it actually is. I left a couple of comments
to help the reviewers.
## Test plan:
- new unit tests
- manual testing:
I ran a couple of search jobs locally (with the janitor job interval set
to 1 min) and checked that
- logs are uploaded to `blobstore-go/buckets/search-jobs`
- repo jobs are deleted from `exhaustive_repo_jobs`
- logs are served from the blobstore after the janitor ran
- downloading logs while the job is running still works
## Changelog
The new background job drastically reduces the size of the
`exhaustive_*` tables and improves performance of the Search Jobs GQL
API.
To ease the transition from older search semantics to keyword search, we
[boost matches on
phrases](https://github.com/sourcegraph/sourcegraph/pull/59940). For
example if you search for `// The vararg opts parameter can include`, we
ensure the match in the method comment is highest, even though there
were no explicit quotes in the query.
We decided to limit this boosting to searches with 3 or more terms. With
2 terms, it's possible there are a ton of phrase matches, which can
drown out high quality 'AND' matches. However, we've seen a few dogfood
examples with fewer terms where boosting would have been really useful.
This PR removes the 3 term restriction on boosting. To mitigate noise,
it updates the phrase matching to only match on word boundaries. It also
switches to matching on the original query string instead of
reconstructing the phrase from terms. That lets us match even when
there's special characters, like `return "disabled"`.
Relates to SPLF-168
## Test plan
Adapted unit tests. Lots of manual testing:
* `invalid ModelID`
* `return "disabled"`
* `func TestClient_do(`
* `// The vararg opts parameter can include functions to configure the`
* `test server` -- this still works quite well, it's a bit fragile but
restricting the matches to word boundaries helped reduce noise
* `bytes buffer`
A large sub-query was duplicated between two queries, making
the code harder to understand. This patch factors that out.
Ancillary changes:
- Added more doc comments
- Tweak tests to make failures easier to follow.
1. Make `sg cloud eph` instance status Reason simple string, as CloudAPI
will take over and return Reason with job URL - no need to parse Reason
anymore.
```sh
go run ./dev/sg cloud eph status --name ff-eph56
⚠️ Running sg with a dev build, following flags have different default value unless explictly set: skip-auto-update, disable-analytics
✅ Ephemeral instance "ff-eph56" status retrieved
Ephemeral instance details:
Name Expires In Instance status Details
ff-eph56 4h37m4.79031s unspecified creation task is already running: https://console.cloud.google.com/workflows/workflow/us-central1/create-instance-c31bd1a4ea84/executions?project=michael-test-03
```
2. Allow re-run create if CloudAPI returns status with Reason - it means
instance is not fully created yet, so user might re-try create -
CloudAPI will ensure more than one create is not running at the same
time.
3. Updated printers with GOTO action for each instance details:
```sh
go run ./dev/sg cloud eph list --all
⚠️ Running sg with a dev build, following flags have different default value unless explictly set: skip-auto-update, disable-analytics
☁️ Fetched 10 instances
Name Expires In Instance status Details
andre-eph-1 30h10m42.989163s unspecified invoke: sg cloud eph status --name andre-eph-1
ff-eph56 4h34m43.989154s unspecified invoke: sg cloud eph status --name ff-eph56
```
## Test plan
Unit tests simplified.
E2e against old and new CloudAPI.
---------
Co-authored-by: William Bezuidenhout <william.bezuidenhout@sourcegraph.com>
Fixes GRAPH-782
Specifically this allows us avoid updating syntactic_indexing_enabled
when we don't have the value available.
This is done to solve the problem where not providing that field crashes
the resolver. I confirmed it by both via tests and via manual testing of
the API
## Test plan
- Added a new test for resolvers specifically to test the extra logic
around this field
Frontend assets are not embedded into the binary anymore instead they're
added to the final container image at `/assets-dist`.
In this PR we check the directory inside the image for the marker
instead of the binary
Closes DINF-176
## Test plan
Tested locally
```
sg release run test --version 1.2.3
👉 [ setup] Finding release manifest in "."
[ setup] No explicit branch name was provided, assuming current branch is the target: main
[ setup] Found manifest for "sourcegraph" (github.com/sourcegraph/sourcegraph)
[ meta] Owners: @sourcegraph/release
[ meta] Repository: github.com/sourcegraph/sourcegraph
👉 [ vars] Variables
[ vars] tag="1.2.3"
[ vars] config="{\"version\":\"v1.2.3\",\"inputs\":\"\",\"type\":\"patch\"}"
[ vars] git.branch="main"
[ vars] is_development="false"
[ vars] version="v1.2.3"
👉 [ reqs] Checking requirements...
[ reqs] ✅ jq
[ reqs] 🔕 Buidkite access token (excluded for test)
[ reqs] 🔕 GitHub Token to submit changelogs (excluded for test)
👉 [ test] Running testing steps for v1.2.3
👉 [ step] Running step "check:frontend and server image contain bundle"
[check:frontend and server image contain bundle] pulling frontend image us.gcr.io/sourcegraph-dev/frontend:insiders
[check:frontend and server image contain bundle] insiders: Pulling from sourcegraph-dev/frontend
[check:frontend and server image contain bundle] Digest: sha256:1256bfb7c64bee0f11a3d0b82af6899f1d3fe22c0d6f3875a877c5f8f8b0e963
[check:frontend and server image contain bundle] Status: Image is up to date for us.gcr.io/sourcegraph-dev/frontend:insiders
[check:frontend and server image contain bundle] us.gcr.io/sourcegraph-dev/frontend:insiders
[check:frontend and server image contain bundle] checking frontend has web-bundle at /assets-dist inside the container
[check:frontend and server image contain bundle] WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
[check:frontend and server image contain bundle] pulling server image us.gcr.io/sourcegraph-dev/server:insiders
[check:frontend and server image contain bundle] insiders: Pulling from sourcegraph-dev/server
[check:frontend and server image contain bundle] Digest: sha256:592c4e94ced4990a3b461eb474d5e7fee9c408d93ba4df44220b22f7d39ea645
[check:frontend and server image contain bundle] Status: Image is up to date for us.gcr.io/sourcegraph-dev/server:insiders
[check:frontend and server image contain bundle] us.gcr.io/sourcegraph-dev/server:insiders
[check:frontend and server image contain bundle] checking server has web-bundle at /assets-dist inside the container
[check:frontend and server image contain bundle] WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
[ step] Step "check:frontend and server image contain bundle" succeeded
```
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
* release: check contiainer directory `/assets-dist` for marker instead
of frontend binary
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Some commands like the
[`batcheshelper-builder`](https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/sg.config.yaml?L821)
aren't long running commands.
This command is used to build and load an image into docker. The `cmd`
section returns an `exit 0`. This behavior combined with
`continueWatchOnExit` results in an infinite loop where the process is
continually restarted because `sg` doesn't know that the process has
finished executing and isn't a long-running process.
https://github.com/user-attachments/assets/e7a027a1-6f93-403f-9240-6a791255fba9
An example of the behavior is shown below as running `sg start batches`
results in the `batcheshelper-builder` command continually restarted.
The fix is quite simple, we return an empty receiver channel when the
process is done executing so that `sg` knows it's done and doesn't
restart the command unless there's a change.
## Test plan
* Manual testing with `go run ./dev/sg start batches` doesn't result in
an infinite loop anymore.
* Add unit tests
## Changelog
During an investigation, we saw that Rockskip was not using scip-ctags
for symbol parsing when applicable. This means that
1. Rockskip is getting less than optimal symbols for certain languages
(like Go)
2. Rockskip is getting no symbols for languages not in universal ctags
(Magik)
This PR attempts to solve this problem but updating Rockskip to re-use
the ctags parser pool logic from symbol service.
### Key Changes
- Update parser pool to be re-usable
- Push common logic for parser type detection into the parser pool
module
- Update rockskip service config to take a parser pool
- Update and add unit/integration tests
## Questions
- What performance impact will using this pooled parser have compared to
its previous behavior of spawning a new ctags process each time?
## Test plan
- [x] Add unit tests
- [x] Update integration tests
- [x] Manually test rockskip
- [x] Manually test symbolservice (in case of regression)
---------
Co-authored-by: Keegan Carruthers-Smith <keegan.csmith@gmail.com>
When exactly 1 auth provider is configured, Sourcegraph redirects users
automatically to the IdP to speed up the sign-in process, so that users
don't have to make an extra click to select the one and only sign-in
provider.
SOAP is a special case though because it is hidden by default, but
enabled on all cloud instances. That caused this auto redirect to never
fire for Cloud, since there are technically two auth providers.
This PR fixes it by checking for the sourcegraph-operator query
parameter which tells the UI to show the magic SOAP auth provider in the
list.
Closes SRC-500
Test plan: Tested on a cloud instance that indeed there is no auto
redirect. Then tested locally with SOAP configured that auto redirects
happen after this PR, and that there is no auto redirect when the
?sourcegraph-operator query parameter is set.
## Changelog
When only a single auth provider is configured, users are again
redirected correctly to the identity provider on Sourcegraph Cloud.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
This PR resolves
[REL-300](https://linear.app/sourcegraph/issue/REL-300/put-update-redirect-into-current-sourcegraph-admin-panel).
We point the Update button in the SiteAdmin sidebar to point to a
different URL (Currently appliance localhost port, needs to be changed)
based on the `APPLIANCE_MANAGED` env var.
Most of the PR is tracking types / config down to the backend. There, a
simple function checks for the existence of this env var and if it
exists returns it's value.
I may have updated extra unnecessary types (not certain) but I was
following the compiler.
Second commit is just updating the storybook type values
We'll need another PR to the Helm chart to activate the env var once we
want to switch people over to pointing to the appliance maintenance UI.
@DaedalusG brought up the good point that even when managed by
Appliance, the Upgrades page still provides valuable information to
administrators, and so we may or may not actually want to leave this the
way it is.
TODO:
- [ ] Change the URL that is pointed to when the env var is active
(listed in a comment)
## Test plan
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
Tested manually
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
- **feat(appliance): change update endpoint based on env var**
- **misc: add type to storybook**
---------
Co-authored-by: Craig Furman <craig.furman@sourcegraph.com>
For Executors on Native Kubernetes deployments, the option to run jobs
in a single pod has been available since Native Kubernetes has been
around.
The purpose of running jobs in a single pod is:
1. Efficiency. Jobs require three steps at least, and without specifying
a single pod, that requires spinning up three pods.
2. Security. For Batch Changes, when jobs are run across several pods,
`git`'s `safe.directory` must be set to avoid untrusted users or
processes injecting code or an attack. Running the job in one pod
removes the need for `safe.directory`.
3. Usability. Because of the need to set `safe.directory`, `root` access
to write to `git`'s global config is required, which means that many
times special configurations and sign-offs from security teams must be
used for Batch Change setups.
This PR takes a step toward using single pod jobs only in enabling them
by default instead of requiring an environment variable to enable them.
The same environment variable that was used to enable them -
`KUBERNETES_SINGLE_JOB_POD` - is still available to disable them by
setting it to `false`.
## Test plan
Bazel and CI for now
## Changelog
Triggering cloud ephemeral builds on main-dry-run leads to unexpected
results which is due to how the eventual pipeline is generated.
Closes DINF-165
### Generated pipeline?
The pipeline gets generated based on _what matches first_. We detect
cloud ephemeral builds if there is an environment variable
`CLOUD_EPHEMERAL=true`. We detect main-dry-runs if the branch prefix is
`main-dry-run`...
Now due to the `main-dry-run` match happening before the cloud ephemeral
match a Cloud Ephemeral build on a main dry run gets detected as _just_
a `main-dry-run` build.
#### Alternatives
Sure we can just move the Cloud Ephemeral match earlier, but it's
difficult to say what else might break. We could also just add the we
force the runtype to always be `CloudEphemeral` if we're on the Cloud
Ephemeral pipeline, but I don't want to make the pipline a special
snowflake and detect the current pipeline just for Cloud Ephemeral.
#### Why deny it the deployment?
Ultimately, `main-dry-run` builds are meant for ... `main` not cloud
ephemeral. People can just switch to their original branch and run `sg
cloud eph deploy` and the branch will be deployed with no issue
## Test plan
Executed locally
```
./sg-test cloud eph build
⚠️ Running sg with a dev build, following flags have different default value unless explictly set: skip-auto-update, disable-analytics
⚠️ Triggering Cloud Ephemeral builds from "main-dry-run" branches are not supported. Try renaming the branch to not have the "main-dry-run" prefix as it complicates the eventual pipeline that gets generated
To rename a branch and launch a cloud ephemeral deployment do:
1. git branch -m "main-dry-run/lol" <my-new-name>
2. git push --set-upstream origin <my-new-name>
3. trigger the build by running sg cloud ephemeral build
FAQ https://www.notion.so/sourcegraph/How-to-deploy-my-branch-on-an-ephemeral-Cloud-instance-dac45846ca2a4e018c802aba37cf6465?pvs=4#20cb92ae27464891a9d03650b4d67cee❌ failed to trigger epehemeral build for branch: main-dry-run branch is not supported
```
## Changelog
* sg: deny deployment of `main-dry-run` branches on Cloud Ephemeral.
* Frontend no longer embeds the assets intead it reads from the local
filesystem assets.
* Generally the frontend and server cmd targets will use the
`//client/web/dist:copy_bundle` target to create a tarball for the
oci_image. `copy_bundle` puts all the assets at `assets-dist`
* For integration tests, frontend and server have the `no_client_bundle`
target variants. For these oci_images, instead of the `tar_bundle` which
is just a tar'd `copy_bundle` we use the `tar_dummy_manifest` which is
just a tar that contains a dummy manifest.
* By default we expect assets to be at `/assets-dist`
* Renamed DevProvider to DirProvider
## Why
By 'breaking' the dependency of frontend requiring assets to be built we
essentially stop a common cache invalidation scenario that happens:
- someone makes a frontend change = assets need to be rebuilt
By decoupling assets from the frontend binary and moving the packing of
assets to the building of the frontend and server images we will have a
better cache hit rate (theoretically).
Thus with this change, when:
* client/web is change and nothing else ... only assets will have to
rebuilt and cached versions of the backend will be used
* if only backend code has changed ... cached assets will be used
Closes DINF-115
## Test plan
✅ sg start - web app opens and can search. Local dev assets get loaded
✅ sg test bazel-integration-test - server image gets built with **only**
dummy web manifest. Also verified by running `sg bazel run
//cmd/server:no_client_bundle.image` and then inspect container
✅ sg test bazel-e2e - server image gets built with bundle and all tests
pass
✅ [main dry
run](https://buildkite.com/sourcegraph/sourcegraph/builds/284042#0190e54c-14d9-419e-95ca-1198dc682048)
## Changelog
- frontend: assets are no longer bundled with binary through `go:embed`.
Instead assets are now added to the frontend container at `assets-dist`.
Typo when copying the flags from somewhere else, names mismatched and
didnt upload 🤦
Also adding in proper BEP emitting, it appears the build_event_log.bin
that was being output was actually for the honeyvent bazel invocation
## Test plan
CI main dry-run
https://buildkite.com/sourcegraph/sourcegraph/builds/285375
## Changelog
For more insights into whats happening during those damn build commands
Also removes emitting to honeycomb, that was my experiment that has
concluded.
## Test plan
CI
## Changelog
The old name 'Bytes' made it sound like it was supposed
to hold a []byte or a similar structure, whereas the type
actually represents a _size_ (in units of bytes).
Automatically generated PR to update package lockfiles for Sourcegraph
base images.
Built from Buildkite run
[#285266](https://buildkite.com/sourcegraph/sourcegraph/builds/285266).
## Test Plan
- CI build verifies image functionality
Co-authored-by: Buildkite <buildkite@sourcegraph.com>
This speeds up precise find usages from about 600ms to 400ms
for a test workload involving 16 precise usages across 5 files.
(Specifically for the `codenav.Service` type)
Fixes GRAPH-781
I have some more thoughts on speeding this up further, but
I'll likely look into that after adding tests.
Follow-up to https://github.com/sourcegraph/devx-support/issues/1130
Moving the doc links to a comment is merely a few lines of GitHub
Script, so it's worth doing it for better merge commit messages.
## Test plan
This PR itself was the test, as you can see below the comments that
resulted from this new GH workflow. Btw, the second one is the one
you'll see in the code here. I went for a oneliner to avoid cluttering
the UI.
This PR exposes per-item relevance score (as assigned by the re-ranker)
in the GraphQL `rankContext` endpoint. Before, we only ordered the
items, but didn't return relevance information.
## Test plan
- tested locally
To implement `select:symbol.enum` filters, we look at each symbol's
ctags 'kind' and check if it matches the filter value `enum`. We
accidentally didn't include 'enum' in this match logic, so all these
symbols were filtered away.
This PR fixes that, and adds a few improvements:
* Use a shared map between `symbol.LSPKind` and `symbol.SelectKind`, to
avoid drift between these two conversions.
* Audit the ctags mapping from
[sourcegraph/zoekt#674](https://github.com/sourcegraph/zoekt/pull/674)
and add other missing kinds (besides enum)
Closes SPLF-178
I think the span will not be terminated if we don't call `endObservation`.
Not the end of the world since this span likely won't be useful,
but we should avoid skipping calls to `endObservation` for consistency.
This patch wires up the newly changed APIs in #64118
to the GraphQL API, enabling precise support in the
usagesForSymbol API. It also handles pagination.
Fixes https://linear.app/sourcegraph/issue/GRAPH-573
## Test plan
Manually tested. Will add automated tests in follow-up PR.
## Changelog
- Adds support for precise code navigation to the experimental
`usagesForSymbol` API, which can be used to implement a
reference panel or similar functionality.
This fixes the current regression of autocomplete in `main`, which
occurs both when (and sometimes when not) using the new
`modelConfiguration` site config option.
In 3cb1c4528d (which lives only in `main`,
not in any official releases - except those images that went to
Self-hosted models EAP customers.) we had a regression where
`CodyLLMConfiguration.provider` would return `"various"`.
This change makes it so that we never return `"various"`, and instead
only return provider values that would be accepted by the autocomplete
provider tests of the client (`create-provider.test.ts`)
This change was based purely on evaluating what the client actually does
with this data, see the code comment (a story / explanation in itself)
for details.
## Test plan
* I am 100% confident the current "various" behavior is absolutely
incorrect, and currently has broken autocomplete in `main` - so this
change (even if it turns out to be slightly incorrect and needs a small bit more work) is a major
improvement over the current state of `main`.
* After merge, I'd like to confirm with a good amount of manual testing
that the various autocomplete providers work as expected.
Note: there is no changelog entry because this regression did not ship yet. It was in `main` prior to this commit, but not in any official releases of Sourcegraph.
## Changelog
N/A
---------
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Co-authored-by: Chris Smith <chrsmith@users.noreply.github.com>
Previously, for providing self-hosted model' configuration (the models
we've tested and believe work well), a site admin would use
configuration like this:
```
"modelConfiguration": {
...
"modelOverridesRecommendedSettings": [
"mistral::v1::mixtral-8x7b-instruct",
"bigcode::v1::starcoder2-7b"
],
}
```
A few problems with this:
1. If you are NOT self-hosting models, you probably really should not be
using this option, as it would set `serverSideConfig` options specific
to self-hosting, but it's naming "recommended settings" which kind of
suggests otherwise!
2. When self-hosting models, there is almost a 1:1 correlation of
`provider` to actual API endpoint (because you have a single endpoint
per model) - so not being able to configure the `mistral` or `bigcode`
parts of the modelref above is problematic (restricts you to hosting
'only one model per provider'). The only escape for this currently is to
abandon the defaults we provide with `modelOverridesRecommendedSettings`
and rewrite it using `modelOverrides` fully yourself.
3. When self-hosting models, needing to configure the
`serverSideConfig.openaicompatible.apiModel` is a really common need -
the most common option probably - but again there's no way to configure
it here, only option is to abandon defaults and rewrite it yourself.
4. If we improve the default values - such as if we learn that a higher
context window size for `mixtral-8x7b-instruct` is better - we currently
don't have a good way to 'release a new version of the defaults' because
the string is a model ref `mistral::v1::mixtral-8x7b-instruct` we'd have
to do this by appending `-v2` to the model name or something. Having
versioning here is important because there are both:
* Breaking changes: if we increase the context window at all, site
admins hosting these models may need to increase limits in their hosted
model deployment - or else the API may just return a hard error ('you
sent me too many tokens')
* Non-breaking changes: if we _decrease_ the context window, Cody
responses will get faster, and it's fine to do. Similarly, adding new
stop sequences may be fine for example.
This PR fixes all of these^ issues by deprecating
`modelOverridesRecommendedSettings` and introducing a new
`selfHostedModels` field which looks like:
```
"modelConfiguration": {
...
"selfHostedModels": [
{
"provider": "mistral",
"model": "mixtral-8x7b-instruct@v1",
"override": {
"serverSideConfig": {
"type": "openaicompatible",
"apiModel": "mixtral-8x7b-instruct-custom!"
}
}
},
{
"provider": "bigcode",
"model": "starcoder2-7b@v1",
"override": {
"serverSideConfig": {
"type": "openaicompatible",
"apiModel": "starcoder2-7b-custom!"
}
}
}
],
}
```
Notably:
* The `provider` part of the model ref is now configurable, enabling
self-hosting more than one model per provider while still benefitting
from our default model configurations.
* `"model": "starcoder2-7b@v1",` is no longer a model ref, but rather a
'default model configuration name' - and has a version associated with
it.
* `override` allows overriding properties of the default `"model":
"starcoder2-7b@v1",` configuration, like the
`serverSideConfig.apiModel`.
## Importance
I'm hoping to ship this to a few customers asap;
* Unblocks customer https://linear.app/sourcegraph/issue/PRIME-447
* Fixes https://linear.app/sourcegraph/issue/PRIME-454 (you can see some
alternatives I considered here before settling on this approach.)
## Test plan
Manually tested for now. Regression tests will come in the near future
and are being tracked on Linear.
## Changelog
Improved configuration functionality for Cody Enterprise with
Self-hosted models.
---------
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
Just another package that is now only used by frontend, so moving it
around to make clearer which service needs this package.
Test plan: Go compiler doesn't complain about moved directory.
This package is no longer required now that everything that depends on
session is inside cmd/frontend. This removes one layer of indirection in
the code nav flow.
Test plan: Just removed aliases and pointed to the original code, and
the Go compiler doesn't complain.
There's no need to track they haven't been called twice, this is a
programming error and should not happen. We don't need to maintain a
global int for that, IMO which just makes it harder to call in tests
etc.
Test plan: Local stack still works, code review.
This PR removes the requirement that a global variable is initialized in
the frontend main function. Instead, we create a data exporter where
it's used.
Test plan: CI passes, code review.
Only frontend uses this package, so moving this package here to make
clearer what service depends on this code.
This package also requires a call to the Init() func which only frontend
does right now, so this also was a footgun where someone might start
using this package outside of frontend, but then it's not correctly
initialized.
Test plan: Just moved a package, go compiler doesn't complain.
Only frontend uses this package, so moving this package here to make
clearer what service depends on this code.
Test plan: Just moved a package, go compiler doesn't complain.
At some point, we decided to serve a 500 error instead of a 302 FOUND
which redirects to the sign in page when redis is down. This usually
happens during a rollout because redis is usually also rolled out at the
same time.
When that happens, users would previously get redirected to the signin
page, and their session is cleared.
So we decided to instead send a 500 status code, and let them explicitly
retry.
However, that didn't stop the execution chain before, so we would send
the 500 status code, but then eventually get to the app handler that
calls http.Redirect with 302 FOUND (which as a side-effect renders a
`Found.` link as an HTML page for old browsers that don't support 302
natively). Since the http status header has already been sent, it
doesn't change the status code to 302 and instead returns a 500 response
with the HTML `Found.` link.
This is the strange error page we've been seeing during rollouts a lot
of times and never knew where it's from.
Test plan: Locally stopped redis while the instance was running and no
longer saw the `Found.` message and instead now get a proper error
message.
This current code sets a global variable which couples the session
package and the app package and the session package cannot work when the
NewHandler method wasn't called yet.
This also used to wait for redis implicitly, without any indication of
doing that.
To be more explicit about that, we now do that in the main function of
frontend instead, and create the session store where required on the
fly.
Test plan: Login still works, integration/E2E tests are passing.
Just a little cleanup which makes the main function a bit shorter and
easier to understand that userpasswd is not different to other authn
providers.
Test plan: Authn with builtin still works in CI integration tests.
This removes the need to register a global URL watcher. Instead, the
conf package now returns a cached version, and also makes a copy of it
so it's impossible to accidentally modify it.
This makes it safe in non-cmd/frontend packages to use this.
Test plan: All tests are still passing.
This doesn't blow up, but golangci-lint complains about it so following
best practices here.
Test plan: Go test still passes, linter doesn't complain anymore.
Since we don't do the enterprise/oss split anymore, this global package
is no longer required and we can move the code to where it's actually
used.
Test plan: Go compiler doesn't complain, and integration tests are still
passing.
This code was using a strange pattern that isn't actually observed or
controlled by our worker mechanisms, so switching it to return proper
goroutines.
Test plan: CI passes, would like a thorough review on the licensecheck
code.
They should not be used outside of cmd/frontend, so making it a frontend
internal package.
While doing that, I realized that there is a coupling dependency between
authz providers and auth (which is authN) providers: GitLab code host
connections can do authz mapping via the usernames of another OIDC or
SAML auth provider
(https://sourcegraph.com/docs/admin/code_hosts/gitlab#administrator-sudo-level-access-token).
It turns out this feature does not work anymore, since at least several
releases, because we don't actually instantiate auth providers outside
of `cmd/frontend` and thus the mapping will never find anything (auth
providers don't explode when queried before init, unlike authz).
This only now became clear as I moved this code, and the dependency
graph was broken, so that's a nice property of these cleanups I guess 😬
Since it doesn't seem to work for quite some time, I opted for removing
it, and added a changelog entry about it. Not sure if that is
sufficient, I raised a thread here:
https://sourcegraph.slack.com/archives/C03K05FCRFH/p1721848436473209.
This would've prevented this change and needed more refactoring as
unfortunately we cannot map an auth provider by the conf type to a
record in the `user_external_accounts` table and need to actually
instantiate it.
Test plan: Compiler doesn't complain, tests still pass.
## Changelog
GitLab code host connections were [able to sync permissions by mapping
Sourcegraph users to GitLab users via the username property of an
external OIDC or SAML
provider](https://sourcegraph.com/docs/admin/code_hosts/gitlab#administrator-sudo-level-access-token)
that is shared across Sourcegraph and GitLab. This integration stopped
working a long time ago, and it has been removed in this release.
Previously, we would store authz providers globally and refresh them
every now and then.
However, creating the providers is fairly cheap (1.3ms in a local trace)
so we should not keep them in memory and remember to not forget to start
the watcher routine.
This will help for multi-tenant Sourcegraph in that providers are now
computed for the context in question, and not held globally. Keeping
potentially 100k authz providers in memory will not scale.
Test plan: Still works, local Jaeger traces are quite acceptable.
This is a register call that is easy to forget. When forgotten, all queries against the repo store will block forever.
In addition, this adds a hard-dependency on conf to every services startup, plus a busy loop. With multi-tenant, this will not work great because authz providers would be a global, and we instead want most things to be ephemeral so they're per-provider. This is a step toward that, but doesn't yet remove the providers global variable.
Good news, it turns out that we don't actually need to register the providers in every service! The reason they were required was to check if zero providers are configured, or if authzbypass mode is enabled.
Authz bypass mode is usually ON, except when there are problems with the authz providers, meaning some authz providers might not be able to sync permissions. Bypassing of permissions is only ever happening if there are ALSO zero providers configured.
So this is basically an optimization for the case where an instance has zero authz configured so that the SQL queries are a bit simpler. This also helps in tests because with bypass mode on and no providers configured, authz enforcement is effectively off in the repo store.
This makes it so that in tests we need to do slightly more work, but also makes for a more realistic test vs at runtime setup. Also, it's highly recommended to use mocks for DB wherever possible in more high-level components to keep tests fast.
To never have a scenario where we accidentally mess up here and enable bypass mode erroneously, this PR drops that entirely. Authz is always enforced, but when a code host connection is unrestricted (i.e., will not spawn a provider) the repos are still visible, so this should be no change over before.
## Test plan
The stack starts and works, and all CI tests are still passing. Code review should help as well.
This PR greatly relaxes the naming restrictions for resource IDs when
using the new `modelconfig` LLM model configuration system.
The original intent was to set a high bar for how we would allow users
to refer to LLM providers, API versions, or models. So that we could
have use those names in URLs and/or display them if no human-friendly
name was supplied. e.g. `opanai::v1::gpt-3.5-turbo`.
However, in practice, this was just a bit too optimistic. We already had
carved out an exception for "API versions" to be less strict, since
there were some cases where we saw those included slashes and things.
And one of our trusted testers immediately ran into problems because the
model name had an `@` in it. (See the linked issue.)
In order to avoid perpetually trying to work around LLM providers and
whatever IDs they use for things, it seemed preferable to just relax our
rules.
With this PR, we will constraint Provider, API Versions, and Model IDs
to only be URL-safe(*). So previously characters like `[]@/+;!` were
prohibited, they are now allowed.) However, we still restrict names to
only include `a-zA-Z` and no unicode characters, and curly braces are
not allowed. (But `[]` and `()` are.
> Pedantically, we do not want to say they are _exactly_ URL safe.
Because we don't perform any sort of escaping (e.g. "%20" -> " ", etc.)
Nor do we allow the `:` which _is_ allowed by the RFC, since we are
using that to delimit parts of the model ref.
Anyways, this should provide enough flexibility to avoid most (hopefully
all) problems users would encounter. While still enforcing some type of
standards with regard to how models are defined.
Fixes [PRIME-451 : invalid ModelID (claude-3-5-sonnet@20240620) after
upgrading to
5.5.2463](https://linear.app/sourcegraph/issue/PRIME-451/invalid-modelid-claude-3-5-sonnet20240620-after-upgrading-to-552463).)
## Test plan
Added tests.
## Changelog
NA.
https://linear.app/sourcegraph/issue/DINF-111/rework-how-we-inject-version-in-our-artifacts
Pros:
- saves having to rebuild `bazel query 'kind("go_library", rdeps(//...,
//internal/version))' | wc -l` == 523 Go packages when stamp variables
cause a rebuild
- Cutting out GoLink action time when stamp changes but code is cached
Cons:
- Binaries themselves are no longer stamped, only knowing their version
info within the context of the docker image
- A tad extra complexity in internal/version/version.go to handle this
new divergence
---
Before:
```
$ bazel aquery --output=summary --include_commandline=false --include_artifacts=false --include_aspects=false --stamp 'inputs(".*volatile-status\.txt", //...)'
Action: 1
Genrule: 2
Rustc: 3
ConvertStatusToJson: 88
GoLink: 383
```
After:
```
$ bazel aquery --output=summary --include_commandline=false --include_artifacts=false --include_aspects=false --stamp 'inputs(".*volatile-status\.txt", //...)'
Mnemonics:
Genrule: 2
Action: 3
Rustc: 3
ConvertStatusToJson: 86
```
## Test plan
Lots of building & rebuilding with stamp flags, comparing execution logs
& times
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
There was some confusion about how a starcoder model would "come out" of
the modelconfig system. Added a unit test and some clarifying comments
to hopefully help out.
## Test plan
Just add tests for the sake of clarifying the code.
## Changelog
NA
closes CLO-527
[context](https://sourcegraph.slack.com/archives/CHXHX7XAS/p1721835847010889)
cloud uses a lot of the GraphQL API to pre-configure instances on
customer behave, and it's very sensitive to breaking changes to the
schema upstream.
currently, we have a [cronjob github
actions](https://github.com/sourcegraph/controller/actions/workflows/srcgql-compat.yaml)
that periodically check the schema compatibility every day.
such approach works but we're always playing catch up and will have to
result in extra work on the product team to re-work the PR. it is much
better to catch this in CI time within the monorepo.
This PR added a new github actions to the monorepo that will run on any
`*.graphql` changes. Then it will remotely trigger the cronjob github
action in sourcegraph/controller to run and poll the result. See test
plan for demo.
## FAQ
### Why not run the test in buildkite or directly in this repo using
github actions?
- sourcegraph/controller is a huge repo forhistorical reason (> 2G) and
cloning it is very expensive
- the repo contains sensitive information, and we don't want to make it
possible to expose it accidentally.
### How does authentication work?
We use a [GitHub
App](https://github.com/organizations/sourcegraph/settings/apps/cloud-srcgql-compat-test-invoker)
with extremely limited permissions. It only permits the workflow to
trigger/read the workflow without any access to the source code itself.
Also, GitHub App installation access token has a life span of 1h, much
better than PAT.
## Test plan
it triggered the job and it worked:
good: https://github.com/sourcegraph/sourcegraph/pull/64094
bad: https://github.com/sourcegraph/sourcegraph/pull/64095
The same changes that we did in
https://github.com/sourcegraph/sourcegraph/pull/64149
but for the Svelte version of the app.
- Update cody web to the `@sourcegraph/cody-web:0.3.0`
- Fix the telemetry name
- Rendering improvements in cody chat UI
- Change status from experimental to beta
## Test plan
- Manual checks over cody web in the svelte version
Closes https://linear.app/sourcegraph/issue/SRC-454/extract-and-propagate-user-ip-address-throughout-the-request-lifecycle
According to [HTTP1.1/RFC 2616](https://www.rfc-editor.org/rfc/rfc2616): Headers may be repeated, and any comma-separated list-headers (like `X-Forwarded-For`) should be treated as a single value.
In section 4.2:
> Multiple message-header fields with the same field-name MAY bepresent in a message if and only if the entire field-value for that header field is defined as a comma-separated list [i.e., #(values)]. **It MUST be possible to combine the multiple header fields into one"field-name: field-value" pair, without changing the semantics of the message, by appending each subsequent field-value to the first, each separated by a comma.** The order in which header fields with the same field-name are received **is therefore significant** to the interpretation of the combined field value, and thus a proxy MUST NOT change the order of these field values when a message is forwarded.
For Example:
For the following HTTP request, it's valid to have multiple instances of x-forwarded-for:
| Header Name | Header Value |
|------------------|---------------------------|
| X-Forwarded-For | 203.0.113.195, 70.41.3.18 |
| X-Forwarded-For | 150.172.238.178 |
| X-Forwarded-For | 123.45.67.89 |
| ... | ...|
That must be interpret-able as `X-Forwarded-For: 203.0.113.195, 70.41.3.18, 150.172.238.178, 123.45.67.89`
Previously, our code used http.Header.Get():
9e26623d90/internal/requestclient/http.go (L81-L95)
However, [func (Header) Get](https://pkg.go.dev/net/http#Header.Get) only returns the first value of the header:
> Get gets the first value associated with the given key. If there are no values associated with the key, Get returns "". ...
In our example, this means that our code would only get `X-Forwarded-For: 203.0.113.195, 70.41.3.18`, which is invalid according to RFC 2616.
## Test plan
There were no unit tests, so I added some.
## Changelog
<!-- OPTIONAL; info at https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c -->
A few things this PR does
- Update cody web to the most updated version and package name which is
(`@sourcegraph/cody-web`)
- Support custom telemetry name for cody web events (on dotcom it should
be `dotcom.web` and `server.web` on the enterprise instance
- The most updated release of Cody Web includes
- fixes for remote LLM models on enterprise
- improvements for performance rendering Chat UI
- telemetry fixes
## Test plan
- Manual checks over Cody Web chat UI (standalone chat page and the blob
UI side panel chat)
Closes https://linear.app/sourcegraph/issue/SRC-453/modify-the-perforce-authorization-provider-to-support-ip-aware-sub
This PR builds on https://github.com/sourcegraph/sourcegraph/pull/64010, and enhances the re-insertion logic.
Before, we'd fail the sync operation outright if we tried to re-insert existing permissions that hadn't been converted from the path only form to the (ip, path) tuple.
Now, when trying to reinsert the existing permissions:
- if it has already been converted (`(path, ip)`) -> we save it back in the database using the UpsertWithUP() method
- if it hasn't been converted (`path` only) -> we save it back in the databse using the existing old plain Upsert method
I accomplish this by using a small interface that encpasulates the data and the insertion logic to use:
```go
type subRepoPermissionsUpserter interface {
// UpsertWithSpec inserts or updates the sub-repository permissions with the data
// stored in the upserter.
UpsertWithSpec(ctx context.Context, store database.SubRepoPermsStore, userID int32, spec api.ExternalRepoSpec) error
}
type ipBasedPermissions struct {
perms *authz.SubRepoPermissionsWithIPs
}
func (u *ipBasedPermissions) UpsertWithSpec(ctx context.Context, store database.SubRepoPermsStore, userID int32, spec api.ExternalRepoSpec) error {
return store.UpsertWithSpecWithIPs(ctx, userID, spec, *u.perms)
}
type pathBasedPermissions struct {
perms *authz.SubRepoPermissions
}
func (u *pathBasedPermissions) UpsertWithSpec(ctx context.Context, store database.SubRepoPermsStore, userID int32, spec api.ExternalRepoSpec) error {
return store.UpsertWithSpec(ctx, userID, spec, *u.perms)
}
var (
_ subRepoPermissionsUpserter = &ipBasedPermissions{}
_ subRepoPermissionsUpserter = &pathBasedPermissions{}
)
```
Now, any code that deals with inserting sub repo permissions now can deal with a `[]subRepoPermissionsUpserter` without having to care about the exact semantics to use. Our re-insertion logic is also now more robust.
## Test plan
New unit tests
## Changelog
<!-- OPTIONAL; info at https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c -->
Adds scip-ctags support for the Hack language.
Noteworthy items
1. I did not add support for modules since they are [not
supported](https://github.com/slackhq/tree-sitter-hack/issues/70) in the
tree-sitter grammar right now.
## Screenshots
## Test plan
- [x] Update unit tests
- [x] Manually validate symbol side bar for indexed commits
- [x] Manually validate symbol side bar for unindexed commits
- [x] Validate symbol search for indexed commits
- [x] Validate symbol search for unindexed commits
The perforce permission syncer has been adapted to now read and save the HOST field from the perforce protections table, which contains the IP address(es) that the path rule that it applies to. It uses the updated sub_repository_rules store methods introduced in https://github.com/sourcegraph/sourcegraph/pull/63811/.
### Notes
- There is some existing logic in the permissions syncer that attempts to re-insert the existing sub_repo_permissions if we encounter a temporary (timeout, etc.) error when syncing. However, there is an edge case: what do we do if the existing permissions don't have an IP address associated with them yet (they were inserted before the updated permission syncer ran)? For simplicity, in this PR I leaned towawrds correctness - I fail the operation outright (I'd rather temporarirly lock someone out rather than accidentally leak information). I implemented a more robust straetgy for this in https://github.com/sourcegraph/sourcegraph/pull/64086.
## Test plan
- The existing unit tests have been adapted to use the new authz.SubRepoPermissionsWithIP structs (I use wildcard IP addresses).
- The big new test to pay attention to is TestScanIPPermissions (and the associated `sample-protects-ip.txt` file).
## Changelog
- The perforce permissions syncer has been updated to save the IP address associated with each sub_repository_permissions rule.
This PR brings back https://github.com/sourcegraph/sgtail back in `sg`,
plus a few adjustments to make it easier to use. I'll archive that repo
once this PR lands.
@camdencheek mentioned you here as you've been the most recent beta
tester, it's more an FYI than a request for a review, though it's
welcome if you want to spend a bit of time reading this.
Closes DINF-155
## Test plan
Locally tested + new unit test + CI
## Changelog
- Adds a new `sg tail` command that provides a better UI to tail and
filter log messages from `sg start --tail`.
1. Use interned strings for scope kinds
This lets us avoid a few String allocations and requires dealing with
fewer lifetimes/borrows
2. Unify tree walks to use `Ancestors`
This means we now consistently apply the fuel limit (we were missing a
few loops before)
3. Use binary search over linear search to find preceding defs
Silly me
4. Removes hacks for skipping entries in the scope tree
We now make sure to never insert references that ought to be skipped in
the first place
I'd recommend reviewing commit by commit, the changes aren't really
related. Just didn't want to spam PRs for this.
## Test plan
Snapshots continue to pass. Verified fuel limit is enforced by creating
loopy trees on purpose. Ran some hyperfines on spring-framework to make
sure perf didn't regress.
Support detecting `search` and `edit` intents - return additional scores
for those two categories.
## Test plan
- tested locally -> use `{
chatIntent(query: "yo", interactionId: "123") {
intent
score
searchScore
editScore
}
}
` as GraphQL payload
In the first UI, dev mode was a webform field. On reflection, it's
probably simpler as an env var, since it is not expected that production
users will enable this.
I'm not actually sure if I can even guarantee the _at least_ part for
syntactic usages right now. It would require particularily pathological
circumstances, but because I have to stay within the given
`context.Context` window, I might end up returning fewer syntactic
results than requested.
I think that should be fixable in the future so I'd still like the spec
to reflect the state we'd like to end up at.
## Test plan
Just a documentation/specification change
For the implementation of precise usagesForSymbol, I need to be
able to access some of these types in the codenav package directly, so move
a bunch of types there to avoid an import cycle: codenav -> resolvers -> codenav.
As the next release of Sourcegraph approaches, and we get our ducks in a
row for rolling out the modelconfig changes, we have cut the ability for
the Sourcegraph backend to poll Cody Gateway for new LLM models.
Instead, if configured to, the only "Sourcegraph-supplied models" will
be what is embedded into the binary at build-time. (See
`internal/modelconfig/embedded`.)
This PR removes any externally facing references to this capability.
Since it wasn't actually implemented yet, this isn't actually going to
change any functionality.
We'll add this capability in the next release, ~September. See
[PRIME-290](https://linear.app/sourcegraph/issue/PRIME-290/feature-sourcegraph-instances-can-automatically-pick-up-new-llms).
## Test plan
NA
## Changelog
NA
Propagates a for-reference-only record of the first `User-Agent` seen
when a request gets into Sourcegraph across services and contexts. This
allows telemetry to try and indicate where a request originates from
(https://github.com/sourcegraph/sourcegraph/pull/64112), rather than
only having the most recent user-agent.
A new header and `requestclient.Client` property
`X-Forwarded-For-User-Agent` and `ForwardedForUserAgent` is used to
explicitly forward this. Strictly speaking I think we're supposed to
just forward `User-Agent` but it looks like in multiple places we
add/clobber the `User-Agent` ourselves.
The gRPC propagator currently sets user-agent on outgoing requests, this
change also makes that consistent with the HTTP transport, such that
both only explicitly propagate `X-Forwarded-For-User-Agent`
## Test plan
Unit tests
Closes
https://linear.app/sourcegraph/issue/GRAPH-774/see-if-we-can-get-away-with-2-visibilities-in-scip-syntax
## Test plan
Snapshots don't change
A couple hyperfine runs suggest this is also slightly quicker
```
spring-framework on HEAD (9409543) [?]
❯ hyperfine --warmup 3 'git archive HEAD | scip-syntax index tar - -l java' 'git archive HEAD | scip-syntax-main index tar - -l java'
Benchmark 1: git archive HEAD | scip-syntax index tar - -l java
Time (mean ± σ): 909.9 ms ± 9.7 ms [User: 7307.6 ms, System: 212.2 ms]
Range (min … max): 902.4 ms … 935.2 ms 10 runs
Benchmark 2: git archive HEAD | scip-syntax-main index tar - -l java
Time (mean ± σ): 939.6 ms ± 6.5 ms [User: 7681.2 ms, System: 210.9 ms]
Range (min … max): 931.2 ms … 952.1 ms 10 runs
Summary
git archive HEAD | scip-syntax index tar - -l java ran
1.03 ± 0.01 times faster than git archive HEAD | scip-syntax-main index tar - -l java
spring-framework on HEAD (9409543) [?]
❯ hyperfine --warmup 3 'git archive HEAD | scip-syntax index tar - -l java' 'git archive HEAD | scip-syntax-main index tar - -l java'
Benchmark 1: git archive HEAD | scip-syntax index tar - -l java
Time (mean ± σ): 916.4 ms ± 10.7 ms [User: 7302.7 ms, System: 217.8 ms]
Range (min … max): 905.4 ms … 939.9 ms 10 runs
Benchmark 2: git archive HEAD | scip-syntax-main index tar - -l java
Time (mean ± σ): 945.4 ms ± 7.9 ms [User: 7684.8 ms, System: 212.6 ms]
Range (min … max): 935.0 ms … 959.3 ms 10 runs
Summary
git archive HEAD | scip-syntax index tar - -l java ran
1.03 ± 0.01 times faster than git archive HEAD | scip-syntax-main index tar - -l java
```
Fixes SPLF-170
This fixes a bug in our status reporting where we would report a job as
"queued" although it should be reported as "processing". In some cases
this led to the state flip-flopping between "queued" and "processing".
This happens, for example, if a worker has just finished a repo-rev job
(and set it to completed) but hasn't yet dequeued a new job. In this
brief period we reported the entire search job as queued.
Test plan:
- updated unit test
- manual testing
I ran the following search job which previously exhibited this behavior
and verified that now it doesn't.
```
context:global r:^github\.com/sourcegraph/sourcegraph$@*refs/heads/* ghp_.+ patterntype:regexp
```
When there is already a global token, then the notice after installing a
personal app is wrong. This PR handles this issue, so that the notice
shows up as expected.
We also fix an alignment issue, where the call to refresh the page would
be in a distinct column, instead of what we'd expect.
## Test plan
Manual testing
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
This patch changes the code so that the core APIs we need
for precise usagesForSymbol actually return (Upload)Usages instead
of just (Upload)Locations, so that we have the usage kind information
as well as the symbol names available. I will be handling the GraphQL
layer for converting the Usages to appropriate Resolvers in a follow-up PR.
## Test plan
Modified a bunch of existing mock tests. 🤷🏽
Add basic health check to switch state to `waitForAdmin` after
Sourcegraph frontend is ready. As noted in the code, this is a temporary
health check and will/should be replaced with something more
comprehensive in the near future.
Wait for admin page successfully appears when Sourcegraph frontend is
"ready":
Co-authored-by: Jacob Pleiness <jdpleiness@users.noreply.github.com>
Co-authored-by: Craig Furman <craig.furman@sourcegraph.com>
This only contains one commit which has a performance improvement
experiment hidden behind an environment variable.
- https://github.com/sourcegraph/zoekt/commit/12ce07a298 index:
experiment to limit ngram lookups for large snippets
Test Plan: CI
If a build is triggered from the web the variable BUILDKITE_BUILD_AUTHOR
is not set which the msp_deploy.sh script requires. This PR uses
BUILDKITE_BUILD_CREATOR as a fallback if _AUTHOR is missing
## Test plan
Tested locally
Closes SRCH-741
Closes SRCH-716
This PR removes the GitHub installation code from the redirect flow to a
webhook-based appraoch. We expect that the GitHub server calls the
webhook when the installation is ready, and therefore shouldn't see the
errors explained in the issues above.
To handle the potential delay until the webhook is called and the
credential is set up, I added a scrappy info notice that the user should
refresh their page:
<img width="928" alt="Screenshot 2024-07-24 at 13 48 24"
src="https://github.com/user-attachments/assets/4d298f2a-d7b8-423b-9e2f-2ae53fbce1ac">
Below is what you see after you refreshed (or if the webhook was called
faster than the user being redirected back to the settings):
<img width="929" alt="Screenshot 2024-07-24 at 13 50 14"
src="https://github.com/user-attachments/assets/b6826158-8561-476d-b20e-e36f8cfb86fd">
I'm able to create PRs for sourcegraph-testing with an app that was
created this way.
<img width="1171" alt="Screenshot 2024-07-24 at 16 16 06"
src="https://github.com/user-attachments/assets/86e20acb-136f-4a46-a33b-bdfdd0d51d71">
I'm seeing an error when getting an access token with a personal github
app to run a batch change, but that will be handled with another PR.
<img width="1053" alt="Screenshot 2024-07-24 at 16 38 38"
src="https://github.com/user-attachments/assets/5655ba91-1ae4-453a-8d5c-1bcdbe34bc17">
## Test plan
Manual testing locally, and more testing to be done on S2 where we have
a more production like environment
## Changelog
- When installing a GitHub app for batch changes, the instance now waits
for a callback from GitHub to complete the installation to avoid issues
from eventual consistency.
---------
Co-authored-by: Peter Guy <peter.guy@sourcegraph.com>
At the heart of the loop for extracting usages across a Sourcegraph
instance is the `extractLocationsFromPosition` function, which
extracts related symbols and source ranges from a single SCIP
Document. (Source ranges for returning to the user directly,
and related symbols to do further lookups, e.g. in the case
of inheritance.)
Since we want to perform matching based on symbol names in the upcoming
precise usagesForSymbol API, and also return symbol names for each
associated source range, this function needs to be updated to:
1. Be able to take a symbol name for doing lookups. This is done using
the new `FindUsagesKey` type which allows two cases - position-based and
symbol-based.
2. Be able to return symbol names associated with every source range.
This is done by creating a new `UsageBuilder` type which somewhat subsumes
the `Location` type. We avoid copying the same 'UploadID' and 'Path'
fields eagerly for clarity; that will be handled by callers in the future when
they mix `UsageBuilder` values across different Documents (by first calling `build`).
For the above, I've introduced a new func `extractRelatedUsagesAndSymbolNames`,
and `extractLocationsFromPosition` delegates to that. In the future,
`extractLocationsFromPosition` will be removed.
Relates to #64041
This refactors the `adminanalytics` package to set the cache explicitly
instead of implicitly relying on the global `redispool.Store`. The
global store obfuscated the dependency and also made testing a bit
awkward.
Test plan:
- new unit test
- I ran a local instance and checked for panics in the logs from the
worker job that updates the cache on startup.
- Checked that the following GQL query returned results
```GQL
query {
site {
analytics {
search(dateRange: LAST_MONTH, grouping: WEEKLY) {
searches {
nodes {
date
count
}
summary {
totalCount
totalUniqueUsers
totalRegisteredUsers
}
}
}
}
}
}
```
- I deleted the cache and ran the GQL query again and verified that
cache had the following new entries
```
1) "adminanalytics:Search:Searches:LAST_MONTH:WEEKLY:nodes"
2) "adminanalytics:Search:Searches:LAST_MONTH:WEEKLY:summary"
```
Adds a read-only "known baddies" list of feature/action combination
where we know the event will produce marshalling or validation errors,
and skips the error-logging step.
Note that the code fails the entire batch the way it works today, but in
practice, none of our clients batch events yet, so this is something to
fix when we start doing that.
## Test plan
```
sg start enterprise
```
```gql
mutation {
telemetry {
recordEvents(
events: [{feature: "cody.completion", action: "persistence:present", source: {client: "VSCode.Cody", clientVersion: "0.14.1"}, parameters: {version: 0, privateMetadata: 12}}]
) {
alwaysNil
}
}
}
```
get an error response, but does not record a log in `sg start` output
Changes in client-side code should not result in expensive rebuilds of
`frontend` locally when using bazel with `sg start`. For some reason,
iBazel still thinks it needs to be rebuilt (possibly it uses `query`
instead of `cquery` and therefore doesnt resolve `selects`), but bazel
underneath determines nothing needs to be rebuilt so theres very little
cost
## Test plan
Command set with just `frontend` in a `bazelCommands` section and then
`sg start <command set>`
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Creates the Wolfi image for the Appliance Maintenance UI
## Test plan
```
bazel test \
//internal/appliance/frontend/maintenance:image_test \
//docker-images/appliance-frontend:image_test
```
**Appliance points ingress-facing service to itself by default**
Not frontend.
**feat(appliance): healthchecker manages ingress-facing service**
Add a new background goroutine to the appliance. It does nothing until a
"begin" channel closes. The idea is that another part of the appliance
will close this channel if the configmap state is set to a post-install
value (or on startup if this is already the case when an appliance
boots).
After this barrier is lifted, the healtchecker periodically checks the
readiness (using k8s conditions) of each pod returned by the frontend
deployment's label selector. If even a single pod is ready, it ensures
that the service points to frontend. Otherwise, it waits for a grace
period, checks again, and if downtime persists, it points the service to
the appliance.
This should cover the following cases:
- The service is pointed to frontend after the admin clicks "go" after
an initial successful install.
- The service is pointed to appliance after frontend downtime that
exceeds the grace period.
- The service is promptly pointed to frontend after downtime ends.
Sorry missed this during the review. No need to allow `rEfErEnCe` as the
capture name. Noticed this while checking the samply profile and seeing
string allocations (gets us back 20ms on spring-framework 🎉)
## Test plan
Covered by existing tests
For precise usagesForSymbols, we want to propagate usages everywhere
(with associated symbol names, not just 'Location' values). This PR
introduces the new Usage type, and unifies the old GetBulkSymbolUsages and
GetMinimalBulkSymbolUsages APIs into a single GetSymbolUsages API.
We convert the Usage values to Location to avoid changing a lot of code
at once.
We also change the DB query to do grouping and aggregation for us
instead of doing it in Go code.
---------
Co-authored-by: Christoph Hegemann <christoph.hegemann@sourcegraph.com>
Fixes GRAPH-649
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
This PR adds non-local references support to Java (and more generally to
our locals handling code).
The main idea is to cherrypick nodes that _can_ contain global or
resolved (see below) references, handle them first, and then mark
everything else as locals.
- Pure Global references are formed from types of definitions that
_can't_ be locals in our code. Currently it's only methods in Java that
we treat as always global references
- Pure local references - these references should only be resolved
against the locals scope tree and definitions within it - never emitting
a non-local references
- Resolved references - these are first resolved as locals, and if that
doesn't succeed, a global reference is emitted. These will be used most
frequently, as TS grammars don't carry enough information for us to
identify more pure global references. For example, in Java's type bounds
`class Hello<N extends Number>`, Number can refer to both the local type
parameter of an enclosing class (in which case we should emit a local
reference), or it can refer to `java.lang.Number` (in which case we
should emit a global reference)
## Test plan
- New snapshot results
- New evaluation results
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
---------
Co-authored-by: Christoph Hegemann <christoph.hegemann@sourcegraph.com>
This moves the `refreshAnalyticsCache` job from `frontend` to `worker`
Notes:
the `adminanalytics` package uses a global cache which made testing
awkward. That's why I introduced the `store()` abstraction to swap the
store at test time.
## Test plan
- new unit test
Closes
https://linear.app/sourcegraph/issue/GRAPH-765/use-surroundingcontent-returned-from-the-search-client
The primary reason for doing this is to avoid sending a bunch of extra
calls for file contents to gitserver, which dominate the profile/trace
after implementing the remaining opts from
https://github.com/sourcegraph/sourcegraph/pull/63971
Also limits the API to only return the line containing the match for now
as discussed with Camden.
We might need to bring back the `lineGetter` for precise usages, but it
should be easy enough to revive at that point /cc @varungandhi-src
## Test plan
Updated unit tests, some manual testing in the API console
With the https://github.com/sourcegraph/sourcegraph/pull/63985/files
PatchRelease is matched before InternalRelease leading to the wrong
build being generated.
We therefore move the Promote and Internal Release runtypes higher in
priority so that they get matched first.
## Test plan
```
export RELEASE_INTERNAL=true
export VERSION="5.5.2463"
go run ./dev/sg ci preview
```
👇🏼
```
go run ./dev/sg ci preview
⚠️ Running sg with a dev build, following flags have different default value unless explictly set: skip-auto-update, disable-analytics
If the current branch were to be pushed, the following pipeline would be run:
Parsed diff:
changed files: [WORKSPACE client/web-sveltekit/BUILD.bazel client/web-sveltekit/playwright.config.ts client/web-sveltekit/src/lib/navigation/GlobalHeader.svelte client/web-
sveltekit/src/routes/[...repo=reporev]/(validrev)/(code)/page.spec.ts client/web/src/cody/chat/new-chat/NewCodyChatPage.tsx client/web/src/cody/sidebar/new-cody-sidebar/NewCodySidebar.tsx
client/web/src/cody/sidebar/new-cody-sidebar/NewCodySidebarWebChat.tsx client/web/src/enterprise/batches/settings/AddCredentialModal.tsx
client/web/src/enterprise/batches/settings/BatchChangesCreateGitHubAppPage.tsx client/web/src/repo/blame/hooks.ts client/web/src/repo/blame/shared.ts cmd/frontend/auth/user.go
cmd/frontend/auth/user_test.go cmd/frontend/internal/codycontext/context.go cmd/frontend/internal/codycontext/context_test.go deps.bzl dev/ci/push_all.sh dev/ci/runtype/runtype.go go.mod go.sum
internal/codeintel/uploads/BUILD.bazel internal/codeintel/uploads/internal/background/backfiller/BUILD.bazel internal/codeintel/uploads/internal/background/backfiller/mocks_test.go
internal/codeintel/uploads/internal/background/commitgraph/BUILD.bazel internal/codeintel/uploads/internal/background/commitgraph/job_commitgraph.go
internal/codeintel/uploads/internal/background/expirer/BUILD.bazel internal/codeintel/uploads/internal/background/expirer/mocks_test.go
internal/codeintel/uploads/internal/background/processor/BUILD.bazel internal/codeintel/uploads/internal/background/processor/mocks_test.go internal/codeintel/uploads/internal/store/BUILD.bazel
internal/codeintel/uploads/internal/store/commitdate.go internal/codeintel/uploads/internal/store/commitdate_test.go internal/codeintel/uploads/internal/store/observability.go
internal/codeintel/uploads/internal/store/store.go internal/codeintel/uploads/mocks_test.go internal/database/migration/shared/data/cmd/generator/consts.go
internal/database/migration/shared/data/stitched-migration-graph.json package.json pnpm-lock.yaml schema/schema.go schema/site.schema.json]
diff changes: "Go, Client, pnpm, Docs, Shell"
The generated build pipeline will now follow, see you next time!
• Detected run type: Internal release
• Detected diffs: Go, Client, pnpm, Docs, Shell
• Computed variables:
• VERSION=5.5.2463
• Computed build steps:
• Aspect Workflow specific steps
• 🤖 Generated steps that include Buildifier, Gazelle, Test and Integration/E2E tests
• Image builds
• :bazel::packer: 🚧 Build executor image
• :bazel: Bazel prechecks & build sg
• :bazel:⏳ BackCompat Tests
• :bazel:🧹 Go mod tidy
• Linters and static analysis
• 🍍:lint-roller: Run sg lint → depends on bazel-prechecks
• Client checks
• :java: Build (client/jetbrains)
• :vscode: Tests for VS Code extension
• :stylelint: Stylelint (all)
• Security Scanning
• Semgrep SAST Scan
• Publish candidate images
• :bazel::docker: Push candidate Images
• End-to-end tests
• :bazel::docker::packer: Executors E2E → depends on bazel-push-images-candidate
• Publish images
• :bazel::packer: ✅ Publish executor image → depends on executor-vm-image:candidate
• :bazel:⤴️ Publish executor binary
• :bazel::docker: Push final images → depends on main::test main::test_2
• Release
• Release tests → depends on bazel-push-images
• Finalize internal release
```
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
This PR is a second attempt at improving push_all.sh, continuing on from
(and inspired by) https://github.com/sourcegraph/sourcegraph/pull/63391.
As a recap, that PR uses
[--script_path](https://bazel.build/reference/command-line-reference#flag--script_path)
to emit a short bash script for every `oci_push` target, which
essentially does minor setup + invokes the executable as if running
`bazel run`.
While the idea in https://github.com/sourcegraph/sourcegraph/pull/63391
was good, it trades concurrent server locking with an equal amount of
overhead in sequentially building the scripts. By observing the
scripts<b>[1]</b> that it would emit, we can notice a few things:
- The path
`/home/noah/.cache/bazel/_bazel_noah/8fd1d20666a46767e7f29541678514a0/execroot/__main__/bazel-out/k8-fastbuild/bin/`
shows up twice, which is the same path that `./bazel-bin` points at
- The script only `cd`'s to a path, unsets some environment variables,
and then executes the underlying script of the target.
The path can be observed to be a combination of bazel-bin, the target's
package (`//cmd/batcheshelper` in this case), as well as the target with
some extra static strings (`candidate_push` with `push_` prefix and
`.sh{,.runfiles}` suffixes for the script & its runfiles respectively).
Knowing this, and assuming that this is reliably so, we can opt to
recreate this manually instead, saving on the hefty overhead of `bazel
run --script_path`.
The current average times for `Push candidate images` and `Push final
images` are ~7m50s and ~8m30s respectively. While the example
main-dry-run build
[here](https://buildkite.com/sourcegraph/sourcegraph/builds/284041#0190e54a-9aaa-471a-81bf-623fce6ffa45)
isnt fully representative of how much rebuilding is required, it sets a
pretty solid 3m20s baseline.
Note this may break with rules_oci changes, but imo thats a small and
very infrequent cost to pay for cleaner log output + shaving a good
piece of time off.
<details><summary><b>[1]</b> A <code>--script_path</code>
example</summary>
```
#!/nix/store/mqc7dqwp046lh41dhs7r7q7192zbliwd-bash/bin/bash
cd /home/noah/.cache/bazel/_bazel_noah/8fd1d20666a46767e7f29541678514a0/execroot/__main__/bazel-out/k8-fastbuild/bin/cmd/batcheshelper/push_candidate_push.sh.runfiles/__main__ && \
exec env \
-u JAVA_RUNFILES \
-u RUNFILES_DIR \
-u RUNFILES_MANIFEST_FILE \
-u RUNFILES_MANIFEST_ONLY \
-u TEST_SRCDIR \
BUILD_WORKING_DIRECTORY=/home/noah/Sourcegraph/sourcegraph \
BUILD_WORKSPACE_DIRECTORY=/home/noah/Sourcegraph/sourcegraph \
/home/noah/.cache/bazel/_bazel_noah/8fd1d20666a46767e7f29541678514a0/execroot/__main__/bazel-out/k8-fastbuild/bin/cmd/batcheshelper/push_candidate_push.sh "$@"
```
</details>
## Test plan
Observe a `sg ci build main-dry-run`
[here](https://buildkite.com/sourcegraph/sourcegraph/builds/284041#0190e54a-9aaa-471a-81bf-623fce6ffa45).
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Adds better observability, and follows more standard patterns. I don't
know why I did it that way a year ago when I implemented this 🤦
Test plan: Code review to find behavior changes that CI cannot cover;
also CI.
Monikers carry a bunch of redundant fields which basically consist of
information parsed out of the symbol name. This patch cleans up
some APIs to work with symbol names directly instead of monikers.
This moves the job from `frontend` to `worker`.
Note:
I believe there was a subtle bug in the original code. We only evaluated
the
feature flag on startup, which means that changes of the feature flag
only took effect after a restart.
## Test plan:
new unit test
Automatically generated PR to update package lockfiles for Sourcegraph
base images.
Built from Buildkite run
[#283904](https://buildkite.com/sourcegraph/sourcegraph/builds/283904).
## Test Plan
- CI build verifies image functionality
Co-authored-by: Buildkite <buildkite@sourcegraph.com>
This patch changes the location querying code so that:
1. We're populating structures corresponding to SCIP instead of LSIF
(with "scheme" and "identifier" inside "MonikerData")
2. Avoid repeatedly allocating a constant string 'scip' for the scheme
only to throw it away later.
3. Makes the two queries and their scanning code more similar for easier
comparison. When I land precise usagesForSymbol, I will de-duplicate
some of the scanning code between these two queries.
I have avoided renaming all of the local variables to avoid creating
more noise.
## Test plan
Covered by existing tests.
Also consolidates mocks to avoid wasting time generating
and compiling them repeatedly, and reducing pollution of
Find Usages results in the editor (yes, I know these can be
filtered out, but it's silly to have multiple copies).
Closes
https://linear.app/sourcegraph/issue/GRAPH-742/simplify-gittreetranslator-implementation
- use unified diff format and 0 context lines to allow translation by
just using hunk headers
- only request hunks per file once, and sync follow-up requests for the
same file
- don't bother LRU caching the full hunk contents, just store 4 int32's
per hunk
- replace linear search through sorted hunks with binary search
Current PR keeps the old PR and just proxies to the new implementation.
Once this PR is reviewed and merged I'll remove the old API and update
all callsites.
## Test plan
New/existing tests
This makes it easier to chart this metric on a daily basis. Transmitting
the count in a rolling 24-hour window is harder.
Note that this new usagestats schema for saved searches has not yet been
released in a patch or stable release.
## Test plan
CI
We need to ensure prod data does not end up in the dev Enterprise Portal
instance once we start syncing. This change adds support for the DevOnly
option in ListSubscriptions and strictly enforces it there (i.e. dev
subscriptions cannot end up in the prod instance either).
I've tried to minimize the changes elsewhere to reduce the impact of
this change for now.
## Test plan
Tests
Implements insert and retrieval of subscription conditions, similar to
#63792
Unlike for licenses, which have a more limited lifecycle (create and
revoke), subscriptions are longer-lived and may be updated frequently.
So the storage layer allows the caller to provide the conditions that
are of interest for recording.
Part of https://linear.app/sourcegraph/issue/CORE-156, but doesn't yet
use it from the API.
Part of https://linear.app/sourcegraph/issue/CORE-100
## Test plan
Integration tests
As we start adding more stuff here, we need a bit more flexibility to
add tests without it becoming overwhelming. The current copy-pasta is
quite difficult to read, this standardizes the expected behaviour for
the Update and List tests.
## Test plan
CI
Fixes [inc-313
](https://sourcegraph.slack.com/archives/C07D8ETFAQP/p1721746352871739?thread_ts=1721745191.614829&cid=C07D8ETFAQP)
It should solve problems with the old auth flow in the new Svelte
version. Currently, the go router doesn't know anything about
post-sign-up flow, which is used in the JB Cody extension. Because of
this, it passes it to the Svelte version which doesn't implement this
route on the client (but the react version did have it on the client
which is why it worked before, before we rolled out Svelte version by
default)
## Test plan
- There is no good way to check this properly, but after this PR you
have to see a react (old) version of the App when you go to the
/post-sigh-up router
Closes [#1110](https://github.com/sourcegraph/devx-support/issues/1110)
Closes DINF-96
We don't print the stdErr when a command fails … in particular when git
fails. Therefore we see very little in the panic of what went wrong.
Explanation:
> There's a weird behavior that occurs where an error isn't accessible
in the err variable
// from a *Cmd executing a git command after calling CombinedOutput().
// This occurs due to how Git handles errors and how the exec package in
Go interprets the command's output.
// Git often writes error messages to stderr, but it might still exit
with a status code of 0 (indicating success).
// In this case, CombinedOutput() won't return an error, but the error
message will be in the out variable.
## Test plan
Manual testing
```go
func main() {
ctx := context.Background()
cmd := exec.CommandContext(ctx, "git", "rev-parse", "--is-inside-work-tree")
out, err := handleGitCommandExec(cmd)
if err != nil {
// er := errors.Wrap(err, fmt.Sprintf("idsdsd: %s", string(out)))
panic(err)
}
fmt.Println("hello", string(out))
}
```
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
We don't appear to make use of the stamp values for any rust_binary
targets, so lets not stamp it to save on cache invalidations
## Test plan
```
$ bazel build --stamp --workspace_status_command=./dev/bazel_stamp_vars.sh $(bazel query 'kind("rust_binary", //...)')
...
$ sha256sum bazel-bin/docker-images/syntax-highlighter/scip-ctags bazel-bin/docker-images/syntax-highlighter/crates/scip-syntax/scip-syntax bazel-bin/docker-images/syntax-highlighter/syntect_server
...
$ bazel build $(bazel query 'kind("rust_binary", //...)')
...
$ sha256sum bazel-bin/docker-images/syntax-highlighter/scip-ctags bazel-bin/docker-images/syntax-highlighter/crates/scip-syntax/scip-syntax bazel-bin/docker-images/syntax-highlighter/syntect_server
...
```
observe identical checksums
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Implements server-side part of AI-132, exposing a single-repo context
endpoint backed by Zoekt.
Differences from the current Enterprise endpoint (that is left
untouched):
- supports a single repo only
- supports passing a repo name, not Sourcegraph-assigned repo ID
- has a shorter, 5s timeout
- reports partial errors (if we hit issues converting search results to
git results, or if some retrievers time out)
## Test plan
- tested locally with
```
{
chatContext(
query: "squirrel"
repo: "github.com/sourcegraph/sourcegraph"
interactionId: "foo"
) {
contextItems {
score
retriever
item {
... on FileChunkContext {
blob {
path
repository {
id
name
}
commit {
oid
}
url
}
startLine
endLine
chunkContent
}
}
}
partialErrors
}
}
```
Pass down semantic information down as much as possible,
delaying lossy conversion to table names till the very end.
I will be introducing the `exhaustive` linter in a future PR
(https://golangci-lint.run/usage/linters/#exhaustive)
so that we can get a static error if you don't switch exhaustively.
For certain types, we do not want the zero-value initialization for structs.
This means we need to trade off readability vs exhaustive initialization
checking, as the Go syntax `Foo{Bar: bar}` is more readable, but doesn't do
exhaustiveness checking, and `Foo{bar}` does check for exhaustiveness but can be less
readable depending on context.
For now, the check is only introduced for one type, and is meant to be
opt-in so that code authors may choose for stricter checking.
Relates to
https://linear.app/sourcegraph/issue/REL-81/service-definition-otel-collector
but does not close it.
Some of the StandardConfig features (notably PodTemplateConfig) assume
that there is only one pod template "thing" per service. The otel
service contains an agent daemonset and a collector deployment, which
have quite different characteristics. We're less likely to paint
ourselves into a corner if we split these services at the config level.
This patch moves the mocks for `codenav/../lsifstore` package to a
separate package `lsifstore/mocks` instead of it living in `codenav`.
The problem is that if you update the `LsifStore` interface, then you
get an error when trying to regenerate mocks, as regeneration happens
in the `codenav` package, and that package's old mocks no longer
satisfy the new interface (so there is an error when looking at other test
files). Moving the codegen to a separate package avoids this problem.
The current description doesn't do a good job of explaining why Search
Jobs is useful and how to create a job.
## Test plan:
visual inspection
Co-authored-by: Erik Seliger <erikseliger@me.com>
This moves the janitor jobs for event logs from `frontend` to `worker`.
Next up: move the other recurring jobs as well.
## Test plan
- new unit tests
- manual testing: the worker dashboard showed the job is running
We only need to two out of three for representing a document in an
upload:
1. Upload root
2. RepoRootRelPath
3. UploadRootRelPath
For simplicity, drop the 3rd one, deriving it dynamically
from the other two as needed.
There are many occurrences flying around, so add some prefix to identify
the fact that some values are used for lookups (whereas other values
are the results of the lookup -- these are not specially marked).
If you look at the code for `bulkMonikerResultsQuery` and
`minimalBulkMonikerResultsQuery`, you'll see that the last
returned value is the `document_path`. The value returned
was directly used an `UploadRootRelPath` elsewhere. So it
makes sense to rename the field from URI to `DocumentPath`.
## Test plan
Covered by existing tests
Builds on
https://github.com/sourcegraph/sourcegraph/pull/63845, and closes
https://linear.app/sourcegraph/issue/REL-14/ensure-pods-roll-when-referenced-secret-changes.
When one of the 3 DB configs changes, an annotation "checksum/auth" on
frontend's deployment's spec.template.metadata should change on next
reconcile. This will cause pods to roll, picking up the new secret
values. It should also cause the top-level annotation configHash to
change, which indicates to the appliance that the kubernetes resource
should be updated.
A similar mechanism is implemented for "checksum/redis", on every
service that uses redis (obtained by grepping the helm chart for the
same annotation).
The reads for `s.streamFailureFunc` and `s.streamError` fields must be
mutex protected since another method may perform writes to those
fields at the same time, and both reads and writes must use the same mutex.
## Test plan
I ran `bazel test //internal/grpc/retry:retry_test --runs_per_test=100`
and no longer see a data race failure, but I see other errors, see
[Slack thread](https://sourcegraph.slack.com/archives/C05EMJM2SLR/p1721484109700539).
In this PR (https://github.com/sourcegraph/sourcegraph/pull/45042), it
is noted that:
> Removed lifecycle configuration from uploadstore, instead relying just
> on our own builtin background worker to expire objects.
As a result, the `TTL` field on the GCS Client was not used anywhere. So
this patch removes the TTL field.
- The `internal/uploadstore` package is renamed to `object` indicating
that it is meant to provide a generic object storage wrapper.
- The `search/exhaustive/uploadstore` package is used in very few places
so I've merged into the `internal/search` package similar to
`internal/embeddings`.
There are a few reasons to do the renaming.
1. The word `upload` in a more general context is ambiguous (just in
`internal/`) - in the codeintel context, it means "SCIP index" but it
can also be interpreted generically ("upload of _some_ data").
2. Better readability - `object.Storage` is much shorter than
`uploadstore.Store`. Additionally, we use the term `Store` A LOT
in the codebase, and usually, these refer to wrappers over some
tables in some DB.
Making things worse, some of our code also has:
```
uploadsstore
"github.com/sourcegraph/sourcegraph/internal/codeintel/uploads/internal/store"
```
And code which says `uploadsstore.Store` (notice the extra `s` 😢), which
is actually a wrapper over some key DB tables like `lsif_uploads`.
- Make it a React function component (not class componnt)
- Remove prop drilling for isSourcegraphDotCom to remove a bunch of code
- fix issue in Back button after clicking to an org from the site admin
orgs page. Repro: 1. Go to /site-admin/organizations. 2. Click on an
org. 3. Click the browser's back button. This was already being done for
the user routes.
## Test plan
Repro described above
Followup from https://github.com/sourcegraph/sourcegraph/pull/63941.
I forgot to make it so site admins could list org members in that PR.
As for the new ability for site admins to see user and org settings:
Site admins could already add themselves to orgs as members and see
settings that way. I considered still keeping it stricter, but it is
valuable for site admins to be able to view settings to help users
troubleshoot.
## Test plan
In dotcom mode, as a site admin, view a user or org (that the site admin
is not a member of). Confirm that the settings can be viewed.
This is the last bit of work before flipping the switch on dotcom.
Implemented based on the behavior @taiyab and I aligned on this morning.
Can definitely still use a little sparkle, but I'll let others follow up
on that.
It adds a toggle button to all pages that we have a svelte version of.
The toggle is off when you're in the react webapp and on when you're in
the svelte webapp. It also updates the copy of the popover to be more
appropriate for the dotcom crowd.
Previously, the GraphQL API on dotcom only let org members query for an
org (through `organization(name: ...)` or otherwise). This made sense as
a strict security safeguard in a world where an org had only private
resources, not public resources.
However, with search contexts, saved searches, and now the new prompt
library, we want orgs on dotcom to be able to create things that (if we
or they intentionally make them public) all users can see, and can see
the association with the org owner. That requires all users to be able
to query for the org and see its name.
We continue to enforce the secrecy of much org data: members (only org
members can list the other members of the org), settings (only org
members can view this).
**But the name, displayName, and existence of an org will now be
considered public.**
## Test plan
In dotcom mode, view an organization.
Anonymous visitors to Sourcegraph.com were unable to list all public
saved searches (at https://sourcegraph.com/saved-searches) and prompts
(at https://sourcegraph.com/prompts) for 2 reasons:
1. The namespace selector (filter) was broken for anonymous users and
returned an error. Fixed this by just handling this case.
2. The GraphQL resolvers for `savedSearches` and `prompts` prevented
anonymous users from listing items because there was no affiliated user
and thus the anonymous user was essentially trying to list all. Fixed
this by adding a new and well-tested code path for listing only public
saved searches or prompts.
## Test plan
In dotcom mode, visit those 2 pages (`/saved-searches` and `/prompts`).
Implements Cody Gateway access in Enterprise Portal DB, such that it
replicates the behaviour it has today, retrieving:
- Quota overrides
- Subscription display name
- Active license info
- Non-revoked, non-expired license keys as hashes
- Revocation + non-expiry replaces the existing mechanism of flagging
licenses as `access_token_enabled`. Since we ended up doing zero-config
for Cody Gateway, the only license hashes that are valid for Cody
Gateway are non-expired licenses - once your license expires you should
be switching to a new license key anyway.
It's fairly similar to the `dotcomdb` shim we built before, but for our
new tables. See https://github.com/sourcegraph/sourcegraph/pull/63792
for the licenses tables.
None of this is going live yet.
Part of https://linear.app/sourcegraph/issue/CORE-100
Part of https://linear.app/sourcegraph/issue/CORE-160
## Test plan
DB integration tests
`sg run enterprise-portal` does the migrations without a hitch
See https://sourcegraph.slack.com/archives/C03K05FCRFH/p1721387628038419
We'll see if this fixes the race condition?
I believe multiple tests are setting the mock configuration object via conf.mock - so hopefully removing the t.Parallel() call fixes the issue.
## Test plan
CI passes
## Changelog
- A race condition in the sub_repo_permissions database store test suite has been fixed.
Adds an equivalent to the curl command we currently share, but in `sg`.
If we add a better API around this later it's just an in-place
replacement.
Similar to https://github.com/sourcegraph/sourcegraph/pull/63883 this
"just works" with zero configuration against SAMS-dev.
Part https://linear.app/sourcegraph/issue/CORE-220, a spike into
polishing some local-dev DX for SAMS.
## Test plan
```
sg sams client create -redirect-uris='https://sourcegraph.test:3443/.auth/callback' robert-testing
```
if you hit an error loading the secret, e.g. targeting the prod
instance, you get a suggestion to get Entitle access:
```
sg sams client create -redirect-uris='https://sourcegraph.test:3443/.auth/callback' -sams='https://accounts.sourcegraph.com' robert-testing
⚠️ Running sg with a dev build, following flags have different default value unless explictly set: skip-auto-update, disable-analytics
👉 Failed to get secret - do you have Entitle access to the "sourcegraph-accounts-prod-csvc" project? See https://sourcegraph.notion.site/Sourcegraph-Accounts-infrastructure-operations-b90a571da30443a8b1e7c31ade3594fb❌ google(sourcegraph-accounts-prod-csvc): failed to get secret "MANAGEMENT_SECRET": rpc error: code = PermissionDenied desc = Permission 'secretmanager.versions.access' denied for resource 'projects/sourcegraph-accounts-prod-csvc/secrets/MANAGEMENT_SECRET/versions/latest' (or it may not exist).
```
## Changelog
- `sg sams client create` can now be used to create IdP clients for
SAMS.
---------
Co-authored-by: Erik Seliger <erikseliger@me.com>
Prototype what it might be like to have `sg` run `sourcegraph-accounts`.
I think this would be a huge improvement over the multi-step,
many-things-to-install process that currently exists for SAMS.
There are a couple of quirks, but I think I'll leave this here for now,
as this is part https://linear.app/sourcegraph/issue/CORE-220, a
timeboxed spike into polishing some local-dev DX for SAMS.
Also see https://github.com/sourcegraph/sourcegraph-accounts/pull/262
## Test plan
```
sg start sourcegraph-accounts
```
Go to 127.0.0.1:9991 and try to log in
The schema file was removed long ago, this removes the Go code for it as
well, as there are no more references to it.
Test plan:
Go compiler doesn't complain about missing symbols.
As it says on the tin - various commands related to SAMS can now target
dev services integrated against SAMS-dev directly. See test plan for
examples.
I've also refactored the `sg sams introspect-token` etc commands in
preparation for introducing more `sg sams` commands - the existing
commands are now collapsed into `sg sams token introspect` and `sg sams
token introspect -p`
Part https://linear.app/sourcegraph/issue/CORE-220, a spike into
polishing some local-dev DX for SAMS.
I also upgrade the glamour library because I noticed the JSON
pretty-printing was no longer colored - the upgrade fixed that
## Test plan
All the below now work with no additional effort:
```sh
# get token details and print a temporary token
sg sams token introspect -p
# list enterprise-portal-dev data
sg enterprise subscription list -member.cody-analytics-viewer 'robert@sourcegraph.com'
```
You can use it against locally running services that connect to SAMS-dev
as well, for example the below also works with no additional
flags/envvars:
```sh
sg start dotcom # includes enterprise-portal
sg enterprise subscription list -enterprise-portal-server=http://localhost:6081
```
## Changelog
- `sg` commands requiring SAMS client credentials now load shared
SAMS-dev client credentials by default.
This adds support to searching for repo metadata with a regex pattern.
Background: repo metadata is a useful feature for shoehorning
business-specific information into the search query language. It allows
tagging repos with arbitrary metadata (think ownership info, quality
info, 3rd-party system IDs, etc.). This ends up being a useful escape
hatch to shim in functionality that is not natively supported in
Sourcegraph.
However it's currently limited to searching with an exact key/value
pair. We've had a few requests to extend this to allow searching by
pattern because it enables ingesting semi-structured data and making it
searchable.
This adds the ability to use a `/.../`-delimited regex pattern to match
against both keys and values. For example,
`repo:has.meta(team:/^my\/org/)`
This PR fixes several issues related to CodyPro/Sourcegraph.com relying
on Sourcegraph-supplied models.
When Sourcegraph.com is _exclusively_ using the modelconfig systems we
can delete a lot of this code, and all of the hard-coded lists of LLM
models. However, in order to support a smoother transition, we need to
support Sourcegraph.com _not_ using Sourcegraph-supplied models, as well
as updating the site configuration so that it does.
This PR makes the following fixes:
- For forwards compatibility, Sourcegraph.com will now support model
references using the "mref format" (a:🅱️:c). Whereas previously it
_only_ would support the legacy format (a/b). This will allow us to
update Cody clients to use mrefs natively.
- Add virtualized models to the hard-coded list of Cody Pro chat models.
If Sourcegraph.com is using Sourcegraph-supplied LLM models, then it
will use ModelID "claude-3-sonnet" to refer to the LLM model with
ModelName "claude-3-sonnet-20240229". We now accept the virtualized
model name inside of `func isAllowedCodyProChatModel(...)`, as well as
"devirtualize" the model name inside of `func
resolveRequestedModel(..)`.
- Use the Cody Pro user's access token for calls to Cody Gateway. This
is a serious bug that is live today. (⚠️😬) For Sourcegraph.com, when we
call Cody Gateway we do _not_ want to authenticate the request using the
Sourcegraph license key. And instead use the end user's credentials.
However, inside of the Cody Gateway completions client, we were not
actually using that access token.
See:
dbf420bb8c/internal/completions/types/types.go (L61-L75)
> I'll try to add a unit test to ensure this behavior before submitting,
but wanted to get the PR sent out for review ASAP.
## Test plan
Added, updated unit tests. Tested manually.
## Changelog
NA
Adds a otel counter to measure how many traced requests we sent to
Fireworks from Cody Gateway -
[context](https://sourcegraph.slack.com/archives/C0729T2PBV2/p1721385268801079?thread_ts=1721029684.522409&cid=C0729T2PBV2)
## Test plan
- tested locally by sending a request without the `X-Fireworks-Genie`
header, with the header but with value != `true` and with the header and
value `true` and observing `fireworks-traced-requests` Prometheus metric
Closes
[CODY-2775](https://linear.app/sourcegraph/issue/CODY-2775/%5Bautocomplete-latency%5D-apply-the-same-timeout-on-the-cody-gateway-side)
Enables client control over the request processing timeout on the server
(both Sourcegraph backend and Cody Gateway). The context timeout is set
to the value provided in the `X-Timeout-Ms` header of the client
request. If the header is not provided, the default context timeout is
used (1 minute on both Sourcegraph backend and Cody Gateway).
Previously, we only had a default timeout on the Sourcegraph backend
side (8 minutes).
Corresponding client change:
- https://github.com/sourcegraph/cody/pull/4921
<!-- 💡 To write a useful PR description, make sure that your description
covers:
- WHAT this PR is changing:
- How was it PREVIOUSLY.
- How it will be from NOW on.
- WHY this PR is needed.
- CONTEXT, i.e. to which initiative, project or RFC it belongs.
The structure of the description doesn't matter as much as covering
these points, so use
your best judgement based on your context.
Learn how to write good pull request description:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e?pvs=4
-->
## Test plan
- Manually tested and confirmed that if the request contains the
`X-Timeout-Ms` header, its value is used. If not, the default maximum
request duration is applied.
- CI
-
<!-- All pull requests REQUIRE a test plan:
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
- Use the provided timeout from request parameters if available;
otherwise use the default maximum request duration (8 minutes)
<!--
1. Ensure your pull request title is formatted as: $type($domain): $what
2. Add bullet list items for each additional detail you want to cover
(see example below)
3. You can edit this after the pull request was merged, as long as
release shipping it hasn't been promoted to the public.
4. For more information, please see this how-to
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c?
Audience: TS/CSE > Customers > Teammates (in that order).
Cheat sheet: $type = chore|fix|feat $domain:
source|search|ci|release|plg|cody|local|...
-->
<!--
Example:
Title: fix(search): parse quotes with the appropriate context
Changelog section:
## Changelog
- When a quote is used with regexp pattern type, then ...
- Refactored underlying code.
-->
This approach is a bit of a departure from how our helm chart works.
That hashes hashes values blocks to determine these annotations. The
issue is, that these values blocks often contain references to secrets,
and therefore don't change when the _content_ of the secret changes.
This PR takes a more general approach of looking for that secret, which
must always eventually exist - whether admin-provisioned or provisioned
by the pgsql service definition - and hashes that.
Previously, organizations would be displayed using their `displayName`,
which was inconsistent and non-canonical. The `displayName` is also
longer, so it's not suited to display in select menus and other similar
places.
For example, when viewing the user menu or creating a batch change or
saved search, the dropdown will now show `sourcegraph` instead of
`Sourcegraph` (where the latter is the display name).
## Test plan
Go to create a batch change or saved search and confirm that org names
not displayNames are shown.
## Changelog
- The user menu and other filter menus now show the names of
organizations, not their "display names", to avoid ambiguity. For
example, these menus will now show `abc-corp` not `ABC Corp` (if the
latter was the `abc-corp`'s display name).
This method can be used to find a common ancestor for many commit SHAs,
to be used by code intel for finding visible uploads.
Closes SRC-485
Test plan: Added unit tests for the several layers (client, grpc,
gitcli).
Previously, a user's or org's settings were protected from unauthorized
access in the GraphQL API by access checks far from the actual
`SettingCascade` and `LatestSettings` implementations in most cases.
This did not present a security issue because on Sourcegraph.com we
prevented users from getting a reference to an org they aren't in, and
user settings had the right manual access checks.
But the access checks are too far away from the actual resolver methods
(so it'd be easy to make a mistake) and were not consistently
implemented. **Now, all checks for view-settings-of-subject access go
through the same function, `settingsSubjectForNodeAndCheckAccess`.**
One substantive security change is that now site admins may NOT view the
settings of an org they are not a member of. This is in line with site
admins on dotcom not being able to see user settings. This is important
because settings might contain secrets in the future (e.g., OpenCtx
provider config). Site admins may still add themselves to the org and
then view the settings, but that creates more of an audit trail and and
we may lock down that as well.
## Test plan
Unit tests, also browse around the UI and ensure there are no code paths
where our assertion triggers.
- skip graphqlbackend tests that use the DB when using `go test -short`
- fix race condition in (settingsResolver).Author
- fix OrgMembers.GetByOrgIDAndUserID mocks: The actual function returns
a non-nil error (`database.ErrOrgMemberNotFound` type) when the user is
not an org member. Our mocks should do the same for correctness.
## Test plan
CI
This PR adds support for two new arguments in the gitserver API:
InterHunkContext and ContextLines.
See the inline docstring for the two, but basically this allows to
request diffs with a custom amount of context lines and hunk merging.
The defaults should be unchanged at `3` for both, according to
https://sourcegraph.com/github.com/git/git/-/blob/diff.c?L60.
Closes SRC-467
Test plan:
Added unit tests.
- Add spacing at bottom of page
- Fix viewerCanAdminister check to return `false` not an error when the
user is not an administrator of a saved search or prompt
- Improve display of prompt description
## Test plan
In dotcom mode, try loading a prompt or saved search in incognito mode.
For some reason, the indication that the text was copied was removed for
secrets. The indication does not show the secret value itself, so I
don't see why that change was made. It is helpful to the user to give
them feedback that the secret was in fact copied.
## Test plan
Click the copy button after creating an access token
We want to support orgs on dotcom for grouping prompts in the Prompt
Library. Note that creating orgs was never actually disabled in the API,
so this was ineffectual anyway.
## Test plan
View the user settings area sidebar in dotcom mode and ensure the new
org button is shown.
Previously, only site admins could see the members of an org, even of
orgs that they were a member of. This restriction does not make any
sense and makes the orgs feature broken on dotcom.
## Test plan
Added test.
I just noticed that the revision pickers on the compare page don't work.
That's because I made a last minute change to rename the parameter but
forgot to acutally update the parameter when constructing the URL.
This also makes two additional changes:
- Remove `{@debug ...}` expression that was accidentally left in (I feel
like Svelte should remove this in prod builds).
- Adds a `display` prop to button group, and the rev picker, which works
the same as in `Button`. Setting `width: 100%` as done previously causes
wrapping on the compare page. I don't think inline elements should be
set to 100% width. But without it the button doesn't stretch in the
sidebar. This new prop allows the caller to influence this behavior.
| Before | After |
|--------|--------|
|
|
|
## Test plan
Tested the compare page and verified that the revision picker stretches
in the sidebar.
This PR is stacked on top of all the prior work @chrsmith has done for
shuffling configuration data around; it implements the new "Self hosted
models" functionality.
## Configuration
Configuring a Sourcegraph instance to use self-hosted models basically
involves adding some configuration like this to the site config (if you
set `modelConfiguration`, you are opting in to the new system which is
in early access):
```
// Setting this field means we are opting into the new Cody model configuration system.
"modelConfiguration": {
// Disable use of Sourcegraph's servers for model discovery
"sourcegraph": null,
// Create two model providers
"providerOverrides": [
{
// Our first model provider "mistral" will be a Huggingface TGI deployment which hosts our
// mistral model for chat functionality.
"id": "mistral",
"displayName": "Mistral",
"serverSideConfig": {
"type": "huggingface-tgi",
"endpoints": [{"url": "https://mistral.example.com/v1"}]
},
},
{
// Our second model provider "bigcode" will be a Huggingface TGI deployment which hosts our
// bigcode/starcoder model for code completion functionality.
"id": "bigcode",
"displayName": "Bigcode",
"serverSideConfig": {
"type": "huggingface-tgi",
"endpoints": [{"url": "http://starcoder.example.com/v1"}]
}
}
],
// Make these two models available to Cody users
"modelOverridesRecommendedSettings": [
"mistral::v1::mixtral-8x7b-instruct",
"bigcode::v1::starcoder2-7b"
],
// Configure which models Cody will use by default
"defaultModels": {
"chat": "mistral::v1::mixtral-8x7b-instruct",
"fastChat": "mistral::v1::mixtral-8x7b-instruct",
"codeCompletion": "bigcode::v1::starcoder2-7b"
}
}
```
More advanced configurations are possible, the above is our blessed
configuration for today.
## Hosting models
Another major component of this work is starting to build up
recommendations around how to self-host models, which ones to use, how
to configure them, etc.
For now, we've been testing with these two on a machine with dual A100s:
* Huggingface TGI (this is a Docker container for model inference, which
provides an OpenAI-compatible API - and is widely popular)
* Two models:
* Starcoder2 for code completion; specifically `bigcode/starcoder2-15b`
with `eetq` 8-bit quantization.
* Mixtral 8x7b instruct for chat; specifically
`casperhansen/mixtral-instruct-awq` which uses `awq` 4-bit quantization.
This is our 'starter' configuration. Other models - specifically other
starcoder 2, and mixtral instruct models - certainly work too, and
higher parameter versions may of course provide better results.
Documentation for how to deploy Huggingface TGI, suggested configuration
and debugging tips - coming soon.
## Advanced configuration
As part of this effort, I have added a quite extensive set of
configuration knobs to to the client side model configuration (see `type
ClientSideModelConfigOpenAICompatible` in this PR)
Some of these configuration options are needed for things to work at a
basic level, while others (e.g. prompt customization) are not needed for
basic functionality, but are very important for customers interested in
self-hosting their own models.
Today, Cody clients have a number of different _autocomplete provider
implementations_ which tie model-specific logic to enable autocomplete,
to a provider. For example, if you use a GPT model through Azure OpenAI,
the autocomplete provider for that is entirely different from what you'd
get if you used a GPT model through OpenAI officially. This can lead to
some subtle issues for us, and so it is worth exploring ways to have a
_generalized autocomplete provider_ - and since with self-hosted models
we _must_ address this problem, these configuration knobs fed to the
client from the server are a pathway to doing that - initially just for
self-hosted models, but in the future possibly generalized to other
providers.
## Debugging facilities
Working with customers in the past to use OpenAI-compatible APIs, we've
learned that debugging can be quite a pain. If you can't see what
requests the Sourcegraph backend is making, and what it is getting
back.. it can be quite painful to debug.
This PR implements quite extensive logging, and a `debugConnections`
flag which can be turned on to enable logging of the actual request
payloads and responses. This is critical when a customer is trying to
add support for a new model, their own custom OpenAI API service, etc.
## Robustness
Working with customers in the past, we also learned that various parts
of our backend `openai` provider were not super robust. For example, [if
more than one message was present it was a fatal
error](https://github.com/sourcegraph/sourcegraph/blob/main/internal/completions/client/openai/openai.go#L305),
or if the SSE stream yielded `{"error"}` payloads, they would go
ignored. Similarly, the SSE event stream parser we use is heavily
tailored towards [the exact response
structure](https://github.com/sourcegraph/sourcegraph/blob/main/internal/completions/client/openai/decoder.go#L15-L19)
which OpenAI's official API returns, and is therefor quite brittle if
connecting to a different SSE stream.
For this work, I have _started by forking_ our
`internal/completions/client/openai` - and made a number of major
improvements to it to make it more robust, handle errors better, etc.
I have also replaced the usage of a custom SSE event stream parser -
which was not spec compliant and brittle - with a proper SSE event
stream parser that recently popped up in the Go community:
https://github.com/tmaxmax/go-sse
My intention is that after more extensive testing, this new
`internal/completions/client/openaicompatible` provider will be more
robust, more correct, and all around better than
`internal/completions/client/openai` (and possibly the azure one) so
that we can just supersede those with this new `openaicompatible` one
entirely.
## Client implementation
Much of the work done in this PR is just "let the site admin configure
things, and broadcast that config to the client through the new model
config system."
Actually getting the clients to respect the new configuration, is a task
I am tackling in future `sourcegraph/cody` PRs.
## Test plan
1. This change currently lacks any unit/regression tests, that is a
major noteworthy point. I will follow-up with those in a future PR.
* However, these changes are **incredibly** isolated, clearly only
affecting customers who opt-in to this new self-hosted models
configuration.
* Most of the heavy lifting (SSE streaming, shuffling data around) is
done in other well-tested codebases.
2. Manual testing has played a big role here, specifically:
* Running a dev instance with the new configuration, actually connected
to Huggingface TGI deployed on a remote server.
* Using the new `debugConnections` mechanism (which customers would use)
to directly confirm requests are going to the right places, with the
right data and payloads.
* Confirming with a new client (changes not yet landed) that
autocomplete and chat functionality work.
Can we use more testing? Hell yeah, and I'm going to add it soon. Does
it work quite well and have small room for error? Also yes.
## Changelog
Cody Enterprise: added a new configuration for self-hosting models.
Reach out to support if you would like to use this feature as it is in
early access.
---------
Signed-off-by: Stephen Gutekanst <stephen@sourcegraph.com>
The Prompt Library lets you create, share, and browse chat prompts for
use with Cody. Prompts are owned by users or organizations, and site
admins can make prompts public so that all users on the instance can see
and use them.
A prompt is just plain text for now, and you can see a list of prompts
in your Prompt Library from within Cody chat
(https://github.com/sourcegraph/cody/pull/4903).
See https://www.loom.com/share/f3124269300c481ebfcbd0a1e300be1b.
Depends on https://github.com/sourcegraph/cody/pull/4903.
## Test plan
Add a prompt on the web. Ensure you can access it from Cody.
## Changelog
- The Prompt Library lets you create, share, and browse chat prompts for
use with Cody. Prompts are owned by users or organizations, and site
admins can make prompts public so that all users on the instance can see
and use them. To use a prompt from your Prompt Library in Cody, select
it in the **Prompts** dropdown in the Cody chat message field.
Closes https://linear.app/sourcegraph/issue/SRC-459/
Closes
This PR adds support for saving and retreiving the IP addressess
associated with each path rule in the sub_repo_permissions store.
It does this by:
**Adding a new permissions type to the internal/authz package**:
1be7df6d79/internal/authz/iface.go (L52-L96)
**Adding new `*WithIPs` versions of all the setter and getter methods**
The new setter methods uses the above `authz.SubRepoPermissionsWithIPs`
type that write to the appropriate `ips` column in the DB.
The new getter methods retrieve the ip addresses associated with each
path entry. However, here there is an additional complication: It's
possible for someone to call the `*WithIPs` getters when the ips column
is still NULL (indicating that the perforce syncer hasn't been updated /
ran in order to save the IP addresses from the protection table yet.
| repo_id | user_id | version | updated_at | paths | ips |
|---------|---------|---------|------------|-------|-----|
| 1 | 1 | 1 | 2023-07-01 10:00:00 | {`"/depot/main/..."`,
`"/depot/dev/..."`, `"-/depot/secret/..."`} | NULL |
| 2 | 1 | 1 | 2023-07-01 11:00:00 | {`"/depot/public/..."`,
`"-/depot/private/..."`} | NULL |
In order to address this, the getters each have a `backfill` boolean
that allows the caller to choose the behavior that they want.
- If `backfill = true`, the paths without IP entries will be returned
with a `*` (wildcard) IP indicating that any client IP address is okay.
(This is effectively the behavior we have today since we don't check IPs
for sub_repo_permisisons). I imagine this can be used when callers don't
care about enforcing IP-based permissions (such as when IP address
enforcement is disabled in site configuration).
- If `backfill = false`, if the IPs column is NULL - an error is
returned instead of backfilling ("The IP addresses associated with this
sub-repository-permissions entry have not been synced yet."). This
allows for callers that care about IP address enforcement to know
_explicitly_ if the IP address information hasn't been updated yet - so
we can't know whether or not the user is able to view the file (e.g when
IP based enforcement is enabled).
**Ensuring that the old setter methods set the IPs column to NULL**:
self-explanatory, if someone uses the non `*WithIP` variants of the
setters, we want to ensure that we zero out that column so that we don't
leave stale / inconsistent information for those Path entries.
---
Overall, the design this adds the new IP address functionality without
having to immediately update all the call sites in the codebase to force
them to interpret all this information (which would make for a
gargantuan PR). Eventually, we should be able to simply delete the old
versions of the setters/getters once the IP address functioanlity has
been threaded through everywhere.
## Test plan
Extensive unit tests.
For each new setter and getter, I added unit tests that tested along all
of the following dimenisons:
- **initial store state**: empty database, database seeded with
permissions with no IP information (paths column only), database seeded
with permissions that have the IP information synced
- **insertion method**: was the data for the test inserted **with IP
information** (using the `withIP` variant of upsert, etc.), or was it
inserted with the old legacy way with no ip information
- **retreieval method**: was the data reterived with the legacy getters
(that don't look at the IP information), with the new IP getters that
either backfill (if the IP information for that paths entry hasn't been
synced yet, it will return an `*` for that entry), or avoids backfilling
(will return the information in the IPs column, or hard-error)?
## Changelog
- The sub_repository_permissions_ database store can now save and
retrieve the IP addresses associated with each path rule.
**Public saved searches will let us make global saved searches for
dotcom and for customers to help them discover and share awesome search
queries!**
Saved searches now have:
- Visibility (public vs. secret). Only site admins may make a saved
search public. Secret saved searches are visible only to their owners
(either a user, or all members of the owning org). A public saved search
can be viewed by everyone on the instance.
- Draft status: If a saved search's "draft" checkbox is checked, that
means that other people shouldn't use that saved search yet. You're
still working on it.
- Timestamps: The last user to update a saved search and the creator of
the saved search are now recorded.
Also adds a lot more tests for saved search UI and backend code.
## Test plan
Create a saved search. Ensure it's in secret visibility to begin with.
As a site admin, make it public. Ensure other users can view it, and no
edit buttons are shown. Try changing visibility back and forth.
## Changelog
- Saved searches can now be made public (by site admins), which means
all users can view them. This is a great way to share useful search
queries with all users of a Sourcegraph instance.
- Saved searches can be marked as a "draft", which is a gentle indicator
that other people shouldn't use it yet.
I noticed that the commit page doesn't render well on mobile when the
commit has a commit message.
This commit refactors how the `Commit` component is rendered, including
on mobile, which affects both the commit**s** and the commit page.
The two most important changes:
- The component now uses CSS grid to be more flexible about how
individual elements are arranged.
- On mobile we don't show expand the message inline anymore but instead
show it full screen. I think that works well for the commits list too
because now you can open and read a longer commit message without having
to scroll the commits list itself.
## Test plan
Manually inspecting the commits and commit pages. Opened a long commit
message to test that the message is properly scrollable.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
Opsgenie alert notifications for critical alerts should be enabled by
default for production projects or where `env.alerting.opsgenie` is set
to true.
Closes CORE-223
## Test plan
Tested locally by running `sg msp gen` for a `prod` env which doesn't
have an alerting config and verifying that notification suppression was
disabled
Set `env.alerting.opsgenie` to false which enabled suppression again.
No changes to `test` environments unless `env.alerting.opsgenie` is set
to true.
This PR modifies the sub_repo_permissions database to store the ip
addresses associated with each Perforce path rule, as part of the IP
based sub repo permissions work.
The new IP column is implemetned as a []text similar to the path column.
The IP address associated with `paths[0]` is stored in the ips column in
`ips[0]`.
For example, the follownig proections table
```
Protections:
read user emily * //depot/elm_proj/...
write group devgrp * //...
write user * 192.168.41.0/24 -//...
write user * [2001:db8:1:2::]/64 -//...
write user joe * -//...
write user lisag * -//depot/...
write user lisag * //depot/doc/...
super user edk * //...
```
turns into the following rows in the sub_repo_permissions table
| repo_id | user_id | version | updated_at | paths | ips |
|---------|---------|---------|------------|-------|-----|
| 1 | 1 | 1 | 2023-07-01 10:00:00 | {`"//depot/elm_proj/..."`} | {`"*"`}
|
| 1 | 2 | 1 | 2023-07-01 10:00:00 | {`"//..."`} | {`"*"`} |
| 1 | 3 | 1 | 2023-07-01 10:00:00 | {`"-//..."`} | {`"192.168.41.0/24"`}
|
| 1 | 4 | 1 | 2023-07-01 10:00:00 | {`"-//..."`} |
{`"2001:db8:1:2::]/64"`} |
| 1 | 5 | 1 | 2023-07-01 10:00:00 | {`"-//..."`} | {`"*"`} |
| 1 | 6 | 1 | 2023-07-01 10:00:00 | {`"-//depot/..."`,
`"//depot/doc/..."`} | {`"*"`, `"*"`} |
| 1 | 7 | 1 | 2023-07-01 10:00:00 | {`"//..."`} | {`"*"`} |
## Test plan
The unit test for the sub_repository_permissions store PR that is built
on this PR:
https://app.graphite.dev/github/pr/sourcegraph/sourcegraph/63811/internal-database-sub_repo_permissions-modify-store-to-be-able-to-insert-ip-based-permissions
## Changelog
- The sub_repo_permissions table now has an ips column to store the
associated IP address associated with each path rule.
The branches page didn't work well on mobile and neither did the tags
page if long tag names were present.
This commit changes how the information is displayed and rendered,
especially on mobile.
I also added additional links to each row to make navigating to relevant
places easier.
## Test plan
Manual inspection of pages in various screen sizes.
This improves the typings to remove some inscrutable inferred types and
some weird type errors if you didn't use React.PropsWithChildren. Also
refactors the code and exposes a new component AuthenticatedUserOnly
that's simpler and can be used when you don't need to do prop
propagation of authenticatedUser.
## Test plan
CI
Contributes to SRCH-738
Notably, this does not yet identify the root cause of SRCH-738, but it
does identify and fix some confounding bugs. It's possible that these
actually also _cause_ some of the issues in SRCH-738, but I wanted to at
least push these to dotcom, where we can reproduce some of the
weirdness. At the very least, it doesn't explain the auth errors being
reported.
The goal of this PR is to increase the stability of web-sveltekit
e2e-tests so that we don't have to rely on manual runs anymore. They
have previously been disabled due to a high number of failures:
https://github.com/sourcegraph/sourcegraph/pull/63874
---
To improve the stability of web-sveltekit e2e-tests, I used `sg ci bazel
test //client/web-sveltekit:e2e_test --runs_per_test=N` with N=5,10,15
to see which tests break under different levels of pressure on the
machine. The logs looked like it was mostly timeouts, that got worse
when increasing N. That means we can check where tests will break due to
timeouts, but we don't really need to raise timeouts so far that it
would work with N=20.
With N=5, 10 we get a good understanding if our timeouts are high
enough.
You can see two CI runs here after applying higher timeouts and skipping
a consistently failing test:
- N=5:
https://buildkite.com/sourcegraph/sourcegraph/builds/283011/waterfall
- N=10:
https://buildkite.com/sourcegraph/sourcegraph/builds/283013/waterfall
---
From logs of some other run that I don't have the link to anymore, we
can see that some tests take up to 50s so a timeout of 60s (instead of
the default 30s) for CI should be a good new ceiling.
```
Slow test file: [chromium] › src/routes/[...repo=reporev]/(validrev)/(code)/-/blob/[...path]/page.spec.ts (48.9s)
--
| Slow test file: [chromium] › src/routes/[...repo=reporev]/(validrev)/(code)/page.spec.ts (45.0s)
| Slow test file: [chromium] › src/routes/search/page.spec.ts (40.6s)
| Slow test file: [chromium] › src/routes/[...repo=reporev]/(validrev)/(code)/-/tree/[...path]/page.spec.ts (31.7s)
| Slow test file: [chromium] › src/routes/layout.spec.ts (31.5s)
```
## Test plan
CI
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
It turns out that Kubernetes objects constructed using client-go don't
know their own TypeMeta. There is code in client-go that figures out
which resource-scoped HTTP path to call on the kube-apiserver by looking
up a mapping of Go object kinds to k8s kinds. A similar facility appears
to be exposed to the user by apiutil.GVKForObject().
Closes
https://linear.app/sourcegraph/issue/REL-275/reconciler-logs-dont-contain-resource-metadata
Secrets fetched from GSM should probably not be stored locally. As we
increase the usage of fetching external secrets, this stuff is
increasingly sensitive, particularly for SAMS stuff - every time it's
used, we should ensure that the user has the required permissions, and
also only store external secrets in-memory.
It looks like several other callsites make use of the persistence of
other secrets e.g. those prompted from users, so this change
specifically targets the `GetExternal` method. Additionally, I also
added a check on load to delete any legacy external secrets that are
stored to disk on load - we can remove this after a few weeks.
## Test plan
Unit tests asserts old behaviour and new desired behaviour
`sg start -cmd cody-gateway` uses external secrets and works as expected
After running `sg`, `sg secret list` has no external secrets anymore
Closes
[DINF-58](https://linear.app/sourcegraph/issue/DINF-58/overwrite-ordering-in-sg)
https://github.com/user-attachments/assets/d8e59a5f-9390-47f7-a6a7-9ccbf97423f8
## Test plan
- Add a `commandset` to the `sg.config.overwrite.yaml`
- This commandset should depend on an existing command in the
`sg.config.yaml` file.
- The commandset should also include an `env var` that should override
what's set in the `command` contained in the `sg.config.yaml` file.
- Running `sg start <commandset name>` should allow the env ordering
matrix shown below
```
Priority: overwrite.command.env > overwrite.commandset.env > command.env > commandset.env.
```
## Changelog
N/A
Delivery Manifest step has started to run `bazel build` commands, in them clobbering our execlog artifacts. We should only emit it for the test buildkite jobs (at least for the time being), as it currently doesnt make sense for e.g. the image push jobs which contain multiple invocations
## Test plan
CI
## Changelog
Follow-up to
https://sourcegraph.slack.com/archives/C07KZF47K/p1720639713491779?thread_ts=1720636753.404169&cid=C07KZF47K
Basically, if we see that the local assets are above 500mb, we just nuke
it. It's a bandage btw. The `.gitkeep` is there so it doesn't break the
build because there's nothing to embed.
@eseliger and @burmudar can you test this a bit further and land it if
it's all good? My tests are good, but I don't want to hastily land
something and go in PTO five seconds before I'm out for two weeks.
## Test plan
Locally tested.
---------
Co-authored-by: William Bezuidenhout <william.bezuidenhout@sourcegraph.com>
Fixes CODY-2888
Previously, Sourcegraph Enterprise instances with context filters
enabled rejected requests from all unknown clients out of concern that
they might not respect context filters. This behavior makes it
incredibly impractical to release now agent-based clients (CLI, Eclipse,
Visual Studio, Neovim, ..) that do respect context filters out of the
box thanks to the reused logic in the Cody agent.
This logic suffers from both false positives and false negatives:
- False negatives: upcoming Cody clients (CLI, Eclipse, Visual Studio)
already support context filters out of the box thanks to using the Cody
agent but they can't send requests unless we add a special case to them.
It may require months for these clients to wait for all Enterprise
instances to upgrade to a version that adds exceptions for their name.
- False positive: a malicious client can always fake that it's
"jetbrains" with a valid version number even if the client doesn't
respect context filters. This gives a false sense of security because it
doesn't prevent malicious traffic from bypassing context filters. In
fact, I am leaning towards using the
Now, with this change, Sourcegraph Enterprise instances only reject
requests from old versions of Cody clients that are known to not support
context filters. This ensures we never have false positives or false
negatives.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
## Test plan
See updated test case which accepts a request from an unknown "sublime"
client.
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Instead of a a maximum of 2 minor versions beyond the deployed
Sourcegraph instance.
The main advantage of removing the 2-minor-version constraint, is that
it allows admins to always be able to update to the latest SG with one
click, instead of having to repeat that process through intermediate
versions if they have fallen far behind.
Otherwise Apollo Client complains that it does not know how to cache the
result because it does not know which repository ID to associate the
result with.
## Test plan
CI
We previously improved the performance of Language Stats Insights by
introducing parallel requests to gitserver:
https://github.com/sourcegraph/sourcegraph/pull/62011
This PR replaces the previous approach where we would iterate through
and request each file from gitserver with an approach where we request
just one archive. This eliminates a lot of network traffic, and gives us
an additional(!) performance improvement of 70-90%.
Even repositories like chromium (42GB) can now be processed (on my
machine in just one minute).
---
Caching: We dropped most of the caching, and kept only the top-level
caching (repo@commit). This means that we only need to compute the
language stats once per commit, and subsequent users/requests can see
the cached data. We dropped the file/directory level caching, because
(1) the code to do that got very complex and (2) we can assume that most
repositories are able to compute within the 5 minutes timeout (which can
be increase via the environment variable `GET_INVENTORY_TIMEOUT`). The
timeout is not bound to the user's request anymore. Before, the frontend
would request the stats up to three times to let the computation move
forward and pick up where the last request aborted. While we still have
this frontend retry mechanism, we don't have to worry about an
abort-and-continue mechanism in the backend.
---
Credits for the code to @eseliger:
https://github.com/sourcegraph/sourcegraph/issues/62019#issuecomment-2119278481
I've taken the diff, and updated the caching methods to allow for more
advanced use cases should we decide to introduce more caching. We can
take that out again if the current caching is sufficient.
Todos:
- [x] Check if CI passes, manual testing seems to be fine
- [x] Verify that insights are cached at the top level
---
Test data:
- sourcegraph/sourcegraph: 9.07s (main) -> 1.44s (current): 74% better
- facebook/react: 17.52s (main) -> 0.87s (current): 95% better
- godotengine/godot: 28.92s (main) -> 1.98s (current): 93% better
- chromium/chromium: ~1 minute: 100% better, because it didn't compute
before
## Changelog
- Language stats queries now request one archive from gitserver instead
of individual file requests. This leads to a huge performance
improvement. Even extra large repositories like chromium are now able to
compute within one minute. Previously they timed out.
## Test plan
- New unit tests
- Plenty of manual testing
Precursor for
https://linear.app/sourcegraph/issue/GRAPH-735/test-syntactic-usages
This PR introduces the `MappedIndex` abstraction, which wraps up an
upload with a target commit. Its APIs then take care of mapping upload
relative paths and repo relative paths, and ranges across commits.
My main motivation for making this change is that I can now test this
logic in isolation (which this PR does), and also have an interface that
is easy to fake and use to test syntactic usages.
## Test plan
Added unit tests for the `MappedIndex` component, manual testing of the
GraphQL APIs show no changes in the syntactic usages output between this
PR and master.
Turns out, `gorm` does not auto-migrate constraints you've removed from
the struct schema. You must drop them by hand, or run the commands to
drop them explicitly.
## Test plan
```
sg start -cmd enterprise-portal
```
```
\d+ enterprise_portal_*
```
then, comment the custom migrations, and run `sg start -cmd
enterprise-portal` again to re-do the migrations. The output of `\d+`
matches. Unexpected constraints are gone.
This commit adds the Svelte version of the compare page. Now that we
have a revision picker this was easy to implement.
A couple of notes:
- The URL parameter is a rest parameter ([...spec]) because revisions
can contain slashes, and we don't encode those.
- I changed the API of the revision selector so that it works on pages
that don't have `resolvedRevision` available (also we never needed the
`repo` property from that object), and to provide custom select
handlers.
- Moved the revision to lib since it's used on various different pages.
- Unlike the React version this version
- Provides forward/backward buttons to browse the commit list
- Uses infinity scrolling to load the next diffs
- Doesn't use a different style for the commit list. I experimented with
adding a "compact" version of the commit but it didn't feel quite right.
I'm sure we'll eventually redesign this.
- The `filePath` parameter is supported (for backwards compatibility)
but we don't use it anywhere atm.
(note that the avatar size has changed since the video was recorded;
it's larger now)
## Test plan
Manual testing.
We recently updated the completions APIs to use the `modelconfig` system
for managing LLM model configuration. Behind the scenes, we
automatically converted the existing site configuration ("completions
config") into the newer format so things work as expected.
However, the GraphQL view of the Sourcegraph instance's LLM
configuration was not updated to use the `modelconfig` system. And so fi
the site admin and opted into using the new-style of configuration data,
the data returned would be all sorts of wrong.
(Because the GraphQL handler looked for the "completions config" part of
the site config, and not the newer "model configuration" section.)
This PR updates the `CodyLLMConfiguration` GraphQL resolver to return
the data from the modelconfig component of the Sourcegraph instance.
Some careful refactoring was needed to avoid a circular dependency in
the Go code. So the resolver's type _declaration_ is in the
`graphqlbackend` package. But it's _definition_ is in
`internal/modelconfig`.
## Test plan
I only tested these changes manually.
If you open the Sourcegraph instance's API console, this is the GraphQL
query to serve all of the data:
```gql
{
site {
codyLLMConfiguration {
chatModel
fastChatModel
completionModel
provider
disableClientConfigAPI
chatModelMaxTokens
fastChatModelMaxTokens
completionModelMaxTokens
}
}
}
```
## Changelog
NA
Part of: https://github.com/sourcegraph/devx-support/issues/1093
If we get 3 errors in a row trying to write to bigquery ... chances are
we are not going to succeed. So we exit early.
## Test plan
CI
## Changelog
- sg: provide a better error message when we fail to insert into
bigquery
- sg: stop puslishing to bigquery if we get 3 errors in a row
Testing for display-name setting which we recently added, and this is
useful in the interim to set display names on the go for subscriptions
EP already tracks.
note: I don't anticipate doing this for every field we make update-able,
especially since the next step(s) will be updating the UI
## Test plan
```
sg enterprise subscription set-name es_4dae04ba-5f5b-431a-b90b-e8e3dd449181 "robert's test subscription"
```
Adds site-config configuration for RFC 969 intent detection, making the
Intent Detection API endpoint and token configurable without code
changes. Additionally, adds an option to hit multiple intent detection
backends with the same query.
Previously, URL was hardcoded in code, so if the backend has changed, we
had to redeploy sourcegraph.com.
As we iterate on intent detection, we want to be able to test multiple
models in parallel, so this PR adds a setting for `extra` backends - if
provided, additional .com -> backend requests will be sent, but the
client-initiated request will not wait for those requests.
Closes AI-128.
## Test plan
- tested locally - add
```
"cody.serverSideContext": {
"intentDetectionAPI": {
"default": {
"url": "http://35.188.42.13:8000/predict/linearv2"
},
"extra": [
{
"url": "http://35.188.42.13:8000/predict/linearv2"
}
]
}
}
```
to `experimentalFeatures` in dev-private.
This is an input element that constraints the input to look like a path
component, used for the names of batch changes (and for prompts in the
prompt library in the future).
Also fixes an invalid regexp (it needs to escape `-` because the `'v'`
flag is used; see
https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/pattern).
## Test plan
Test against existing batch change create form name input. Try invalid
inputs.
This was added to the old global navbar but not the one one by accident
(by me).
## Test plan
Ensure that the new global navbar shows a Saved Searches link.
Ideally, we would introduce copies of the old names, deprecate the old
ones, migrate the frontend code, and cut over after a couple of minor
releases.
I don't have time for that now, so just updating the docs a bit for clarity.
Just a couple of things I noticed while working on unrelated tasks.
1) Removes the unused `Separator` component, which has been replaced by
the Panel API
2) Makes the sourcegraph mark a proper icon so it can be used via the
`Icon` component like all our other icons.
While attempting to use `observeIntersection` with the new references
panel, I was running into performance issues with large lists of
references (the API does not actually paginate yet). When I took a look
at the profile, a good chunk of the time came from finding the nearest
scrolling ancestor, specifically the `overflowY` part, which requires
computing large chunks of the layout.
So this changes the `observeIntersection` action to take an explicit
target container so we don't need to do any searching for a scroll
container. For the example that I was debugging, this reduced the time
it took to render the list ~5x. Additionally, it establishes a cache for
all created `IntersectionObserver`s rather than just the root observer.
S2 Cody Web is broken at the moment. New client-config handlers fail
with 401 status because we don't send custom headers, this works for gql
queries since they all are POST requests and the browser automatically
sends an Origin header for them and this is enough for our auth
middleware to check cookies, but with client-config which is rest it's
not the case and we should send `X-Requested-Client: Sourcegraph` header
to make our auth middleware to pass this query correctly
Note that this problem doesn't exist in local builds since we proxy all
requests and add `X-Requested-Client: Sourcegraph` in dev server.
See Cody latest build PR for more details
https://github.com/sourcegraph/cody/pull/4898
## Test plan
- CI is passing
`sg run` is supposed to be deprecated in favour of `sg start -cmd`, but
the `sg start` completions don't work with `-cmd` like `sg run` does.
This change updates `sg start` completion to check for the `-cmd` flag,
and if it is provided, offer completions for commands instead of
command_sets_ (the default `sg start` behaviour).
## Test plan
<img width="1023" alt="image"
src="https://github.com/user-attachments/assets/9b887180-f58f-4aef-9dbb-718c71ba15e6">
<img width="1077" alt="image"
src="https://github.com/user-attachments/assets/927b4562-fce1-48c0-a8c5-453bfc60fe35">
## Changelog
- Completions for `sg start -cmd` now offer valid suggestions.
Noticed several `Usage` using newlines, which makes `-h` output pretty
annoying to read as it breaks up the formatting. It tickled me enough to
put a formatting check against it, and update the existing usages that
were incorrect, to use `Description` or `UsageText` instead :-)
## Test plan
CI, `sg -h` is pretty(er) again (but still very long)
- Don't show the Tools menu on dotcom for now, since there is only 1
item. Users can still access saved searches in their user menu.
- Fix an issue where non-site admins on dotcom would see an error at
`/saved-searches` until they changed the Owner filter.
- Other minor code cleanups.
## Test plan
Try being a non-site admin and ensure that `/saved-searches` works.
The appliance checks owner references, and only deletes (or updates)
resources whose owner references matches the configmap being reconciled.
The current user interface to external databases, is for the admin to
create secrets with a well-known name out of band (e.g. "pgsql-auth")
and then disable the relevant backing services (e.g. pgsql). This commit
fixes a bug in which the appliance would have deleted such secrets,
leaving the admin unable to use external databases.
Discovered while investigating
https://linear.app/sourcegraph/issue/REL-14/ensure-pods-roll-when-referenced-secret-changes,
but does not close it.
This PR upgrades the cody web experimental package to 0.2.5, in the new
version we fixed
- Telemetry problem with init extension-related events (we don't send
install extension events anymore)
- Most recent updates on LLM availability for enterprise instances
## Test plan
- CI is green
- Manual check on basic Cody Web functionality (highly recommended)
This PR adds better gating for disabling the code monitors, notebooks,
search jobs and own features.
## Test plan
Ran locally and verified the features are gone when the env var is set.
Ran again without those and they worked.
---------
Co-authored-by: Stefan Hengl <stefan@sourcegraph.com>
Fixes CODY-2661
Previously, only the JetBrains and Neovim Cody plugins had dedicated
pages to authorize access token redirects to a localhost port. Other
clients like the Cody CLI and Eclipse plugin reused the JetBrains page,
which technically works but is problematic because the UI says the user
is authorizing "JetBrains" when in reality they're authorizing other
clients. This PR fixes the problem by adding dedicated pages for the
Cody CLI, Eclipse plugin and Visual Studio extension.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
## Test plan
Didn't test this locally because it takes me a long time to setup Docker
access. The diff is small and should be low-risk to merge. I manually
searched for 'JETBRAINS' in this directory and confirmed there are no
remaining special cases for JetBrains.
<!-- REQUIRED; info at
https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles
-->
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
This commit is in preparation for other work. It refactors how we work
with resolved repository and resolved revision information.
The idea was that the top level repo loader should try to resolve the
repository information and error if that wasn't possible. Not being able
to resolve the revision was accepted, hence this check:
```js
// still render the main repo navigation and header
if (!isRevisionNotFoundErrorLike(repoError)) {
error(400, asError(repoError))
}
```
However the way it was implemented meant that we wouldn't pass any
resolved repository information to the sub-pages/layouts when the
revision couldn't be resolved, which seems wrong.
With these changes, the top level repo loader now provides a
`resolvedRepository` object (where `commit`/`changelist` might be unset)
and the `(validrev)` loader creates the `resolvedRevision` object, just
how its sub-pages/layouts expect.
And instead of returning error objects from `resolveRepoRevision` and
checking them in the loader we throw errors/redirects directly in that
function. IMO that makes the whole flow easier to understand.
## Test plan
Manual testing, CI integration tests
In preparation for other work, this commit substantially refactors the
"infinity query" store implementation. The internals have been changed
completely which allows us to simplify its public API.
- Simpler configuration, especially merging previous and next results.
- Restoration support. So far pages/components had to implement
restoring the state of an infinity store on their own. Now the
restoration strategy is part of the configuration. Pages/components only
have to get an opaque snapshot via `store.capture()` and restore it via
`store.restore(snapshot)`.
- More predictable state. It wasn't always obvious if the store content
stale data e.g. during restoring data. Now `data` will only be set when
the data is 'fresh'.
- Smarter 'incremental restoration' strategy. This strategy makes
multiple requests to restore the previous state. It makes multiple
requests because normally requests are cached and there this is fast.
When the data is not cached though there is a noticable delay due to
waterfall requests. Now we use a simple heuristic to determine whether
or not GraqhQL data might be cached. If not we make a single request to
restore the state.
For review I suggest to turn whitespace changes off.
## Test plan
Manual testing, unit tests.
Previously, we would attempt to recreate the clone URL of a repo based
on event data. This is a lossy matching, and can cause events to get
rejected, although we have the repo cloned.
This PR changes the matching to instead use the external ID of the repo,
which we already store in the repo table in a separate column.
Closes SRC-40
Test plan:
Tests still pass, set up webhooks locally and they still matched (but
only tried GitHub).
- Remove long-deprecated and long-ineffective notifications for saved
searches (removed in
de8ae5ee28
2.5 years ago). Note that code monitors were the replacement for saved
searches and work great.
- Clean up UI.
- Make the UI global instead of in the user/org area.
- Convert React class components to function components.
- Add default `patterntype:` because it's required.
- Use `useQuery` and `useMutation` instead of `requestGraphQL`.
- Use a single namespace `owner` GraphQL arg instead of separating out
`userID` and `orgID`.
- Clean up GraphQL resolver code and factor out common auth checking.
- Support transferring ownership of saved searches among owners (the
user's own user account and the orgs they're a member of).
(I know this is not in Svelte.)
SECURITY: There is one substantive change. Site admins may now view any
user's and any org's saved searches. This is so that they can audit and
delete them if needed.
## Test plan
Try creating, updating, and deleting saved searches, and transferring
ownership of them.
## Changelog
- Improved the saved searches feature, which lets you save search
queries to easily reuse them later and share them with other people in
an organization.
- Added the ability to transfer ownership of a saved search to a user's
organizations or from an organization to a user's account.
- Removed a long-deprecated and ineffective settings
`search.savedQueries` field. You can manage saved searches in a user's
or organization's profile area (e.g., at `/user/searches`).
This is a refactor with a few incidental user-facing changes (eg not
showing a sometimes-incorrect total count in the UI).
---
Make `usePageSwitcherPagination` and `useShowMorePaginationUrl` support
storing filter and query params, not just pagination params, in the URL.
This is commonly desired behavior, and there are many ways we do it
across the codebase. This attempts to standardize how it's done. It does
not update all places this is done to standardize them yet.
Previously, you could use the `options: { useURL: true}` arg to
`usePageSwitcherPagination` and `useShowMorePaginationUrl`. This was not
good because it only updated the pagination URL querystring params and
not the filter params. Some places had a manual way to update the filter
params, but it was incorrect (reloading the page would not get you back
to the same view state) and had a lot of duplicated code. There was
actually no way to have everything (filters and pagination params)
updated in the URL all together, except using the deprecated
`<FilteredConnection>`.
Now, callers that want the URL to be updated with the connection state
(including pagination *and* filters) do:
```typescript
const connectionState = useUrlSearchParamsForConnectionState(filters)
const { ... } = usePageSwitcherPagination({ query: ..., state: connectionState}) // or useShowMorePaginationUrl
```
Callers that do not want the connection state to be reflected in the URL
can just not pass any `state:` value.
This PR also has some other refactors:
- remove `<ConnectionSummary first>` that was used in an erroneous
calculation. It was only used as a hack to determine the `totalCount` of
the connection. This is usually returned in the connection result itself
and that is the only value that should be used.
- remove `?visible=N` param from some connection pages, just use
`?first=N`. This was intended to make it so that if you reloaded or
navigated directly to a page that had a list, subsequently fetching the
next page would only get `pageSize` additional records (eg 20), not
`first` (which is however many records were already showing) for
batch-based navigation. This is not worth the additional complexity that
`?visible=` introduces, and is not clearly desirable even.
- 2 other misc. ones that do not affect user-facing behavior (see commit
messages)
## Test plan
Visit the site admin repositories and packages pages. Ensure all filter
options work and pagination works.
This PR stubs out the URI needed for the React UI to interface with the
appliance, as well as removed the previously implemented UI and
components of the React UI that were only around for a demo.
A number of helper and safety methods have also been added for
interfacing with JSON reads/writes and handling common errors.
While the HTTP handlers are still only stubs, this PR was growing in
size so I decided to cut it here and break apart the rest in upcoming
PRs. React UI is able to parse status and auth correctly at this time.
<!-- PR description tips:
https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e
-->
## Test plan
Unit tests
## Changelog
<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Implements CRUD on the new licenses DB. I had to make significant
changes from the initial setup after spending more time working on this.
There's lots of schema changes but that's okay, as we have no data yet.
As in the RPC design, this is intended to accommodate new "types" of
licensing in the future, and so the DB is structured as such as well.
There's also feedback that context around license management events is
very useful - this is encoded in the conditions table, and can be
extended to include more types of conditions in the future.
Part of https://linear.app/sourcegraph/issue/CORE-158
Part of https://linear.app/sourcegraph/issue/CORE-100
## Test plan
Integration tests
Locally, running `sg run enterprise-portal` indicates migrations proceed
as expected
For simple, MVP integrations with Telemetry Gateway, and also as a
reference. See example - this can be consumed with:
```go
import telemetrygatewayv1 "github.com/sourcegraph/sourcegraph/lib/telemetrygateway/v1"
```
## Test plan
n/a
While testing the modelconfig system working end-to-end with the data
coming from the site configuration, I ran into a handful of minor
issues.
They are all kinda subtle so I'll just leave comments to explain the
what and why.
## Test plan
Added new unit tests.
## Changelog
NA
This PR adds experimental endpoints implementing for context ranking and
storage, as required by RFC 969.
Also removes the deprecated and unused `getCodyIntent` query (we have
`chatIntent`).
## Test plan
- tested locally, not integrated with any clients
## Linked Issues
- Closes https://github.com/sourcegraph/sourcegraph/issues/35027
## Motivation and Context:
<!--- Why is this change required? What problem does it solve? -->
- To improve the UX by displaying the invalid token error message on the
UI to the users while adding a code host connection
## Existing Issue Root Cause:
- After the submit button is clicked while adding a code host
connection, the existing code navigates to the
`/site-admin/external-services/${data.addExternalService.id}` page (the
page which displays information of the newly added code host connection)
before the error message could even be displayed on the current page
## Changes Made:
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. List any
dependencies that are required for this change. -->
- Modified the onSubmit function (the one which is called when the
submit button is clicked on the code host connection addition page) to
only navigate to the
`/site-admin/external-services/${data.addExternalService.id}` page
(newly added code host information page) only if no error/warning is
returned from the gRPC call which validates the token of the code host
added
## Type of change:
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Refactoring (altering code without changing its external
behaviour)
- [ ] Documentation change
- [ ] Other
## Checklist:
- [x] Development completed
- [x] Comments added to code (wherever necessary)
- [x] Documentation updated (if applicable)
- [x] Tested changes locally
## Follow-up tasks (if any):
- None
## Test Plan
<!--- Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce. -->
- Had setup the codebase locally and tested the entire flow locally
end-to-end
- Screen recording which shows the invalid token error message on the UI
in the code host connection addition flow:
https://github.com/sourcegraph/sourcegraph/assets/51479159/1857f32d-56a6-42c1-af88-ea3c9edc46a5
## Additional Comments
- Please let me know if any further changes are required elsewhere
In Sveltekit, we respect the setting `search.defaultPatternType` for the
initial search, but if you set then unset the regexp toggle, it always
reverts to `keyword`. Now we revert back to the default when a toggle is
unset.
This is important because some customers have disabled keyword search,
and for 5.5 we've directed them to use `search.defaultPatternType:
standard`.
## Test plan
Manually tested search + toggles with following config:
* No settings
* `search.defaultPatternType: standard`, `keyword`, and `regexp`
This PR if what the past dozen or so
[cleanup](https://github.com/sourcegraph/sourcegraph/pull/63359),
[refactoring](https://github.com/sourcegraph/sourcegraph/pull/63731),
and [test](https://github.com/sourcegraph/sourcegraph/pull/63761) PRs
were all about: using the new `modelconfig` system for the completion
APIs.
This will enable users to:
- Use the new site config schema for specifying LLM configuration, added
in https://github.com/sourcegraph/sourcegraph/pull/63654. Sourcegraph
admins who use these new site config options will be able to support
many more LLM models and providers than is possible using the older
"completions" site config.
- For Cody Enterprise users, we no longer ignore the
`CodyCompletionRequest.Model` field. And now support users specifying
any LLM model (provided it is "supported" by the Sourcegraph instance).
Beyond those two things, everything should continue to work like before.
With any existing "completions" configuration data being converted into
the `modelconfig` system (see
https://github.com/sourcegraph/sourcegraph/pull/63533).
## Overview
In order to understand how this all fits together, I'd suggest reviewing
this PR commit-by-commit.
### [Update internal/completions to use
modelconfig](e6b7eb171e)
The first change was to update the code we use to serve LLM completions.
(Various implementations of the `types.CompletionsProvider` interface.)
The key changes here were as follows:
1. Update the `CompletionRequest` type to include the `ModelConfigInfo`
field (to make the new Provider and Model-specific configuration data
available.)
2. Rename the `CompletionRequest.Model` field to
`CompletionRequest.RequestedModel`. (But with a JSON annotation to
maintain compatibility with existing callers.) This is to catch any bugs
related to using the field directly, since that is now almost guaranteed
to be a mistake. (See below.)
With these changes, all of the `CompletionProvider`s were updated to
reflect these changes.
- Any situation where we used the
`CompletionRequest.Parameters.RequestedModel` should now refer to
`CompletionRequest.ModelConfigInfo.Model.ModelName`. The "model name"
being the thing that should be passed to the API provider, e.g.
`gpt-3.5-turbo`.
- In some situations (`azureopenai`) we needed to rely on the Model ID
as a more human-friendly identifier. This isn't 100% accurate, but will
match the behavior we have today. A long doc comment calls out the
details of what is wrong with that.
- In other situations (`awsbedrock`, `azureopenai`) we read the new
`modelconfig` data to configure the API provider (e.g.
`Azure.UseDeprecatedAPI`), or surface model-specific metadata (e.g. AWS
Provisioned Throughput ARNs). While the code is a little clunky to avoid
larger refactoring, this is the heart and soul of how we will be writing
new completion providers in the future. That is, taking specific
configuration bags with whatever data that is required.
### [Fix bugs in
modelconfig](75a51d8cb5)
While we had lots of tests for converting the existing "completions"
site config data into the `modelconfig.ModelConfiguration` structure,
there were a couple of subtle bugs that I found while testing the larger
change.
The updated unit tests and comments should make that clear.
### [Update frontend/internal/httpapi/completions to use
modelconfig](084793e08f)
The final step was to update the HTTP endpoints that serve the
completion requests. There weren't any logic changes here, just
refactoring how we lookup the required data. (e.g. converting the user's
requested model into an actual model found in the site configuration.)
We support Cody clients sending either "legacy mrefs" of the form
`provider/model` like before, or the newer mref
`provider::apiversion::model`. Although it will likely be a while before
Cody clients are updated to only use the newer-style model references.
The existing unit tests for the competitions APIs just worked, which was
the plan. But for the few changes that were required I've added comments
to explain the situation.
### [Fix: Support requesting models just by their
ID](99715feba6)
> ... We support Cody clients sending either "legacy mrefs" of the form
`provider/model` like before ...
Yeah, so apparently I lied 😅 . After doing more testing, the extension
_also_ sends requests where the requested model is just `"model"`.
(Without the provider prefix.)
So that now works too. And we just blindly match "gtp-3.5-turbo" to the
first mref with the matching model ID, such as
"anthropic::unknown::gtp-3.5-turbo".
## Test plan
Existing unit tests pass, added a few tests. And manually tested my Sg
instance configured to act as both "dotcom" mode and a prototypical Cody
Enterprise instance.
## Changelog
Update the Cody APIs for chat or code completions to use the "new style"
model configuration. This allows for great flexibility in configuring
LLM providers and exposing new models, but also allows Cody Enterprise
users to select different models for chats.
This will warrant a longer, more detailed changelog entry for the patch
release next week. As this unlocks many other exciting features.
This adds two new components for the common situation of needing to
display a styled path.
- `DisplayPath` handles splitting, coloring, spacing, slotting in a file
icon, adding a copy button, and ensuring that no spaces get introduced
when copying the path manually
- `ShrinkablePath` is built on top of `DisplayPath` and adds the ability
to collapse path elements into a dropdown menu
These are used in three places:
- The file header. There should be no change in behavior except maybe a
simplified DOM. This makes use of the "Shrinkable" version of the
component.
- The file search result header. This required carefully ensuring that
the text content of the node is exactly equal to the path so that the
character offsets are correct.
- The file popover, where it is used for both the repo name (unlinkified
version) and the file name (linkified version).
Fixes SRCH-718
Fixes SRCH-690
The "Exclude" options in the filter panels are very useful, but many are specific to Go. This change generalizes them so they apply in many more cases:
* All files with suffix `_test` plus extension (covers Go, Python, some Ruby, C++, C, more)
* All files with suffix `.test` plus extension (covers Javascript, some Ruby)
* Ruby specs
* Third party folders (common general naming pattern)
Relates to SPLF-70
We register ctrl+backspace to go to the repository root, but that should
not trigger when an input field, such as the fuzzy finder, is focused.
Fixes srch-681
## Test plan
Manual testing.
## Linked Issues
- Closes https://github.com/sourcegraph/sourcegraph/issues/38348
## Motivation and Context:
<!--- Why is this change required? What problem does it solve? -->
- Improves the UX of the password reset flow
## Changes Made:
<!--- Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context. List any
dependencies that are required for this change. -->
- Made changes to the following 2 flows:
- On the new password entry screen:
- Added a text which displays the email of the account for which the
password change request has been raised
- Added a back button to allow the users to go back to the previous
email entry screen if the account they want to reset the password for is
different
- On the sign-in screen which comes after successful password reset
request completion:
- Made changes to auto-populate the email text-box with the email linked
to the account on which the password reset request was completed
recently
## Type of change:
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] Refactoring (altering code without changing its external
behaviour)
- [ ] Documentation change
- [ ] Other
## Checklist:
- [x] Development completed
- [ ] Comments added to code (wherever necessary)
- [ ] Documentation updated (if applicable)
- [x] Tested changes locally
## Follow-up tasks (if any):
- None
## Test Plan
<!--- Please describe the tests that you ran to verify your changes.
Provide instructions so we can reproduce. -->
- Setup the codebase locally along with configuring a custom SMTP to
enable the email delivery functionality with the help of this
documentation:
https://docs.sourcegraph.com/admin/config/email#configuring-sourcegraph-to-send-email-using-another-provider
- Tested the entire flow locally end-to-end.
- Screen recording of the password reset screen where the current email
ID is added along with a back button:
https://github.com/sourcegraph/sourcegraph/assets/51479159/a79fc338-ace0-4281-86d2-de7cc68eae20
- Screen recording of the sign-in screen after password reset is
successfully done where the email ID is auto-populated in the text-box:
https://github.com/sourcegraph/sourcegraph/assets/51479159/be7db65d-9421-4621-a1e9-a04a546b9757
## Additional Comments
- Please let me know if I need to make any further design changes from
the frontend side or any API contract related changes from the backend
side
---------
Co-authored-by: Vincent <evict@users.noreply.github.com>
Co-authored-by: Shivasurya <s.shivasurya@gmail.com>
This PR overhauls the UI a bunch to make it look more in line with other
pages, and fixes various smaller papercuts and bugs.
Closes SRC-377
Test plan:
Added storybooks and made sure existing ones still look good, created,
updated, deleted various webhooks locally.
The background publisher was started regardless if analytics was
disabled or not. This PR makes it so that we only publish analytics if
it is enabled.
To make it work and not duplicate the disabled analytics check, I moved
the usershell + background context creation to happen earlier.
## Test plan
CI and tested locally
## Changelog
* sg - only start the analytics background publisher when analytics are
enabled
---------
Co-authored-by: Jean-Hadrien Chabran <jh@chabran.fr>
I went through all call sites of the 3 search APIs (Stream API, GQL API,
SearchClient (internal)) and made sure that the query syntax version is
set to "V3".
Why?
Without this change, a new default search syntax version might have
caused a change in behavior for some of the call sites.
## Test plan
- No functional change, so relying mostly on CI
- The codeintel GQL queries set the patternType explicitly, so this
change is a NOP.
I tested manually
- search based code intel sends GQL requests with version "V3"
- repo badge still works
- compute GQL returns results
A couple of tweaks to the commit / diff view:
- Linking both file paths in the header for renamed files
- Collapse renamed file diffs without changes by default
- Move "no changes" out of `FileDiffHunks` to not render a border around
the test.
- Add description for binary files
- Adjust line height and font size to match what we use in the file view
- Added the `visibly-hidden` utility class to render content for a11y
purposes (I didn't test the changes I made with a screenreader though)
Contributes to SRCH-523
## Test plan
Manual testing
The OTEL upgrade https://github.com/sourcegraph/sourcegraph/pull/63171
bumps the `prometheus/common` package too far via transitive deps,
causing us to generate configuration for alertmanager that altertmanager
doesn't accept, at least until the alertmanager project cuts a new
release with a newer version of `promethues/common`.
For now we forcibly downgrade with a replace. Everything still builds,
so we should be good to go.
## Test plan
`sg start` and `sg run prometheus`. On `main`, editing
`observability.alerts` will cause Alertmanager to refuse to accept the
generated configuration. With this patch, all is well it seems - config
changes go through as expected. This is a similar test plan for
https://github.com/sourcegraph/sourcegraph/pull/63329
## Changelog
- Fix Prometheus Alertmanager configuration failing to apply
`observability.alerts` from site config
`REDIS_ENDPOINT` is now registered by gateway, but it is also registered
in `internal/redispool` as the fallback for when the other values are
not set.
The real fix would be to not have env vars in that package, and instead
each service creates one instance of each of those two in their `cmd/`,
but that's a lot of work so short-term fixing it by reading the fallback
using os.Getenv.
Test plan:
`sg run cody-gateway` doesn't panic.
---------
Co-authored-by: Jean-Hadrien Chabran <jh@chabran.fr>
**chore(appliance): extract constant for configmap name**
To the reconciler, this is just a value, but to higher-level packages
like appliance, there is a single configmap that is an entity. Let's
make sure all high-level orchestration packages can reference our name
for it. This could itself be extracted to injected config if there was a
motivation for it.
**chore(appliance): extract NewRandomNamespace() in k8senvtest**
From reconciler tests, so that we can reuse it in self-update tests.
**feat(appliance): self-update**
Add a worker thread to the appliance that periodically polls release
registry for newer versions, and updates its own Kubernetes deployment.
If the APPLIANCE_DEPLOYMENT_NAME environment variable is not set, this
feature is disabled. This PR will be accompanied by one to the
appliance's helm chart to add this variable by default.
**fix(appliance): only self-update 2 minor versions above deployed SG**
**chore(appliance): self-update integration test extra case**
Check that self-update doesn't run when SG is not yet deployed.
https://linear.app/sourcegraph/issue/REL-212/appliance-can-self-upgrade
This PR fixes an important bug in #62976, where we didn't properly map the
symbol line match to the return type. Instead, we accidentally treated symbol
matches like file matches and returned the start of the file.
## Test plan
Add new unit test for symbol match conversion. Extensive manual testing.
Instead of fetching the file for every node, this passes in a
request-scoped cache to minimize the number of gitserver roundtrips, but
does no fetching if `surroundingContent` is not requested by the caller.
This commit removes files/dependencies that we are not using (anymore).
In the case of `@sourcegraph/wildcard` we never want to import
dependencies from it, but have done so accidentally in the past. I hope
that we can prevent this by removing it from dependencies (and we don't
need anyway).
## Test plan
`pnpm build` and CI
This PR fixes the following:
- Handles source range translation in the occurrences API
(Fixes https://linear.app/sourcegraph/issue/GRAPH-705)
- Handles range translation when comparing with document occurrences in
search-based and syntactic usagesForSymbol implementations
Throwing this PR up in its current state as I think adding the bulk
conversion
API will be a somewhat complex task, so we should split them into
separate
PRs anyways, and I don't have time to continue working on this right
now.
Some design notes:
- We want to avoid passing around full CompletedUpload and RequestState
objects,
which is why I chose to create a smaller UploadSummary type and decided
to pass
around GitTreeTranslator as that is the minimal thing we need to handle
range re-mapping.
- Yes, this PR increases the surface of the UploadLike type, but I think
it's still quite manageable.
## Test plan
manual testing, existing tests on gittreetranslator
---------
Co-authored-by: Christoph Hegemann <christoph.hegemann@sourcegraph.com>
Removes the `sg telemetry` command that pertains to the legacy V1
exporter that is specific to Cloud instances.
I got asked about this recently, and especially with the new `sg
analytics` for usage of the `sg` CLI, this has the potential to be
pretty confusing.
Part of https://linear.app/sourcegraph/issue/CORE-104
## Test plan
n/a
## Changelog
- `sg`: the deprecated `sg telemetry` command for allowlisting export of
V1 telemetry from Cloud instances has been removed. Use telemetry V2
instead.
A couple of minor changes to minimize the diff for "large completions
API refactoring".
Most changes are just a refactoring of the `openai` completions
provider, which I apparently missed in
https://github.com/sourcegraph/sourcegraph/pull/63731. (There are still
some smaller tweaks that can be made to the `fireworks` or `google`
completion providers, but they aren't as meaningful.
This PR also removes a couple of unused fields and methods. e.g.
`types.CompletionRequestParameters::Prompt`. There was a comment to the
effect of it being long since deprecated, and it is no longer read
anywhere on the server side. So I'm assuming that a green CI/CD build
means it is safe to remove.
## Test plan
CI/CD
## Changelog
NA
This PR adds more unit tests for the "Chat Completions" HTTP endpoint.
The goal is to have unit tests for more of the one-off quirks that we
support today, so that we can catch any regressions when refactoring
this code.
This PR adds _another layer_ of test infrastructure to use to streamline
writing completion tests. (Since they are kinda involved, and are
mocking out multiple interactions, it's kinda necessary.)
It introduces a new data type `completionsRequestTestData` which
contains all of the "inputs" to the test case, as well as some of the
things we want to validate.
```go
type completionsRequestTestData struct {
SiteConfig schema.SiteConfiguration
UserCompletionRequest types.CodyCompletionRequestParameters
WantRequestToLLMProvider map[string]any
WantRequestToLLMProviderPath string
ResponseFromLLMProvider map[string]any
WantCompletionResponse types.CompletionResponse
}
```
Then to run one of these tests, you just call the new function:
```go
func runCompletionsTest(t *testing.T, infra *apiProviderTestInfra, data completionsRequestTestData) {
```
With this, the new pattern for completion tests is of the form:
```go
func TestProviderX(t *testing.T) {
// Return a valid site configuration, and the expected API request body
// we will send to the LLM API provider X.
getValidTestData := func() completionsRequestTestData {
...
}
t.Run("TestDataIsValid", func(t *testing.T) {
// Just confirm that the stock test data works as expected,
// without any test-specific modifications.
data := getValidTestData()
runCompletionsTest(t, infra, data)
})
}
```
And then, for more sophisticated tests, we would just overwrite whatever
subset of fields are necessary from the stock test data.
For example, testing the way AWS Bedrock provisioned throughput ARNs get
reflected in the completions API can be done by creating a function to
return the specific site configuration data, and then:
```go
t.Run("Chat", func(t *testing.T) {
data := getValidTestData()
data.SiteConfig.Completions = getProvisionedThroughputSiteConfig()
// The chat model is using provisioned throughput, so the
// URLs are different.
data.WantRequestToLLMProviderPath = "/model/arn:aws:bedrock:us-west-2:012345678901:provisioned-model/abcdefghijkl/invoke"
runCompletionsTest(t, infra, data)
})
t.Run("FastChat", func(t *testing.T) {
data := getValidTestData()
data.SiteConfig.Completions = getProvisionedThroughputSiteConfig()
data.UserCompletionRequest.Fast = true
// The fast chat model does not have provisioned throughput, and
// so the request path to bedrock just has the model's name. (No ARN.)
data.WantRequestToLLMProviderPath = "/model/anthropic.claude-v2-fastchat/invoke"
runCompletionsTest(t, infra, data)
})
```
## Test plan
Added more unit tests.
## Changelog
NA
Closes srch-494
This adds the search query syntax introduction component to the search
homepage. I tried to replicate the React version as closely as possible.
I originally wanted to reuse the logic to generate the example sections
but since it had dependencies on wildcard I duplicated it instead.
Notable additional changes:
- Added a `value` method to the temporary settings store to make it
easier to get the current value of the settings store. It only resolves
(or rejects) once the data is loaded.
- Extended the tabs component to not show the tab header if there is
only a single panel. This makes it easier for consumers to render tabs
conditionally.
- Added the `ProductStatusBadge` component
- Various style adjustments
For reference, the relevant parts of the React version are in
https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/client/branded/src/search-ui/components/useQueryExamples.tsx
and
https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph/-/blob/client/branded/src/search-ui/components/QueryExamples.tsx
## Test plan
Manual testing. I manually set the value received from temporary
settings to `null` (in code) to force trigger the compute logic.
Fixes srch-589
This implements the 'y' shortcut to navigate to the permalink page, just
like we have in the React app.
According to aria guidelines it should be possible to disable single
character shortcuts but this actually never worked for the 'y' shortcut
because it was implemented differently. So adding it without the option
to turn it off is at least not a regression.
For reference this the code handling the shortcut in the React app:
https://sourcegraph.sourcegraph.com/github.com/sourcegraph/sourcegraph@d9dff1191a3bad812aa5b50315b8e77ee0e40e55/-/blob/client/web/src/repo/actions/CopyPermalinkAction.tsx?L64-77
When initially implementing this I noticed that there is a slight delay
between pressing 'y' and the URL/UI updating. That's because SvelteKit
has to wait for `ResolveRepoRevision` call to resolve before updating
the page. A new request is made because the URL changes which triggers
the data loader. But we already know that such a request will return the
same data, so the request is unnecessary. To avoid the second call I
added another caching layer.
I also noticed that the last commit info would be reloaded which is
unnecessary. I changed the implementation to use the resolved revision
instead of the revision from the URL. Now the request will be properly
cached on the commit ID and the implementation is also much simpler.
## Test plan
Manual testing.
Closes
https://linear.app/sourcegraph/issue/SRC-463/error-handling-for-smtp-config-issues
Before, when someone created or deleted an access token, we'd block on
sending the email before we'd return to the user.
However, if an error occurred when sending an email - the error was only
logged - **not** returned to the caller.
This PR makes three changes:
1. We now sending the email in a background goroutine. This makes it so
that the caller doesn't have to wait for the email to be sent before
getting the access token back. This isn't a behavior change since we
only logged any SMTP errors and **deliberatly avoided** forwarding that
error to the caller. That behavior doesn't change here.
1. If an error occurs while sending the access token email, we now have
a specialized log message indicating that the user's SMTP configuration
might be incorrect.
1. The log error that's printed is now an `ERROR` instead of a `WARN`
level.
## Test plan
1. On local dev, I edited the credentials in the [dev-private site
configuration file
](67ee9b3a69/enterprise/dev/site-config.json (L8-L11))to
point to an invalid port.
2. I attempted to create an access token, and after only 30 seconds I
saw this error print in the terminal:
```
[ frontend] WARN schemaResolver.CreateAccessToken graphqlbackend/access_tokens.go:150 Failed to send email to inform user of access token creation. (This error might indicate that your SMTP connection settings are incorrect. Please check your site configuration.) {"userID": 1, "error": "establishing TCP connection to \"smtp.gmail.com:57\": dial tcp 142.250.101.109:57: i/o timeout"}
```
3. I fixed the broken SMTP configuration, and deleted the access token.
I saw it suscessfully send an email:
## Changelog
- When creating or deleting an access token, we no longer wait for the
email to be sent before returning to the caller. Instead, we now send it
in the background.
Before, the smtp.Dial function had no cancellation mechanism - meaning
that you could be waiting for several minutes to try to establish a
connection before it gives up.
I work around this by establishing a TCP connection myself to the
appropriate address, and using
[net.DialContext](https://pkg.go.dev/net#Dialer.DialContext) to cancel
the dial process if either:
1. The parent context cancels
2. 30 seconds have passed
Afterwards, we construct the smtp.Client ourselves using
[smtp.NewClient](https://pkg.go.dev/net/smtp#NewClient)
This only changes what happens when we try to establish a connection -
it doesn't change any behavior around sending emails afterwards.
## Test plan
Manual testing.
1. On local dev, I edited the credentials in the [dev-private site
configuration file
](67ee9b3a69/enterprise/dev/site-config.json (L8-L11))to
point to an invalid port.
2. I attempted to create an access token, and after only30 seconds I saw
this error print in the terminal:
```
[ frontend] WARN schemaResolver.CreateAccessToken graphqlbackend/access_tokens.go:127 Failed to send email to inform user of access token creation {"userID": 1, "error": "establishing TCP connection to \"smtp.gmail.com:57\": dial tcp 74.125.137.109:57: i/o timeout"}
```
## Changelog
- Instead of waiting forever, we wait at most 30 seconds before giving
up when trying to connect to the configured mail server when sending an
email.
about: Report problems and unexpected behavior (Code Search ONLY)
title: ''
labels: ''
assignees: ''
---
<!-- Please submit all feedback or bug reports for Cody in the [VS Code](https://github.com/sourcegraph/cody) or [JetBrains](https://github.com/sourcegraph/jetbrains) repo. -->
- **Sourcegraph version:**<!-- the version of Sourcegraph or "Sourcegraph.com" -->
- **Platform information:**<!-- OS version, cloud provider, web browser version, Docker version, etc., depending on the issue -->
## :x: Cloud Controller GraphQL Compatability Test Result
[sourcegraph/controller](https://github.com/sourcegraph/controller) uses the GraphQL API to perform automation. The compatibility test has failed and this pull request may have introduced breaking changes to the GraphQL schema.
Next steps:
- Review the GitHub Actions [workflow logs](${process.env.RUN_URL}) for more details.
- Reach out to the Cloud Ops team to resolve the issue before merging this pull request.
${commentMarker}
`
}else {
message = `
## :white_check_mark: Cloud Controller GraphQL Compatability Test Result
[sourcegraph/controller](https://github.com/sourcegraph/controller) uses the GraphQL API to perform automation. The compatibility test has passed.
Learn more from [workflow logs](${process.env.RUN_URL}).
@ -37,6 +37,7 @@ All notable changes to Sourcegraph are documented in this file.
- The default and recommended chat model for Anthropic and Cody Gateway configurations is now `claude-3-sonnet-20240229`. [#62757](https://github.com/sourcegraph/sourcegraph/pull/62757)
- The default and recommended autocomplete model for Cody Gateway configurations is now `fireworks/starcoder`. [#62757](https://github.com/sourcegraph/sourcegraph/pull/62757)
- Code Insights: Language Stats Insights performance improved by another 70-90%. It's now able to handle repositories above 40 GB. [#62946](https://github.com/sourcegraph/sourcegraph/pull/62946)
- The keyword search toggle has been removed from the search results page. [Keyword search](https://sourcegraph.com/docs/code-search/queries#keyword-search-default) is now enabled by default for all searches in the Sourcegraph web app. [#63584](https://github.com/sourcegraph/sourcegraph/pull/63584)
return'**Built-in predicate**. Search only inside repositories that satisfy the specified `path:` and `content:` filters. `path:` and `content:` filters should be regular expressions.'
- [Report a bug](https://github.com/sourcegraph/sourcegraph/issues/new?labels=team/integrations,vscode-extension&title=VSCode+Bug+report:+&projects=Integrations%20Project%20Board)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
